Fix Docker build:

- get correct Image ID from the build.
- set docker tag correctly supporting tag and digest
- add concurrency limit to github action ci jobs

.circleci/config.yml

@@ -379,15 +379,14 @@ commands:
           command: |
             make docker_compose_config
       - run:
-          name: Build docker image and push to repo
+          name: Build docker image (push = << parameters.push >>)
           command: |
-            docker version
-            docker login -u "${DOCKERHUB_USER}" -p "${DOCKERHUB_PASS}"
             make build_docker_image
       - run:
           name: Print Digest
           command: |
-            cat << parameters.output_file >> | jq -r '.web."containerimage.digest"'
+            docker images
+            make docker_image_id
 
   better_checkout:
     description: circle ci checkout step on steroids
@@ -634,9 +633,9 @@ jobs:
     steps:
       - checkout
       - make_release:
-          image_tag: circle-${CIRCLE_BRANCH}
+          image_tag: "circle-${CIRCLE_BRANCH}"
           # explicitly don't push
-          push: false
+          push: true
 
   release-master:
     <<: *defaults-release
@@ -665,7 +664,7 @@ workflows:
       - devhub
       - main
       # Uncomment if you want to test the docker build
-      # - build-image
+      - build-image
       - reviewers-and-zadmin
       - es-tests
       - localization

.github/actions/build-docker/action.yml

@@ -72,15 +72,12 @@ runs:
           # The production build
           # type=raw,value=latest,enable={{is_default_branch}}
 
-    - name: Define environment variables
-      shell: bash
-      run: |
-        echo "DOCKER_VERSION=${{ steps.meta.outputs.version }}" >> $GITHUB_ENV
-        echo "DOCKER_COMMIT=${{ github.sha }}" >> $GITHUB_ENV
-        echo "VERSION_BUILD_URL=${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" >> $GITHUB_ENV
-
     - name: Create .env and version.json files
       shell: bash
+      env:
+        DOCKER_VERSION: ${{ steps.meta.outputs.version }}
+        DOCKER_COMMIT: ${{ github.sha }}
+        VERSION_BUILD_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
       run: make setup
 
     - name: Build Image
@@ -95,5 +92,4 @@ runs:
       id: digest
       shell: bash
       run: |
-        echo '${{ steps.build.outputs.metadata }}' > metadata.json
-        echo "digest=$(jq -r '.web."containerimage.digest"' metadata.json )" >> $GITHUB_OUTPUT
+        echo "digest=$(make docker_image_id)" >> $GITHUB_OUTPUT

.github/actions/run-docker/action.yml

@@ -2,9 +2,13 @@ name: 'Docker Run Action'
 description: 'Run a command in a new container'
 inputs:
   version:
-    description: 'The version of the image to run. Supports tag or digest'
+    description: 'The version of the image to run. '
     required: true
     default: 'local'
+  digest:
+    description: 'The build digest of the image to run. Overrides version.'
+    required: true
+    default: ''
   run:
     description: 'Run command in container'
     required: true
@@ -28,6 +32,7 @@ runs:
       shell: bash
       env:
         DOCKER_VERSION: ${{ inputs.version }}
+        DOCKER_DIGEST: ${{ inputs.digest }}
         COMPOSE_FILE: ${{ inputs.compose_file }}
         DOCKER_SERVICES: ${{ inputs.services }}
         HOST_UID: ${{ steps.id.outputs.id }}
@@ -37,9 +42,6 @@ runs:
           exit 1
         fi
 
-        # Setup host
-        make setup
-
         # Start the specified services
         make up
 

.github/workflows/verify-docker-image.yml

@@ -5,6 +5,10 @@ on:
     branches:
       - master
 
+concurrency:
+  group: verify-docker-image
+  cancel-in-progress: true
+
 jobs:
   docker_config_check:
     runs-on: ubuntu-latest

Makefile-os

@@ -7,7 +7,6 @@
 DOCKER_BUILDER ?= container
 DOCKER_PROGRESS ?= auto
 DOCKER_PUSH ?= false
