Reorganize make up command: (#22938)

- Updated docker-compose files to use named volumes instead of bind mounts for better portability and consistency.
- Introduced a new healthcheck script to monitor the status of the Celery worker and web service.
- Removed deprecated healthcheck configurations from services in docker-compose.yml.
- Adjusted Makefile to better separate the up recipe and include new healtch check in initialize recipe
- Cleaned up Dockerfile by linking package.json and package-lock.json to the correct paths.

.github/workflows/_test_check.yml

@@ -65,7 +65,8 @@ jobs:
             version: ${{ matrix.version }}
             compose_file: ${{ matrix.compose_file }}
           EOF
-      - uses: ./.github/actions/run-docker
+      - name: ${{ matrix.version == 'local' && 'Uncached Build' || 'Pull' }} Check
+        uses: ./.github/actions/run-docker
         # Set environment variables that are expected to be ignored
         env:
           DOCKER_COMMIT: 'not-expected'
@@ -74,6 +75,13 @@ jobs:
           version: ${{ matrix.version }}
           compose_file: ${{ matrix.compose_file }}
           run: make check
+      - name: Cached Build Check
+        uses: ./.github/actions/run-docker
+        if: ${{ matrix.version == 'local' }}
+        with:
+          version: ${{ matrix.version }}
+          compose_file: ${{ matrix.compose_file }}
+          run: echo true
 
   test_make_docker_configuration:
     runs-on: ubuntu-latest

Dockerfile

@@ -57,10 +57,6 @@ chown -R olympia:olympia /deps
 ln -s /deps/bin/uwsgi /usr/bin/uwsgi
 ln -s /usr/bin/uwsgi /usr/sbin/uwsgi
 
-# link to the package*.json at ${HOME} so npm can install in /deps
-ln -s ${HOME}/package.json /deps/package.json
-ln -s ${HOME}/package-lock.json /deps/package-lock.json
-
 # Create the storage directory and the test file to verify nginx routing
 mkdir -p ${HOME}/storage
 chown -R olympia:olympia ${HOME}/storage
@@ -109,8 +105,8 @@ RUN \
     # Files required to install pip dependencies
     --mount=type=bind,source=./requirements/prod.txt,target=${HOME}/requirements/prod.txt \
     # Files required to install npm dependencies
-    --mount=type=bind,source=package.json,target=${HOME}/package.json \
-    --mount=type=bind,source=package-lock.json,target=${HOME}/package-lock.json \
+    --mount=type=bind,source=package.json,target=/deps/package.json \
+    --mount=type=bind,source=package-lock.json,target=/deps/package-lock.json \
     # Mounts for caching dependencies
     --mount=type=cache,target=${PIP_CACHE_DIR},uid=${OLYMPIA_UID},gid=${OLYMPIA_UID} \
     --mount=type=cache,target=${NPM_CACHE_DIR},uid=${OLYMPIA_UID},gid=${OLYMPIA_UID} \
@@ -126,8 +122,8 @@ RUN \
     --mount=type=bind,source=./requirements/prod.txt,target=${HOME}/requirements/prod.txt \
     --mount=type=bind,source=./requirements/dev.txt,target=${HOME}/requirements/dev.txt \
     # Files required to install npm dependencies
-    --mount=type=bind,source=package.json,target=${HOME}/package.json \
-    --mount=type=bind,source=package-lock.json,target=${HOME}/package-lock.json \
+    --mount=type=bind,source=package.json,target=/deps/package.json \
+    --mount=type=bind,source=package-lock.json,target=/deps/package-lock.json \
     # Mounts for caching dependencies
     --mount=type=cache,target=${PIP_CACHE_DIR},uid=${OLYMPIA_UID},gid=${OLYMPIA_UID} \
     --mount=type=cache,target=${NPM_CACHE_DIR},uid=${OLYMPIA_UID},gid=${OLYMPIA_UID} \

Makefile-docker

@@ -108,6 +108,7 @@ initialize: ## ensure database exists
 	@echo "Initializing data..."
 	@echo "args: $(ARGS)"
 	$(PYTHON_COMMAND) ./manage.py initialize $(ARGS)
+	./scripts/healthcheck.py
 
 PYTEST_SRC := src/olympia/
 

Makefile-os

@@ -25,8 +25,6 @@ ifneq ($(INIT_LOAD),)
 	INITIALIZE_ARGS += --load $(INIT_LOAD)
 endif
 
-
-
 DOCKER_BAKE_ARGS := \
 	--file docker-bake.hcl \
 	--file .env \
@@ -43,7 +41,6 @@ DOCKER_COMPOSE_ARGS := \
 	--remove-orphans \
 	--no-build \
 	--quiet-pull \
-	--renew-anon-volumes
 
 # Paths should be cleaned before mounting .:/data/olympia
 # These are files which should be sourced from the container
@@ -150,17 +147,23 @@ docker_clean_build_cache: ## Remove buildx build cache
 .PHONY: clean_docker
 clean_docker: docker_compose_down docker_mysqld_volume_remove docker_clean_images docker_clean_volumes docker_clean_build_cache ## Remove all docker resources taking space on the host machine
 
-.PHONY: docker_compose_up
-docker_compose_up: docker_mysqld_volume_create ## Start the docker containers
+.PHONY: up_pre
+up_pre: setup docker_pull_or_build ## Pre-up the environment, setup files, volumes and host state
+
+.PHONY: up_start
+up_start: docker_mysqld_volume_create ## Start the docker containers
 	docker compose up $(DOCKER_COMPOSE_ARGS) $(ARGS)
 
-.PHONY: up
-up: setup docker_pull_or_build docker_compose_up docker_clean_images docker_clean_volumes ## Create and start docker compose
-	# Explicitly run initialize via the web container as make can get confused
-	# both routing the command to the web container and
-	# routing the command to the proper target.
+.PHONY: up_post
+up_post: docker_clean_images docker_clean_volumes ## Post-up the environment, setup files, volumes and host state
+# Explicitly run initialize via the web container as make can get confused
+# both routing the command to the web container and
+# routing the command to the proper target.
 	docker compose exec --user olympia web make -f Makefile-docker initialize ARGS=$(shell echo "'$(INITIALIZE_ARGS)'")
 
+.PHONY: up
+up: up_pre up_start up_post ## Up the environment
+
 .PHONY: down
 down: docker_compose_down docker_clean_images docker_clean_volumes ## Stop the docker containers and clean up non-peristent dangling resources
 

docker-compose.ci.yml

@@ -4,7 +4,7 @@ services:
       - HOST_UID=9500
     volumes:
       - storage:/data/olympia/storage
-      - /data/olympia
+      - data_olympia_production:/data/olympia
 
   worker:
     extends:
@@ -17,9 +17,11 @@ services:
 
   nginx:
     volumes:
+      - data_olympia_production:/srv
       - storage:/srv/storage
     depends_on:
       - olympia_volumes
 
 volumes:
   storage:
+  data_olympia_production:

docker-compose.yml

@@ -50,6 +50,7 @@ services:
     command: ["sleep", "infinity"]
     volumes:
       - *site-static-mount
+      - data_olympia_development:/data/olympia
   worker:
     <<: *olympia
     command: [
@@ -63,24 +64,12 @@ services:
       "celery -A olympia.amo.celery:app worker -E -c 2 --loglevel=INFO",
     ]
     volumes:
-      - .:/data/olympia
-
+      - data_olympia_development:/data/olympia
     extra_hosts:
      - "olympia.test:127.0.0.1"
     restart: on-failure:5
-    # entrypoint.sh takes some time
-    # we can wait for services to be up and running
-    healthcheck:
-      test: ["CMD-SHELL", "DJANGO_SETTINGS_MODULE=olympia celery -A olympia.amo.celery status"]
-      # The interval is 90s after the start period of 60s
-      interval: 90s
-      # 3 failed attempts result in container failure
-      retries: 3
-      # While starting, ping faster to get the container healthy as soon as possible
-      start_interval: 1s
-      # The start period is 60s
-      start_period: 120s
     depends_on:
+      - olympia_volumes
       - mysqld
       - elasticsearch
       - redis
@@ -91,29 +80,21 @@ services:
   web:
     extends:
       service: worker
-    healthcheck:
-      test: ["CMD-SHELL", "curl --fail --show-error --include --location http://127.0.0.1:8002/__version__"]
-      retries: 3
-      interval: 90s
-      start_interval: 1s
-      start_period: 120s
     command:
       - uwsgi --ini /data/olympia/docker/uwsgi.ini
     volumes:
       # Don't mount generated files. They only exist in the container
       # and would otherwiser be deleted by mounting the cwd volume above
-      - /data/olympia/static-build
+      - data_static_build:/data/olympia/static-build
       - *site-static-mount
-      - ./package.json:/deps/package.json
-      - ./package-lock.json:/deps/package-lock.json
     depends_on:
       - olympia_volumes
 
   nginx:
     image: nginx
     volumes:
-      - ./docker/nginx/addons.conf:/etc/nginx/conf.d/addons.conf
-      - .:/srv
+      - data_nginx:/etc/nginx/conf.d
+      - data_olympia_development:/srv
       - *site-static-mount
     ports:
       - "80:80"
@@ -138,9 +119,22 @@ services:
     - "3306:3306"
     volumes:
       - data_mysqld:/var/lib/mysql
+    command:
+      # Optimize for development speed over durability
+      - --innodb-flush-log-at-trx-commit=0
+      - --innodb-buffer-pool-size=64M
+      - --innodb-log-buffer-size=8M
+      - --innodb-log-file-size=32M
+      # Skip DNS lookups
+      - --skip-name-resolve
+      # Disable performance schema for faster startup
+      - --performance-schema=OFF
     healthcheck:
-      test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "--silent"]
+      test: ["CMD-SHELL", "mysql -u root --silent --execute='SELECT 1;'"]
       start_interval: 1s
