1289249 - Overhaul desktop-build image/task interaction with hg.mozilla.org

Status: RESOLVED FIXED

Description

[Gregory Szorc [:gps]]

This is follow-up work from bug 1247168. We want the desktop-build image/task to use robustcheckout, stop using tc-vcs, using caches appropriately, etc.

Comment 1

[Gregory Szorc [:gps]]

Created attachment 8774585 [details]
Bug 1289249 - Remove REGISTRY and VERSION files from android-gradle-build;

AFAICT these files were cargo culted: we don't need these files because
the Docker images is built as part of the task graph.

Review commit: https://reviewboard.mozilla.org/r/67038/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67038/

Comment 2

[Gregory Szorc [:gps]]

Created attachment 8774586 [details]
Bug 1289249 - Unify desktop-build-upd versions;

After this commit, all consumers of the desktop-build-upd image are
using the same version.

Review commit: https://reviewboard.mozilla.org/r/67040/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67040/

Comment 3

[Gregory Szorc [:gps]]

Created attachment 8774587 [details]
Bug 1289249 - Remove centos6-build-upd image;

This image was introduced in bug 1203158. The intent of the image
was to minimize layer sizes so people with slower internet connections
would only have to download a minimal layer update whenever package
versions were bumped.

Fast forward a year and I don't think we're actually using images this
way. We don't really have people downloading images in bulk to run
them locally. Instead, I think people are more likely to obtain a
one-click loaner to obtain an instance running the image. I view the
centos6-build-upd image as avoidable complexity standing in the way
of future refactoring of various build images. So, this commit
eliminates it.

Review commit: https://reviewboard.mozilla.org/r/67042/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67042/

Comment 4

[Gregory Szorc [:gps]]

Created attachment 8774588 [details]
Bug 1289249 - Move environment variable declaration to build.yml;

In preparation for eliminating Docker image inheritance for the
desktop-build image, we move declaring of environment variables
from the base image's Dockerfile to build.yml, which is inherited at
run-time for every task using the desktop-build image.

Review commit: https://reviewboard.mozilla.org/r/67044/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67044/

Comment 5

[Gregory Szorc [:gps]]

Created attachment 8774589 [details]
Bug 1289249 - Inline hgrc into system-setup.sh;

Continuing to minimize centos6-build's Dockerfile so we can eliminate
the image.

Review commit: https://reviewboard.mozilla.org/r/67046/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67046/

Comment 6

[Gregory Szorc [:gps]]

Created attachment 8774590 [details]
Bug 1289249 - Remove centos6-build image;

The system-setup.sh script has been moved to the "desktop-build.sh"
recipe and included in each of the Dockerfiles that referenced it.
The remaining lines from centos6-build's Dockerfile have
essentially been inlined into the Dockerfiles that previously inherited
from centos6-build.

As of this change, we no longer require someone with TaskCluster
privileges to upload a base image to the Docker Registry: it is
possible to make changes to the environment used to build Firefox
in-tree. You can easily experiment with new build environments by
pushing to Try.

Review commit: https://reviewboard.mozilla.org/r/67048/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67048/

Comment 7

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8774585 [details]
Bug 1289249 - Remove REGISTRY and VERSION files from android-gradle-build;

https://reviewboard.mozilla.org/r/67038/#review64540

Comment 8

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8774586 [details]
Bug 1289249 - Unify desktop-build-upd versions;

https://reviewboard.mozilla.org/r/67040/#review64542

Comment 9

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8774587 [details]
Bug 1289249 - Remove centos6-build-upd image;

https://reviewboard.mozilla.org/r/67042/#review64544

Comment 10

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8774588 [details]
Bug 1289249 - Move environment variable declaration to build.yml;

https://reviewboard.mozilla.org/r/67044/#review64538

::: taskcluster/ci/legacy/tasks/build.yml:42
(Diff revision 1)
> +      # process. These are taken from the GNU su manual.
> +      LOGNAME: 'worker'
> +      HOME: '/home/worker'
> +      HOSTNAME: 'taskcluster-worker'
> +      SHELL: '/bin/bash'
> +      USER: 'worker'

It feels very wrong to have this in the task definition.  The effect is correct, but this is essentially machine configuration.

I would feel more comfortable duplicating the removed lines in all of the Dockerfiles, even at he cost of not being very DRY.