-DOCKER_OUTPUT ?=
 DOCKER_COMMIT ?= $(shell git rev-parse HEAD || echo "commit")
 VERSION_BUILD_URL ?= build
 BUILDX_BAKE_COMMAND := docker buildx bake web
@@ -84,10 +83,6 @@ else
 	BUILDX_BAKE_COMMAND += --load
 endif
 
-ifneq ($(DOCKER_OUTPUT),)
-	BUILDX_BAKE_COMMAND += --metadata-file=$(DOCKER_OUTPUT)
-endif
-
 .PHONY: docker_compose_config
 docker_compose_config: ## Show the docker compose configuration
 	@docker compose config web --format json
@@ -112,6 +107,15 @@ docker_mysqld_volume_create: ## Create the mysqld volume
 docker_mysqld_volume_remove: ## Remove the mysqld volume
 	docker volume rm $(DOCKER_MYSQLD_VOLUME)
 
+# DOCKER_TAG is defined in .env
+# or can be passed as an argument or environment variable
+DOCKER_TAG ?= $(shell grep DOCKER_TAG .env | cut -d '=' -f 2)
+
+.PHONY: docker_image_id
+docker_image_id: ## get the digest of the image
+	@echo "image: $(DOCKER_TAG)"
+	@echo "id: $(shell docker inspect $(DOCKER_TAG) | jq -r '.[0].Id')"
+
 .PHONY: docker_compose_down
 docker_compose_down: ## Stop the docker containers
 	docker compose down --rmi local --remove-orphans --volumes

docker-compose.yml

@@ -27,7 +27,7 @@ x-env-mapping: &env
 services:
   worker: &worker
     <<: *env
-    image: mozilla/addons-server${DOCKER_VERSION:-}
+    image: ${DOCKER_TAG:-}
     build:
       context: .
       dockerfile: Dockerfile

scripts/setup.py

@@ -53,24 +53,30 @@ def git_ref():
     return get_value('DOCKER_COMMIT', git_ref)
 
 
-def clean_docker_version(docker_version):
-    # For DOCKER_VERSION, we support defining a version tag or a digest.
-    # Digest allows us to guarantee an image from a specific build is used in ci.
-
-    # first check if the value in DOCKER_VERSION starts with : or @
-    # if so, remove it, so we can re-evaluate the version.
-    if docker_version[0] in [':', '@']:
-        docker_version = docker_version[1:]
-
-    # if the new value starts with sha256, it is a digest, otherwise a tag
-    if docker_version.startswith('sha256'):
-        # add a @ at the beginning of DOCKER_VERSION
-        docker_version = '@' + docker_version
+def get_docker_tag():
+    image_name = 'mozilla/addons-server'
+    version = os.environ.get('DOCKER_VERSION')
+    digest = os.environ.get('DOCKER_DIGEST')
+
+    tag = f'{image_name}:local'
+
+    if digest:
+        tag = f'{image_name}@{digest}'
+    elif version:
+        tag = f'{image_name}:{version}'
     else:
-        # add a : at the beginning of DOCKER_VERSION
-        docker_version = ':' + docker_version
+        tag = get_value('DOCKER_TAG', tag)
+        # extract version or digest from existing tag
+        if '@' in tag:
+            digest = tag.split('@')[1]
+        elif ':' in tag:
+            version = tag.split(':')[1]
+
+    print('Docker tag: ', tag)
+    print('version: ', version)
+    print('digest: ', digest)
 
-    return docker_version
+    return tag, version, digest
 
 
 # Env file should contain values that are referenced in docker-compose*.yml files
@@ -86,13 +92,11 @@ def clean_docker_version(docker_version):
 # 3. the value defined in the environment variable
 # 4. the value defined in the make args.
 