+      timeout: 2s
+      start_period: 10s
+      retries: 3
 
   elasticsearch:
     image: docker.elastic.co/elasticsearch/elasticsearch:7.17.3
@@ -173,9 +167,9 @@ services:
   autograph:
     image: mozilla/autograph:3.3.2
     platform: linux/amd64
-    command: /go/bin/autograph -c /autograph_localdev_config.yaml
+    command: /go/bin/autograph -c /data/autograph/autograph_localdev_config.yaml
     volumes:
-      - ./scripts/autograph_localdev_config.yaml:/autograph_localdev_config.yaml
+      - data_autograph:/data/autograph
 
   addons-frontend:
     <<: *env
@@ -196,13 +190,39 @@ services:
 
 networks:
   default:
+    driver: bridge
+    enable_ipv6: false
 
 volumes:
   # Volumes for static files that should not be
   # mounted from the host.
+  data_static_build:
   data_site_static:
+  # Volume for rabbitmq/redis to avoid anonymous volumes
+  data_rabbitmq:
+  data_redis:
+  data_olympia_development:
+    driver: local
+    driver_opts:
+      type: none
+      o: bind
+      device: ${PWD}
   data_mysqld:
     # Keep this value in sync with Makefile-os
     # External volumes must be manually created/destroyed
     name: addons-server_data_mysqld
     external: true
