Fix the mounts....

- Removed the deprecated docker-compose.ci.yml file, consolidating configurations into docker-compose.yml. - Updated environment variable mappings to use OLYMPIA_UID and OLYMPIA_MOUNT for improved clarity and consistency. - Enhanced entrypoint script to adjust user IDs based on the new environment variables. - Modified setup.py to determine the appropriate olympia mount based on the target environment. - Updated GitHub Actions workflows to reflect changes in environment variable usage and remove references to the old compose file.
mozilla · Dec 17, 2024 · fc63c15 · fc63c15
1 parent 079cc5b
commit fc63c15
Show file tree

Hide file tree

Showing 12 changed files with 186 additions and 174 deletions.
diff --git a/.github/actions/run-docker/action.yml b/.github/actions/run-docker/action.yml
@@ -12,17 +12,17 @@ inputs:
   run:
     description: 'Run command in container'
     required: true
-  compose_file:
-    description: 'The docker-compose file to use'
-    required: false
-    default: 'docker-compose.yml:docker-compose.ci.yml'
   logs:
     description: 'Show logs'
     required: false
   data_backup_skip:
     description: 'Skip data backup'
     required: false
     default: 'true'
+  mount:
+    description: 'Mount olympia files from host'
+    required: false
+    default: 'production'
 
 runs:
   using: 'composite'
@@ -34,15 +34,14 @@ runs:
 
     - name: Run Docker Container
       shell: bash
-      env:
-        DOCKER_VERSION: ${{ inputs.version }}
-        DOCKER_DIGEST: ${{ inputs.digest }}
-        COMPOSE_FILE: ${{ inputs.compose_file }}
-        HOST_UID: ${{ steps.id.outputs.id }}
-        DATA_BACKUP_SKIP: ${{ inputs.data_backup_skip }}
       run: |
         # Start the specified services
-        make up
+        make up \
+          DOCKER_VERSION="${{ inputs.version }}" \
+          DOCKER_DIGEST="${{ inputs.digest }}" \
+          OLYMPIA_UID="${{ steps.id.outputs.id }}" \
+          OLYMPIA_MOUNT="${{ inputs.mount }}" \
+          DATA_BACKUP_SKIP="${{ inputs.data_backup_skip }}"
 
         # Exec the run command in the container
         # quoted 'EOF' to prevent variable expansion

diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml
@@ -41,29 +41,24 @@ jobs:
           -
             name: Needs Locale Compilation
             services: ''
-            compose_file: docker-compose.yml:docker-compose.ci.yml
             run: |
               make compile_locales
               make test_needs_locales_compilation
           -
             name: Static Assets
             services: ''
-            compose_file: docker-compose.yml:docker-compose.ci.yml
             run: make test_static_assets
           -
             name: Internal Routes
             services: ''
-            compose_file: docker-compose.yml:docker-compose.ci.yml
             run: make test_internal_routes_allowed
           -
             name: Elastic Search
             services: ''
-            compose_file: docker-compose.yml:docker-compose.ci.yml
             run: make test_es_tests
           -
             name: Codestyle
             services: web
-            compose_file: docker-compose.yml:docker-compose.ci.yml
             run: make lint-codestyle
     steps:
       - uses: actions/checkout@v4
@@ -73,5 +68,4 @@ jobs:
           version: ${{ inputs.version }}
           digest: ${{ inputs.digest }}
           services: ${{ matrix.services }}
-          compose_file: ${{ matrix.compose_file }}
           run: ${{ matrix.run }}
diff --git a/.github/workflows/_test_check.yml b/.github/workflows/_test_check.yml
@@ -45,16 +45,16 @@ jobs:
     runs-on: ubuntu-latest
     name: |
       version: '${{ matrix.version }}' |
-      compose_file: '${{ matrix.compose_file }}'
+      mount: '${{ matrix.mount }}'
     strategy:
       fail-fast: false
       matrix:
         version:
           - local
           - ${{ inputs.version }}
-        compose_file:
-          - docker-compose.yml
-          - docker-compose.yml:docker-compose.ci.yml
+        mount:
+          - development
+          - production
     steps:
       - uses: actions/checkout@v4
       - shell: bash
@@ -63,7 +63,7 @@ jobs:
           cat <<EOF
             Values passed to the action:
             version: ${{ matrix.version }}
-            compose_file: ${{ matrix.compose_file }}
+            mount: ${{ matrix.mount }}
           EOF
       - name: ${{ matrix.version == 'local' && 'Uncached Build' || 'Pull' }} Check
         uses: ./.github/actions/run-docker
@@ -73,14 +73,14 @@ jobs:
           DOCKER_VERSION: 'not-expected'
         with:
           version: ${{ matrix.version }}
-          compose_file: ${{ matrix.compose_file }}
+          mount: ${{ matrix.mount }}
           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 }}
+          mount: ${{ matrix.mount }}
           run: echo true
 
   test_make_docker_configuration:

diff --git a/.github/workflows/_test_main.yml b/.github/workflows/_test_main.yml
@@ -88,7 +88,7 @@ jobs:
           services: ''
           digest: ${{ inputs.digest }}
           version: ${{ inputs.version }}
-          compose_file: docker-compose.yml
+          mount: development
           run: |
             split="--splits ${{ needs.test_config.outputs.splits }}"
             group="--group ${{ matrix.group }}"

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -88,7 +88,7 @@ jobs:
         with:
           digest: ${{ needs.build.outputs.digest }}
           version: ${{ needs.build.outputs.version }}
-          compose_file: docker-compose.yml
+          mount: development
           run: |
             make docs
 
@@ -139,7 +139,7 @@ jobs:
         with:
           digest: ${{ needs.build.outputs.digest }}
           version: ${{ needs.build.outputs.version }}
-          compose_file: docker-compose.yml
+          mount: development
           run: make extract_locales
 
       - name: Push Locales

diff --git a/docker-compose.ci.yml b/docker-compose.ci.yml
diff --git a/docker-compose.yml b/docker-compose.yml
@@ -20,9 +20,22 @@ x-env-mapping: &env
     - HISTIGNORE=ls:exit:"cd .."
     - HISTCONTROL=erasedups
     - CIRCLECI
-    - HOST_UID
-    - DEBUG
     - DATA_BACKUP_SKIP
+    # Map values saved in the .env file to the expected environment variables.
+    - OLYMPIA_UID=${HOST_UID}
+    - OLYMPIA_MOUNT=${HOST_MOUNT}
+
+# Value is determined by the setup.py script
+# production: (data_olympia):/data/olympia/storage
+# development: (./):/data/olympia/storage
+x-olympia-mount: &olympia-mount
+  ${OLYMPIA_MOUNT_SOURCE:?}:/data/olympia
+
+# Value is determined by the setup.py script
+# production: (data_olympia)_storage:/data/olympia/storage
+# development: (./)_storage:/data/olympia/storage
+x-storage-mount: &storage-mount
+  ${OLYMPIA_MOUNT_SOURCE:?}_storage:/data/olympia/storage
 
 x-site-static-mount: &site-static-mount
   data_site_static:/data/olympia/site-static
@@ -49,7 +62,10 @@ services:
     # so we just sleep indefinitely instead.
     command: ["sleep", "infinity"]
     volumes:
+      # used by: web, worker, nginx
+      - *olympia-mount
       - *site-static-mount
+      - *storage-mount
   worker:
     <<: *olympia
     command: [
@@ -63,11 +79,13 @@ services:
       "celery -A olympia.amo.celery:app worker -E -c 2 --loglevel=INFO",
     ]
     volumes:
-      - .:/data/olympia
+      - *olympia-mount
+      - *storage-mount
     extra_hosts:
      - "olympia.test:127.0.0.1"
     restart: on-failure:5
     depends_on:
+      - olympia_volumes
       - mysqld
       - elasticsearch
       - redis
@@ -85,15 +103,14 @@ services:
       # and would otherwiser be deleted by mounting the cwd volume above
       - data_static_build:/data/olympia/static-build
       - *site-static-mount
-    depends_on:
-      - olympia_volumes
 
   nginx:
     image: nginx
     volumes:
       - data_nginx:/etc/nginx/conf.d
-      - .:/srv
+      - *olympia-mount
       - *site-static-mount
+      - *storage-mount
     ports:
       - "80:80"
     networks:
@@ -196,6 +213,16 @@ volumes:
   # mounted from the host.
   data_static_build:
   data_site_static:
+  # Volume for the production olympia mounts
+  # allowing to conditionally mount directories
+  # from the host or from the image to <path>
+  # in the running docker container.
+  # If DATA_OLYMPIA_MOUNT_PREFIX matches (data_olympia)
+  # then we use the production volume mounts. Otherwise
+  # it will map to the current directory ./<name>
+  # (data_olympia)<name>:/<path>
+  data_olympia:
+  data_olympia_storage:
   # Volume for rabbitmq/redis to avoid anonymous volumes
   data_rabbitmq:
   data_redis:

diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh
@@ -6,7 +6,7 @@
 ### id of the olympia user sometimes should match the host user's id
 ### to avoid permission issues with mounted volumes.
 
-set -ue
+set -xue
 
 if [[ $(id -u) -ne 0 ]]; then
   echo "This script must be run as root"
@@ -18,13 +18,26 @@ OLYMPIA_USER="olympia"
 function get_olympia_uid() { echo "$(id -u "$OLYMPIA_USER")"; }
 function get_olympia_gid() { echo "$(id -g "$OLYMPIA_USER")"; }
 
-if [[ -n "${HOST_UID:-}" ]]; then
-  usermod -u ${HOST_UID} ${OLYMPIA_USER}
-  echo "${OLYMPIA_USER} UID: ${OLYMPIA_UID} -> ${HOST_UID}"
+OLD_OLYMPIA_UID=$(get_olympia_uid)
+
+# If the olympia user's uid is different in the container than from the build,
+# we need to update the olympia user's uid to match the new one.
+if [[ "${OLYMPIA_UID}" != "${OLD_OLYMPIA_UID}" ]]; then
+  usermod -u ${OLYMPIA_UID} ${OLYMPIA_USER}
+  echo "${OLYMPIA_USER} UID: ${OLD_OLYMPIA_UID} -> ${OLYMPIA_UID}"
+fi
+
+NEW_OLYMPIA_UID=$(get_olympia_uid)
+OLYMPIA_ID_STRING="${NEW_OLYMPIA_UID}:$(get_olympia_gid)"
+
+# If we are on production mode, update the ownership of /data/olympia and /deps to match the new id
+if [[ "${OLYMPIA_MOUNT}" == "production" ]]; then
+  echo "Updating ownership of /data/olympia and /deps to ${OLYMPIA_ID_STRING}"
+  chown -R ${OLYMPIA_ID_STRING} /data/olympia /deps
 fi
 
 cat <<EOF | su -s /bin/bash $OLYMPIA_USER
-  echo "Running command as ${OLYMPIA_USER} $(get_olympia_uid):$(get_olympia_gid)"
+  echo "Running command as ${OLYMPIA_USER} ${OLYMPIA_ID_STRING}"
   set -xue
   $@
 EOF
diff --git a/scripts/setup.py b/scripts/setup.py
@@ -65,6 +65,21 @@ def get_docker_tag():
 
     return tag, version, digest
 
+def get_olympia_mount(docker_target):
+    """
+    When running on production targets, the user can specify the olympia mount
+    to one of the valid values: development or production. In development, we
+    hard code the values to ensure we have necessary files and permissions.
+    """
+    olympia_mount = os.environ.get('OLYMPIA_MOUNT', docker_target)
+    olympia_mount_source = './'
+
+    if docker_target == 'production' and olympia_mount == 'production':
+        # This is the prefix of the volume name.
+        olympia_mount_source = 'data_olympia'
+
+    return olympia_mount, olympia_mount_source
+
 
 # Env file should contain values that are referenced in docker-compose*.yml files
 # so running docker compose commands produce consistent results in terminal and make.
@@ -94,17 +109,8 @@ def main():
 
     is_production = docker_target == 'production'
 
-    # The default value for which compose files to use is based on the target
-    # but can be freely overridden by the user.
-    # E.g running a production image in development mode with source code changes
-    compose_file = get_value(
-        'COMPOSE_FILE',
-        (
-            'docker-compose.yml:docker-compose.ci.yml'
-            if is_production
-            else 'docker-compose.yml'
-        ),
-    )
+    olympia_uid = os.getuid()
+    olympia_mount, olympia_mount_source = get_olympia_mount(docker_target)
 
     # DEBUG is special, as we should allow the user to override it
     # but we should not set a default to the previously set value but instead
@@ -113,10 +119,18 @@ def main():
 
     set_env_file(
         {
-            'COMPOSE_FILE': compose_file,
             'DOCKER_TAG': docker_tag,
             'DOCKER_TARGET': docker_target,
-            'HOST_UID': get_value('HOST_UID', os.getuid()),
+            # We save olympia_* values as host_* values to ensure that
+            # inputs can be recieved via environment variables, but that
+            # docker compose only reads the values explicitly set in the .env file.
+            # These values are mapped back to the olympia_* values in the environment
+            # of the container so everywhere they can be referenced as the user expects.
+            'HOST_UID': olympia_uid,
+            'HOST_MOUNT': olympia_mount,
+            # Save the docker compose volume name
+            # to use as the source of the /data/olympia volume
+            'OLYMPIA_MOUNT_SOURCE': olympia_mount_source,
             'DEBUG': debug,
         }
     )

diff --git a/settings.py b/settings.py
@@ -18,7 +18,8 @@
 # So if the value is anything other than "production" we are in development mode.
 DEV_MODE = DOCKER_TARGET != 'production'
 
-HOST_UID = os.environ.get('HOST_UID')
+OLYMPIA_UID = os.environ.get('OLYMPIA_UID')
+OLYMPIA_MOUNT = os.environ.get('OLYMPIA_MOUNT')
 
 WSGI_APPLICATION = 'olympia.wsgi.application'
 

diff --git a/src/olympia/lib/settings_base.py b/src/olympia/lib/settings_base.py
@@ -72,7 +72,8 @@ def path(*folders):
 DEV_MODE = False
 
 # Host info that is hard coded for production images.
-HOST_UID = None
+OLYMPIA_UID = None
+OLYMPIA_MOUNT = None
 
 # Used to determine if django should serve static files.
 # For local deployments we want nginx to proxy static file requests to the