-# Some variables have special formatting applied, such as DOCKER_VERSION
-# this can be defined in an optional third argument to this function, as a function.
-docker_version = clean_docker_version(get_value('DOCKER_VERSION', 'local'))
+docker_tag, docker_version, docker_digest = get_docker_tag()
 
 set_env_file(
     {
-        'DOCKER_VERSION': docker_version,
+        'DOCKER_TAG': docker_tag,
         'HOST_UID': get_value('HOST_UID', os.getuid()),
         'SUPERUSER_EMAIL': get_value(
             'SUPERUSER_EMAIL', git_config('user.email', 'admin@mozilla.com')
@@ -108,7 +112,8 @@ def clean_docker_version(docker_version):
 with open('version.json', 'w') as f:
     data = {
         'commit': git_ref(),
-        'version': docker_version[1:],
+        'version': docker_version,
+        'digest': docker_digest,
         'build': build,
         'source': 'https://github.com/mozilla/addons-server',
     }

tests/make/make.spec.js

@@ -112,57 +112,59 @@ function standardPermutations(name, defaultValue) {
   ];
 }
 
-const testCases = [
-  {
-    name: 'DOCKER_VERSION',
-    file: undefined,
-    env: undefined,
-    expected: ':local',
-  },
+describe.each([
   {
-    name: 'DOCKER_VERSION',
-    file: 'file',
-    env: undefined,
-    expected: ':file',
+    version: undefined,
+    digest: undefined,
+    tag: undefined,
+    expected: 'mozilla/addons-server:local',
   },
   {
-    name: 'DOCKER_VERSION',
-    file: undefined,
-    env: 'env',
-    expected: ':env',
+    version: 'version',
+    digest: undefined,
+    tag: undefined,
+    expected: 'mozilla/addons-server:version',
   },
   {
-    name: 'DOCKER_VERSION',
-    file: 'file',
-    env: 'env',
-    expected: ':env',
+    version: undefined,
+    digest: 'sha256:digest',
+    tag: undefined,
+    expected: 'mozilla/addons-server@sha256:digest',
   },
-  // Test that if the prefix already exists, it is not duplicated
   {
-    name: 'DOCKER_VERSION',
-    file: ':local',
-    env: undefined,
-    expected: ':local',
+    version: 'version',
+    digest: 'sha256:digest',
+    tag: undefined,
+    expected: 'mozilla/addons-server@sha256:digest',
   },
-  // Test that if the prefix already exists, it is not duplicated
   {
-    name: 'DOCKER_VERSION',
-    file: '@sha256:local',
-    env: undefined,
-    expected: '@sha256:local',
+    version: 'version',
+    digest: 'sha256:digest',
+    tag: 'previous',
+    expected: 'mozilla/addons-server@sha256:digest',
   },
   {
-    name: 'DOCKER_VERSION',
-    file: 'sha256:local',
-    env: undefined,
-    expected: '@sha256:local',
-  },
-  {
-    name: 'DOCKER_VERSION',
-    file: undefined,
-    env: '@sha256:local',
-    expected: '@sha256:local',
+    version: undefined,
+    digest: undefined,
+    tag: 'previous',
+    expected: 'previous',
   },
+])('DOCKER_TAG', ({ version, digest, tag, expected }) => {
+  it(`version:${version}_digest:${digest}_tag:${tag}`, () => {
+    fs.writeFileSync(envPath, '');
+    runSetup({
+      DOCKER_VERSION: version,
+      DOCKER_DIGEST: digest,
+      DOCKER_TAG: tag,
+    });
+
+    const actual = readEnvFile('DOCKER_TAG');
+    expect(actual).toStrictEqual(expected);
+  });
+});
+
+const testCases = [
+  ...standardPermutations('DOCKER_TAG', 'mozilla/addons-server:local'),
   ...standardPermutations('HOST_UID', process.getuid().toString()),
   ...standardPermutations('SUPERUSER_EMAIL', gitConfigUserEmail()),
   ...standardPermutations('SUPERUSER_USERNAME', gitConfigUserName()),
@@ -171,7 +173,6 @@ const testCases = [
 describe.each(testCases)('.env file', ({ name, file, env, expected }) => {
   it(`name:${name}_file:${file}_env:${env}`, () => {
     fs.writeFileSync(envPath, file ? `${name}=${file}` : '');
-    process.env[name] = env;
 
     runSetup({ [name]: env });