From b3d0a224f17bc91579255b109308c38520ca355e Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Tue, 12 Nov 2024 16:32:27 +0100 Subject: [PATCH] Sync storage with make up flow (#22805) * Sync storage with make up flow * data_load should clean storage directory --- .dockerignore | 8 +--- .github/actions/run-docker/action.yml | 6 ++- .github/workflows/_test.yml | 2 + .gitignore | 7 +-- Dockerfile | 4 ++ Makefile-docker | 2 + docker-compose.ci.yml | 8 ++++ docker/nginx/addons.conf | 2 +- src/olympia/amo/management/__init__.py | 33 ++++++++++++++ .../amo/management/commands/data_load.py | 4 ++ .../amo/management/commands/data_seed.py | 39 +++++++++++++++- src/olympia/amo/tests/test_commands.py | 45 ++++++++++++++++++- storage/files/.gitkeep | 0 storage/shared_storage/tmp/addons/.gitignore | 1 - storage/shared_storage/tmp/data/.gitkeep | 1 - .../shared_storage/tmp/file_viewer/.gitkeep | 1 - .../tmp/guarded-addons/.gitkeep | 1 - storage/shared_storage/tmp/icon/.gitkeep | 1 - storage/shared_storage/tmp/log/.gitkeep | 1 - .../tmp/persona_header/.gitkeep | 0 storage/shared_storage/tmp/preview/.gitkeep | 1 - storage/shared_storage/tmp/test/.gitkeep | 1 - storage/shared_storage/tmp/uploads/.gitkeep | 1 - storage/shared_storage/uploads/.check | 1 - storage/shared_storage/uploads/.gitkeep | 0 25 files changed, 142 insertions(+), 28 deletions(-) delete mode 100644 storage/files/.gitkeep delete mode 100644 storage/shared_storage/tmp/addons/.gitignore delete mode 100644 storage/shared_storage/tmp/data/.gitkeep delete mode 100644 storage/shared_storage/tmp/file_viewer/.gitkeep delete mode 100644 storage/shared_storage/tmp/guarded-addons/.gitkeep delete mode 100644 storage/shared_storage/tmp/icon/.gitkeep delete mode 100644 storage/shared_storage/tmp/log/.gitkeep delete mode 100644 storage/shared_storage/tmp/persona_header/.gitkeep delete mode 100644 storage/shared_storage/tmp/preview/.gitkeep delete mode 100644 storage/shared_storage/tmp/test/.gitkeep delete mode 100644 storage/shared_storage/tmp/uploads/.gitkeep delete mode 100644 storage/shared_storage/uploads/.check delete mode 100644 storage/shared_storage/uploads/.gitkeep diff --git a/.dockerignore b/.dockerignore index a50ec16c2a..10e1a2d627 100644 --- a/.dockerignore +++ b/.dockerignore @@ -48,12 +48,7 @@ src/olympia/discovery/strings.jinja2 static-build/* static/css/node_lib/* static/js/node_lib/* -storage/files/* -storage/git-storage/* -storage/guarded-addons/* -storage/mlbf/* -storage/shared_storage/* -storage/sitemaps/* +storage tmp/* # Additionally ignore these files from the docker build that are not in .gitignore @@ -62,7 +57,6 @@ tmp/* .github docs private -storage docker-bake.hcl docker-compose*.yml Dockerfile* diff --git a/.github/actions/run-docker/action.yml b/.github/actions/run-docker/action.yml index a1029964e2..cf7bafaa31 100644 --- a/.github/actions/run-docker/action.yml +++ b/.github/actions/run-docker/action.yml @@ -19,6 +19,10 @@ inputs: logs: description: 'Show logs' required: false + data_backup_skip: + description: 'Skip data backup' + required: false + default: 'true' runs: using: 'composite' @@ -35,7 +39,7 @@ runs: DOCKER_DIGEST: ${{ inputs.digest }} COMPOSE_FILE: ${{ inputs.compose_file }} HOST_UID: ${{ steps.id.outputs.id }} - DATA_BACKUP_SKIP: true + DATA_BACKUP_SKIP: ${{ inputs.data_backup_skip }} run: | # Start the specified services make up diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml index 260e220e50..cf2477cbf5 100644 --- a/.github/workflows/_test.yml +++ b/.github/workflows/_test.yml @@ -70,6 +70,7 @@ jobs: services: web nginx compose_file: docker-compose.yml:docker-compose.ci.yml run: make check + data_backup_skip: true steps: - uses: actions/checkout@v4 - name: Test (${{ matrix.name }}) @@ -80,3 +81,4 @@ jobs: services: ${{ matrix.services }} compose_file: ${{ matrix.compose_file }} run: ${{ matrix.run }} + data_backup_skip: ${{ matrix.data_backup_skip || 'true' }} diff --git a/.gitignore b/.gitignore index ee75887a99..08eb3abdd8 100644 --- a/.gitignore +++ b/.gitignore @@ -47,12 +47,7 @@ src/olympia/discovery/strings.jinja2 static-build/* static/css/node_lib/* static/js/node_lib/* -storage/files/* -storage/git-storage/* -storage/guarded-addons/* -storage/mlbf/* -storage/shared_storage/* -storage/sitemaps/* +storage tmp/* # End of .gitignore. Please keep this in sync with the top section of .dockerignore diff --git a/Dockerfile b/Dockerfile index a1e58b08ab..63e27e9585 100644 --- a/Dockerfile +++ b/Dockerfile @@ -60,6 +60,10 @@ 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 EOF USER olympia:olympia diff --git a/Makefile-docker b/Makefile-docker index 05d87b8fcf..4c1a2ce80c 100644 --- a/Makefile-docker +++ b/Makefile-docker @@ -53,6 +53,8 @@ check_django: ## check if the django app is configured properly .PHONY: check_nginx check_nginx: ## check if the nginx config for local development is configured properly + mkdir -p /data/olympia/storage/shared_storage/uploads + echo "OK" > /data/olympia/storage/shared_storage/uploads/.check @if [ "$$(curl -sf http://nginx/user-media/.check)" != "OK" ]; then echo "Requesting http://nginx/user-media/.check failed"; exit 1; fi @echo "Nginx user-media configuration looks correct." diff --git a/docker-compose.ci.yml b/docker-compose.ci.yml index 519116402e..fb093e2576 100644 --- a/docker-compose.ci.yml +++ b/docker-compose.ci.yml @@ -8,4 +8,12 @@ services: web: extends: service: worker + volumes: + - storage:/data/olympia/storage + nginx: + volumes: + - storage:/srv/storage + +volumes: + storage: diff --git a/docker/nginx/addons.conf b/docker/nginx/addons.conf index d4493caee7..97eb1678f1 100644 --- a/docker/nginx/addons.conf +++ b/docker/nginx/addons.conf @@ -7,7 +7,7 @@ server { location /data/olympia/storage/ { internal; - alias /srv/user-media/; + alias /srv/storage/; } location /static/ { diff --git a/src/olympia/amo/management/__init__.py b/src/olympia/amo/management/__init__.py index 86ac1a9b04..1ba869ecaa 100644 --- a/src/olympia/amo/management/__init__.py +++ b/src/olympia/amo/management/__init__.py @@ -150,6 +150,26 @@ class ProcessObjectsCommand(BaseCommand): ts.apply_async() +storage_structure = { + 'files': '', + 'shared_storage': { + 'tmp': { + 'addons': '', + 'data': '', + 'file_viewer': '', + 'guarded-addons': '', + 'icon': '', + 'log': '', + 'persona_header': '', + 'preview': '', + 'test': '', + 'uploads': '', + }, + 'uploads': '', + }, +} + + class BaseDataCommand(BaseCommand): # Settings for django-dbbackup data_backup_dirname = os.path.abspath(os.path.join(settings.ROOT, 'backups')) @@ -190,3 +210,16 @@ class BaseDataCommand(BaseCommand): ) os.makedirs(path, exist_ok=True) + + def _clean_storage(self, root: str, dir_dict: dict[str, str | dict]) -> None: + for key, value in dir_dict.items(): + curr_path = os.path.join(root, key) + if isinstance(value, dict): + self._clean_storage(curr_path, value) + else: + shutil.rmtree(curr_path, ignore_errors=True) + os.makedirs(curr_path, exist_ok=True) + + def clean_storage(self): + self.logger.info('Cleaning storage...') + self._clean_storage(settings.STORAGE_ROOT, storage_structure) diff --git a/src/olympia/amo/management/commands/data_load.py b/src/olympia/amo/management/commands/data_load.py index 663ff652d8..4973409f63 100644 --- a/src/olympia/amo/management/commands/data_load.py +++ b/src/olympia/amo/management/commands/data_load.py @@ -1,5 +1,6 @@ import os +from django.core.cache import cache from django.core.management import call_command from django.core.management.base import CommandError @@ -36,6 +37,9 @@ class Command(BaseDataCommand): if not os.path.exists(storage_path): raise CommandError(f'Storage backup not found: {storage_path}') + cache.clear() + self.clean_storage() + call_command( 'mediarestore', input_path=storage_path, diff --git a/src/olympia/amo/management/commands/data_seed.py b/src/olympia/amo/management/commands/data_seed.py index 713504cb80..07ac5ac6ec 100644 --- a/src/olympia/amo/management/commands/data_seed.py +++ b/src/olympia/amo/management/commands/data_seed.py @@ -1,3 +1,6 @@ +import os +import shutil + from django.conf import settings from django.core.management import call_command @@ -10,6 +13,39 @@ class Command(BaseDataCommand): 'generated add-ons, and data from AMO production.' ) + def _clean_storage(self, root: str, dir_dict: dict[str, str | dict]) -> None: + for key, value in dir_dict.items(): + curr_path = os.path.join(root, key) + if isinstance(value, dict): + self._clean_storage(curr_path, value) + else: + shutil.rmtree(curr_path, ignore_errors=True) + os.makedirs(curr_path, exist_ok=True) + + def clean_storage(self): + self.logger.info('Cleaning storage...') + self._clean_storage( + settings.STORAGE_ROOT, + { + 'files': '', + 'shared_storage': { + 'tmp': { + 'addons': '', + 'data': '', + 'file_viewer': '', + 'guarded-addons': '', + 'icon': '', + 'log': '', + 'persona_header': '', + 'preview': '', + 'test': '', + 'uploads': '', + }, + 'uploads': '', + }, + }, + ) + def handle(self, *args, **options): num_addons = 10 num_themes = 5 @@ -18,6 +54,7 @@ class Command(BaseDataCommand): self.logger.info('Resetting database...') call_command('flush', '--noinput') + self.clean_storage() # reindex --wipe will force the ES mapping to be re-installed. call_command('reindex', '--wipe', '--force', '--noinput') call_command('migrate', '--noinput') @@ -43,4 +80,4 @@ class Command(BaseDataCommand): call_command('generate_default_addons_for_frontend') call_command('data_dump', '--name', self.data_backup_init) - call_command('reindex', '--wipe', '--force', '--noinput') + call_command('data_load', '--name', self.data_backup_init) diff --git a/src/olympia/amo/tests/test_commands.py b/src/olympia/amo/tests/test_commands.py index 9a09ca44c6..18fabb2a7b 100644 --- a/src/olympia/amo/tests/test_commands.py +++ b/src/olympia/amo/tests/test_commands.py @@ -13,7 +13,7 @@ import pytest from freezegun import freeze_time from olympia.addons.models import Preview -from olympia.amo.management import BaseDataCommand +from olympia.amo.management import BaseDataCommand, storage_structure from olympia.amo.management.commands.get_changed_files import ( collect_addon_icons, collect_addon_previews, @@ -622,6 +622,40 @@ class TestBaseDataCommand(BaseTestDataCommand): mock_exists.assert_called_with(backup_path) mock_makedirs.assert_called_with(backup_path, exist_ok=True) + @mock.patch('olympia.amo.management.shutil.rmtree') + @mock.patch('olympia.amo.management.os.makedirs') + def test_clean_storage(self, mock_makedirs, mock_rmtree): + self.base_data_command.clean_storage() + + def walk_keys(root, dir_dict): + for key, value in dir_dict.items(): + if isinstance(value, dict): + walk_keys(os.path.join(root, key), value) + else: + keys.append(os.path.join(root, key)) + + keys = [] + walk_keys(settings.STORAGE_ROOT, storage_structure) + + assert keys == [ + os.path.join(settings.STORAGE_ROOT, 'files'), + os.path.join(settings.STORAGE_ROOT, 'shared_storage/tmp/addons'), + os.path.join(settings.STORAGE_ROOT, 'shared_storage/tmp/data'), + os.path.join(settings.STORAGE_ROOT, 'shared_storage/tmp/file_viewer'), + os.path.join(settings.STORAGE_ROOT, 'shared_storage/tmp/guarded-addons'), + os.path.join(settings.STORAGE_ROOT, 'shared_storage/tmp/icon'), + os.path.join(settings.STORAGE_ROOT, 'shared_storage/tmp/log'), + os.path.join(settings.STORAGE_ROOT, 'shared_storage/tmp/persona_header'), + os.path.join(settings.STORAGE_ROOT, 'shared_storage/tmp/preview'), + os.path.join(settings.STORAGE_ROOT, 'shared_storage/tmp/test'), + os.path.join(settings.STORAGE_ROOT, 'shared_storage/tmp/uploads'), + os.path.join(settings.STORAGE_ROOT, 'shared_storage/uploads'), + ] + + for key in keys: + assert mock.call(key, ignore_errors=True) in mock_rmtree.mock_calls + assert mock.call(key, exist_ok=True) in mock_makedirs.mock_calls + class TestDumpDataCommand(BaseTestDataCommand): def setUp(self): @@ -694,6 +728,13 @@ class TestLoadDataCommand(BaseTestDataCommand): with pytest.raises(CommandError): call_command('data_load') + @mock.patch('olympia.amo.management.commands.data_load.os.path.exists') + @mock.patch('olympia.amo.management.commands.data_load.cache.clear') + def test_clear_cache(self, mock_clear_cache, mock_exists): + mock_exists.return_value = True + call_command('data_load', name='test_backup') + mock_clear_cache.assert_called_once() + @mock.patch('olympia.amo.management.commands.data_load.os.path.exists') def test_loads_correct_path(self, mock_exists): mock_exists.return_value = True @@ -771,6 +812,6 @@ class TestSeedDataCommand(BaseTestDataCommand): self.mock_commands.generate_themes(5), self.mock_commands.generate_default_addons_for_frontend, self.mock_commands.data_dump(self.base_data_command.data_backup_init), - self.mock_commands.reindex, + self.mock_commands.data_load(self.base_data_command.data_backup_init), ], ) diff --git a/storage/files/.gitkeep b/storage/files/.gitkeep deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/storage/shared_storage/tmp/addons/.gitignore b/storage/shared_storage/tmp/addons/.gitignore deleted file mode 100644 index 72e8ffc0db..0000000000 --- a/storage/shared_storage/tmp/addons/.gitignore +++ /dev/null @@ -1 +0,0 @@ -* diff --git a/storage/shared_storage/tmp/data/.gitkeep b/storage/shared_storage/tmp/data/.gitkeep deleted file mode 100644 index 9096300f21..0000000000 --- a/storage/shared_storage/tmp/data/.gitkeep +++ /dev/null @@ -1 +0,0 @@ -stub file to create an empty dir diff --git a/storage/shared_storage/tmp/file_viewer/.gitkeep b/storage/shared_storage/tmp/file_viewer/.gitkeep deleted file mode 100644 index 9096300f21..0000000000 --- a/storage/shared_storage/tmp/file_viewer/.gitkeep +++ /dev/null @@ -1 +0,0 @@ -stub file to create an empty dir diff --git a/storage/shared_storage/tmp/guarded-addons/.gitkeep b/storage/shared_storage/tmp/guarded-addons/.gitkeep deleted file mode 100644 index 9096300f21..0000000000 --- a/storage/shared_storage/tmp/guarded-addons/.gitkeep +++ /dev/null @@ -1 +0,0 @@ -stub file to create an empty dir diff --git a/storage/shared_storage/tmp/icon/.gitkeep b/storage/shared_storage/tmp/icon/.gitkeep deleted file mode 100644 index 9096300f21..0000000000 --- a/storage/shared_storage/tmp/icon/.gitkeep +++ /dev/null @@ -1 +0,0 @@ -stub file to create an empty dir diff --git a/storage/shared_storage/tmp/log/.gitkeep b/storage/shared_storage/tmp/log/.gitkeep deleted file mode 100644 index 9096300f21..0000000000 --- a/storage/shared_storage/tmp/log/.gitkeep +++ /dev/null @@ -1 +0,0 @@ -stub file to create an empty dir diff --git a/storage/shared_storage/tmp/persona_header/.gitkeep b/storage/shared_storage/tmp/persona_header/.gitkeep deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/storage/shared_storage/tmp/preview/.gitkeep b/storage/shared_storage/tmp/preview/.gitkeep deleted file mode 100644 index 9096300f21..0000000000 --- a/storage/shared_storage/tmp/preview/.gitkeep +++ /dev/null @@ -1 +0,0 @@ -stub file to create an empty dir diff --git a/storage/shared_storage/tmp/test/.gitkeep b/storage/shared_storage/tmp/test/.gitkeep deleted file mode 100644 index 9096300f21..0000000000 --- a/storage/shared_storage/tmp/test/.gitkeep +++ /dev/null @@ -1 +0,0 @@ -stub file to create an empty dir diff --git a/storage/shared_storage/tmp/uploads/.gitkeep b/storage/shared_storage/tmp/uploads/.gitkeep deleted file mode 100644 index 9096300f21..0000000000 --- a/storage/shared_storage/tmp/uploads/.gitkeep +++ /dev/null @@ -1 +0,0 @@ -stub file to create an empty dir diff --git a/storage/shared_storage/uploads/.check b/storage/shared_storage/uploads/.check deleted file mode 100644 index d86bac9de5..0000000000 --- a/storage/shared_storage/uploads/.check +++ /dev/null @@ -1 +0,0 @@ -OK diff --git a/storage/shared_storage/uploads/.gitkeep b/storage/shared_storage/uploads/.gitkeep deleted file mode 100644 index e69de29bb2..0000000000