Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync storage with make up flow #22805

Merged
merged 2 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -62,7 +57,6 @@ tmp/*
.github
docs
private
storage
docker-bake.hcl
docker-compose*.yml
Dockerfile*
Expand Down
6 changes: 5 additions & 1 deletion .github/actions/run-docker/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }})
Expand All @@ -80,3 +81,4 @@ jobs:
services: ${{ matrix.services }}
compose_file: ${{ matrix.compose_file }}
run: ${{ matrix.run }}
data_backup_skip: ${{ matrix.data_backup_skip || 'true' }}
7 changes: 1 addition & 6 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions Makefile-docker
Original file line number Diff line number Diff line change
Expand Up @@ -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."

Expand Down
8 changes: 8 additions & 0 deletions docker-compose.ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,12 @@ services:
web:
extends:
service: worker
volumes:
- storage:/data/olympia/storage

nginx:
volumes:
- storage:/srv/storage

volumes:
storage:
2 changes: 1 addition & 1 deletion docker/nginx/addons.conf
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ server {

location /data/olympia/storage/ {
internal;
alias /srv/user-media/;
alias /srv/storage/;
}

location /static/ {
Expand Down
33 changes: 33 additions & 0 deletions src/olympia/amo/management/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
Expand Down Expand Up @@ -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)
4 changes: 4 additions & 0 deletions src/olympia/amo/management/commands/data_load.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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,
Expand Down
39 changes: 38 additions & 1 deletion src/olympia/amo/management/commands/data_seed.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import os
import shutil

from django.conf import settings
from django.core.management import call_command

Expand All @@ -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
Expand All @@ -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')
Expand All @@ -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)
45 changes: 43 additions & 2 deletions src/olympia/amo/tests/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
],
)
Empty file removed storage/files/.gitkeep
Empty file.
1 change: 0 additions & 1 deletion storage/shared_storage/tmp/addons/.gitignore

This file was deleted.

1 change: 0 additions & 1 deletion storage/shared_storage/tmp/data/.gitkeep

This file was deleted.

1 change: 0 additions & 1 deletion storage/shared_storage/tmp/file_viewer/.gitkeep

This file was deleted.

1 change: 0 additions & 1 deletion storage/shared_storage/tmp/guarded-addons/.gitkeep

This file was deleted.

1 change: 0 additions & 1 deletion storage/shared_storage/tmp/icon/.gitkeep

This file was deleted.

1 change: 0 additions & 1 deletion storage/shared_storage/tmp/log/.gitkeep

This file was deleted.

Empty file.
1 change: 0 additions & 1 deletion storage/shared_storage/tmp/preview/.gitkeep

This file was deleted.

1 change: 0 additions & 1 deletion storage/shared_storage/tmp/test/.gitkeep

This file was deleted.

1 change: 0 additions & 1 deletion storage/shared_storage/tmp/uploads/.gitkeep

This file was deleted.

1 change: 0 additions & 1 deletion storage/shared_storage/uploads/.check

This file was deleted.

Empty file.
Loading