+++ This bug was initially created as a clone of Bug #895186 +++ I noticed a small error (incorrect screen height) in the avd definitions that we are using. I created a new set of avds with the error corrected, verified the change, and ran the full suite of tests on a loaner against the new avds. I found no difference in test results, but hope that we might be able to enable a few more tests with this change. Given that I have not identified a benefit to this change, I understand if we want to not act on this bug at this time, but if it is easy to update, why not? New avds are at: http://people.mozilla.org/~gbrown/test-avds-sep19.tar.gz

Adding dustin as well so he can see this on his radar. The original deployment happened on bug 895186.

Thanks! Can we dup or link this to bug 913011, where we need to refactor the deployment anyway (to use debs and/or rpms instead of puppet file resources)?

I can take this bug after the summit.

gbrown: can we post the avd definitions somewhere public? dustin: do you have any recommended place to put this file somewhere public? I want to make it available so developers could run the jobs locally.

If we can distribute them, they should go in the public puppetagain repository. Note, again, bug 913011, though. They should be packaged as RPMs, and not installed as large tarballs like they are now.

I would like to change our use for test-x86.tar.gz. This is how it currently works: * a machine grabs test-x86.tar.gz from private puppet and places it on /home/cltbld/avds/test-x86.tar.gz * on every job, we remove /home/cltbld/.android/avd and we untar test-x86.tar.gz again to the same location (clean state) What I would like to change to is this: * a machine grabs test-x86.tar.gz from *somewhere* where a developer could also reach it ** the machine will have to determine if the file is already on disk and skip it if need be *** this way we don't need to download it again on every run ** I think I'm fine if we have to place it behind LDAP (to avoid legal concerns) ** could we have a setup where developers would need to provide LDAP while the releng machines wouldn't need it? ** I am not sure if we can place it somewhere public since some of the files are taken directly from the android sdk (e.g. kernel-qemu, ramdisk.img, system.img) ** We would need a legal bug to determine if we can distribute them * on every job, we remove /home/cltbld/.android/avd and we untar it again to the same location (clean state) dustin: * where would it make more sense to place test-x86.tar.gz instead of /home/cltbld/avds? what is the concern? * I'm not very familiar with rpm files; we don't really install anything from test-x86.tar.gz; what is the reason to create an RPM? If we move away from puppet would this still be needed? I will also ask in bug 917361 if it mandatory to make the jobs runnable outside of the releng infra. Another idea would be if we could ask the person running the job to create its own avd definitions by pointing them to the documentation: https://bugzilla.mozilla.org/show_bug.cgi?id=894507#c19 Or if we could remove the components that are part of Android SDK installation and point directly to it. Here's what the tar ball contains: x kernel-qemu x ramdisk.img x system.img x test-x86-1.avd/ x test-x86-1.avd/cache.img x test-x86-1.avd/userdata-qemu.img x test-x86-1.avd/userdata.img x test-x86-1.avd/hardware-qemu.ini x test-x86-1.avd/emulator-user.ini x test-x86-1.avd/sdcard.img x test-x86-1.avd/config.ini x test-x86-1.ini x test-x86-2.avd/ x test-x86-2.avd/cache.img x test-x86-2.avd/userdata-qemu.img x test-x86-2.avd/userdata.img x test-x86-2.avd/hardware-qemu.ini x test-x86-2.avd/emulator-user.ini x test-x86-2.avd/sdcard.img x test-x86-2.avd/config.ini x test-x86-2.ini x test-x86-3.avd/ x test-x86-3.avd/cache.img x test-x86-3.avd/userdata-qemu.img x test-x86-3.avd/userdata.img x test-x86-3.avd/hardware-qemu.ini x test-x86-3.avd/emulator-user.ini x test-x86-3.avd/sdcard.img x test-x86-3.avd/config.ini x test-x86-3.ini x test-x86-4.avd/ x test-x86-4.avd/cache.img x test-x86-4.avd/userdata-qemu.img x test-x86-4.avd/userdata.img x test-x86-4.avd/hardware-qemu.ini x test-x86-4.avd/emulator-user.ini x test-x86-4.avd/sdcard.img x test-x86-4.avd/config.ini x test-x86-4.ini

