Docker digest #22260
Docker digest #22260
Conversation
KevinMind
force-pushed
the
docker-digest
branch
3 times, most recently
from
678392c
to
087aad2
Compare
| @@ -2,18 +2,6 @@ | |||
| # Our makefile makes use of docker compose commands. Our config files rely on environment variables | |||
| # both for passing configuration to the containers as well as configuring the compose file itself. | |||
| # Variables referenced in docker-compose*.yml should be read from .env, exported and saved in .env | |||
|
|
|||
| define ENV_VAR | |||
There was a problem hiding this comment.
This logic was already very hard to follow, and now with the requirement to additionally prefix the DOCKER_VERSION value, it makes a lot of sense to move this to a python script.
…n readable python
| @@ -160,7 +160,9 @@ volumes: | |||
| data_redis: | |||
| data_elasticsearch: | |||
| data_mysqld: | |||
| name: ${DOCKER_MYSQLD_VOLUME:-} | |||
| # Keep this value in sync with Makefile-os | |||
There was a problem hiding this comment.
This value is covered explicitly with tests and should realistically never change. We can ditch the env variable.
| const fileContent = fs.readFileSync(path.join(rootPath, file), { | ||
| encoding: 'utf-8', | ||
| }); | ||
| const { stdout: rawConfig } = spawnSync( |
There was a problem hiding this comment.
We can call docker explicitly here to test the scenario where a user doesn't use make and must rely only on .env or env variables.
| ]; | ||
| } | ||
|
|
||
| const testCases = [ |
There was a problem hiding this comment.
This format for the env variable test is much more flexible and simple. Also results in less code.
| @@ -23,6 +11,7 @@ DOCKER_OUTPUT ?= | |||
| DOCKER_COMMIT ?= $(shell git rev-parse HEAD || echo "commit") | |||
| VERSION_BUILD_URL ?= build | |||
| BUILDX_BAKE_COMMAND := docker buildx bake web | |||
| override DOCKER_MYSQLD_VOLUME = addons-server_data_mysqld | |||
There was a problem hiding this comment.
Yes so we can create or destroy the volume.
Co-authored-by: Mathieu Pillard <diox@users.noreply.github.com>
Fixes: mozilla/addons#14797
Description
This PR adds support for running our docker compose services via the build digest instead of the version. Version is still supported in cases where you want to run on "latest" or the latest build of a tag for example.
The PR achieves several discrete and related things:
Context
This PR implements a fix for a bug that was discovered during the preparation of #22250 and should be a prerequisite for continuing.
Coincidentally and (I believe) beneficially, we now totally decouple the specification of .env values used in docker-compose substitution and the make file. This prevents over-reliance of make commands on these values as they are "special" and shouldn't be used outside of substituting values in compose.
An important edge case that is covered is that we need to prefix DOCKER_VERSION. This is because the way you specify an image version is different for tags or digests.
mozilla/addons-server@sha2456: or mozilla/addons-server:latest
Tags use
:and digests use@. This behavior is tested and should work regardless of how you specify the DOCKER_VERSION at run time.Testing
You can test locally running the test suite or play around with the specifications.
Relevant values are noted in the test suite here (https://github.com/mozilla/addons-server/pull/22260/files#diff-1904f45ef3db1fec0d686e0545e32db0dca4c7c4151c56ca56a4f803f96e194bR115-R169)