Closed
Bug 1289249
Opened 7 years ago
Closed 6 years ago
Overhaul desktop-build image/task interaction with hg.mozilla.org
Categories
(Firefox Build System :: Task Configuration, task)
Firefox Build System
Task Configuration
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla52
People
(Reporter: gps, Assigned: gps)
References
(Blocks 1 open bug)
Details
Attachments
(13 files, 10 obsolete files)
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
gps
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
dustin
:
review+
|
Details |
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.
Assignee | ||
Comment 1•7 years ago
|
||
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/
Attachment #8774585 -
Flags: review?(dustin)
Attachment #8774586 -
Flags: review?(dustin)
Attachment #8774587 -
Flags: review?(dustin)
Attachment #8774588 -
Flags: review?(dustin)
Attachment #8774589 -
Flags: review?(dustin)
Attachment #8774590 -
Flags: review?(dustin)
Assignee | ||
Comment 2•7 years ago
|
||
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/
Assignee | ||
Comment 3•7 years ago
|
||
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/
Assignee | ||
Comment 4•7 years ago
|
||
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/
Assignee | ||
Comment 5•7 years ago
|
||
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/
Assignee | ||
Comment 6•7 years ago
|
||
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•7 years ago
|
||
Comment on attachment 8774585 [details] Bug 1289249 - Remove REGISTRY and VERSION files from android-gradle-build; https://reviewboard.mozilla.org/r/67038/#review64540
Attachment #8774585 -
Flags: review?(dustin) → review+
Updated•7 years ago
|
Attachment #8774586 -
Flags: review?(dustin) → review+
Comment 8•7 years ago
|
||
Comment on attachment 8774586 [details] Bug 1289249 - Unify desktop-build-upd versions; https://reviewboard.mozilla.org/r/67040/#review64542
Comment 9•7 years ago
|
||
Comment on attachment 8774587 [details] Bug 1289249 - Remove centos6-build-upd image; https://reviewboard.mozilla.org/r/67042/#review64544
Attachment #8774587 -
Flags: review?(dustin) → review+
Updated•7 years ago
|
Attachment #8774588 -
Flags: review?(dustin) → review-
Comment 10•7 years ago
|
||
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•7 years ago
|
||
Comment on attachment 8774589 [details] Bug 1289249 - Inline hgrc into system-setup.sh; https://reviewboard.mozilla.org/r/67046/#review64548
Attachment #8774589 -
Flags: review?(dustin) → review+
Comment 12•7 years ago
|
||
Comment on attachment 8774590 [details] Bug 1289249 - Remove centos6-build image; https://reviewboard.mozilla.org/r/67048/#review64550
Attachment #8774590 -
Flags: review?(dustin) → review+
Assignee | ||
Comment 13•7 years ago
|
||
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 :/
Assignee | ||
Comment 14•7 years ago
|
||
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.
Updated•7 years ago
|
Blocks: thunder-try
Assignee | ||
Comment 15•6 years ago
|
||
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.
No longer depends on: 1289812
Assignee | ||
Updated•6 years ago
|
Attachment #8774585 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8774586 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8774587 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8774588 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8774589 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8774590 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 27•6 years ago
|
||
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 hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 37•6 years ago
|
||
mozreview-review |
Comment on attachment 8796010 [details] Bug 1289249 - Use vendored tooltool in desktop-build image; https://reviewboard.mozilla.org/r/81996/#review80716
Attachment #8796010 -
Flags: review?(dustin) → review+
Comment 38•6 years ago
|
||
mozreview-review |
Comment on attachment 8796011 [details] Bug 1289249 - Extract tooltool_fetch to its own shell script; https://reviewboard.mozilla.org/r/81998/#review80718
Attachment #8796011 -
Flags: review?(dustin) → review+
Comment 39•6 years ago
|
||
mozreview-review |
Comment on attachment 8796012 [details] Bug 1289249 - Install Mercurial 3.9.1 in desktop-build; https://reviewboard.mozilla.org/r/82000/#review80720
Attachment #8796012 -
Flags: review?(dustin) → review+
Comment 40•6 years ago
|
||
mozreview-review |
Comment on attachment 8796013 [details] Bug 1289249 - Add volumes for Mercurial checkouts; https://reviewboard.mozilla.org/r/82002/#review80722
Attachment #8796013 -
Flags: review?(dustin) → review+
Comment 41•6 years ago
|
||
mozreview-review |
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.
Attachment #8796014 -
Flags: review?(dustin)
Comment 42•6 years ago
|
||
mozreview-review |
Comment on attachment 8796015 [details] Bug 1289249 - Add run-task to desktop-build image; https://reviewboard.mozilla.org/r/82006/#review80728
Attachment #8796015 -
Flags: review?(dustin) → review+
Comment 43•6 years ago
|
||
mozreview-review |
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 :(
Attachment #8796016 -
Flags: review?(dustin) → review+
Comment 44•6 years ago
|
||
mozreview-review |
Comment on attachment 8796017 [details] Bug 1289249 - Use run-task in spidermonkey tasks; https://reviewboard.mozilla.org/r/82010/#review80734
Attachment #8796017 -
Flags: review?(dustin) → review+
Assignee | ||
Comment 45•6 years ago
|
||
CC sfink as a courtesy since I'm touching spidermonkey automation.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8796014 -
Attachment is obsolete: true
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 85•6 years ago
|
||
Comment on attachment 8796017 [details] Bug 1289249 - Use run-task in spidermonkey tasks; Need a new review since this changed a bit.
Attachment #8796017 -
Flags: review+ → review?(dustin)
Assignee | ||
Comment 86•6 years ago
|
||
Comment on attachment 8796012 [details] Bug 1289249 - Install Mercurial 3.9.1 in desktop-build; Need a new review since this changed.
Attachment #8796012 -
Flags: review+ → review?(dustin)
Assignee | ||
Comment 87•6 years ago
|
||
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•6 years ago
|
||
mozreview-review |
Comment on attachment 8796012 [details] Bug 1289249 - Install Mercurial 3.9.1 in desktop-build; https://reviewboard.mozilla.org/r/82000/#review80970
Attachment #8796012 -
Flags: review?(dustin) → review+
Comment 89•6 years ago
|
||
mozreview-review |
Comment on attachment 8796372 [details] Bug 1289249 - Set permissions of parent directory during --chown-recursive; https://reviewboard.mozilla.org/r/82252/#review80972
Attachment #8796372 -
Flags: review?(dustin) → review+
Comment 90•6 years ago
|
||
mozreview-review |
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)
Attachment #8796373 -
Flags: review?(dustin) → review+
Comment 91•6 years ago
|
||
mozreview-review |
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!
Attachment #8796374 -
Flags: review?(dustin) → review+
Comment 92•6 years ago
|
||
mozreview-review |
Comment on attachment 8796375 [details] Bug 1289249 - Ensure cwd is /home/worker; https://reviewboard.mozilla.org/r/82258/#review80982
Attachment #8796375 -
Flags: review?(dustin) → review+
Comment 93•6 years ago
|
||
mozreview-review |
Comment on attachment 8796376 [details] Bug 1289249 - Use vendored tooltool.py in SpiderMonkey tasks; https://reviewboard.mozilla.org/r/82260/#review80984
Attachment #8796376 -
Flags: review?(dustin) → review+
Comment 94•6 years ago
|
||
mozreview-review |
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 :)
Attachment #8796018 -
Flags: review?(dustin) → review+
Comment 95•6 years ago
|
||
mozreview-review |
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 hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8796019 -
Attachment is obsolete: true
Assignee | ||
Updated•6 years ago
|
Attachment #8796020 -
Attachment is obsolete: true
Comment 109•6 years ago
|
||
Pushed by gszorc@mozilla.com: 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
Assignee | ||
Comment 110•6 years ago
|
||
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 hidden (mozreview-request) |
Comment 112•6 years ago
|
||
Pushed by gszorc@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ac89a649cbb9 Use os.lchown; r=garndt
Assignee | ||
Updated•6 years ago
|
Attachment #8796715 -
Attachment is obsolete: true
Attachment #8796715 -
Flags: review?(garndt)
Assignee | ||
Updated•6 years ago
|
Attachment #8796017 -
Flags: review?(dustin) → review+
Comment 113•6 years ago
|
||
bugherder |
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
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Updated•5 years ago
|
Product: TaskCluster → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•