1290531 - Build Docker images with Python-generated context

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Gregory Szorc [:gps]]

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.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Bug 1290531 - Add mach taskcluster-build-image command;

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/

Comment 2

[Gregory Szorc [:gps]]

Attachment: Bug 1290531 - Remove unused find_registry();

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/

Comment 3

[Gregory Szorc [:gps]]

Attachment: Bug 1290531 - Move docker validation from build.sh to Python;

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/

Comment 4

[Gregory Szorc [:gps]]

Attachment: Bug 1290531 - Move image name verification to Python;

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

Comment 5

[Gregory Szorc [:gps]]

Attachment: Bug 1290531 - Move image tag resolution to Python;

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/

Comment 6

[Gregory Szorc [:gps]]

Attachment: Bug 1290531 - Remove tester-device Docker image;

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/

Comment 7

[Gregory Szorc [:gps]]

Attachment: Bug 1290531 - Remove support for building with custom build.sh;

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/

Comment 8

[Gregory Szorc [:gps]]

Attachment: Bug 1290531 - Invoke docker from Python, remove build.sh;

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/

Comment 9

[Gregory Szorc [:gps]]

Attachment: Bug 1290531 - Build Docker images from custom tar contexts;

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/

Comment 10

[Gregory Szorc [:gps]]

Attachment: Bug 1290531 - Move tooltool.py into shared directory;

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 11

[Gregory Szorc [:gps]]

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/

Comment 12

[Gregory Szorc [:gps]]

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 13

[Gregory Szorc [:gps]]

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 14

[Gregory Szorc [:gps]]

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 15

[Gregory Szorc [:gps]]

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/

Comment 16

[Gregory Szorc [:gps]]

Attachment: Bug 1290531 - Use checkout-gecko-and-run from decision task;

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/

Comment 17

[Dustin J. Mitchell [:dustin] (he/him)]

(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 18

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8776128 [details]
Bug 1290531 - Add mach taskcluster-build-image command;

https://reviewboard.mozilla.org/r/68030/#review65384

Comment 19

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8776129 [details]
Bug 1290531 - Remove unused find_registry();

https://reviewboard.mozilla.org/r/68032/#review65396

Comment 20

[Dustin J. Mitchell [:dustin] (he/him)]

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.

Comment 21

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8776131 [details]
Bug 1290531 - Move image name verification to Python;

https://reviewboard.mozilla.org/r/68036/#review65402

Comment 22

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8776132 [details]
Bug 1290531 - Move image tag resolution to Python;

https://reviewboard.mozilla.org/r/68038/#review65404

Comment 23

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8776134 [details]
Bug 1290531 - Remove support for building with custom build.sh;

https://reviewboard.mozilla.org/r/68042/#review65406

Comment 24

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8776135 [details]
Bug 1290531 - Invoke docker from Python, remove build.sh;

https://reviewboard.mozilla.org/r/68044/#review65410

Comment 25

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8776136 [details]
Bug 1290531 - Build Docker images from custom tar contexts;

https://reviewboard.mozilla.org/r/68046/#review65412

Comment 26

[Dustin J. Mitchell [:dustin] (he/him)]

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>

Comment 27

[Dustin J. Mitchell [:dustin] (he/him)]

lol at the mess of HTML :(

Comment 28

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8776137 [details]
Bug 1290531 - Move tooltool.py into shared directory;

https://reviewboard.mozilla.org/r/68048/#review65416

Comment 29

[Gregory Szorc [:gps]]

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 30

[Gregory Szorc [:gps]]

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 31

[Gregory Szorc [:gps]]

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 32

[Gregory Szorc [:gps]]

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 33

[Gregory Szorc [:gps]]

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 34

[Gregory Szorc [:gps]]

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 35

[Gregory Szorc [:gps]]

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 36

[Gregory Szorc [:gps]]

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 37

[Gregory Szorc [:gps]]

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 38

[Gregory Szorc [:gps]]

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/

Comment 39

[Pulsebot]

Pushed by [e-mail]:
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

Comment 40

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b6eff3784cf2
https://hg.mozilla.org/mozilla-central/rev/4509921b0cfd
https://hg.mozilla.org/mozilla-central/rev/95aeafbfec09
https://hg.mozilla.org/mozilla-central/rev/a1f4555494b7
https://hg.mozilla.org/mozilla-central/rev/e20bc9e7b388
https://hg.mozilla.org/mozilla-central/rev/1532b022e19c
https://hg.mozilla.org/mozilla-central/rev/9b290fa0e180
https://hg.mozilla.org/mozilla-central/rev/4ec50d432877
https://hg.mozilla.org/mozilla-central/rev/d61de5431643
https://hg.mozilla.org/mozilla-central/rev/ed95bb78b383