+  # Volume for nginx configuration
+  data_nginx:
+    driver: local
+    driver_opts:
+      type: none
+      o: bind
+      device: ${PWD}/docker/nginx
+  # Volume for autograph configuration
+  data_autograph:
+    driver: local
+    driver_opts:
+      type: none
+      o: bind
+      device: ${PWD}/docker/autograph

scripts/healthcheck.py

@@ -0,0 +1,50 @@
+#!/usr/bin/env python3
+
+import os
+import subprocess
+import sys
+import time
+
+
+env = os.environ.copy()
+
+env['DJANGO_SETTINGS_MODULE'] = 'olympia'
+
+
+def worker_healthcheck():
+    subprocess.run(
+        ['celery', '-A', 'olympia.amo.celery', 'status'],
+        env=env,
+        stdout=subprocess.DEVNULL,
+    )
+
+
+def web_healthcheck():
+    subprocess.run(
+        [
+            'curl',
+            '--fail',
+            '--show-error',
+            '--include',
+            '--location',
+            '--silent',
+            'http://127.0.0.1:8002/__version__',
+        ],
+        stdout=subprocess.DEVNULL,
+    )
+
+
+TIME = time.time()
+TIMEOUT = 60
+SLEEP = 1
+
+while time.time() - TIME < TIMEOUT:
+    try:
+        worker_healthcheck()
+        web_healthcheck()
+        print('OK')
+        sys.exit(0)
+    except Exception as e:
+        print(f'Error: {e}')
+        time.sleep(SLEEP)
+        SLEEP *= 2

