Sync storage with make up flow (#22805)

* Sync storage with make up flow

* data_load should clean storage directory

.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*

.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

.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' }}

.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

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

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."
 

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:

docker/nginx/addons.conf

@@ -7,7 +7,7 @@ server {
 
     location /data/olympia/storage/ {
         internal;
-        alias /srv/user-media/;
+        alias /srv/storage/;
     }
 
     location /static/ {

src/olympia/amo/management/__init__.py

@@ -150,6 +150,26 @@ def handle(self, *args, **options):
             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 @@ def make_dir(self, name: str, force: bool = False) -> None:
                 )
 
         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)

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 @@ def handle(self, *args, **options):
         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,

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 @@ def handle(self, *args, **options):
 
         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 @@ def handle(self, *args, **options):
         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)

src/olympia/amo/tests/test_commands.py

@@ -13,7 +13,7 @@
 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 @@ def test_make_dir_non_existing_path(self, mock_makedirs, mock_exists):
         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 @@ def test_missing_name(self):
         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 @@ def test_default(self):
                 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),
             ],
         )