Use Bakefile to build image (#22611)

* Use Bakefile to build image

* TMP: better tests and lint fix

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

@@ -66,12 +66,13 @@ runs:
 
     - name: Create .env and version.json files
       shell: bash
-      env:
-        DOCKER_TARGET: ${{ inputs.target }}
-        DOCKER_VERSION: ${{ steps.meta.outputs.version }}
-        DOCKER_COMMIT: ${{ steps.context.outputs.git_sha }}
-        DOCKER_BUILD: ${{ steps.context.outputs.git_build_url }}
-      run: make setup
+      run: |
+        echo "DOCKER_TARGET=${{ inputs.target }}" >> $GITHUB_ENV
+        echo "DOCKER_VERSION=${{ steps.meta.outputs.version }}" >> $GITHUB_ENV
+        echo "DOCKER_COMMIT=${{ steps.context.outputs.git_sha }}" >> $GITHUB_ENV
+        echo "DOCKER_BUILD=${{ steps.context.outputs.git_build_url }}" >> $GITHUB_ENV
+
+        make setup
 
     - name: Build Image
       id: build
@@ -80,19 +81,16 @@ runs:
         targets: web
         files: |
           docker-compose.yml
+          docker-bake.hcl
+          .env
           ${{ steps.meta.outputs.bake-file-tags }}
           ${{ steps.meta.outputs.bake-file-labels }}
           ${{ steps.meta.outputs.bake-file-annotations }}
         push: ${{ inputs.push }}
-        set: |
-          *.cache-from=type=gha
-          *.cache-to=type=gha,mode=max,ignore-error=true
 
     - name: Get image digest
       id: build_meta
       shell: bash
-      env:
-        BUILDX_BAKE_METADATA_FILE: metadata.json
       run: |
         echo '${{ steps.build.outputs.metadata }}' > metadata.json
         echo "digest=$(cat metadata.json | jq -r '.web."containerimage.digest"')" >> $GITHUB_OUTPUT

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

@@ -28,9 +28,17 @@ runs:
       run: |
         echo "id=$(id -u)" >> $GITHUB_OUTPUT
 
+    - name: Set up Docker Buildx
+      id: buildx
+      uses: docker/setup-buildx-action@v1
+      with:
+        version: latest
+        buildkitd-flags: --debug
+
     - name: Run Docker Container
       shell: bash
       env:
+        DOCKER_BUILDER: ${{ steps.buildx.outputs.name}}
         DOCKER_VERSION: ${{ inputs.version }}
         DOCKER_DIGEST: ${{ inputs.digest }}
         COMPOSE_FILE: ${{ inputs.compose_file }}

.github/workflows/_test.yml

@@ -41,7 +41,9 @@ jobs:
           -
             name: Needs Locale Compilation
             services: ''
-            run: make test_needs_locales_compilation
+            run: |
+              make compile_locales
+              make test_needs_locales_compilation
           -
             name: Static Assets
             services: ''

.github/workflows/ci.yml

@@ -125,6 +125,27 @@ jobs:
             echo 'a * is born'
             echo 'wow an array []'
 
+      - name: Verify Build Metadata
+        uses: ./.github/actions/run-docker
+        env:
+          DOCKER_COMMIT: should_not_be_this
+        with:
+          digest: ${{ needs.build.outputs.digest }}
+          version: ${{ needs.build.outputs.version }}
+          run: |
+            expected_version="${{ needs.build.outputs.version }}"
+            expected_commit="${{ github.sha }}"
+
+            if [ "$DOCKER_COMMIT" != "$expected_commit" ]; then
+              echo "DOCKER_COMMIT: '$DOCKER_COMMIT' is not equal to '$expected_commit'"
+              exit 1
+            fi
+
+            if [ "$DOCKER_VERSION" != "$expected_version" ]; then
+              echo "DOCKER_VERSION: '$DOCKER_VERSION' is not equal to '$expected_version'"
+              exit 1
+            fi
+
   docs_build:
     runs-on: ubuntu-latest
     needs: build

Makefile-os

@@ -6,18 +6,21 @@
 
 DOCKER_BUILDER ?= container
 DOCKER_PROGRESS ?= auto
-DOCKER_PUSH ?=
-BUILDX_BAKE_METADATA_FILE ?=
 export DOCKER_COMMIT ?=
 export DOCKER_BUILD ?=
 export DOCKER_VERSION ?=
-BUILDX_BAKE_COMMAND := docker buildx bake web
 override DOCKER_MYSQLD_VOLUME = addons-server_data_mysqld
 
 override BACKUPS_DIR = $(shell pwd)/backups
 override EXPORT_DIR = $(BACKUPS_DIR)/$(shell date +%Y%m%d%H%M%S)
 RESTORE_DIR ?= $(BACKUPS_DIR)/$(shell ls -1 backups | sort -r | head -n 1)
 
+DOCKER_COMPOSE_ARGS := \
+	-d \
+	--wait \
+	--remove-orphans \
+	--quiet-pull \
+
 # Paths should be cleaned before mounting .:/data/olympia
 # These are files which should be sourced from the container
 # or should be fresh on every run of the project
@@ -83,35 +86,39 @@ create_docker_builder: ## Create a custom builder for buildkit to efficiently bu
 		--name $(DOCKER_BUILDER) \
 		--driver=docker-container
 
-BUILDX_BAKE_COMMAND += \
---progress=$(DOCKER_PROGRESS) \
---builder=$(DOCKER_BUILDER) \
-
-ifneq ($(DOCKER_PUSH),)
-	BUILDX_BAKE_COMMAND += --push
-else
-	BUILDX_BAKE_COMMAND += --load
-endif
-
-ifneq ($(BUILDX_BAKE_METADATA_FILE),)
-	BUILDX_BAKE_COMMAND += --metadata-file=$(BUILDX_BAKE_METADATA_FILE)
-endif
-
 .PHONY: docker_compose_config
 docker_compose_config: ## Show the docker compose configuration
 	@docker compose config web --format json
 
-.PHONY: docker_build_args
-docker_build_args: ## Show the docker build configuration
-	@echo $(BUILDX_BAKE_COMMAND)
-
-.PHONY: docker_build_config
-docker_build_config:
-	@$(BUILDX_BAKE_COMMAND) --print
-
-.PHONY: build_docker_image
-build_docker_image: create_docker_builder ## Build the docker image
-	$(BUILDX_BAKE_COMMAND)
+.PHONY: docker_build_web ## Build the docker images using buildx bake
+docker_build_web: create_docker_builder
+	docker buildx bake \
+	--file docker-bake.hcl \
+	--file .env \
+	--builder $(DOCKER_BUILDER) \
+	--progress $(DOCKER_PROGRESS) \
+	$(ARGS); \
+
+.PHONY: docker_pull_web ## Pull the latest docker image using current tag
+docker_pull_web:
+	docker compose pull web --policy always
+
+.PHONY: docker_pull_or_build ## Pull or build the docker image based on the image version
+docker_pull_or_build:
+# If the image is tagged with version "local" then we should build the image before running
+# docker compose up. The image will be available to docker compose, skipping a pull attempt.
+# This is useful for local development where the image is built and tagged with "local".
+# Also for CI/CID pipelines on forks where we cannot pull the image and must build locally.
+# If the image is tagged with a version other than "local" then we should skip the build
+# and let docker compose pull the image instead. This is useful for CI/CD pipelines where
+# the image is already built and pushed to a registry.
+	@IMAGE=$$(docker compose config web --format json | jq -r '.services.web.image'); \
+	echo "image: $$IMAGE"; \
+	if echo "$$IMAGE" | grep -q ":local"; then \
+		$(MAKE) docker_build_web; \
+	else \
+		$(MAKE) docker_pull_web; \
+	fi
 
 .PHONY: docker_mysqld_volume_create
 docker_mysqld_volume_create: ## Create the mysqld volume
@@ -146,26 +153,10 @@ clean_docker: docker_compose_down docker_mysqld_volume_remove docker_clean_image
 
 .PHONY: docker_compose_up
 docker_compose_up: docker_mysqld_volume_create ## Start the docker containers
-	docker compose up $(DOCKER_SERVICES) -d --wait --remove-orphans --quiet-pull $(ARGS)
-	docker compose rm -f olympia
-
-.PHONY: docker_extract_deps
-docker_extract_deps: ## Extract dependencies from the docker image to a local volume mount
-# Run a fresh container from the base image to install deps. Since /deps is
-# shared via a volume in docker-compose.yml, this installs deps for both web
-# and worker containers, and does so without requiring the containers to be up.
-# We just create dummy empty package.json and package-lock.json in deps/ so
-# that docker compose doesn't create dummy ones itself, as they would be owned
-# by root. They don't matter: the ones at the root directory are mounted
-# instead.
-	touch deps/package.json
-	touch deps/package-lock.json
-	# mounting ./deps:/deps effectively removes dependencies from the /deps directory in the container
-	# running `update_deps` will install the dependencies in the /deps directory before running
-	docker compose run --rm --quiet-pull web make update_deps
+	docker compose up $(DOCKER_SERVICES) $(DOCKER_COMPOSE_ARGS) $(ARGS)
 
 .PHONY: up
-up: setup docker_mysqld_volume_create docker_compose_up docker_clean_images docker_clean_volumes ## Create and start docker compose
+up: setup docker_pull_or_build docker_compose_up docker_clean_images docker_clean_volumes ## Create and start docker compose
 
 .PHONY: down
 down: docker_compose_down docker_clean_images docker_clean_volumes ## Stop the docker containers and clean up non-peristent dangling resources

docker-bake.hcl

@@ -0,0 +1,32 @@
+group "default" {
+  targets = ["web"]
+}
+
+variable DOCKER_BUILD {}
+variable DOCKER_COMMIT {}
+variable DOCKER_VERSION {}
+variable DOCKER_TARGET {}
+variable DOCKER_TAG {}
+
+target "web" {
+  context = "."
+  dockerfile = "Dockerfile"
+  target = "${DOCKER_TARGET}"
+  tags = ["${DOCKER_TAG}"]
+  platforms = ["linux/amd64"]
+  args = {
+	DOCKER_COMMIT = "${DOCKER_COMMIT}"
+	DOCKER_VERSION = "${DOCKER_VERSION}"
+	DOCKER_BUILD = "${DOCKER_BUILD}"
+  }
+  pull = true
+  cache-to = [
+    "type=gha,mode=max,scope=mozilla/addons-server/web"
+  ]
+  cache-from = ["type=gha,scope=mozilla/addons-server/web"]
+
+  output = [
+    "type=docker",
+  ]
+
+}

docker-compose.yml

@@ -25,8 +25,10 @@ x-env-mapping: &env
 x-olympia: &olympia
   <<: *env
   image: ${DOCKER_TAG:-}
-  # Ignore any linting saying we have an invalid value.
-  pull_policy: ${DOCKER_PULL_POLICY:-}
+  # We don't want docker compose to manage the image for us.
+  # We sometimes build the image locally and sometimes pull from a registry
+  # but docker compose should always assume the image is available.
+  pull_policy: never
   # We drop down to a different user through entrypoint.sh, but starting as
   # root allows us to fix the ownership of files generated at image build
   # time through the ./docker/entrypoint.sh script.
@@ -37,17 +39,6 @@ x-olympia: &olympia
 services:
   olympia:
     <<: *olympia
-    build:
-      args:
-        DOCKER_COMMIT: ${DOCKER_COMMIT:-}
-        DOCKER_VERSION: ${DOCKER_VERSION:-}
-        DOCKER_BUILD: ${DOCKER_BUILD:-}
-      context: .
-      dockerfile: Dockerfile
-      target: ${DOCKER_TARGET:-}
-      x-bake:
-        pull: true
-        platforms: linux/amd64
     volumes:
       - data_deps:/deps
       - ./package.json:/deps/package.json

scripts/setup.py

@@ -7,7 +7,7 @@ def set_env_file(values):
     with open('.env', 'w') as f:
         print('Environment:')
         for key, value in values.items():
-            f.write(f'{key}={value}\n')
+            f.write(f'{key}="{value}"\n')
             print(f'{key}={value}')
 
 
@@ -18,7 +18,7 @@ def get_env_file():
         with open('.env', 'r') as f:
             for line in f:
                 key, value = line.strip().split('=', 1)
-                env[key] = value
+                env[key] = value.strip('"')
     return env
 
 
@@ -76,16 +76,14 @@ def get_docker_tag():
 
 docker_tag, docker_version, docker_digest = get_docker_tag()
 
-# set pull_policy of web/worker containers based on the specified tag
-# for digest or non `local` versions, we should avoid building and pull aggressively
-docker_pull_policy = 'always' if docker_digest or docker_version != 'local' else 'build'
+docker_target = get_value('DOCKER_TARGET', 'development')
+compose_file = get_value('COMPOSE_FILE', ('docker-compose.yml'))
 
 set_env_file(
     {
-        'COMPOSE_FILE': get_value('COMPOSE_FILE', ('docker-compose.yml')),
+        'COMPOSE_FILE': compose_file,
         'DOCKER_TAG': docker_tag,
-        'DOCKER_TARGET': get_value('DOCKER_TARGET', 'development'),
-        'DOCKER_PULL_POLICY': docker_pull_policy,
+        'DOCKER_TARGET': docker_target,
         'HOST_UID': get_value('HOST_UID', os.getuid()),
     }
 )