Could we set these variables in the shell scripts run within these scripts?  Possibly by sourcing some #%include'd script?

Comment 11

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8774589 [details]
Bug 1289249 - Inline hgrc into system-setup.sh;

https://reviewboard.mozilla.org/r/67046/#review64548

Comment 12

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8774590 [details]
Bug 1289249 - Remove centos6-build image;

https://reviewboard.mozilla.org/r/67048/#review64550

Comment 13

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/67048/#review64666

Thanks for the reviews. Unfortunately, I'm going to hold off landing this series until we have a better understanding of image determinism. As it stands, pushing this series will likely break Valgrind due to newer system packages being pulled in :/

Comment 14

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/67044/#review64538

> It feels very wrong to have this in the task definition.  The effect is correct, but this is essentially machine configuration.
> 
> I would feel more comfortable duplicating the removed lines in all of the Dockerfiles, even at he cost of not being very DRY.
> 
> Could we set these variables in the shell scripts run within these scripts?  Possibly by sourcing some #%include'd script?

Yes, this patch made me feel a bit dirty as well.

I think sourcing from a #%include'd script is a good solution.

Comment 15

[Gregory Szorc [:gps]]

I can do this without needing the base image to be updated. It will result in some code duplication. But that's far easier than consolidating images.

Comment 16

[Gregory Szorc [:gps]]

Created attachment 8796010 [details]
Bug 1289249 - Use vendored tooltool in desktop-build image;

Vendoring: more reliable, more determinism, more secure.

Review commit: https://reviewboard.mozilla.org/r/81996/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81996/

Comment 17

[Gregory Szorc [:gps]]

Created attachment 8796011 [details]
Bug 1289249 - Extract tooltool_fetch to its own shell script;

I will add more stuff to common.sh in the future in order to justify
its existence. Not in this bug though.

Review commit: https://reviewboard.mozilla.org/r/81998/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/81998/

Comment 18

[Gregory Szorc [:gps]]

Created attachment 8796012 [details]
Bug 1289249 - Install Mercurial 3.9.1 in desktop-build;

As part of this, we had to teach install-mercurial.sh to detect
CentOS and download and install the proper RPM. We also added a
"system setup" shell script. This matches the conventions used
for the desktop-test images.

The RPM uploaded to tooltool is the same RPM we use on
reviewboard-hg.mozilla.org, which also runs CentOS 6. It should
"just work."

Review commit: https://reviewboard.mozilla.org/r/82000/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82000/

Comment 19

[Gregory Szorc [:gps]]

Created attachment 8796013 [details]
Bug 1289249 - Add volumes for Mercurial checkouts;

In preparation for switching desktop-build to use run-task and
its VCS management.

Review commit: https://reviewboard.mozilla.org/r/82002/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82002/

Comment 20

[Gregory Szorc [:gps]]

Created attachment 8796014 [details]
Bug 1289249 - Hardcode worker uid and gid;

When I was testing the desktop-build image's conversion to use
run-task, I ran into permissions problems on Try accessing files
in the hg-shared cache. While multiple tasks were running as
worker:worker, it appeared that Ubuntu containers were running
as uid 1000 and CentOS containers were running as uid 500. Since
file permissions are based on UID and not username, this was
causing permissions failures.

We /could/ change run-task to chown the hg-shared cache. But
iterating a directory with 100,000+ files takes precious time.
Let's normalize on a consistent uid/gid value instead.

Review commit: https://reviewboard.mozilla.org/r/82004/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82004/

Comment 21

[Gregory Szorc [:gps]]

Created attachment 8796015 [details]
Bug 1289249 - Add run-task to desktop-build image;

run-task is our new universal wrapper for executing tasks. Add it
to desktop-build.

Review commit: https://reviewboard.mozilla.org/r/82006/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82006/

Comment 22

[Gregory Szorc [:gps]]

Created attachment 8796016 [details]
Bug 1289249 - Make build-linux.sh executable;

Currently, it is executed via another shell script (build.sh from
the desktop-build image). build.sh is going away and this will be
the main script called by run-task. It already has a shebang. So
make it executable.

Review commit: https://reviewboard.mozilla.org/r/82008/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82008/

Comment 23

[Gregory Szorc [:gps]]

Created attachment 8796017 [details]
Bug 1289249 - Use run-task in spidermonkey tasks;

This is a pretty straightforward port.