dustin: what about if we added a developer to the BuildVPN when needed so they could retrieve on their local machine the avd definitions from tooltool?

(In reply to Armen Zambrano [:armenzg] (Release Engineering) (EDT/UTC-4) from comment #6) > Or if we could remove the components that are part of Android SDK > installation and point directly to it. I think that would work for kernel-qemu, ramdisk.img, and userdata.img -- they are identical to the images found in $(ANDROID_SDK)/system-images/android-17/x86. But we have customized system.img; we have basically taken the system.img from the SDK, added sutagent, watcher, and su to it, and then put the resulting image in the tar archive. sdcard.img can be generated by $(ANDROID-SDK)/tools/mksdcard. We need distinct copies of userdata.img, sdcard.img, cache.img, and userdata-qemu.img for each avd (each concurrent emulator instance). I don't know much about cache.img or userdata-qemu.ini otherwise.

s/userdata-qemu.ini/userdata-qemu.img/

I initially opted for using avds in part because I thought it would be portable: simple instructions would allow anyone to re-create our x86 emulator environment using standard Android tools. Unfortunately, https://bugzilla.mozilla.org/show_bug.cgi?id=894507#c19 is anything but simple now. Also, I thought the avd definition could be copied as an alternative to following the procedure, but it seems like those avd definitions must contain full paths, which really limits their portability. A possible alternative to avds is capturing all of the configuration on the emulator command line, but there are a lot of parameters to specify: all the paths to the .img files, gpu emulation and other options, and device characteristics (memory, screen size, etc, etc.)

Right now, the AVD is deployed by puppet. While it doesn't download the file on every run of puppet, it does perform a very slow md5 sum on the file on both the puppet master and the slave on every run, which is slow and causes high load on the puppet masters. We've added a new puppet master to handle this load, but need to find an alternative solution soon. Two options I can think of: * package the AVD in an RPM/DEB and deploy it that way (probably under /builds instead of /home) -- this would get around the md5 issue, but requires some work to upgrade the AVD * put the AVD on tooltool, and download it in such a way that clobbers don't wipe it out -- this makes it easy to use different AVDs on different branches, while getting local caching Tooltool is currently 100% private. There are outstanding releng bugs to fix that, but even so the public repository would not be LDAP-protected. I suspect that explicit BuildVPN access is going to be the best way to let devs download the private, licensed AVD, but we could certainly put a stripped-down version (perhaps with a shell script to build it back up?) on the public tooltool, once that public tooltool is implemented.

For now I will focus on moving it to the private tooltool as it moves us away from puppet.

(In reply to Geoff Brown [:gbrown] from comment #0) > New avds are at: http://people.mozilla.org/~gbrown/test-avds-sep19.tar.gz I am going to start experimenting with this tar ball. Should I expect a new one any time soon?

I don't have any plans for updating the avd definitions.

I'm thinking of placing the files in here instead: http://talos-bundles.pvt.build.mozilla.org/mobile/test-avds-sep19.tar.gz dustin, any objections? The reason to suggest that place is because I don't want to be having to developed tooltool for unit tests as well as disliking strongly the storage format (digest + manifests). I would be fine if I was allowed to put the file under sha512 without having to store it as the other files.

How often would that download occur? The file is very large, and seldom changed, so I would like to avoid frequent downloads from any service. Is this a talos bundle? If not, it shouldn't be hosted on a vhost named talos-bundles. Tooltool has its issues, but the solution is to fix those, not re-re-invent the wheel. If you're suggesting doing some kind of sha512 check and only downloading from talos-bundles if the checksum doesn't match, then you're re-inventing tooltool :) I haven't gotten a good indication of how often this file changes, but given the date in your filename that is more than a month ago, I'm assuming it's not terribly frequent. So I think the two suggestions in comment 11 (OS package or tooltool) are still the front-runners.

(In reply to Dustin J. Mitchell [:dustin] from comment #16) > How often would that download occur? The file is very large, and seldom > changed, so I would like to avoid frequent downloads from any service. > > Is this a talos bundle? If not, it shouldn't be hosted on a vhost named > talos-bundles. > > Tooltool has its issues, but the solution is to fix those, not re-re-invent > the wheel. If you're suggesting doing some kind of sha512 check and only > downloading from talos-bundles if the checksum doesn't match, then you're > re-inventing tooltool :) > > I haven't gotten a good indication of how often this file changes, but given > the date in your filename that is more than a month ago, I'm assuming it's > not terribly frequent. So I think the two suggestions in comment 11 (OS > package or tooltool) are still the front-runners. I was not planning to do a checksum. We only download the file once. I can use tooltool as the storage place, however, I would not be using the manifests approach + tooltool client (if that is how tooltool works).

Well, that would miss the advantages of tooltool, then :) If the file is only downloaded once, let's go with packaging it as an RPM/DEB. I realize we're beginning to go in circles here. I'm happy to discuss this one-on-one if you'd prefer.

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.

(In reply to Geoff Brown [:gbrown] from comment #14) > I don't have any plans for updating the avd definitions. Now there is bug 933840 -- distributing sutagent 1.20 to tegras and pandas. The new sutagent likely won't be fully tested for another week or two, and even then, it is not essential that the Android x86 tests use sutagent 1.20 immediately. However, probably within the next month or so we would want/need to update the avds for this (avds contain system.img, which has sutagent installed).

hey, I guessed right the week of the 11th :) Any new AVD definitions? I can work with what I have and add a new one when we need.

I don't have anything new. sutagent 1.20 has not been tested yet. I have not found any problems with Graydon's latest avds, in bug 910092. I suggest you use either my definitions from Comment 0, or Graydon's latest from bug 910092 -- your choice.

I deployed graydon's tar ball: http://people.mozilla.org/~graydon/AVDs-x86-android-4.2_r1-build-2013-11-13-ubuntu.tar.gz to tooltool: http://tooltool.pvt.build.mozilla.org/build/sha512/3b2d18eb0194d82c70c5ee17487ccbac309f9b2e9839fe7ca4a27a9a06f6338bb24394476da78559685d99151fccc85fdde03297aa73ee2f7fb3183e11925c4d

What do you think? I run it with this: /tools/buildbot/bin/python scripts/scripts/androidx86_emulator_unittest.py --cfg android/androidx86.py --test-suite robocop-1 --download-cacheable-artifacts --setup-avds I assume I could move other things there like host_utils_url. I would prefer something a little more sophisticated. For now I will work with this and try to test graydon's code. I can re-factor with your feedback once you get to it.

Comment on attachment 831811 [details] [diff] [review] cache artifacts Wfm. I'd be fine with this in the setup-avds action; the additional action is fine too. Do we want the avd tarball defined globally, rather than per-tree?

gbrown, graydon: wrt to avd tar balls: do we need to have per branch support? should we try to cross that bridge when we get there?

(In reply to Armen Zambrano [:armenzg] (Release Engineering) (EDT/UTC-4) from comment #26) > gbrown, graydon: wrt to avd tar balls: do we need to have per branch > support? should we try to cross that bridge when we get there? Just for information we don't have per branch android images for tegras or pandas, so while a good ideal, its not a requirement [imho]

I think the tegra/panda analogy is a good one: one image for all branches is good enough.

I'm pushing it now to Ash after local testing. We should see results in 40 minutes. https://tbpl.mozilla.org/?tree=Ash&jobname=Android%204.2%20x86

This patch cannot land until the previous lands and stays on production. This patch will need a follow up patch to remove the whole x86.pp file plus the reference on init.pp.

Comment on attachment 832382 [details] [diff] [review] cache avds + graydon's changes It worked on Ash.

Comment on attachment 832382 [details] [diff] [review] cache avds + graydon's changes >--- a/configs/android/androidx86.py >+++ b/configs/android/androidx86.py >--- a/scripts/androidx86_emulator_unittest.py >+++ b/scripts/androidx86_emulator_unittest.py If we are removing the x86 from the Class and emulator names, should we be removing the x86 from the file names of: androidx86.py and androidx86_emulator_unittest.py too? >+ ".avds_dir": "/home/cltbld/.android", just so I understand, prior to this: c[".avds_dir"] == "/home/cltbld/.android/avd". This means we now want to remove the '.android' tree and unpack the avd tar ball right into '.android' during setup_avds() right? I am assuming this is fine because '.android' previously had nothing other than the 'avd' dir in it and we are not removing important preexisting files? > def setup_avds(self): ... >- if os.path.exists(os.path.join(self.config[".avds_dir"], "test-x86-1.avd")): >- self.rmtree(self.config[".avds_dir"]) >- avds_path = self.config["avds_path"] >+ c = self.config ... > self.mkdir_p(self.config[".avds_dir"]) >- self.unpack(avds_path, self.config[".avds_dir"]) >+ self.unpack(avd_tar_ball_path, self.config[".avds_dir"]) Tiny tiny nit. Since we defined 'c', should we put it the start of setup_avds() and replace all the 'self.config' uses? >+ def download_cacheable_artifacts(self): ... >+ c = self.config >+ artifacts = c["tooltool_cacheable_artifacts"] >+ for artifact_name in artifacts.keys(): >+ file_name = artifacts[artifact_name][0] >+ file_shasum = artifacts[artifact_name][1] >+ file_path = os.path.join(c["tooltool_cache_path"], file_name) >+ file_url = os.path.join(c["tooltool_url"], file_shasum) >+ if not os.path.exists(file_path): >+ self.download_file(file_url, file_path, c["tooltool_cache_path"]) Another tiny nit, to save us automatically assigning file_url for each artifact, can we move it within the os.path.exists(file_path) condition block since we only need it if we reach there? Also, I think passing the third arg parent_dir=c["tooltool_cache_path"] in download_file() just means we: if parent_dir: file_name = os.path.join(parent_dir, file_name) which, since file_path is already assigned as a join of c["tooltool_cache_path"] and file_name (an absolute path), having that third argument will do nothing? Sorry for the review delay. r+ once you assert that removing the 'avd' from '/home/cltbld/.android/avd' is intentional + safe. The nits were just me trying really hard to find something :)

I wonder, why are we putting these in ~cltbld at all? If that's part of the way the emulators work, that's fine, but otherwise it would make a lot more sense to have them under /builds, where all of the other data, code, and binaries from our builds go. This isn't a blocker by any means, just a question.

As far as I can tell, this is part of the way the emulator works -- it expects avd definitions in $HOME/.android/avd. You can specify an avd name when starting the emulator, but not the location to look for the name. :(

(In reply to Geoff Brown [:gbrown] from comment #35) > As far as I can tell, this is part of the way the emulator works -- it > expects avd definitions in $HOME/.android/avd. You can specify an avd name > when starting the emulator, but not the location to look for the name. :( It looks in $ANDROID_SDK_HOME, which defaults to ~/.android but can be pointed anywhere. Note however: if you change it from "/home/cltbld/.android" to any other location, the AVD tarballs will have to be regenerated, since unfortunately each AVD dir inside the tarball has a corresponding config file that hard-codes that string (necessarily -- I tried several variants of relative naming and none seemed to work).

It's probably not worth such effort to change it. /home and /builds are the same partition, so there's no practical difference - just a matter of hygiene.

(In reply to Jordan Lund (:jlund) from comment #33) > >--- a/scripts/androidx86_emulator_unittest.py > >+++ b/scripts/androidx86_emulator_unittest.py > > If we are removing the x86 from the Class and emulator names, should we > be removing the x86 from the file names of: androidx86.py and > androidx86_emulator_unittest.py too? > I've landed a change on buildbot-configs to match renaming the "scripts" (not the config). For Android 2.3 we will use the same script with a different config. > >+ ".avds_dir": "/home/cltbld/.android", > > just so I understand, prior to this: c[".avds_dir"] == > "/home/cltbld/.android/avd". This means we now want to remove the '.android' > tree and unpack the avd tar ball right into '.android' during setup_avds() > right? I am assuming this is fine because '.android' previously had nothing > other than the 'avd' dir in it and we are not removing important preexisting > files? > It is all intended. Thanks for checking. > Tiny tiny nit. Since we defined 'c', should we put it the start of > setup_avds() and replace all the 'self.config' uses? > Done. > Another tiny nit, to save us automatically assigning file_url for each > artifact, can we move it within the os.path.exists(file_path) condition > block since we only need it if we reach there? > Also, I think passing the third arg parent_dir=c["tooltool_cache_path"] in > download_file() just means we: > if parent_dir: > file_name = os.path.join(parent_dir, file_name) > which, since file_path is already assigned as a join of > c["tooltool_cache_path"] and file_name (an absolute path), having that third > argument will do nothing? > Done. Good catch. Much cleaner. > Sorry for the review delay. r+ once you assert that removing the 'avd' from > '/home/cltbld/.android/avd' is intentional + safe. The nits were just me > trying really hard to find something :) I'm taking the r+ as I replied to the question as being intentional.

(In reply to Jordan Lund (:jlund) from comment #33) > Also, I think passing the third arg parent_dir=c["tooltool_cache_path"] in > download_file() just means we: > if parent_dir: > file_name = os.path.join(parent_dir, file_name) > which, since file_path is already assigned as a join of > c["tooltool_cache_path"] and file_name (an absolute path), having that third > argument will do nothing? > The third argument is intentional. If you look a little further in base/script.py you will see this: if create_parent_dir: parent_dir = os.path.dirname(file_path) self.mkdir_p(parent_dir, error_level=error_level) Since we start from the state of a non-existent parent_dir, we create it by passing it to the function.

Live in production as of 95 mins. ago.

something[s] here made it to production

> Since we start from the state of a non-existent parent_dir, we create it by > passing it to the function. aha! thanks, I did not catch that parent_dir itself would not exist yet. :)

Comment on attachment 832382 [details] [diff] [review] cache avds + graydon's changes I think I need more verbosity for the "already cached scenario". Already cached scenario: 11:10:12 INFO - ##### 11:10:12 INFO - ##### Running download-cacheable-artifacts step. 11:10:12 INFO - ##### 11:10:12 INFO - Running pre-action listener: _resource_record_pre_action 11:10:12 INFO - Running main action method: download_cacheable_artifacts 11:10:12 INFO - Running post-action listener: _resource_record_post_action 11:10:12 INFO - ##### 11:10:12 INFO - ##### Running setup-avds step. 11:10:12 INFO - ##### First time scenario: 11:07:54 INFO - ##### 11:07:54 INFO - ##### Running download-cacheable-artifacts step. 11:07:54 INFO - ##### 11:07:54 INFO - Running pre-action listener: _resource_record_pre_action 11:07:54 INFO - Running main action method: download_cacheable_artifacts 11:07:54 INFO - mkdir: /builds/slave/talos-slave/cached 11:07:54 INFO - Downloading http://tooltool.pvt.build.mozilla.org/build/sha512/3b2d18eb0194d82c70c5ee17487ccbac309f9b2e9839fe7ca4a27a9a06f6338bb24394476da78559685d99151fccc85fdde03297aa73ee2f7fb3183e11925c4d to /builds/slave/talos-slave/cached/AVDs-x86-android-4.2_r1-build-2013-11-13-ubuntu.tar.gz 11:07:54 INFO - retry: Calling <bound method AndroidEmulatorTest._download_file of <__main__.AndroidEmulatorTest object at 0x2b1a810>> with args: ('http://tooltool.pvt.build.mozilla.org/build/sha512/3b2d18eb0194d82c70c5ee17487ccbac309f9b2e9839fe7ca4a27a9a06f6338bb24394476da78559685d99151fccc85fdde03297aa73ee2f7fb3183e11925c4d', '/builds/slave/talos-slave/cached/AVDs-x86-android-4.2_r1-build-2013-11-13-ubuntu.tar.gz'), kwargs: {}, attempt #1 11:07:59 INFO - Downloaded 561274118 bytes. 11:07:59 INFO - Running post-action listener: _resource_record_post_action 11:07:59 INFO - ##### I see this command: 11:10:12 INFO - Running command: ['tar', 'zxfv', '/builds/slave/talos-slave/cached/AVDs-x86-android-4.2_r1-build-2013-11-13-ubuntu.tar.gz', '-C', '/home/cltbld/.android']

Comment on attachment 832387 [details] [diff] [review] remove avd deployed through puppet checked-in and live in production: https://hg.mozilla.org/build/puppet/rev/1927bae55726 https://hg.mozilla.org/build/puppet/rev/24ba0c844fb1

We can land the last patch next week once we hope all Linux 64-bit have removed the tar ball.

Armen: can you or did you already also deploy the armv7a AVDs to tooltool? http://people.mozilla.org/~graydon/AVDs-armv7a-android-2.3.7_r1-build-2013-11-13-ubuntu.tar.gz http://people.mozilla.org/~graydon/AVDs-armv7a-android-4.2_r1-build-2013-11-13-ubuntu.tar.gz I'm not sure what to put in config/androidarm.py

Please have a look at this patch to see how to adjust androidarm.py: http://hg.mozilla.org/build/mozharness/diff/1508818fa61b/configs/android/androidx86.py I dislike this sha512 setup but that's what we have. In reality, this setup should come with some in-tree manifests like this: http://hg.mozilla.org/mozilla-central/file/fa0e6916f88c/b2g/config/hamachi/releng-hamachi.tt However, I decided not to do the real tooltool design but something half-way there. In other words, the file did not need to be stored as a sha512 in the webhost but I'm following that approach just in case we decide one day to move completely to the tooltool approach. You probably will want specifically this in your patch: "avd_tar_ball": ("AVDs-armv7a-android-2.3.7_r1-build-2013-11-13-ubuntu.tar.gz ", "ac8a99fe0d120520de7e165899140cad074479fbaf3c4fcadb0d4ccb873bda0d249f88132df0828b86844d6df02aabce911316ace32bba7e94cf8dffdec77669"), [armenzg@relengwebadm.private.scl3 ~]$ FILE=AVDs-armv7a-android-2.3.7_r1-build-2013-11-13-ubuntu.tar.gz [armenzg@relengwebadm.private.scl3 ~]$ export SHA512=`openssl sha512 $FILE | cut -d' ' -f2` [armenzg@relengwebadm.private.scl3 ~]$ sudo mv -i $FILE /mnt/netapp/relengweb/tooltool/pvt/build/sha512/${SHA512} [armenzg@relengwebadm.private.scl3 ~]$ sudo chmod 644 /mnt/netapp/relengweb/tooltool/pvt/build/sha512/${SHA512} [armenzg@relengwebadm.private.scl3 ~]$ ls -l /mnt/netapp/relengweb/tooltool/pvt/build/sha512/${SHA512} -rw-r--r-- 1 armenzg armenzg 219296692 Dec 4 07:05 /mnt/netapp/relengweb/tooltool/pvt/build/sha512/ac8a99fe0d120520de7e165899140cad074479fbaf3c4fcadb0d4ccb873bda0d249f88132df0828b86844d6df02aabce911316ace32bba7e94cf8dffdec77669 [armenzg@relengwebadm.private.scl3 ~]$ FILE=AVDs-armv7a-android-4.2_r1-build-2013-11-13-ubuntu.tar.gz [armenzg@relengwebadm.private.scl3 ~]$ export SHA512=`openssl sha512 $FILE | cut -d' ' -f2` [armenzg@relengwebadm.private.scl3 ~]$ sudo mv -i $FILE /mnt/netapp/relengweb/tooltool/pvt/build/sha512/${SHA512} [armenzg@relengwebadm.private.scl3 ~]$ sudo chmod 644 /mnt/netapp/relengweb/tooltool/pvt/build/sha512/${SHA512} [armenzg@relengwebadm.private.scl3 ~]$ ls -l /mnt/netapp/relengweb/tooltool/pvt/build/sha512/${SHA512} -rw-r--r-- 1 armenzg armenzg 489044600 Dec 4 07:05 /mnt/netapp/relengweb/tooltool/pvt/build/sha512/96f3d3de0ab63ff87e6e789f026c13f5914d340d64db2ee9d77ebeef789e82bcfd7b15705f0c53d25454dfcfcec96c5848e2565bfd39b4e3c9fac848442f9adb

Comment on attachment 832390 [details] [diff] [review] follow up patch to remove avd logic from puppet http://hg.mozilla.org/build/puppet/rev/a9afc8b5eb72 We don't have anymore references of avds on puppet.