diff --git a/.github/actions/run-docker/action.yml b/.github/actions/run-docker/action.yml index 1619f498eb9..dea9fdd6490 100644 --- a/.github/actions/run-docker/action.yml +++ b/.github/actions/run-docker/action.yml @@ -23,10 +23,6 @@ inputs: description: 'Docker target to run (development|production)' required: false default: 'production' - mount: - description: 'Mount host files at runtime? (development|production)' - required: false - default: 'production' deps: description: 'Which dependencies to install at runtime? (development|production)' required: false @@ -44,7 +40,6 @@ runs: DOCKER_DIGEST="${{ inputs.digest }}" \ DOCKER_TARGET="${{ inputs.target }}" \ OLYMPIA_UID="$(id -u)" \ - OLYMPIA_MOUNT="${{ inputs.mount }}" \ OLYMPIA_DEPS="${{ inputs.deps }}" \ DATA_BACKUP_SKIP="${{ inputs.data_backup_skip }}" \ DOCKER_WAIT="true" diff --git a/.github/workflows/_test_check.yml b/.github/workflows/_test_check.yml index eebd0f90028..88ec6f90ecb 100644 --- a/.github/workflows/_test_check.yml +++ b/.github/workflows/_test_check.yml @@ -46,7 +46,6 @@ jobs: name: | version: '${{ matrix.version }}' | target: '${{ matrix.target }}' | - mount: '${{ matrix.mount }}' | deps: '${{ matrix.deps }}' strategy: fail-fast: false @@ -57,9 +56,6 @@ jobs: target: - development - production - mount: - - development - - production deps: - development - production @@ -72,7 +68,6 @@ jobs: Values passed to the action: version: ${{ matrix.version }} target: ${{ matrix.target }} - mount: ${{ matrix.mount }} deps: ${{ matrix.deps }} EOF - name: ${{ matrix.version == 'local' && 'Uncached Build' || 'Pull' }} Check @@ -84,7 +79,6 @@ jobs: with: version: ${{ matrix.version }} target: ${{ matrix.target }} - mount: ${{ matrix.mount }} deps: ${{ matrix.deps }} run: make check - name: Cached Build Check @@ -93,7 +87,6 @@ jobs: with: version: ${{ matrix.version }} target: ${{ matrix.target }} - mount: ${{ matrix.mount }} deps: ${{ matrix.deps }} run: echo true diff --git a/.github/workflows/_test_main.yml b/.github/workflows/_test_main.yml index 38457e047a1..cd69c91de69 100644 --- a/.github/workflows/_test_main.yml +++ b/.github/workflows/_test_main.yml @@ -88,7 +88,6 @@ jobs: services: '' digest: ${{ inputs.digest }} version: ${{ inputs.version }} - mount: development deps: development run: | split="--splits ${{ needs.test_config.outputs.splits }}" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d782354e334..743afda17c0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,7 +87,6 @@ jobs: with: digest: ${{ needs.build.outputs.digest }} version: ${{ needs.build.outputs.version }} - mount: development deps: development target: development run: | @@ -140,7 +139,6 @@ jobs: with: digest: ${{ needs.build.outputs.digest }} version: ${{ needs.build.outputs.version }} - mount: development deps: development run: make extract_locales diff --git a/docker-compose.yml b/docker-compose.yml index 0f1ee62aa9a..7e4caaaf81f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -45,10 +45,7 @@ services: command: ["sleep", "infinity"] volumes: # used by: web, worker, nginx - - ${HOST_MOUNT_SOURCE:?}:/data/olympia - - ${HOST_MOUNT_SOURCE:?}deps:/data/olympia/deps - - ./site-static:/data/olympia/site-static - - ./storage:/data/olympia/storage + - ./:/data/olympia worker: <<: *olympia command: [ @@ -63,9 +60,7 @@ services: "celery -A olympia.amo.celery:app worker -E -c 2 --loglevel=INFO", ] volumes: - - ${HOST_MOUNT_SOURCE:?}:/data/olympia - - ${HOST_MOUNT_SOURCE:?}deps:/data/olympia/deps - - ./storage:/data/olympia/storage + - ./:/data/olympia extra_hosts: - "olympia.test:127.0.0.1" restart: on-failure:5 @@ -93,17 +88,12 @@ services: interval: 30s retries: 3 start_interval: 1s - volumes: - - ./static-build:/data/olympia/static-build - - ./site-static:/data/olympia/site-static nginx: image: nginx volumes: - data_nginx:/etc/nginx/conf.d - - ${HOST_MOUNT_SOURCE:?}:/srv - - ./site-static:/srv/site-static - - ./storage:/srv/storage + - ./:/srv ports: - "80:80" networks: @@ -202,16 +192,6 @@ networks: enable_ipv6: false volumes: - # Volumes for the production olympia mounts - # allowing to conditionally mount directories - # from the host or from the image to - # in the running docker container. - # If OLYMPIA_MOUNT_SOURCE matches (data_olympia_) - # then we use the production volume mounts. Otherwise - # it will map to the current directory ./ - # (data_olympia_):/ - data_olympia_: - data_olympia_deps: # Volume for rabbitmq/redis to avoid anonymous volumes data_rabbitmq: data_redis: diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 1a8bf6764fd..06aca156b3c 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -30,12 +30,6 @@ fi NEW_HOST_UID=$(get_olympia_uid) OLYMPIA_ID_STRING="${NEW_HOST_UID}:$(get_olympia_gid)" -# If we are on production mode, update the ownership of /data/olympia to match the new id -if [[ "${HOST_MOUNT}" == "production" ]]; then - echo "Updating ownership of /data/olympia to ${OLYMPIA_ID_STRING}" - chown -R ${OLYMPIA_ID_STRING} /data/olympia -fi - cat < { + if (!acc.length) return values.map((value) => ({ [key]: value })); + return acc.flatMap((obj) => + values.map((value) => ({ ...obj, [key]: value })), + ); + }, []); +} + describe('docker-compose.yml', () => { afterAll(() => { clearEnv(); }); - describe.each([ - ['development', 'development'], - ['development', 'production'], - ['production', 'development'], - ['production', 'production'], - ])('DOCKER_TARGET=%s, OLYMPIA_MOUNT=%s', (DOCKER_TARGET, OLYMPIA_MOUNT) => { - const isProdTarget = DOCKER_TARGET === 'production'; - const isProdMount = OLYMPIA_MOUNT === 'production'; - const isProdMountTarget = isProdMount && isProdTarget; + describe.each( + permutations({ + DOCKER_TARGET: ['development', 'production'], + DOCKER_VERSION: ['local', 'latest'], + }), + )('\n%s\n', (config) => { + const { DOCKER_TARGET, DOCKER_VERSION } = config; const inputValues = { DOCKER_TARGET, - OLYMPIA_MOUNT, - DOCKER_TAG: 'mozilla/addons-server:tag', + DOCKER_VERSION, DEBUG: 'debug', DATA_BACKUP_SKIP: 'skip', }; it('.services.(web|worker) should have the correct configuration', () => { const { - services: { web, worker }, + config: { + services: { web, worker }, + }, + env: { DOCKER_TAG }, } = getConfig(inputValues); for (let service of [web, worker]) { - expect(service.image).toStrictEqual(inputValues.DOCKER_TAG); + expect(service.image).toStrictEqual(DOCKER_TAG); expect(service.pull_policy).toStrictEqual('never'); expect(service.user).toStrictEqual('root'); expect(service.platform).toStrictEqual('linux/amd64'); @@ -100,17 +112,14 @@ describe('docker-compose.yml', () => { ); expect(service.volumes).toEqual( expect.arrayContaining([ - expect.objectContaining({ - source: isProdMountTarget ? 'data_olympia_' : expect.any(String), - target: '/data/olympia', - }), expect.objectContaining({ source: expect.any(String), - target: '/data/olympia/storage', + target: '/data/olympia', }), ]), ); - const { OLYMPIA_MOUNT, ...environmentOutput } = inputValues; + const { DOCKER_VERSION, DOCKER_TARGET, ...environmentOutput } = + inputValues; expect(service.environment).toEqual( expect.objectContaining({ ...environmentOutput, @@ -119,24 +128,13 @@ describe('docker-compose.yml', () => { // We excpect not to pass the input values to the container expect(service.environment).not.toHaveProperty('OLYMPIA_UID'); } - - expect(web.volumes).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - source: expect.any(String), - target: '/data/olympia/static-build', - }), - expect.objectContaining({ - source: expect.any(String), - target: '/data/olympia/site-static', - }), - ]), - ); }); it('.services.nginx should have the correct configuration', () => { const { - services: { nginx }, + config: { + services: { nginx }, + }, } = getConfig(inputValues); // nginx is mapped from http://olympia.test to port 80 in /etc/hosts on the host expect(nginx.ports).toStrictEqual([ @@ -154,19 +152,10 @@ describe('docker-compose.yml', () => { source: 'data_nginx', target: '/etc/nginx/conf.d', }), - // mapping for local host directory to /data/olympia - expect.objectContaining({ - source: isProdMountTarget ? 'data_olympia_' : expect.any(String), - target: '/srv', - }), - expect.objectContaining({ - source: expect.any(String), - target: '/srv/site-static', - }), - // mapping for local host directory to /data/olympia/storage + // mapping for /data/olympia/ directory to /srv expect.objectContaining({ source: expect.any(String), - target: '/srv/storage', + target: '/srv', }), ]), ); @@ -174,7 +163,9 @@ describe('docker-compose.yml', () => { it('.services.*.volumes duplicate volumes should be defined on services.olympia_volumes.volumes', () => { const key = 'olympia_volumes'; - const { services } = getConfig(inputValues); + const { + config: { services }, + } = getConfig(inputValues); // all volumes defined on any service other than olympia const volumesMap = new Map(); // volumes defined on the olympia service, any dupes in other services should be here also @@ -219,7 +210,9 @@ describe('docker-compose.yml', () => { }); it('.services.*.volumes does not contain anonymous or unnamed volumes', () => { - const { services } = getConfig(inputValues); + const { + config: { services }, + } = getConfig(inputValues); for (let [name, config] of Object.entries(services)) { for (let volume of config.volumes ?? []) { if (!volume.bind && !volume.source) { @@ -238,7 +231,9 @@ describe('docker-compose.yml', () => { // and should not be able to deviate from the state at build time. it('.services.(web|worker).environment excludes build info variables', () => { const { - services: { web, worker }, + config: { + services: { web, worker }, + }, } = getConfig({ ...inputValues, ...Object.fromEntries(EXCLUDED_KEYS.map((key) => [key, 'filtered'])), @@ -255,16 +250,12 @@ describe('docker-compose.yml', () => { const failKeys = [ // Invalid docker tag leads to docker not parsing the image 'DOCKER_TAG', - // Value is read directly as the volume source for /data/olympia and must be valid - 'HOST_MOUNT_SOURCE', ]; const ignoreKeys = [ // Ignored because these values are explicitly mapped to the host_* values 'OLYMPIA_UID', - 'OLYMPIA_MOUNT', // Ignored because the HOST_UID is always set to the host user's UID 'HOST_UID', - 'HOST_MOUNT', ]; const defaultEnv = runSetup(); const customValue = 'custom'; @@ -281,7 +272,9 @@ describe('docker-compose.yml', () => { } else if (ignoreKeys.includes(key)) { it('variable should be ignored', () => { const { - services: { web, worker }, + config: { + services: { web, worker }, + }, } = getConfig({ ...defaultEnv, [key]: customValue }); for (let service of [web, worker]) { expect(service.environment).not.toEqual( @@ -294,7 +287,9 @@ describe('docker-compose.yml', () => { } else { it('variable should be overriden based on the input', () => { const { - services: { web, worker }, + config: { + services: { web, worker }, + }, } = getConfig({ ...defaultEnv, [key]: customValue }); for (let service of [web, worker]) { expect(service.environment).toEqual( diff --git a/tests/make/test_setup.py b/tests/make/test_setup.py index d4d94b559e3..ed08f7aa8b4 100644 --- a/tests/make/test_setup.py +++ b/tests/make/test_setup.py @@ -2,7 +2,7 @@ import unittest from unittest import mock -from scripts.setup import get_docker_tag, main +from scripts.setup import get_docker_image_meta, main from tests import override_env @@ -10,8 +10,6 @@ 'DOCKER_TAG', 'DOCKER_TARGET', 'HOST_UID', - 'HOST_MOUNT', - 'HOST_MOUNT_SOURCE', 'OLYMPIA_DEPS', 'DEBUG', ] @@ -35,83 +33,103 @@ def setUp(self): @override_env() class TestGetDockerTag(BaseTestClass): def test_default_value_is_local(self): - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'mozilla/addons-server:local') + self.assertEqual(target, 'development') self.assertEqual(version, 'local') self.assertEqual(digest, None) + with override_env(DOCKER_TARGET='production'): + _, target, _, _ = get_docker_image_meta() + self.assertEqual(target, 'production') + @override_env(DOCKER_VERSION='test') def test_version_overrides_default(self): - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'mozilla/addons-server:test') + self.assertEqual(target, 'production') self.assertEqual(version, 'test') self.assertEqual(digest, None) @override_env(DOCKER_DIGEST='sha256:123') def test_digest_overrides_version_and_default(self): - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'mozilla/addons-server@sha256:123') + self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'sha256:123') with override_env(DOCKER_VERSION='test', DOCKER_DIGEST='sha256:123'): - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'mozilla/addons-server@sha256:123') + self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'sha256:123') @override_env(DOCKER_TAG='image:latest') def test_tag_overrides_default_version(self): - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'image:latest') + self.assertEqual(target, 'production') self.assertEqual(version, 'latest') self.assertEqual(digest, None) with override_env(DOCKER_TAG='image:latest', DOCKER_VERSION='test'): - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'image:test') + self.assertEqual(target, 'production') self.assertEqual(version, 'test') self.assertEqual(digest, None) @override_env(DOCKER_TAG='image@sha256:123') def test_tag_overrides_default_digest(self): - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'image@sha256:123') + self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'sha256:123') with mock.patch.dict(os.environ, {'DOCKER_DIGEST': 'test'}): - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'image@test') + self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'test') def test_version_from_env_file(self): self.mock_get_env_file.return_value = {'DOCKER_TAG': 'image:latest'} - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'image:latest') + self.assertEqual(target, 'production') self.assertEqual(version, 'latest') self.assertEqual(digest, None) def test_digest_from_env_file(self): self.mock_get_env_file.return_value = {'DOCKER_TAG': 'image@sha256:123'} - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'image@sha256:123') + self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'sha256:123') @override_env(DOCKER_VERSION='') def test_default_when_version_is_empty(self): - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'mozilla/addons-server:local') + self.assertEqual(target, 'development') self.assertEqual(version, 'local') self.assertEqual(digest, None) + with override_env(DOCKER_VERSION='', DOCKER_TARGET='production'): + _, target, _, _ = get_docker_image_meta() + self.assertEqual(target, 'production') + @override_env(DOCKER_DIGEST='', DOCKER_TAG='image@sha256:123') def test_default_when_digest_is_empty(self): self.mock_get_env_file.return_value = {'DOCKER_TAG': 'image@sha256:123'} - tag, version, digest = get_docker_tag() + tag, target, version, digest = get_docker_image_meta() self.assertEqual(tag, 'image@sha256:123') + self.assertEqual(target, 'production') self.assertEqual(version, None) self.assertEqual(digest, 'sha256:123') @@ -164,44 +182,6 @@ def test_debug_override(self): self.assert_set_env_file_called_with(DEBUG='test') -@override_env() -class TestHostMount(BaseTestClass): - def test_default_host_mount(self): - main() - self.assert_set_env_file_called_with( - HOST_MOUNT='development', HOST_MOUNT_SOURCE='./' - ) - - @override_env(DOCKER_TARGET='production') - def test_host_mount_set_by_docker_target(self): - main() - self.assert_set_env_file_called_with( - HOST_MOUNT='production', HOST_MOUNT_SOURCE='data_olympia_' - ) - - @override_env(DOCKER_TARGET='production', OLYMPIA_MOUNT='test') - def test_invalid_host_mount_set_by_env_ignored(self): - main() - self.assert_set_env_file_called_with( - HOST_MOUNT='production', HOST_MOUNT_SOURCE='data_olympia_' - ) - - @override_env(DOCKER_TARGET='development', OLYMPIA_MOUNT='production') - def test_host_mount_overriden_by_development_target(self): - main() - self.assert_set_env_file_called_with( - HOST_MOUNT='development', HOST_MOUNT_SOURCE='./' - ) - - @override_env(DOCKER_TARGET='production') - def test_host_mount_from_file_ignored(self): - self.mock_get_env_file.return_value = {'HOST_MOUNT': 'development'} - main() - self.assert_set_env_file_called_with( - HOST_MOUNT='production', HOST_MOUNT_SOURCE='data_olympia_' - ) - - @override_env() class TestOlympiaDeps(BaseTestClass): def test_default_olympia_deps(self):