From 7e1fbc271ca82ebee155198bcdb881a9600ea4c6 Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Tue, 24 Sep 2024 20:49:21 +0200 Subject: [PATCH 1/4] Integrate initialize command into make up flow Rename "initialize_data" to "initialize" Remove DOCKER_SERVICES as dangerous option given hard dependency on mysqld and elasticsearch. Better timing for mysql healthcheck --- .github/actions/run-docker/action.yml | 7 +- Makefile-docker | 60 +------- Makefile-os | 27 ++-- docker-compose.yml | 11 ++ docs/topics/development/data_management.md | 74 ++++++++-- docs/topics/development/makefile_commands.md | 10 +- .../development/setup_and_configuration.md | 68 +++++---- settings.py | 1 + .../amo/management/commands/create_db.py | 57 -------- .../amo/management/commands/initialize.py | 58 ++++++++ src/olympia/amo/tests/test_commands.py | 129 ++++++++++++++++++ 11 files changed, 323 insertions(+), 179 deletions(-) delete mode 100644 src/olympia/amo/management/commands/create_db.py create mode 100644 src/olympia/amo/management/commands/initialize.py diff --git a/.github/actions/run-docker/action.yml b/.github/actions/run-docker/action.yml index c45cd675300..a1029964e29 100644 --- a/.github/actions/run-docker/action.yml +++ b/.github/actions/run-docker/action.yml @@ -12,10 +12,6 @@ inputs: run: description: 'Run command in container' required: true - services: - description: 'List of services to start' - required: false - default: 'web' compose_file: description: 'The docker-compose file to use' required: false @@ -23,6 +19,7 @@ inputs: logs: description: 'Show logs' required: false + runs: using: 'composite' steps: @@ -37,8 +34,8 @@ runs: DOCKER_VERSION: ${{ inputs.version }} DOCKER_DIGEST: ${{ inputs.digest }} COMPOSE_FILE: ${{ inputs.compose_file }} - DOCKER_SERVICES: ${{ inputs.services }} HOST_UID: ${{ steps.id.outputs.id }} + DATA_BACKUP_SKIP: true run: | # Start the specified services make up diff --git a/Makefile-docker b/Makefile-docker index fe6214d9c39..55b03e33e91 100644 --- a/Makefile-docker +++ b/Makefile-docker @@ -62,22 +62,6 @@ data_dump: data_load: ./manage.py data_load $(ARGS) -.PHONY: initialize_db -initialize_db: ## create a new database - rm -rf ./user-media/* ./tmp/* - ./manage.py create_db --force - ./manage.py migrate --noinput - # Seed the database with initial data - ./manage.py data_seed - -.PHONY: reindex_data -reindex_data: ## reindex the data in elasticsearch - $(PYTHON_COMMAND) manage.py reindex --force --noinput - -.PHONY: update_db -update_db: ## run the database migrations - $(PYTHON_COMMAND) manage.py migrate --noinput - .PHONY: update_assets update_assets: # Copy files required in compress_assets to the static folder @@ -87,45 +71,6 @@ update_assets: # Collect static files: This MUST be run last or files will be missing $(PYTHON_COMMAND) manage.py collectstatic --noinput -.PHONY: update -update: update_db update_assets ## update the dependencies, the database, and assets - -.PHONY: reindex -reindex: ## reindex everything in elasticsearch, for AMO - $(PYTHON_COMMAND) manage.py reindex $(ARGS) - -.PHONY: setup-ui-tests -setup-ui-tests: - rm -rf ./user-media/* ./tmp/* - # Reset the database and fake database migrations - $(PYTHON_COMMAND) manage.py create_db --force - $(PYTHON_COMMAND) manage.py migrate --noinput - - # Reindex - $(PYTHON_COMMAND) manage.py reindex --force --noinput --wipe - - # Let's load some initial data and import mozilla-product versions - $(PYTHON_COMMAND) manage.py loaddata initial.json - $(PYTHON_COMMAND) manage.py loaddata zadmin/users - $(PYTHON_COMMAND) manage.py loaddata src/olympia/access/fixtures/initial.json - $(PYTHON_COMMAND) manage.py import_prod_versions - - # Create a proper superuser that can be used to access the API - $(PYTHON_COMMAND) manage.py waffle_switch super-create-accounts on --create - $(PYTHON_COMMAND) manage.py waffle_switch activate-autograph-signing on --create - $(PYTHON_COMMAND) manage.py generate_addons --app firefox $(NUM_ADDONS) - $(PYTHON_COMMAND) manage.py generate_addons --app android $(NUM_ADDONS) - $(PYTHON_COMMAND) manage.py generate_themes $(NUM_THEMES) - $(PYTHON_COMMAND) manage.py generate_default_addons_for_frontend - - # Now that addons have been generated, reindex. - $(PYTHON_COMMAND) manage.py reindex --force --noinput - -.PHONY: perf-tests -perf-tests: setup-ui-tests - $(PIP_COMMAND) install --progress-bar=off --no-deps -r requirements/perftests.txt - locust --no-web -c 1 -f tests/performance/locustfile.py --host "http://olympia.test" - .PHONY: lint lint: ## lint the code ruff check . @@ -148,7 +93,10 @@ dbshell: ## connect to a database shell $(PYTHON_COMMAND) ./manage.py dbshell .PHONY: initialize -initialize: initialize_db update_assets reindex_data ## init the dependencies, the database, and assets +initialize: ## ensure database exists + @echo "Initializing data..." + @echo "args: $(ARGS)" + $(PYTHON_COMMAND) ./manage.py initialize $(ARGS) PYTEST_SRC := src/olympia/ diff --git a/Makefile-os b/Makefile-os index 20c85dd0432..fa981627a30 100644 --- a/Makefile-os +++ b/Makefile-os @@ -11,11 +11,22 @@ export DEBUG ?= True export DOCKER_COMMIT ?= export DOCKER_BUILD ?= export DOCKER_VERSION ?= +export DATA_BACKUP_SKIP ?= override DOCKER_MYSQLD_VOLUME = addons-server_data_mysqld -override BACKUPS_DIR = $(shell pwd)/backups -override EXPORT_DIR = $(BACKUPS_DIR)/$(shell date +%Y%m%d%H%M%S) -RESTORE_DIR ?= $(BACKUPS_DIR)/$(shell ls -1 backups | sort -r | head -n 1) +INITIALIZE_ARGS ?= +INIT_CLEAN ?= +INIT_LOAD ?= + +ifneq ($(INIT_CLEAN),) + INITIALIZE_ARGS += --clean +endif + +ifneq ($(INIT_LOAD),) + INITIALIZE_ARGS += --load $(INIT_LOAD) +endif + + DOCKER_BAKE_ARGS := \ --file docker-bake.hcl \ @@ -135,18 +146,18 @@ clean_docker: docker_compose_down docker_mysqld_volume_remove docker_clean_image .PHONY: docker_compose_up docker_compose_up: docker_mysqld_volume_create ## Start the docker containers - docker compose up $(DOCKER_SERVICES) $(DOCKER_COMPOSE_ARGS) $(ARGS) + docker compose up $(DOCKER_COMPOSE_ARGS) $(ARGS) .PHONY: up up: setup docker_pull_or_build docker_compose_up docker_clean_images docker_clean_volumes ## Create and start docker compose + # Explicitly run initialize via the web container as make can get confused + # both routing the command to the web container and + # routing the command to the proper target. + docker compose exec --user olympia web make -f Makefile-docker initialize ARGS=$(shell echo "'$(INITIALIZE_ARGS)'") .PHONY: down down: docker_compose_down docker_clean_images docker_clean_volumes ## Stop the docker containers and clean up non-peristent dangling resources -.PHONY: initialize_docker -initialize_docker: up - docker compose exec --user olympia web make initialize - %: ## This directs any other recipe (command) to the web container's make. docker compose exec --user olympia web make $(MAKECMDGOALS) ARGS="$(shell echo $(ARGS))" diff --git a/docker-compose.yml b/docker-compose.yml index cf3c637f96a..196ad8c143f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -22,6 +22,7 @@ x-env-mapping: &env - CIRCLECI - HOST_UID - DEBUG + - DATA_BACKUP_SKIP x-olympia: &olympia <<: *env @@ -74,6 +75,13 @@ services: start_interval: 1s # The start period is 60s start_period: 120s + depends_on: + - mysqld + - elasticsearch + - redis + - memcached + - rabbitmq + - autograph web: <<: *worker @@ -116,6 +124,9 @@ services: - "3306:3306" volumes: - data_mysqld:/var/lib/mysql + healthcheck: + test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "--silent"] + start_interval: 1s elasticsearch: image: docker.elastic.co/elasticsearch/elasticsearch:7.17.3 diff --git a/docs/topics/development/data_management.md b/docs/topics/development/data_management.md index e39f263b0da..8d46d00c87d 100644 --- a/docs/topics/development/data_management.md +++ b/docs/topics/development/data_management.md @@ -10,30 +10,76 @@ The project uses persistent data volumes to store MySQL data. This ensures that The use of an external mount allows for manual management of the data lifecycle. This ensures that data is preserved even if you run `make down`. By defining the MySQL data volume as external, it decouples the data lifecycle from the container lifecycle, allowing you to manually manage the data. -## Data Population +## Data Initialization -The `make initialize_docker` command handles initial data population, including creating the database, running migrations, and seeding the database. +When you run `make up` make will run the `initialize` command for you. This command will check if the database exists, and if the elasticsearch index exists. -If you already have running containers, you can just run `make initialize` to reset the database, populate data, and reindex. +If they don't exist it will create them. This command can be run manually as well. -- **Database Initialization**: +```sh +make initialize +``` + +This command is responsible for ensuring your local mysql database is migrated, seeded, loaded with data and indexed. +There are a number of different ways to execute this command. In most cases, the default behavior is what you want. +But there are a few additional edge cases that it supports. + +### Clean the database ```sh - make initialize_docker + make initialize INIT_CLEAN=true ``` -- **Command Breakdown**: - - **`make up`**: Starts the Docker containers. - - **`make initialize`**: Runs database migrations and seeds the database with initial data. + This will force the database to be recreated, and re-initialized. + +### Load a data backup + + ```sh + make initialize [INIT_LOAD=] + ``` + + This command will load a data backup from a specified path. The optional `INIT_LOAD` argument allows you to + specify the path to the data backup file. If not specified, the initialize command will determine if + data should be loaded based on the current state of the databse, and will load the `_init` data backup. + +### Skip seeding + +```sh +make initialize INIT_SKIP_SEED=true +``` + +This will skip the seeding of the database. This can be useful in CI or if you specifically +want to avoid touching the previous data or creating a new _init backup. + +### Skip index recreation + +```sh +make initialize INIT_SKIP_INDEX=true +``` + +This will skip the recreation of the elasticsearch index. This can be useful in CI or if you specifically +want to avoid touching the previous elasticsearch index. + +> NOTE: if your database is modified significantly and you don't re-index elasticsearch you could end up with +> a broken addons-frontend. + +## Data seeding + +`addons-server` uses a a data seeding mechanism to populate the database with the initial data. This data is used to +bootstrap the database with addons and other data to enable development. + +The data seed is treted just like a data backup with a special name `_init`. To recreate the dataseed run: + +```sh +make seed_data +``` -The `make initialize` command, executed as part of `make initialize_docker`, performs the following steps: +This will flush the current database, remove the _init backup directory if it exists, run the seed commands, +and finally dump the data back into the _init backup directory. -1. **Create Database**: Sets up the initial database schema. -2. **Run Migrations**: Applies any pending database migrations. -3. **Seed Database**: Inserts initial data into the database. -4. **Reindex**: Rebuilds the search index in Elasticsearch. +The _init backup is used to populate the database with initial data during the initialization process. -## Exporting and Loading Data Snapshots +## Data backups You can export and load data snapshots to manage data states across different environments or for backup purposes. The Makefile provides commands to facilitate this. diff --git a/docs/topics/development/makefile_commands.md b/docs/topics/development/makefile_commands.md index f7b24809f5d..4b5a03e3be9 100644 --- a/docs/topics/development/makefile_commands.md +++ b/docs/topics/development/makefile_commands.md @@ -102,15 +102,7 @@ A common benefit of using Makefiles in this manner is the ability to coordinate make data_restore ``` -2. **`initialize_docker`**: - - **Purpose**: Sets up the initial Docker environment, including database initialization and data population. - - **Usage**: - - ```sh - make initialize_docker - ``` - -3. **`build_docker_image`**: +2. **`build_docker_image`**: - **Purpose**: Builds the Docker image using BuildKit and Bake. - **Usage**: diff --git a/docs/topics/development/setup_and_configuration.md b/docs/topics/development/setup_and_configuration.md index 605e2d925aa..6bf83b4ca98 100644 --- a/docs/topics/development/setup_and_configuration.md +++ b/docs/topics/development/setup_and_configuration.md @@ -15,49 +15,31 @@ Follow these steps to get started: cd addons-server ``` -(running-for-the-first-time)= -## Running for the first time - -When running the project for the first time, execute: - -```sh -make initialize_docker -``` - -This command will run: - -- `make up` to start the Docker containers. -- `make initialize` to set up the initial Docker environment, including database initialization and data population. -Detailed steps for `make initialize` will be covered in Section 6 on Data Management. - -If you run `make up` without running `make initialize` the docker compose services will be running, but you will not have a database -and the app might crash or otherwise be unusable. - -Similarly, you can run `make initialize` even after you have an up and running environment, but this will totally reset your database -as if you were running the application fresh. - -## Updating your environment +## Running the docker compose project > TLDR; Just run `make up`. The _make up_ command ensures all necessary files are created on the host and starts the Docker Compose project, -including volumes, containers, and networks. It is meant to be run frequently whenever you want to bring your environment "up". +including volumes, containers, networks, databases and indexes. +It is meant to be run frequently whenever you want to bring your environment "up". Here's a high-level overview of what _make up_ does: ```make .PHONY: up -up: setup docker_pull_or_build docker_compose_up docker_clean_images docker_clean_volumes ## Create and start docker compose +up: setup docker_pull_or_build docker_compose_up docker_clean_images docker_clean_volumes data ``` - **setup**: Creates configuration files such as `.env` and `version.json`. - **docker_pull_or_build**: Pulls or builds the Docker image based on the image version. - **docker_compose_up**: Starts the Docker containers defined in [docker-compose.yml][docker-compose]. - **docker_clean_images** and **docker_clean_volumes**: Cleans up unused Docker images and volumes. +- **data**: Ensures the database, seed, and index are created. -What happens if you run `make up` when your environment is already running? -This will result in all services and volumes being recreated as if starting them for the first time, -and will clear any local state from the containers. The `make up` command is {ref}`idempotent ` so you can run it over and over. +What happens if you run `make up` when your environment is already running?. +Well that depends on what is changed since the last time you ran it. +Because `make up` is {ref}`idempotent ` it will only run the commands that are necessary to bring your environment up to date. +If nothing has changed, nothing will happen because your environment is already in the desired state. ## Shutting down your environment @@ -70,6 +52,19 @@ Running `make down` will free up resources on your machine and can help if your A common solution to many problems is to run `make down && make up`. +> NOTE: When you run make down, it will clear all volumes except the data_mysqld volume. +> This is where your database and other persisted data is stored. +> If you want to start fresh, you can delete the data_mysqld volume. + +```sh +make down +make docker_mysqld_volume_remove # Remove the mysql database volume +make up +``` + +If you want to completely nuke your environment and start over as if you had just cloned the repo, +you can run `make clean_docker`. This will `make down` and remove all docker resources taking space on the host machine. + ### Accessing the Development App - Add the following entry to your `/etc/hosts` file to access **addons-server** via a local domain: @@ -219,11 +214,11 @@ Another way to find out what's wrong is to run `docker compose logs`. ### Getting "Programming error [table] doesn't exist"? -Make sure you've run the `make initialize_docker` step as {ref}`detailed ` in the initial setup instructions. +Make sure you've run `make up`. ### ConnectionError during initialize (elasticsearch container fails to start) -When running `make initialize_docker` without a working elasticsearch container, you'll get a ConnectionError. Check the logs with `docker compose logs`. If elasticsearch is complaining about `vm.max_map_count`, run this command on your computer or your docker-machine VM: +When running `make up` without a working elasticsearch container, you'll get a ConnectionError. Check the logs with `docker compose logs`. If elasticsearch is complaining about `vm.max_map_count`, run this command on your computer or your docker-machine VM: ```sh sudo sysctl -w vm.max_map_count=262144 @@ -233,7 +228,7 @@ This allows processes to allocate more [memory map areas](https://stackoverflow. ### Connection to elasticsearch timed out (elasticsearch container exits with code 137) -`docker compose up -d` brings up all containers, but running `make initialize_docker` causes the elasticsearch container to go down. Running `docker compose ps` shows _Exited (137)_ against it. +`docker compose up -d` brings up all containers, but running `make up` causes the elasticsearch container to go down. Running `docker compose ps` shows _Exited (137)_ against it. Update default settings in Docker Desktop - we suggest increasing RAM limit to at least 4 GB in the Resources/Advanced section and click on "Apply and Restart". @@ -386,3 +381,16 @@ make: *** [docker_pull_or_build] Error 2 ``` To fix this error, run `docker context use default` to switch to the default builder context. + +### Failing make up due to invalid or failing migrations + +Every time you run `make up` it will run migrations. If you have failing migrations, +this will cause the make command to fail. However, if migrations are running, it means the containers are already up. + +You can inspect and fix the migration and then run `make up` again to start the re-start the containers. + +Inspecting the database can be done via: + +```bash +make dbshell +``` diff --git a/settings.py b/settings.py index e25c2738301..0c38f88a4c0 100644 --- a/settings.py +++ b/settings.py @@ -27,6 +27,7 @@ DBBACKUP_CONNECTOR_MAPPING = { 'olympia.core.db.mysql': 'dbbackup.db.mysql.MysqlDumpConnector', } +DATA_BACKUP_SKIP = os.environ.get('DATA_BACKUP_SKIP', False) # Override logging config to enable DEBUG logs for (almost) everything. LOGGING['root']['level'] = logging.DEBUG diff --git a/src/olympia/amo/management/commands/create_db.py b/src/olympia/amo/management/commands/create_db.py deleted file mode 100644 index 16308e34c8f..00000000000 --- a/src/olympia/amo/management/commands/create_db.py +++ /dev/null @@ -1,57 +0,0 @@ -import logging - -from django.conf import settings -from django.core.management.base import BaseCommand, CommandError - -import MySQLdb as mysql - - -class Command(BaseCommand): - """Based on django_extension's reset_db command but simplifed and with - support for all character sets defined in settings.""" - - help = 'Creates the database for this project.' - - def add_arguments(self, parser): - super().add_arguments(parser) - parser.add_argument( - '--force', action='store_true', help='Drops any existing database first.' - ) - - def handle(self, *args, **options): - """ - Create the database. - """ - db_info = settings.DATABASES.get('default') - - engine = db_info.get('ENGINE').split('.')[-1] - if engine != 'mysql': - raise CommandError('create_db only supports mysql databases') - - database_name = db_info.get('NAME') - kwargs = { - 'user': db_info.get('USER'), - 'passwd': db_info.get('PASSWORD'), - 'host': db_info.get('HOST'), - } - if db_info.get('PORT'): - kwargs['port'] = int(db_info.get('PORT')) - connection = mysql.connect(**kwargs) - - if options.get('force'): - drop_query = 'DROP DATABASE IF EXISTS `%s`' % database_name - else: - drop_query = None - - character_set = db_info.get('OPTIONS').get('charset', 'utf8mb4') - create_query = 'CREATE DATABASE `{}` CHARACTER SET {}'.format( - database_name, - character_set, - ) - if drop_query: - logging.info('Executing... "' + drop_query + '"') - connection.query(drop_query) - logging.info('Executing... "' + create_query + '"') - connection.query(create_query) - - logging.info('Reset successful.') diff --git a/src/olympia/amo/management/commands/initialize.py b/src/olympia/amo/management/commands/initialize.py new file mode 100644 index 00000000000..59a7cb28d34 --- /dev/null +++ b/src/olympia/amo/management/commands/initialize.py @@ -0,0 +1,58 @@ +import logging + +from django.conf import settings +from django.core.management import call_command + +from .. import BaseDataCommand + + +class Command(BaseDataCommand): + """ + Ensures the database has the correct state. + """ + + help = 'Creates, seeds, and indexes the database.' + + def add_arguments(self, parser): + super().add_arguments(parser) + parser.add_argument( + '--clean', action='store_true', help='Reset the database with fresh data' + ) + parser.add_argument( + '--load', + type=str, + help='Optionally load data from a backup.', + ) + + def local_admin_exists(self): + from olympia.users.models import UserProfile + + return UserProfile.objects.filter(email=settings.LOCAL_ADMIN_EMAIL).exists() + + def handle(self, *args, **options): + """ + Create the database. + """ + # We need to support skipping loading/seeding when desired. + # Like in CI environments where you don't want to load data every time. + if settings.DATA_BACKUP_SKIP: + logging.info( + 'Skipping seeding and loading data because DATA_BACKUP_SKIP is set' + ) + return + + clean = options.get('clean') + load = options.get('load') + logging.info(f'options: {options}') + + # We always migrate the DB. + logging.info('Migrating...') + call_command('migrate', '--noinput') + + # If we specify a specifi backup, simply load that. + if load: + call_command('data_load', '--name', load) + # Otherwise DB empty or we are explicitly cleaning, then reseed. + # This will load the initial data and reindex elasticsearch. + elif clean or not self.local_admin_exists(): + call_command('data_seed') diff --git a/src/olympia/amo/tests/test_commands.py b/src/olympia/amo/tests/test_commands.py index 9b5cf35b42c..f78355048a1 100644 --- a/src/olympia/amo/tests/test_commands.py +++ b/src/olympia/amo/tests/test_commands.py @@ -31,6 +31,7 @@ from olympia.files.models import File, files_upload_to_callback from olympia.git.utils import AddonGitRepository from olympia.hero.models import PrimaryHeroImage +from olympia.users.models import UserProfile from olympia.versions.models import VersionPreview, source_upload_path @@ -411,6 +412,134 @@ def _assert_commands_called_in_order(self, mock_call_command, expected_commands) ) +@override_settings(DATA_BACKUP_SKIP=False) +class TestInitializeDataCommand(BaseTestDataCommand): + def setUp(self): + super().setUp() + patches = ( + ( + 'mock_call_command', + 'olympia.amo.management.commands.initialize.call_command', + ), + ) + + self.mocks = {} + + for mock_name, mock_path in patches: + patcher = mock.patch(mock_path) + self.addCleanup(patcher.stop) + self.mocks[mock_name] = patcher.start() + + UserProfile.objects.filter(email=settings.LOCAL_ADMIN_EMAIL).delete() + + def with_local_admin(self): + user_factory( + username=settings.LOCAL_ADMIN_USERNAME, email=settings.LOCAL_ADMIN_EMAIL + ) + + @override_settings(DATA_BACKUP_SKIP=True) + def test_handle_with_skip_data_initialize(self): + """ + Test running the 'initialize' command with the DATA_BACKUP_SKIP flag set. + Expected: nothing happens. + """ + call_command('initialize') + self._assert_commands_called_in_order( + self.mocks['mock_call_command'], + [], + ) + + @override_settings(DATA_BACKUP_SKIP=True) + def test_handle_with_load_argument_and_skip_data_initialize(self): + """ + Test running the 'initialize' command with both '--load' argument + and DATA_BACKUP_SKIP flag. Expected: nothing happens. + """ + call_command('initialize', load='test') + self._assert_commands_called_in_order( + self.mocks['mock_call_command'], + [], + ) + + def test_handle_with_clean_and_load_arguments(self): + """ + Test running the 'initialize' command with both '--clean' and '--load' + arguments. Expected: Command should prioritize '--load' and perform + migration, loading. + """ + name = 'test' + call_command('initialize', clean=True, load=name) + self._assert_commands_called_in_order( + self.mocks['mock_call_command'], + [ + self.mock_commands.migrate, + self.mock_commands.data_load(name), + ], + ) + + def test_handle_with_clean_argument_no_local_admin(self): + """ + Test running the 'initialize' command with the '--clean' argument + when no local admin exists. Expected: The database is migrated, seeded + with fresh data, loaded from backup if specified, and reindexed. + """ + call_command('initialize', clean=True) + self._assert_commands_called_in_order( + self.mocks['mock_call_command'], + [ + self.mock_commands.migrate, + self.mock_commands.data_seed, + ], + ) + + def test_handle_without_clean_or_load_with_local_admin(self): + """ + Test running the 'initialize' command without '--clean' or '--load' + arguments when a local admin exists. Expected: The database is migrated + and reindexed without seeding or loading data. + """ + self.with_local_admin() + call_command('initialize') + self._assert_commands_called_in_order( + self.mocks['mock_call_command'], + [ + self.mock_commands.migrate, + ], + ) + + def test_handle_without_clean_or_load_without_local_admin(self): + """ + Test running the 'initialize' command without '--clean' or '--load' + arguments when no local admin exists. Expected: The database is + migrated, seeded with initial data, and reindexed. + """ + call_command('initialize') + self._assert_commands_called_in_order( + self.mocks['mock_call_command'], + [ + self.mock_commands.migrate, + self.mock_commands.data_seed, + ], + ) + + def test_handle_migration_failure(self): + """ + Test running the 'initialize' command when the 'migrate' command fails. + Expected: The command exits with an error and does not proceed to seeding + or loading data. + """ + self.mocks['mock_call_command'].side_effect = Exception('test') + with pytest.raises(Exception) as context: + call_command('initialize') + assert 'test' in str(context.value) + self._assert_commands_called_in_order( + self.mocks['mock_call_command'], + [ + self.mock_commands.migrate, + ], + ) + + class TestBaseDataCommand(BaseTestDataCommand): def setUp(self): super().setUp() From 53a6caad141e6974c5fa6c05682752e40eb28789 Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Tue, 8 Oct 2024 14:37:46 +0200 Subject: [PATCH 2/4] Partial revert of setup-ui-tests --- Makefile-docker | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Makefile-docker b/Makefile-docker index 55b03e33e91..645547b0f63 100644 --- a/Makefile-docker +++ b/Makefile-docker @@ -71,6 +71,18 @@ update_assets: # Collect static files: This MUST be run last or files will be missing $(PYTHON_COMMAND) manage.py collectstatic --noinput + +# TOOD: remove this after we migrate addons-frontned to not depend on it. +.PHONY: setup-ui-tests +setup-ui-tests: + rm -rf ./user-media/* ./tmp/* + + $(PYTHON_COMMAND) manage.py loaddata src/olympia/access/fixtures/initial.json + + $(PYTHON_COMMAND) manage.py waffle_switch super-create-accounts on --create + $(PYTHON_COMMAND) manage.py waffle_switch activate-autograph-signing on --create + + .PHONY: lint lint: ## lint the code ruff check . From 4443a74376b3356cee6a9ad606b098bc6eaf2e3e Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Tue, 8 Oct 2024 15:44:40 +0200 Subject: [PATCH 3/4] Fix initialization should always reindex --- src/olympia/amo/management/commands/data_seed.py | 1 + src/olympia/amo/management/commands/initialize.py | 7 +++++-- src/olympia/amo/tests/test_commands.py | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/olympia/amo/management/commands/data_seed.py b/src/olympia/amo/management/commands/data_seed.py index a5478811bef..713504cb80d 100644 --- a/src/olympia/amo/management/commands/data_seed.py +++ b/src/olympia/amo/management/commands/data_seed.py @@ -43,3 +43,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') diff --git a/src/olympia/amo/management/commands/initialize.py b/src/olympia/amo/management/commands/initialize.py index 59a7cb28d34..4976ef9190a 100644 --- a/src/olympia/amo/management/commands/initialize.py +++ b/src/olympia/amo/management/commands/initialize.py @@ -52,7 +52,10 @@ def handle(self, *args, **options): # If we specify a specifi backup, simply load that. if load: call_command('data_load', '--name', load) - # Otherwise DB empty or we are explicitly cleaning, then reseed. - # This will load the initial data and reindex elasticsearch. + # If DB empty or we are explicitly cleaning, then reseed. elif clean or not self.local_admin_exists(): call_command('data_seed') + # 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') diff --git a/src/olympia/amo/tests/test_commands.py b/src/olympia/amo/tests/test_commands.py index f78355048a1..9a09ca44c6f 100644 --- a/src/olympia/amo/tests/test_commands.py +++ b/src/olympia/amo/tests/test_commands.py @@ -504,6 +504,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, ], ) @@ -770,5 +771,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, ], ) From fee3eef7578a9549aca645a814367f4d21514b12 Mon Sep 17 00:00:00 2001 From: Kevin Meinhardt Date: Tue, 8 Oct 2024 16:59:01 +0200 Subject: [PATCH 4/4] TMP: fix --- Makefile-docker | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Makefile-docker b/Makefile-docker index 645547b0f63..0c870b1aea7 100644 --- a/Makefile-docker +++ b/Makefile-docker @@ -75,13 +75,7 @@ update_assets: # TOOD: remove this after we migrate addons-frontned to not depend on it. .PHONY: setup-ui-tests setup-ui-tests: - rm -rf ./user-media/* ./tmp/* - - $(PYTHON_COMMAND) manage.py loaddata src/olympia/access/fixtures/initial.json - - $(PYTHON_COMMAND) manage.py waffle_switch super-create-accounts on --create - $(PYTHON_COMMAND) manage.py waffle_switch activate-autograph-signing on --create - + @echo "This is a deprecated target, please stop using it." .PHONY: lint lint: ## lint the code