Review commit: https://reviewboard.mozilla.org/r/82010/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82010/

Comment 24

[Gregory Szorc [:gps]]

Created attachment 8796018 [details]
Bug 1289249 - Use run-task for mozharness jobs on docker-worker;

Now that we have all the files in place, we switch to using
run-task for all tasks executing on desktop-build.

For now, we preserve the existing filesystem path of the checkout
because modifying it is a non-trivial amount of work.

Review commit: https://reviewboard.mozilla.org/r/82012/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82012/

Comment 25

[Gregory Szorc [:gps]]

Created attachment 8796019 [details]
Bug 1289249 - Use run-task in marionette-harness task

desktop-build just gained run-task. build.sh will soon be deleted
from it. Port marionette-harness to run-task so we can delete
build.sh and friends from desktop-build.

Review commit: https://reviewboard.mozilla.org/r/82014/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82014/

Comment 26

[Gregory Szorc [:gps]]

Created attachment 8796020 [details]
Bug 1289249 - Remove legacy scripts for running tasks in desktop-build

run-task replaces build.sh. So build.sh has been removed.

checkout-sources.sh was only used by build.sh and since build.sh is
no more, checkout-sources.sh has been removed.

checkout-script.sh was added in bug 1179893 as a way to easily
run ad-hoc tasks from random files in a repository. Now that we have
run-task and it can get a gecko checkout efficiently, I think
run-task now satisfies this use case sufficiently. So let's kill
checkout-script.sh and recommend people use run-task. If people
really want the functionality to obtain a single file without
a full checkout, we can re-add this script or add the functionality
to run-task.

We also remove old VCS foo from the TC transform.

Review commit: https://reviewboard.mozilla.org/r/82016/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82016/

Comment 27

[Gregory Szorc [:gps]]

I'm still ironing out the conversion of the tasks to run-task on Try. I should have a complete series ready for review by mid-day Thursday.

I may have to make some additional changes to accommodate uid normalization. So if you want to hold off on review, I don't blame you.

Comment 28

[Gregory Szorc [:gps]]

Comment on attachment 8796012 [details]
Bug 1289249 - Install Mercurial 3.9.1 in desktop-build;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82000/diff/1-2/

Comment 29

[Gregory Szorc [:gps]]

Comment on attachment 8796013 [details]
Bug 1289249 - Add volumes for Mercurial checkouts;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82002/diff/1-2/

Comment 30

[Gregory Szorc [:gps]]

Comment on attachment 8796014 [details]
Bug 1289249 - Hardcode worker uid and gid;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82004/diff/1-2/

Comment 31

[Gregory Szorc [:gps]]

Comment on attachment 8796015 [details]
Bug 1289249 - Add run-task to desktop-build image;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82006/diff/1-2/

Comment 32

[Gregory Szorc [:gps]]

Comment on attachment 8796016 [details]
Bug 1289249 - Make build-linux.sh executable;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82008/diff/1-2/

Comment 33

[Gregory Szorc [:gps]]

Comment on attachment 8796017 [details]
Bug 1289249 - Use run-task in spidermonkey tasks;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82010/diff/1-2/

Comment 34

[Gregory Szorc [:gps]]

Comment on attachment 8796018 [details]
Bug 1289249 - Use run-task for mozharness jobs on docker-worker;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82012/diff/1-2/

Comment 35

[Gregory Szorc [:gps]]

Comment on attachment 8796019 [details]
Bug 1289249 - Use run-task in marionette-harness task

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82014/diff/1-2/

Comment 36

[Gregory Szorc [:gps]]

Comment on attachment 8796020 [details]
Bug 1289249 - Remove legacy scripts for running tasks in desktop-build

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82016/diff/1-2/

Comment 37

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796010 [details]
Bug 1289249 - Use vendored tooltool in desktop-build image;

https://reviewboard.mozilla.org/r/81996/#review80716

Comment 38

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796011 [details]
Bug 1289249 - Extract tooltool_fetch to its own shell script;

https://reviewboard.mozilla.org/r/81998/#review80718

Comment 39

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796012 [details]
Bug 1289249 - Install Mercurial 3.9.1 in desktop-build;

https://reviewboard.mozilla.org/r/82000/#review80720

Comment 40

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796013 [details]
Bug 1289249 - Add volumes for Mercurial checkouts;

