913011 - Clean up AVD handling

Status: RESOLVED DUPLICATE of bug 919812

(Infrastructure & Operations :: RelOps: Puppet, task)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

Bug 895186 landed AVD handling in Puppet.  There are a few issues with the implementation:

Comment on bug 895186 attachment 796728 [details] [diff] [review] [diff] [review]
[puppet] add android-tools v2

::: modules/androidemulator/manifests/init.pp
@@ +5,5 @@
> +    case $::operatingsystem {
> +        Ubuntu: {
> +	    # We want it on Ubuntu
> +	    include packages::mozilla::android_sdk18
> +	}

This should have a default line, or just remove the $::operatingsystem conditional and let the package class handle failing on other platforms.

::: modules/packages/manifests/mozilla/android_sdk18.pp
@@ +5,5 @@
> +class packages::mozilla::android_sdk18 {
> +    case $::operatingsystem {
> +        Ubuntu: {
> +            package {
> +                # Built from https://github.com/rail/android-sdk

Sorry, one more thing on this ptach - like screenresolution, this should probably be moved to a Mozilla repo.  It gets back to the still-unsolved problem of how to version Debian packaging scripts.

::: modules/androidemulator/manifests/x86.pp
@@ +18,5 @@
> +		    source => "http://${config::data_server}/repos/private/avds/test-x86-aug6.tar.gz",
> +		    owner  => $users::builder::username,
> +		    group  => $users::builder::group;
> +	    }
> +	}

Will this HTTP source cause the entire tarball, which is quite large, to be downloaded on every puppet run?  That could be very painful.  Could this be downloaded with tooltool instead, or wrapped in a .deb?  At any rate, it isn't really a repository right now, so probably doesn't belong under repos/.

Do these have to be installed in $HOME?  We're not doing anything else in that directory anymore - could this be in /build or /tools instead?

Finally, where does this tarball come from?  How could an external user upgrade it, or a developer figure out what's in it, or a future relenger upgrade it to a newer version than aug6?

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

also, tabs in modules/androidemulator/manifests/x86.pp

Comment 2

[Justin Wood (:Callek)]

(In reply to Dustin J. Mitchell [:dustin] from comment #0)
> Bug 895186 landed AVD handling in Puppet.  There are a few issues with the
> implementation:
> 
> Comment on bug 895186 attachment 796728 [details] [diff] [review]
> [review]
> [puppet] add android-tools v2
> 
> ::: modules/androidemulator/manifests/init.pp
> @@ +5,5 @@
> > +    case $::operatingsystem {
> > +        Ubuntu: {
> > +	    # We want it on Ubuntu
> > +	    include packages::mozilla::android_sdk18
> > +	}
> 
> This should have a default line, or just remove the $::operatingsystem
> conditional and let the package class handle failing on other platforms.

I am doing the androidemulator for all hardware talos hosts, while we only wanted to install the avd on Ubuntu. Its not a failure for other hw talos platforms. I don't see a problem with that here.

> ::: modules/packages/manifests/mozilla/android_sdk18.pp
> @@ +5,5 @@
> > +class packages::mozilla::android_sdk18 {
> > +    case $::operatingsystem {
> > +        Ubuntu: {
> > +            package {
> > +                # Built from https://github.com/rail/android-sdk
> 
> Sorry, one more thing on this ptach - like screenresolution, this should
> probably be moved to a Mozilla repo.  It gets back to the still-unsolved
> problem of how to version Debian packaging scripts.

I agree, but we needed this now, and I absolutely did not want to block this bug on figuring out a debian setup. And I'm not a git-expert (quite the opposite really) so happy if we move this elsewhere, I just won't backout any of this for that.

> ::: modules/androidemulator/manifests/x86.pp
> @@ +18,5 @@
> > +		    source => "http://${config::data_server}/repos/private/avds/test-x86-aug6.tar.gz",
> > +		    owner  => $users::builder::username,
> > +		    group  => $users::builder::group;
> > +	    }
> > +	}
> 
> Will this HTTP source cause the entire tarball, which is quite large, to be
> downloaded on every puppet run?

Two things, it would, but I didn't realize that. But to make matters worse current puppet entirely fails this since we can't do a raw source=> with an http (or https), so I switched to puppet:/// which works

> Could this be downloaded with tooltool instead, or wrapped in a .deb?

Shipping it via tooltool is the future goal here, however there are two blockers there that made a puppet deployment ideal:
(a) tooltool is not currently used on any testers, so we would need to deploy and test the tooltool scripts there.
(b) these avds are unlikely to change often, and are VERY large, so we'd need some sort of local caching in place for these files for tooltool to use, but our testers currently pretty much blow away their normal test dirs at every run. So we don't get a for-free caching here, as we do on Build machine dep builds.

> At any rate, it
> isn't really a repository right now, so probably doesn't belong under repos/.

I did not see any means to stuff anything private outside of repos/ at this time, if there is a means and I just missed it feel free to let me know.

In theory I don't think it is *fully* private needing, but aiui we can't redist any android emulator images which we generate from the android sdk. I'd want a legal review before we allowed these avds to be shipped externally.

> Do these have to be installed in $HOME?  We're not doing anything else in
> that directory anymore - could this be in /build or /tools instead?

For the current design/testing of the avd system, the actual *used* avds are in $HOME/.android/avds which is a required hardcode [I have ideas on how we can change that, but requires much more testing]
While the actual place this puppet manifest installs the avd tarball does not need to be in $HOME I chose $HOME strictly because the test-dir is right next to it but hidden, it helps preserve correlation. Ideally we would not stuff it in $HOME at all, but given the above hardcode that is indeed required I think this makes sense.

> Finally, where does this tarball come from?  How could an external user
> upgrade it, or a developer figure out what's in it, or a future relenger
> upgrade it to a newer version than aug6?

This will be on ATeam (specifically) gbrown to document, and I'll help press on him to document it. I have not pressed on it yet, because until armen's Bug 895186 is complete and live we don't know for sure if this is the final avd tarball (though we expect it to be).

(In reply to Dustin J. Mitchell [:dustin] from Bug 895186 comment #92)
> Oh, and why is $install_avds "yes"/"no" instead of boolean?

Just because in my brief testing I didn't see boolean working, I admit its entirely possible I just fubar'ed syntax. So I went with what I could get working faster. I have no preference here.

(In reply to Dustin J. Mitchell [:dustin] from comment #1)
> also, tabs in modules/androidemulator/manifests/x86.pp

So, given the above I will be treating this (tabs) and the boolean flag for install_avds as the only action items.

If you object to any of the other items, after my explanation, can we try and have a quick vidyo chat about them (and possibly roping in one third-party of your choice to tie-break if need be) at which point we'll mark in here what the resolution of chat was and act accordingly. We can file bug(s) for said ideal states as discussed of course regardless of the "is current state ok" decision.

- needinfo for confirmation to my understanding/plan here

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

(In reply to Justin Wood (:Callek) from comment #2)
> I am doing the androidemulator for all hardware talos hosts, while we only
> wanted to install the avd on Ubuntu. Its not a failure for other hw talos
> platforms. I don't see a problem with that here.

My concern is that, with this change, we have a manifest with

  include androidemulator

which leads one to think "oh, that has the android emulator installed" on, say, an OS X talos host.  Yet, it doesn't.

I'm not sure what you mean about installing the emulator everywhere - is that going to happen in future commits?

> I agree, but we needed this now, and I absolutely did not want to block this
> bug on figuring out a debian setup. And I'm not a git-expert (quite the
> opposite really) so happy if we move this elsewhere, I just won't backout
> any of this for that.

That sounds fine.

> Two things, it would, but I didn't realize that. But to make matters worse
> current puppet entirely fails this since we can't do a raw source=> with an
> http (or https), so I switched to puppet:/// which works

OK - that still checksums on the master on every run, so it may cause high load, but it seems not to have, so we're OK for now.  I didn't see the backout and re-land when I first noted this problem.

> Shipping it via tooltool is the future goal here, however there are two
> blockers there that made a puppet deployment ideal:
> (a) tooltool is not currently used on any testers, so we would need to
> deploy and test the tooltool scripts there.
> (b) these avds are unlikely to change often, and are VERY large, so we'd
> need some sort of local caching in place for these files for tooltool to
> use, but our testers currently pretty much blow away their normal test dirs
> at every run. So we don't get a for-free caching here, as we do on Build
> machine dep builds.

And even there, we get very little caching, so tooltool still uses an inordinate amount of bandwidth :(

It sounds like a .deb is the best plan going forward.

> > At any rate, it
> > isn't really a repository right now, so probably doesn't belong under repos/.
> 
> I did not see any means to stuff anything private outside of repos/ at this
> time, if there is a means and I just missed it feel free to let me know.

This would really deserve its own data tree:
  https://wiki.mozilla.org/ReleaseEngineering/PuppetAgain/Data

> In theory I don't think it is *fully* private needing, but aiui we can't
> redist any android emulator images which we generate from the android sdk.
> I'd want a legal review before we allowed these avds to be shipped
> externally.

OK - then we should have some instructions for how we generate those emulator images, either in the wiki with a link in the source, or straight in the source (as for XCode).

> For the current design/testing of the avd system, the actual *used* avds are
> in $HOME/.android/avds which is a required hardcode [I have ideas on how we
> can change that, but requires much more testing]
> While the actual place this puppet manifest installs the avd tarball does
> not need to be in $HOME I chose $HOME strictly because the test-dir is right
> next to it but hidden, it helps preserve correlation. Ideally we would not
> stuff it in $HOME at all, but given the above hardcode that is indeed
> required I think this makes sense.

Sounds good.  A comment to this effect in the source would be good too.

> This will be on ATeam (specifically) gbrown to document, and I'll help press
> on him to document it. I have not pressed on it yet, because until armen's
> Bug 895186 is complete and live we don't know for sure if this is the final
> avd tarball (though we expect it to be).

OK - links back to what I said above about giving a reproduction recipe for binaries we can't make available.

> (In reply to Dustin J. Mitchell [:dustin] from Bug 895186 comment #92)
> > Oh, and why is $install_avds "yes"/"no" instead of boolean?
> 
> Just because in my brief testing I didn't see boolean working, I admit its
> entirely possible I just fubar'ed syntax. So I went with what I could get
> working faster. I have no preference here.

Multiple forms of booleans is going to be very confusing.  I think it's worth the time to un-fubar true/false.

> So, given the above I will be treating this (tabs) and the boolean flag for
> install_avds as the only action items.

One outstanding question (first paragraph) and some future items that should probably keep this bug open:
 * build emulators as DEBs, or if that's impossible, move them to their own data tree
 * document how emulators are constructed

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

and one more
 * move android-sdk18 into a mozilla repo and make it clear how to build it

Comment 5

[Justin Wood (:Callek)]

(In reply to Dustin J. Mitchell [:dustin] from comment #3)
> (In reply to Justin Wood (:Callek) from comment #2)
> > Two things, it would, but I didn't realize that. But to make matters worse
> > current puppet entirely fails this since we can't do a raw source=> with an
> > http (or https), so I switched to puppet:/// which works
> 
> OK - that still checksums on the master on every run, so it may cause high
> load, but it seems not to have, so we're OK for now.  I didn't see the
> backout and re-land when I first noted this problem.

So in my #buildduty skim I saw a few instances of puppet in scl1 flapping. If this could likely be said cause I'm open to suggestions on a short-term how to reduce the load

e.g.
Tue 22:24:56 PDT [4365] releng-puppet2.build.scl1.mozilla.com:load is WARNING: WARNING - load average: 13.76, 11.67, 8.01 (http://m.allizom.org/load)

:dustin, does this sound like the most-likely cause of the repeated load, or is there something else that could be causing it here?

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

It's certainly possible.  Check the ganglia graphs to see if those spikes started when you landed the AVD changes.  That said, the scl1 puppetmasters also get a lot of load from the images on the mozpool servers, although there are only 11 of those.

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

I just increased the CPU count on releng-puppet2.build.scl1 to 4.

Comment 8

[Armen [:armenzg]]

I will be working on this on the week of the 11th.
I'm on buildduty next week and bug 929048 will take the rest of this week.

Comment 9

[Armen [:armenzg]]

Work is underway on bug 919812 to deal with this.