tests/make/make.spec.js

@@ -76,11 +76,7 @@ describe('docker-compose.yml', () => {
         expect(service.extra_hosts).toStrictEqual(['olympia.test=127.0.0.1']);
         expect(service.restart).toStrictEqual('on-failure:5');
         // Each service should have a healthcheck
-        expect(service.healthcheck.test).not.toBeUndefined();
-        expect(service.healthcheck.interval).toStrictEqual('1m30s');
-        expect(service.healthcheck.retries).toStrictEqual(3);
-        expect(service.healthcheck.start_interval).toStrictEqual('1s');
-        expect(service.healthcheck.start_period).toStrictEqual('2m0s');
+        expect(service).not.toHaveProperty('healthcheck');
         // each service should have a command
         expect(service.command).not.toBeUndefined();
         // each service should have the same dependencies
@@ -124,11 +120,11 @@ describe('docker-compose.yml', () => {
       expect(web.volumes).toEqual(
         expect.arrayContaining([
           expect.objectContaining({
-            type: 'volume',
+            source: 'data_static_build',
             target: '/data/olympia/static-build',
           }),
           expect.objectContaining({
-            type: 'volume',
+            source: 'data_site_static',
             target: '/data/olympia/site-static',
           }),
         ]),
@@ -154,8 +150,8 @@ describe('docker-compose.yml', () => {
         expect.arrayContaining([
           // mapping for nginx conf.d adding addon-server routing
           expect.objectContaining({
-            source: expect.any(String),
-            target: '/etc/nginx/conf.d/addons.conf',
+            source: 'data_nginx',
+            target: '/etc/nginx/conf.d',
           }),
           // mapping for local host directory to /data/olympia
           expect.objectContaining({
@@ -220,6 +216,25 @@ describe('docker-compose.yml', () => {
         }
       }
     });
+
+    it('.services.*.volumes does not contain anonymous or unnamed volumes', () => {
+      const { services } = getConfig({ COMPOSE_FILE });
+      for (let [name, config] of Object.entries(services)) {
+        for (let volume of config.volumes ?? []) {
+          if (volume.bind) {
+            throw new Error(
+              `'.services.${name}.volumes' contains anonymous bind mount: ` +
+                `'${volume.source}:${volume.target}'. Please use a named volume mount instead.`,
+            );
+          } else if (!volume.source) {
+            throw new Error(
+              `'.services.${name}.volumes' contains unnamed volume mount: ` +
+                `'${volume.target}'. Please use a named volume mount instead.`,
+            );
+          }
+        }
+      }
+    });
   });
 
   // these keys require special handling to prevent runtime errors in make setup