Skip to content

Commit

Permalink
Sync storage with make up flow
Browse files Browse the repository at this point in the history
  • Loading branch information
KevinMind committed Oct 30, 2024
1 parent e85aba5 commit 6d2fdaf
Show file tree
Hide file tree
Showing 27 changed files with 99 additions and 45 deletions.
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
7 changes: 7 additions & 0 deletions docker-compose.ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ services:
web:
extends:
service: worker
volumes:
- storage:/data/olympia/storage

nginx:
volumes:
- storage:/srv/storage

volumes:
data_olympia:
storage:
10 changes: 1 addition & 9 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ services:
# and would otherwiser be deleted by mounbting data_olympia
- /data/olympia/static-build
- /data/olympia/site-static
- storage:/data/olympia/storage
- ./package.json:/deps/package.json
- ./package-lock.json:/deps/package-lock.json
extra_hosts:
Expand Down Expand Up @@ -99,8 +98,7 @@ services:
image: nginx
volumes:
- ./docker/nginx/addons.conf:/etc/nginx/conf.d/addons.conf
- ./static:/srv/static
- storage:/srv/user-media
- ./:/srv
ports:
- "80:80"
networks:
Expand Down Expand Up @@ -205,9 +203,3 @@ volumes:
type: none
o: bind
device: ${PWD}
storage:
driver: local
driver_opts:
type: none
o: bind
device: ./storage
4 changes: 2 additions & 2 deletions 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 All @@ -20,7 +20,7 @@ server {
}

location /user-media/ {
alias /srv/user-media/shared_storage/uploads/;
alias /srv/storage/shared_storage/uploads/;
}

location ~ ^/api/ {
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)
2 changes: 1 addition & 1 deletion src/olympia/amo/management/commands/initialize.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,4 @@ def handle(self, *args, **options):
# We should reindex even if no data is loaded/modified
# because we might have a fresh instance of elasticsearch
else:
call_command('reindex', '--wipe', '--force', '--noinput')
call_command('reindex', '--noinput', '--skip-if-exists')
11 changes: 6 additions & 5 deletions src/olympia/amo/tests/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,8 @@ class Commands:
data_seed = mock.call('data_seed')

flush = mock.call('flush', '--noinput')
reindex = mock.call('reindex', '--wipe', '--force', '--noinput')
reindex_force_wipe = mock.call('reindex', '--wipe', '--force', '--noinput')
reindex_skip_if_exists = mock.call('reindex', '--noinput', '--skip-if-exists')
load_initial_data = mock.call('loaddata', 'initial.json')
import_prod_versions = mock.call('import_prod_versions')
createsuperuser = mock.call(
Expand Down Expand Up @@ -504,7 +505,7 @@ def test_handle_without_clean_or_load_with_local_admin(self):
self.mocks['mock_call_command'],
[
self.mock_commands.migrate,
self.mock_commands.reindex,
self.mock_commands.reindex_skip_if_exists,
],
)

Expand Down Expand Up @@ -709,7 +710,7 @@ def test_loads_correct_path(self, mock_exists):
[
self.mock_commands.db_restore(db_path),
self.mock_commands.media_restore(storage_path),
self.mock_commands.reindex,
self.mock_commands.reindex_force_wipe,
],
)

Expand Down Expand Up @@ -760,7 +761,7 @@ def test_default(self):
self.mocks['mock_call_command'],
[
self.mock_commands.flush,
self.mock_commands.reindex,
self.mock_commands.reindex_force_wipe,
self.mock_commands.migrate,
self.mock_commands.load_initial_data,
self.mock_commands.import_prod_versions,
Expand All @@ -771,6 +772,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),
],
)
11 changes: 10 additions & 1 deletion src/olympia/search/management/commands/reindex.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ def add_arguments(self, parser):
help=('Do not ask for confirmation before wiping. Default: False'),
default=False,
)
parser.add_argument(
'--skip-if-exists',
action='store_true',
help=('Skip the reindex if the index already exists.'),
default=False,
)

def accepted_keys(self):
return ', '.join(settings.ES_INDEXES.keys())
Expand All @@ -129,7 +135,6 @@ def handle(self, *args, **kwargs):
"""
force = kwargs['force']

if Reindexing.objects.is_reindexing() and not force:
raise CommandError('Indexation already occurring - use --force to bypass')

Expand All @@ -141,6 +146,10 @@ def handle(self, *args, **kwargs):
)
self.stdout.write('Starting the reindexation for %s.' % alias)

if kwargs['skip_if_exists'] and ES.indices.exists(index=alias):
self.stdout.write('Index %s already exists. Skipping reindex.' % alias)
return

if kwargs['wipe']:
skip_confirmation = kwargs['noinput']
confirm = ''
Expand Down
21 changes: 19 additions & 2 deletions src/olympia/search/tests/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def get_indices_aliases(cls):
items.sort()
return items

def _test_reindexation(self, wipe=False):
def _test_reindexation(self, wipe=False, skip_if_exists=False):
# Current indices with aliases.
old_indices = self.get_indices_aliases()

Expand All @@ -113,7 +113,11 @@ def run(self):
# alias in setUpClass.
time.sleep(1)
management.call_command(
'reindex', wipe=wipe, noinput=True, stdout=self.stdout
'reindex',
wipe=wipe,
noinput=True,
skip_if_exists=skip_if_exists,
stdout=self.stdout,
)

t = ReindexThread()
Expand All @@ -125,6 +129,10 @@ def run(self):
while t.is_alive() and not Reindexing.objects.is_reindexing():
connection._commit()

if skip_if_exists:
assert not Reindexing.objects.is_reindexing()
return

if not wipe:
# We should still be able to search in the foreground while the
# reindex is being done in the background. We should also be able
Expand Down Expand Up @@ -229,3 +237,12 @@ def test_create_workflow_addons(self):
for addons.
"""
self._test_workflow('default')

@mock.patch('olympia.search.management.commands.reindex.ES')
def test_reindex_skip_if_exists(self, mock_es):
"""
Test reindex command with skip_if_exists option when index already exists.
"""
mock_es.indices.exists.return_value = True
self._test_reindexation(skip_if_exists=True)
assert mock_es.indices.create.call_count == 0
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.

0 comments on commit 6d2fdaf

Please sign in to comment.