https://reviewboard.mozilla.org/r/82002/#review80722

Comment 41

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796014 [details]
Bug 1289249 - Hardcode worker uid and gid;

https://reviewboard.mozilla.org/r/82004/#review80726

This is going to introduce a period of instability where we have some new running as uid 1000 and old builds running as uid 500.  If those are sharing caches, then for as long as we have the old jobs hanging around, we're going to have conflicts.

I wonder if we couldn't reduce this by changing to uid 500 instead?  I don't know why there are Ubuntu tasks running on the same workerTypes as CentOS tasks, but there certainly should be *fewer* such Ubuntu builds.

Also, what think ye about having run-tasks optionally chown things, but only if the top-level directory has the wrong uid?  In other words, assume that if /home/worker/hg-shared has the right owner, then there's no need to examine its contents.

Comment 42

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796015 [details]
Bug 1289249 - Add run-task to desktop-build image;

https://reviewboard.mozilla.org/r/82006/#review80728

Comment 43

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796016 [details]
Bug 1289249 - Make build-linux.sh executable;

https://reviewboard.mozilla.org/r/82008/#review80730

r+ assuming this is just a chmod.  it's making mozreview barf :(

Comment 44

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796017 [details]
Bug 1289249 - Use run-task in spidermonkey tasks;

https://reviewboard.mozilla.org/r/82010/#review80734

Comment 45

[Gregory Szorc [:gps]]

CC sfink as a courtesy since I'm touching spidermonkey automation.

Comment 46

[Gregory Szorc [:gps]]

Created attachment 8796372 [details]
Bug 1289249 - Set permissions of parent directory during --chown-recursive;

os.walk() won't explicitly yield the root directory. So we need to
update it explicitly when doing a recursive chown.

Review commit: https://reviewboard.mozilla.org/r/82252/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82252/

Comment 47

[Gregory Szorc [:gps]]

Created attachment 8796373 [details]
Bug 1289249 - Make version control interaction generic;

Previously, we assumed we only could have a single version control
checkout: Gecko/Firefox. The code reflected this by not passing
arguments to the vcs_checkout function.

Upcoming commits will introduce the need to perform a checkout of the
build/tools repository. In preparation for this, refactor the VCS
functionality so it is generic and can work on any repo.

Review commit: https://reviewboard.mozilla.org/r/82254/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82254/

Comment 48

[Gregory Szorc [:gps]]

Created attachment 8796374 [details]
Bug 1289249 - Add argument to perform build/tools checkout;

Build tasks currently require a checkout of the build/tools repository.
I wish this weren't true and that all files references from this repo
were part of mozilla-central or tooltool, but that's how things are.

In preparation for running build tasks with run-task, teach run-task
to perform a checkout of the build/tools repo. Ideally we'd support
configuring the URL to this repository. But I'm not implementing that
since I'd prefer we stop relying on the build/tools repo.

Review commit: https://reviewboard.mozilla.org/r/82256/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82256/

Comment 49

[Gregory Szorc [:gps]]

Created attachment 8796375 [details]
Bug 1289249 - Ensure cwd is /home/worker;

When we switch to run-task in a subsequent commit, we can no longer
rely on cwd being /home/worker. So have build-linux.sh change the cwd
explicitly.

Review commit: https://reviewboard.mozilla.org/r/82258/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82258/

Comment 50

[Gregory Szorc [:gps]]

Created attachment 8796376 [details]
Bug 1289249 - Use vendored tooltool.py in SpiderMonkey tasks;

Previously, SpiderMonkey tasks were using tc-vcs to clone the tooltool
repository so they could have access to tooltool.py. tooltool.py is
already in the execution environment courtesy of being part of the
source checkout. So use it from there.

One less dependency on a 3rd party service at run-time. One less
point for a MITM attack against our automation.

Review commit: https://reviewboard.mozilla.org/r/82260/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82260/

Comment 51

[Gregory Szorc [:gps]]

Comment on attachment 8796010 [details]
Bug 1289249 - Use vendored tooltool in desktop-build image;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81996/diff/1-2/

Comment 52

[Gregory Szorc [:gps]]

Comment on attachment 8796011 [details]
Bug 1289249 - Extract tooltool_fetch to its own shell script;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81998/diff/1-2/

Comment 53

[Gregory Szorc [:gps]]

