Closed Bug 1290531 Opened 8 years ago Closed 8 years ago

Build Docker images with Python-generated context

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(10 files, 1 obsolete file)

(deleted), text/x-review-board-request
dustin
: review+
Details
(deleted), text/x-review-board-request
dustin
: review+
Details
(deleted), text/x-review-board-request
dustin
: review+
Details
(deleted), text/x-review-board-request
dustin
: review+
Details
(deleted), text/x-review-board-request
dustin
: review+
Details
(deleted), text/x-review-board-request
dustin
: review+
Details
(deleted), text/x-review-board-request
dustin
: review+
Details
(deleted), text/x-review-board-request
dustin
: review+
Details
(deleted), text/x-review-board-request
dustin
: review+
Details
(deleted), text/x-review-board-request
dustin
: review+
Details
In bug 1288567, I introduced special Dockerfile syntax to include files from throughout the source directory. This syntax is only recognized by the Docker image building mechanism in the image_builder Docker image because testing/docker/build.sh doesn't have access to the Docker build context generated by Python. We need to update testing/docker/build.sh to use the Python-generated build context so building Docker images with it behaves like building Docker images via the image_builder Docker image. Since I'm responsible for effectively breaking testing/docker/build.sh, I'll take this bug.
Docker image building will soon need to use Python in order to produce the image build context archive. As the first step towards this, we introduce a Python function that calls out to build.sh. We also implement a mach command that calls it so we can test the functionality. I'm not keen about introducing a new mach command. I'd prefer to have a sub-command instead. I'm not sure what all uses `mach taskcluster-load-image`. Perhaps we could make a `taskcluster` top-level command. Or perhaps we could fold these image commands into `mach taskgraph`? Either way, the mach side of this isn't terribly important to the commit series: most of the code will live inside a Python module outside of mach. Review commit: https://reviewboard.mozilla.org/r/68030/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68030/
Attachment #8776128 - Flags: review?(dustin)
Attachment #8776129 - Flags: review?(dustin)
Attachment #8776130 - Flags: review?(dustin)
Attachment #8776131 - Flags: review?(dustin)
Attachment #8776132 - Flags: review?(dustin)
Attachment #8776134 - Flags: review?(dustin)
Attachment #8776135 - Flags: review?(dustin)
Attachment #8776136 - Flags: review?(dustin)
Attachment #8776137 - Flags: review?(dustin)
We're about to make significant changes to this file. Nuke an unused function to make diffs easier to reason about. Review commit: https://reviewboard.mozilla.org/r/68032/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68032/
Now that we have a mach command and Python code for doing Docker image building, we can start moving code from build.sh to Python. We start with searching for and validating the `docker` binary works. Review commit: https://reviewboard.mozilla.org/r/68034/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68034/
We already had code for resolving the image registry and tag. We refactored it slightly to be more useful then changed build.sh to accept the tag as an argument. At this point, build.sh is basically a wrapper around `docker`. But there's a special case for executing custom "build.sh" files we need to eliminate first... Review commit: https://reviewboard.mozilla.org/r/68038/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68038/
There are no references to tester-device in tree or in the gaia repo. Dustin says it might be a relic of the past. Since it appears to be unused, remove it. The reason I found this is because it is the only thing using a custom "build.sh" to create Docker images. I'm rewriting the Docker image building functionality and tester-device is a one-off interfering with that work. Making it go away is the easiest way to unblock me. Review commit: https://reviewboard.mozilla.org/r/68040/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68040/
Now that tester-device is gone, there are no more images using custom build.sh scripts and that feature can be deleted. Yay simplicity. Review commit: https://reviewboard.mozilla.org/r/68042/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68042/
build.sh had been reduced to invoking `docker`. We move that invocation to Python and remove build.sh. Long live build.sh! Review commit: https://reviewboard.mozilla.org/r/68044/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68044/
Now that Docker image building is called from Python, we can start to do advanced stuff with it. With this commit, we switch from building Docker images directly from the source directory ("the Docker way") to using our custom Docker image build contexts. The main advantage of this is that locally-built Docker images can now use our custom Dockerfile syntax to include extra files in the build context! The code for building a Docker image from a context has been extracted to its own standalone function. I have nefarious plans for this in the future, such as the ability to override the FROM syntax to specify URLs of images. This would allow us to host base images on our own server, which removes a dependency on Docker Hub and improves determinism, since images on Docker Hub change all the time. Review commit: https://reviewboard.mozilla.org/r/68046/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68046/
The reason tooltool.py was vendored in testing/docker/decision was because locally built Docker images were using vanilla `docker build` and didn't know about our special Dockerfile syntax to allow the inclusion of images from outside the directory where the Dockerfile was located. Now that locally-built Docker images know of our special Dockerfile syntax, we can include files from anywhere. So, move tooltool.py to a shared directory, away from the decision image. I didn't bump the version of the decision image because there are a few more things I want to do to this image, such as have it use the `checkout-gecko-and-run` script instead of its own script. I think I'll do that in a separate bug, however. Review commit: https://reviewboard.mozilla.org/r/68048/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68048/
Comment on attachment 8776133 [details] Bug 1290531 - Remove tester-device Docker image; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68040/diff/1-2/
Attachment #8776133 - Flags: review?(dustin)
Comment on attachment 8776134 [details] Bug 1290531 - Remove support for building with custom build.sh; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68042/diff/1-2/
Comment on attachment 8776135 [details] Bug 1290531 - Invoke docker from Python, remove build.sh; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68044/diff/1-2/
Comment on attachment 8776136 [details] Bug 1290531 - Build Docker images from custom tar contexts; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68046/diff/1-2/
Comment on attachment 8776137 [details] Bug 1290531 - Move tooltool.py into shared directory; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68048/diff/1-2/
This commit does a lot. But it's really not too difficult to comprehend once you focus on the final state, which is basically the same as the "lint" image and derived tasks. Before, the "decision" image contained a "checkout-gecko" script and "run-action" and "run-decision" scripts. The latter 2 invoked the first script. The "checkout-gecko-and-run" script basically does what the combination of these scripts were doing before. So we switch to it. While we're here, we also replaced the custom Mercurial installation in this image with the shared install-mercurial.sh script. The system-setup.sh script for the decision image is now short and sweet. The YAML files for tasks using this image have been updated to use "checkout-gecko-and-run." We no longer have to pass an environment variable to hold command arguments. So we revert to putting these arguments inline in the task's command. Yay. Finally, the path to the Gecko checkout has been changed from /home/worker/workspace to /home/worker/checkouts to match changes made in bug 1289643. The Docker image has been bumped accordingly. Review commit: https://reviewboard.mozilla.org/r/68094/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68094/
Attachment #8776190 - Flags: review?(dustin)
Depends on: 1289643
Attachment #8776190 - Attachment is obsolete: true
Attachment #8776190 - Flags: review?(dustin)
(In reply to Gregory Szorc [:gps] from comment #1) > I'm not keen about introducing a new mach command. I'd prefer to have > a sub-command instead. I'm not sure what all uses > `mach taskcluster-load-image`. Perhaps we could make a `taskcluster` > top-level command. Or perhaps we could fold these image commands into > `mach taskgraph`? `mach taskcluster <subcommands>` sounds good to me. I kept `taskcluster-load-image` around as I'd blogged about it under that name, that's all.
Attachment #8776128 - Flags: review?(dustin) → review+
Attachment #8776129 - Flags: review?(dustin) → review+
Comment on attachment 8776130 [details] Bug 1290531 - Move docker validation from build.sh to Python; https://reviewboard.mozilla.org/r/68034/#review65398 ::: taskcluster/taskgraph/docker.py:76 (Diff revision 1) > Output from image building process will be printed to stdout. > """ > + docker_bin = which.which('docker') > + > + # Verify that Docker is working. > + # TODO Check minimum Docker version. I think we can drop this comment.
Attachment #8776130 - Flags: review?(dustin) → review+
Attachment #8776131 - Flags: review?(dustin) → review+
Attachment #8776132 - Flags: review?(dustin) → review+
Comment on attachment 8776134 [details] Bug 1290531 - Remove support for building with custom build.sh; https://reviewboard.mozilla.org/r/68042/#review65406
Attachment #8776134 - Flags: review?(dustin) → review+
Comment on attachment 8776135 [details] Bug 1290531 - Invoke docker from Python, remove build.sh; https://reviewboard.mozilla.org/r/68044/#review65410
Attachment #8776135 - Flags: review?(dustin) → review+
Attachment #8776136 - Flags: review?(dustin) → review+
Comment on attachment 8776133 [details] Bug 1290531 - Remove tester-device Docker image; https://reviewboard.mozilla.org/r/68040/#review65414 <p>I checked with greg:</p> <p>&lt;•garndt&gt; I'm fairly certain that could be removed now<br /> &lt;•garndt&gt; that was used for testing in the remote device lab, which I think is no longer happening</p>
Attachment #8776133 - Flags: review?(dustin) → review+
lol at the mess of HTML :(
Attachment #8776137 - Flags: review?(dustin) → review+
Comment on attachment 8776128 [details] Bug 1290531 - Add mach taskcluster-build-image command; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68030/diff/1-2/
Comment on attachment 8776129 [details] Bug 1290531 - Remove unused find_registry(); Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68032/diff/1-2/
Comment on attachment 8776130 [details] Bug 1290531 - Move docker validation from build.sh to Python; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68034/diff/1-2/
Comment on attachment 8776131 [details] Bug 1290531 - Move image name verification to Python; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68036/diff/1-2/
Comment on attachment 8776132 [details] Bug 1290531 - Move image tag resolution to Python; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68038/diff/1-2/
Comment on attachment 8776133 [details] Bug 1290531 - Remove tester-device Docker image; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68040/diff/2-3/
Comment on attachment 8776134 [details] Bug 1290531 - Remove support for building with custom build.sh; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68042/diff/2-3/
Comment on attachment 8776135 [details] Bug 1290531 - Invoke docker from Python, remove build.sh; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68044/diff/2-3/
Comment on attachment 8776136 [details] Bug 1290531 - Build Docker images from custom tar contexts; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68046/diff/2-3/
Comment on attachment 8776137 [details] Bug 1290531 - Move tooltool.py into shared directory; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68048/diff/2-3/
Pushed by gszorc@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b6eff3784cf2 Add mach taskcluster-build-image command; r=dustin https://hg.mozilla.org/integration/autoland/rev/4509921b0cfd Remove unused find_registry(); r=dustin https://hg.mozilla.org/integration/autoland/rev/95aeafbfec09 Move docker validation from build.sh to Python; r=dustin https://hg.mozilla.org/integration/autoland/rev/a1f4555494b7 Move image name verification to Python; r=dustin https://hg.mozilla.org/integration/autoland/rev/e20bc9e7b388 Move image tag resolution to Python; r=dustin https://hg.mozilla.org/integration/autoland/rev/1532b022e19c Remove tester-device Docker image; r=dustin https://hg.mozilla.org/integration/autoland/rev/9b290fa0e180 Remove support for building with custom build.sh; r=dustin https://hg.mozilla.org/integration/autoland/rev/4ec50d432877 Invoke docker from Python, remove build.sh; r=dustin https://hg.mozilla.org/integration/autoland/rev/d61de5431643 Build Docker images from custom tar contexts; r=dustin https://hg.mozilla.org/integration/autoland/rev/ed95bb78b383 Move tooltool.py into shared directory; r=dustin
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: