Closed Bug 1290531 Opened 3 years ago Closed 3 years ago

Build Docker images with Python-generated context

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(10 files, 1 obsolete file)

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
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
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.
Comment on attachment 8776128 [details]
Bug 1290531 - Add mach taskcluster-build-image command;

https://reviewboard.mozilla.org/r/68030/#review65384
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+
Comment on attachment 8776131 [details]
Bug 1290531 - Move image name verification to Python;

https://reviewboard.mozilla.org/r/68036/#review65402
Attachment #8776131 - Flags: review?(dustin) → review+
Comment on attachment 8776132 [details]
Bug 1290531 - Move image tag resolution to Python;

https://reviewboard.mozilla.org/r/68038/#review65404
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 :(
Comment on attachment 8776137 [details]
Bug 1290531 - Move tooltool.py into shared directory;

https://reviewboard.mozilla.org/r/68048/#review65416
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.