Comment on attachment 8796012 [details]
Bug 1289249 - Install Mercurial 3.9.1 in desktop-build;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82000/diff/2-3/

Comment 54

[Gregory Szorc [:gps]]

Comment on attachment 8796013 [details]
Bug 1289249 - Add volumes for Mercurial checkouts;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82002/diff/2-3/

Comment 55

[Gregory Szorc [:gps]]

Comment on attachment 8796015 [details]
Bug 1289249 - Add run-task to desktop-build image;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82006/diff/2-3/

Comment 56

[Gregory Szorc [:gps]]

Comment on attachment 8796016 [details]
Bug 1289249 - Make build-linux.sh executable;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82008/diff/2-3/

Comment 57

[Gregory Szorc [:gps]]

Comment on attachment 8796017 [details]
Bug 1289249 - Use run-task in spidermonkey tasks;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82010/diff/2-3/

Comment 58

[Gregory Szorc [:gps]]

Comment on attachment 8796018 [details]
Bug 1289249 - Use run-task for mozharness jobs on docker-worker;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82012/diff/2-3/

Comment 59

[Gregory Szorc [:gps]]

Comment on attachment 8796019 [details]
Bug 1289249 - Use run-task in marionette-harness task

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82014/diff/2-3/

Comment 60

[Gregory Szorc [:gps]]

Comment on attachment 8796020 [details]
Bug 1289249 - Remove legacy scripts for running tasks in desktop-build

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82016/diff/2-3/

Comment 61

[Gregory Szorc [:gps]]

Comment on attachment 8796376 [details]
Bug 1289249 - Use vendored tooltool.py in SpiderMonkey tasks;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82260/diff/1-2/

Comment 62

[Gregory Szorc [:gps]]

Comment on attachment 8796013 [details]
Bug 1289249 - Add volumes for Mercurial checkouts;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82002/diff/3-4/

Comment 63

[Gregory Szorc [:gps]]

Comment on attachment 8796015 [details]
Bug 1289249 - Add run-task to desktop-build image;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82006/diff/3-4/

Comment 64

[Gregory Szorc [:gps]]

Comment on attachment 8796016 [details]
Bug 1289249 - Make build-linux.sh executable;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82008/diff/3-4/

Comment 65

[Gregory Szorc [:gps]]

Comment on attachment 8796017 [details]
Bug 1289249 - Use run-task in spidermonkey tasks;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82010/diff/3-4/

Comment 66

[Gregory Szorc [:gps]]

Comment on attachment 8796018 [details]
Bug 1289249 - Use run-task for mozharness jobs on docker-worker;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82012/diff/3-4/

Comment 67

[Gregory Szorc [:gps]]

Comment on attachment 8796019 [details]
Bug 1289249 - Use run-task in marionette-harness task

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82014/diff/3-4/

Comment 68

[Gregory Szorc [:gps]]

Comment on attachment 8796020 [details]
Bug 1289249 - Remove legacy scripts for running tasks in desktop-build

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82016/diff/3-4/

Comment 69

[Gregory Szorc [:gps]]

Comment on attachment 8796018 [details]
Bug 1289249 - Use run-task for mozharness jobs on docker-worker;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82012/diff/4-5/

Comment 70

[Gregory Szorc [:gps]]

Comment on attachment 8796019 [details]
Bug 1289249 - Use run-task in marionette-harness task

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82014/diff/4-5/

Comment 71

[Gregory Szorc [:gps]]

Comment on attachment 8796020 [details]
Bug 1289249 - Remove legacy scripts for running tasks in desktop-build

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82016/diff/4-5/

Comment 72

[Gregory Szorc [:gps]]

Comment on attachment 8796374 [details]
Bug 1289249 - Add argument to perform build/tools checkout;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82256/diff/1-2/

Comment 73

[Gregory Szorc [:gps]]

Comment on attachment 8796375 [details]
Bug 1289249 - Ensure cwd is /home/worker;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82258/diff/1-2/

Comment 74

[Gregory Szorc [:gps]]

Comment on attachment 8796010 [details]
Bug 1289249 - Use vendored tooltool in desktop-build image;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81996/diff/2-3/

Comment 75

[Gregory Szorc [:gps]]

Comment on attachment 8796011 [details]
Bug 1289249 - Extract tooltool_fetch to its own shell script;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81998/diff/2-3/

Comment 76

[Gregory Szorc [:gps]]

Comment on attachment 8796012 [details]
Bug 1289249 - Install Mercurial 3.9.1 in desktop-build;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82000/diff/3-4/

Comment 77

[Gregory Szorc [:gps]]

Comment on attachment 8796376 [details]
Bug 1289249 - Use vendored tooltool.py in SpiderMonkey tasks;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82260/diff/2-3/

Comment 78

[Gregory Szorc [:gps]]

Comment on attachment 8796013 [details]
Bug 1289249 - Add volumes for Mercurial checkouts;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82002/diff/4-5/

Comment 79

[Gregory Szorc [:gps]]

Comment on attachment 8796015 [details]
Bug 1289249 - Add run-task to desktop-build image;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82006/diff/4-5/

Comment 80

[Gregory Szorc [:gps]]

Comment on attachment 8796016 [details]
Bug 1289249 - Make build-linux.sh executable;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82008/diff/4-5/

Comment 81

[Gregory Szorc [:gps]]

Comment on attachment 8796017 [details]
Bug 1289249 - Use run-task in spidermonkey tasks;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82010/diff/4-5/

Comment 82

[Gregory Szorc [:gps]]

Comment on attachment 8796018 [details]
Bug 1289249 - Use run-task for mozharness jobs on docker-worker;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82012/diff/5-6/

Comment 83

[Gregory Szorc [:gps]]

Comment on attachment 8796019 [details]
Bug 1289249 - Use run-task in marionette-harness task

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82014/diff/5-6/

Comment 84

[Gregory Szorc [:gps]]

Comment on attachment 8796020 [details]
Bug 1289249 - Remove legacy scripts for running tasks in desktop-build

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82016/diff/5-6/

Comment 85

[Gregory Szorc [:gps]]

Comment on attachment 8796017 [details]
Bug 1289249 - Use run-task in spidermonkey tasks;

Need a new review since this changed a bit.

Comment 86

[Gregory Szorc [:gps]]

Comment on attachment 8796012 [details]
Bug 1289249 - Install Mercurial 3.9.1 in desktop-build;

Need a new review since this changed.

Comment 87

[Gregory Szorc [:gps]]

I'm pretty confident in this series up to the last 2 commits. A try push is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=d58fb76ff1b832b8fa6e5db236d5702c5085dca6.

If I get r+ on everything, I'll just discard the top 2 commits for a follow-up bug. There are some other random build-related tasks not using run-task, so a follow-up is needed anyway. I would also like to refactor where checkouts are stored on disk, since right now we need to perform a full checkout on every task. Again, it should be deferred to a follow-up.

Comment 88

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796012 [details]
Bug 1289249 - Install Mercurial 3.9.1 in desktop-build;

https://reviewboard.mozilla.org/r/82000/#review80970

Comment 89

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796372 [details]
Bug 1289249 - Set permissions of parent directory during --chown-recursive;

https://reviewboard.mozilla.org/r/82252/#review80972

Comment 90

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796373 [details]
Bug 1289249 - Make version control interaction generic;

https://reviewboard.mozilla.org/r/82254/#review80976

::: testing/docker/recipes/run-task:226
(Diff revision 1)
>                  set_dir_permissions(os.path.join(root, d), uid, gid)
>  
>              for f in files:
>                  os.chown(os.path.join(root, f), uid, gid)
>  
> -    checkout = args.vcs_checkout
> +    def prepare_checkout_dir(checkout):

why is this a nested function (not that I'm opposed .. just seems superfluous here)

Comment 91

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796374 [details]
Bug 1289249 - Add argument to perform build/tools checkout;

https://reviewboard.mozilla.org/r/82256/#review80980

::: testing/docker/recipes/run-task:244
(Diff revision 2)
>          # And that it is owned by the appropriate user/group.
>          os.chown('/home/worker/hg-shared', uid, gid)
>          os.chown(os.path.dirname(checkout), uid, gid)
>  
>      prepare_checkout_dir(args.vcs_checkout)
> +    prepare_checkout_dir(args.tools_checkout)

Ah, that's why!

::: testing/docker/recipes/run-task:284
(Diff revision 2)
>  
> +    if args.tools_checkout:
> +        vcs_checkout(b'https://hg.mozilla.org/build/tools',
> +                     args.tools_checkout,
> +                     # Always check out the latest commit on default branch.
> +                     # This is non-deterministic!

So much fun!

Comment 92

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796375 [details]
Bug 1289249 - Ensure cwd is /home/worker;

https://reviewboard.mozilla.org/r/82258/#review80982

Comment 93

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796376 [details]
Bug 1289249 - Use vendored tooltool.py in SpiderMonkey tasks;

https://reviewboard.mozilla.org/r/82260/#review80984

Comment 94

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796018 [details]
Bug 1289249 - Use run-task for mozharness jobs on docker-worker;

https://reviewboard.mozilla.org/r/82012/#review80988

::: taskcluster/taskgraph/transforms/job/mozharness.py:84
(Diff revision 6)
>      run = job['run']
>  
>      worker = taskdesc['worker']
>      worker['implementation'] = job['worker']['implementation']
>  
>      # running via mozharness assumes desktop-build (which contains build.sh)

You're correct about this assumption :)

Comment 95

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8796019 [details]
Bug 1289249 - Use run-task in marionette-harness task

https://reviewboard.mozilla.org/r/82014/#review80990

I'd really prefer this not use the desktop-build image, since it's a test.  This is a fine fix too, though.

Comment 96

[Gregory Szorc [:gps]]

Comment on attachment 8796372 [details]
Bug 1289249 - Set permissions of parent directory during --chown-recursive;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82252/diff/1-2/

Comment 97

[Gregory Szorc [:gps]]

Comment on attachment 8796373 [details]
Bug 1289249 - Make version control interaction generic;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82254/diff/1-2/

Comment 98

[Gregory Szorc [:gps]]

Comment on attachment 8796374 [details]
Bug 1289249 - Add argument to perform build/tools checkout;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82256/diff/2-3/

Comment 99

[Gregory Szorc [:gps]]

Comment on attachment 8796375 [details]
Bug 1289249 - Ensure cwd is /home/worker;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82258/diff/2-3/

Comment 100

[Gregory Szorc [:gps]]

Comment on attachment 8796010 [details]
Bug 1289249 - Use vendored tooltool in desktop-build image;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81996/diff/3-4/

Comment 101

[Gregory Szorc [:gps]]

Comment on attachment 8796011 [details]
Bug 1289249 - Extract tooltool_fetch to its own shell script;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/81998/diff/3-4/

Comment 102

[Gregory Szorc [:gps]]

Comment on attachment 8796012 [details]
Bug 1289249 - Install Mercurial 3.9.1 in desktop-build;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82000/diff/4-5/

Comment 103

[Gregory Szorc [:gps]]

Comment on attachment 8796376 [details]
Bug 1289249 - Use vendored tooltool.py in SpiderMonkey tasks;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82260/diff/3-4/

Comment 104

[Gregory Szorc [:gps]]

Comment on attachment 8796013 [details]
Bug 1289249 - Add volumes for Mercurial checkouts;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82002/diff/5-6/

Comment 105

[Gregory Szorc [:gps]]

Comment on attachment 8796015 [details]
Bug 1289249 - Add run-task to desktop-build image;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82006/diff/5-6/

Comment 106

[Gregory Szorc [:gps]]

Comment on attachment 8796016 [details]
Bug 1289249 - Make build-linux.sh executable;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82008/diff/5-6/

Comment 107

[Gregory Szorc [:gps]]

Comment on attachment 8796017 [details]
Bug 1289249 - Use run-task in spidermonkey tasks;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82010/diff/5-6/

Comment 108

[Gregory Szorc [:gps]]

Comment on attachment 8796018 [details]
Bug 1289249 - Use run-task for mozharness jobs on docker-worker;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82012/diff/6-7/

Comment 109

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6426086c4c0a
Set permissions of parent directory during --chown-recursive; r=dustin
https://hg.mozilla.org/integration/autoland/rev/db1b85faa8b0
Make version control interaction generic; r=dustin
https://hg.mozilla.org/integration/autoland/rev/ab876459cc14
Add argument to perform build/tools checkout; r=dustin
https://hg.mozilla.org/integration/autoland/rev/a0aaa5f8ec30
Ensure cwd is /home/worker; r=dustin
https://hg.mozilla.org/integration/autoland/rev/e774ca6c841a
Use vendored tooltool in desktop-build image; r=dustin
https://hg.mozilla.org/integration/autoland/rev/2a1ef50facaa
Extract tooltool_fetch to its own shell script; r=dustin
https://hg.mozilla.org/integration/autoland/rev/fb9bc2020790
Install Mercurial 3.9.1 in desktop-build; r=dustin
https://hg.mozilla.org/integration/autoland/rev/3a51bf317438
Use vendored tooltool.py in SpiderMonkey tasks; r=dustin
https://hg.mozilla.org/integration/autoland/rev/c6bd1f367375
Add volumes for Mercurial checkouts; r=dustin
https://hg.mozilla.org/integration/autoland/rev/c650a2f6f8fc
Add run-task to desktop-build image; r=dustin
https://hg.mozilla.org/integration/autoland/rev/149da08ff1c9
Make build-linux.sh executable; r=dustin
https://hg.mozilla.org/integration/autoland/rev/f1160fac6760
Use run-task in spidermonkey tasks; r=dustin
https://hg.mozilla.org/integration/autoland/rev/5df2521786e1
Use run-task for mozharness jobs on docker-worker; r=dustin

Comment 110

[Gregory Szorc [:gps]]

This caused "random" bustage on OS X and Android tasks in TC.

On OS X, we'd see things like:

[chown 2016-09-30T20:23:32.327781Z] recursively changing ownership of /home/worker/workspace to worker:worker
Traceback (most recent call last):
  File "/home/worker/bin/run-task", line 295, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/home/worker/bin/run-task", line 226, in main
    os.chown(os.path.join(root, f), uid, gid)
OSError: [Errno 2] No such file or directory: '/home/worker/workspace/build/src/MacOSX10.7.sdk/Library/Frameworks'

On Android build tasks we'd see something similar:

[setup 2016-09-30T20:23:20.831072Z] run-task started
[chown 2016-09-30T20:23:20.835950Z] recursively changing ownership of /home/worker/workspace to worker:worker
Traceback (most recent call last):
  File "/home/worker/bin/run-task", line 295, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/home/worker/bin/run-task", line 226, in main
    os.chown(os.path.join(root, f), uid, gid)
OSError: [Errno 2] No such file or directory: '/home/worker/workspace/build/src/java_home/jre/lib/audio/default.sf2'

The introduction of run-task introduced a recursive chown of various directories, including the workspace. It certainly looks like something else on the worker is removing the workspace while the task is iterating over it and chowning files. I suspect we uncovered a TaskCluster platform bug.

Comment 111

[Gregory Szorc [:gps]]

Created attachment 8796715 [details]
Bug 1289249 - Ignore ENOENT when chowning;

When we switched builds to run-task, we started receiving tons of
"random" failures on OS X and some Android build tasks where the
recursive chown was failing to chown() files due to ENOENT that
a directory list had previously said existed. The most likely
explanation for this is that something outside of the task is
deleting files in the cache while the task is running. Another
explanation might be a Docker/AUFS bug.

Until we figure out the root cause, ignore ENOENT errors when
performing the recursive chown. To help us debug the issue, we
log when such an error occurs. If we see warnings for hundreds
or thousands of files in the same directories, that's a good
sign something else is racing to delete the directory tree.

Review commit: https://reviewboard.mozilla.org/r/82464/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82464/

Comment 112

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac89a649cbb9
Use os.lchown; r=garndt

Comment 113

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6426086c4c0a
https://hg.mozilla.org/mozilla-central/rev/db1b85faa8b0
https://hg.mozilla.org/mozilla-central/rev/ab876459cc14
https://hg.mozilla.org/mozilla-central/rev/a0aaa5f8ec30
https://hg.mozilla.org/mozilla-central/rev/e774ca6c841a
https://hg.mozilla.org/mozilla-central/rev/2a1ef50facaa
https://hg.mozilla.org/mozilla-central/rev/fb9bc2020790
https://hg.mozilla.org/mozilla-central/rev/3a51bf317438
https://hg.mozilla.org/mozilla-central/rev/c6bd1f367375
https://hg.mozilla.org/mozilla-central/rev/c650a2f6f8fc
https://hg.mozilla.org/mozilla-central/rev/149da08ff1c9
https://hg.mozilla.org/mozilla-central/rev/f1160fac6760
https://hg.mozilla.org/mozilla-central/rev/5df2521786e1
https://hg.mozilla.org/mozilla-central/rev/ac89a649cbb9