diff options
author | Chris Meyers <chrismeyersfsu@users.noreply.github.com> | 2023-11-17 19:33:08 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-11-17 19:33:08 +0100 |
commit | 2ac304d2899f3f2b13c97b2ad30fcf91d8fe90ee (patch) | |
tree | e9210bc752d5cd4c3fadf57946dbdafa350c2f2e | |
parent | Upgrade doc requirements (#14669) (diff) | |
download | awx-23.5.0.tar.xz awx-23.5.0.zip |
allow pytest --migrations to succeed (#14663)23.5.0
* allow pytest --migrations to succeed
* We actually subvert migrations from running in test via pytest.ini
--no-migrations option. This has led to bit rot for the sqlite
migrations happy path. This changeset pays off that tech debt and
allows for an sqlite migration happy path.
* This paves the way for programatic invocation of individual migrations
and weaving of the creation of resources (i.e. Instance, Job Template,
etc). With this, a developer can instantiate various database states,
trigger a migration, assert the state of the db, and then have pytest
rollback all of that.
* I will note that in practice, running these migrations is dog shit
slow BUT this work also opens up the possibility of saving and
re-using sqlite3 database files. Normally, caching is not THE answer
and causes more harm than good. But in this case, our migrations are
mostly write-once (I say mostly because this change set violates
that :) so cache invalidation isn't a major issue.
* functional test for migrations on sqlite
* We commonly subvert running migrations in test land. Test land uses
sqlite. By not constantly exercising this code path it atrophies. The
smoke test here is to continuously exercise that code path.
* Add ci test to run migration tests separately, they take =~ 2-3
minutes each on my laptop.
* The smoke tests also serves as an example of how to write migration
tests.
* run migration tests in ci
-rw-r--r-- | .github/workflows/ci.yml | 2 | ||||
-rw-r--r-- | Makefile | 6 | ||||
-rw-r--r-- | awx/main/migrations/0006_v320_release.py | 5 | ||||
-rw-r--r-- | awx/main/migrations/0050_v340_drop_celery_tables.py | 33 | ||||
-rw-r--r-- | awx/main/migrations/0113_v370_event_bigint.py | 9 | ||||
-rw-r--r-- | awx/main/migrations/0144_event_partitions.py | 8 | ||||
-rw-r--r-- | awx/main/migrations/0185_move_JSONBlob_to_JSONField.py | 32 | ||||
-rw-r--r-- | awx/main/migrations/0186_drop_django_taggit.py | 6 | ||||
-rw-r--r-- | awx/main/migrations/_sqlite_helper.py | 61 | ||||
-rw-r--r-- | awx/main/tests/functional/test_migrations.py | 44 | ||||
-rw-r--r-- | requirements/requirements_dev.txt | 1 |
11 files changed, 177 insertions, 30 deletions
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7612aa61c2..7bc82741e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,8 @@ jobs: tests: - name: api-test command: /start_tests.sh + - name: api-migrations + command: /start_tests.sh test_migrations - name: api-lint command: /var/lib/awx/venv/awx/bin/tox -e linters - name: api-swagger @@ -324,6 +324,12 @@ test: cd awxkit && $(VENV_BASE)/awx/bin/tox -re py3 awx-manage check_migrations --dry-run --check -n 'missing_migration_file' +test_migrations: + if [ "$(VENV_BASE)" ]; then \ + . $(VENV_BASE)/awx/bin/activate; \ + fi; \ + PYTHONDONTWRITEBYTECODE=1 py.test -p no:cacheprovider --migrations -m migration_test $(PYTEST_ARGS) $(TEST_DIRS) + ## Runs AWX_DOCKER_CMD inside a new docker container. docker-runner: docker run -u $(shell id -u) --rm -v $(shell pwd):/awx_devel/:Z --workdir=/awx_devel $(DEVEL_IMAGE_NAME) $(AWX_DOCKER_CMD) diff --git a/awx/main/migrations/0006_v320_release.py b/awx/main/migrations/0006_v320_release.py index f1dd01d708..a3059f1a6e 100644 --- a/awx/main/migrations/0006_v320_release.py +++ b/awx/main/migrations/0006_v320_release.py @@ -9,6 +9,7 @@ from django.conf import settings # AWX import awx.main.fields from awx.main.models import Host +from ._sqlite_helper import dbawaremigrations def replaces(): @@ -131,9 +132,11 @@ class Migration(migrations.Migration): help_text='If enabled, Tower will act as an Ansible Fact Cache Plugin; persisting facts at the end of a playbook run to the database and caching facts for use by Ansible.', ), ), - migrations.RunSQL( + dbawaremigrations.RunSQL( sql="CREATE INDEX host_ansible_facts_default_gin ON {} USING gin(ansible_facts jsonb_path_ops);".format(Host._meta.db_table), reverse_sql='DROP INDEX host_ansible_facts_default_gin;', + sqlite_sql=dbawaremigrations.RunSQL.noop, + sqlite_reverse_sql=dbawaremigrations.RunSQL.noop, ), # SCM file-based inventories migrations.AddField( diff --git a/awx/main/migrations/0050_v340_drop_celery_tables.py b/awx/main/migrations/0050_v340_drop_celery_tables.py index 5c98d7bc7c..ce34d81ef3 100644 --- a/awx/main/migrations/0050_v340_drop_celery_tables.py +++ b/awx/main/migrations/0050_v340_drop_celery_tables.py @@ -3,24 +3,27 @@ from __future__ import unicode_literals from django.db import migrations +from ._sqlite_helper import dbawaremigrations + +tables_to_drop = [ + 'celery_taskmeta', + 'celery_tasksetmeta', + 'djcelery_crontabschedule', + 'djcelery_intervalschedule', + 'djcelery_periodictask', + 'djcelery_periodictasks', + 'djcelery_taskstate', + 'djcelery_workerstate', + 'djkombu_message', + 'djkombu_queue', +] +postgres_sql = ([("DROP TABLE IF EXISTS {} CASCADE;".format(table))] for table in tables_to_drop) +sqlite_sql = ([("DROP TABLE IF EXISTS {};".format(table))] for table in tables_to_drop) + class Migration(migrations.Migration): dependencies = [ ('main', '0049_v330_validate_instance_capacity_adjustment'), ] - operations = [ - migrations.RunSQL([("DROP TABLE IF EXISTS {} CASCADE;".format(table))]) - for table in ( - 'celery_taskmeta', - 'celery_tasksetmeta', - 'djcelery_crontabschedule', - 'djcelery_intervalschedule', - 'djcelery_periodictask', - 'djcelery_periodictasks', - 'djcelery_taskstate', - 'djcelery_workerstate', - 'djkombu_message', - 'djkombu_queue', - ) - ] + operations = [dbawaremigrations.RunSQL(p, sqlite_sql=s) for p, s in zip(postgres_sql, sqlite_sql)] diff --git a/awx/main/migrations/0113_v370_event_bigint.py b/awx/main/migrations/0113_v370_event_bigint.py index d9f4ce1d97..7795d4cc65 100644 --- a/awx/main/migrations/0113_v370_event_bigint.py +++ b/awx/main/migrations/0113_v370_event_bigint.py @@ -2,6 +2,8 @@ from django.db import migrations, models, connection +from ._sqlite_helper import dbawaremigrations + def migrate_event_data(apps, schema_editor): # see: https://github.com/ansible/awx/issues/6010 @@ -24,6 +26,11 @@ def migrate_event_data(apps, schema_editor): cursor.execute(f'ALTER TABLE {tblname} ALTER COLUMN id TYPE bigint USING id::bigint;') +def migrate_event_data_sqlite(apps, schema_editor): + # TODO: cmeyers fill this in + return + + class FakeAlterField(migrations.AlterField): def database_forwards(self, *args): # this is intentionally left blank, because we're @@ -37,7 +44,7 @@ class Migration(migrations.Migration): ] operations = [ - migrations.RunPython(migrate_event_data), + dbawaremigrations.RunPython(migrate_event_data, sqlite_code=migrate_event_data_sqlite), FakeAlterField( model_name='adhoccommandevent', name='id', diff --git a/awx/main/migrations/0144_event_partitions.py b/awx/main/migrations/0144_event_partitions.py index efdcbb37fc..8813ae67e5 100644 --- a/awx/main/migrations/0144_event_partitions.py +++ b/awx/main/migrations/0144_event_partitions.py @@ -1,5 +1,7 @@ from django.db import migrations, models, connection +from ._sqlite_helper import dbawaremigrations + def migrate_event_data(apps, schema_editor): # see: https://github.com/ansible/awx/issues/9039 @@ -59,6 +61,10 @@ def migrate_event_data(apps, schema_editor): cursor.execute('DROP INDEX IF EXISTS main_jobevent_job_id_idx') +def migrate_event_data_sqlite(apps, schema_editor): + return None + + class FakeAddField(migrations.AddField): def database_forwards(self, *args): # this is intentionally left blank, because we're @@ -72,7 +78,7 @@ class Migration(migrations.Migration): ] operations = [ - migrations.RunPython(migrate_event_data), + dbawaremigrations.RunPython(migrate_event_data, sqlite_code=migrate_event_data_sqlite), FakeAddField( model_name='jobevent', name='job_created', diff --git a/awx/main/migrations/0185_move_JSONBlob_to_JSONField.py b/awx/main/migrations/0185_move_JSONBlob_to_JSONField.py index 42bab8eb87..b2efeecfb3 100644 --- a/awx/main/migrations/0185_move_JSONBlob_to_JSONField.py +++ b/awx/main/migrations/0185_move_JSONBlob_to_JSONField.py @@ -3,6 +3,8 @@ import awx.main.models.notifications from django.db import migrations, models +from ._sqlite_helper import dbawaremigrations + class Migration(migrations.Migration): dependencies = [ @@ -104,11 +106,12 @@ class Migration(migrations.Migration): name='deleted_actor', field=models.JSONField(null=True), ), - migrations.RunSQL( + dbawaremigrations.RunSQL( """ ALTER TABLE main_activitystream RENAME setting TO setting_old; ALTER TABLE main_activitystream ALTER COLUMN setting_old DROP NOT NULL; """, + sqlite_sql="ALTER TABLE main_activitystream RENAME setting TO setting_old", state_operations=[ migrations.RemoveField( model_name='activitystream', @@ -121,11 +124,12 @@ class Migration(migrations.Migration): name='setting', field=models.JSONField(blank=True, default=dict), ), - migrations.RunSQL( + dbawaremigrations.RunSQL( """ ALTER TABLE main_job RENAME survey_passwords TO survey_passwords_old; ALTER TABLE main_job ALTER COLUMN survey_passwords_old DROP NOT NULL; """, + sqlite_sql="ALTER TABLE main_job RENAME survey_passwords TO survey_passwords_old", state_operations=[ migrations.RemoveField( model_name='job', @@ -138,11 +142,12 @@ class Migration(migrations.Migration): name='survey_passwords', field=models.JSONField(blank=True, default=dict, editable=False), ), - migrations.RunSQL( + dbawaremigrations.RunSQL( """ ALTER TABLE main_joblaunchconfig RENAME char_prompts TO char_prompts_old; ALTER TABLE main_joblaunchconfig ALTER COLUMN char_prompts_old DROP NOT NULL; """, + sqlite_sql="ALTER TABLE main_joblaunchconfig RENAME char_prompts TO char_prompts_old", state_operations=[ migrations.RemoveField( model_name='joblaunchconfig', @@ -155,11 +160,12 @@ class Migration(migrations.Migration): name='char_prompts', field=models.JSONField(blank=True, default=dict), ), - migrations.RunSQL( + dbawaremigrations.RunSQL( """ ALTER TABLE main_joblaunchconfig RENAME survey_passwords TO survey_passwords_old; ALTER TABLE main_joblaunchconfig ALTER COLUMN survey_passwords_old DROP NOT NULL; """, + sqlite_sql="ALTER TABLE main_joblaunchconfig RENAME survey_passwords TO survey_passwords_old;", state_operations=[ migrations.RemoveField( model_name='joblaunchconfig', @@ -172,11 +178,12 @@ class Migration(migrations.Migration): name='survey_passwords', field=models.JSONField(blank=True, default=dict, editable=False), ), - migrations.RunSQL( + dbawaremigrations.RunSQL( """ ALTER TABLE main_notification RENAME body TO body_old; ALTER TABLE main_notification ALTER COLUMN body_old DROP NOT NULL; """, + sqlite_sql="ALTER TABLE main_notification RENAME body TO body_old", state_operations=[ migrations.RemoveField( model_name='notification', @@ -189,11 +196,12 @@ class Migration(migrations.Migration): name='body', field=models.JSONField(blank=True, default=dict), ), - migrations.RunSQL( + dbawaremigrations.RunSQL( """ ALTER TABLE main_unifiedjob RENAME job_env TO job_env_old; ALTER TABLE main_unifiedjob ALTER COLUMN job_env_old DROP NOT NULL; """, + sqlite_sql="ALTER TABLE main_unifiedjob RENAME job_env TO job_env_old", state_operations=[ migrations.RemoveField( model_name='unifiedjob', @@ -206,11 +214,12 @@ class Migration(migrations.Migration): name='job_env', field=models.JSONField(blank=True, default=dict, editable=False), ), - migrations.RunSQL( + dbawaremigrations.RunSQL( """ ALTER TABLE main_workflowjob RENAME char_prompts TO char_prompts_old; ALTER TABLE main_workflowjob ALTER COLUMN char_prompts_old DROP NOT NULL; """, + sqlite_sql="ALTER TABLE main_workflowjob RENAME char_prompts TO char_prompts_old", state_operations=[ migrations.RemoveField( model_name='workflowjob', @@ -223,11 +232,12 @@ class Migration(migrations.Migration): name='char_prompts', field=models.JSONField(blank=True, default=dict), ), - migrations.RunSQL( + dbawaremigrations.RunSQL( """ ALTER TABLE main_workflowjob RENAME survey_passwords TO survey_passwords_old; ALTER TABLE main_workflowjob ALTER COLUMN survey_passwords_old DROP NOT NULL; """, + sqlite_sql="ALTER TABLE main_workflowjob RENAME survey_passwords TO survey_passwords_old", state_operations=[ migrations.RemoveField( model_name='workflowjob', @@ -240,11 +250,12 @@ class Migration(migrations.Migration): name='survey_passwords', field=models.JSONField(blank=True, default=dict, editable=False), ), - migrations.RunSQL( + dbawaremigrations.RunSQL( """ ALTER TABLE main_workflowjobnode RENAME char_prompts TO char_prompts_old; ALTER TABLE main_workflowjobnode ALTER COLUMN char_prompts_old DROP NOT NULL; """, + sqlite_sql="ALTER TABLE main_workflowjobnode RENAME char_prompts TO char_prompts_old", state_operations=[ migrations.RemoveField( model_name='workflowjobnode', @@ -257,11 +268,12 @@ class Migration(migrations.Migration): name='char_prompts', field=models.JSONField(blank=True, default=dict), ), - migrations.RunSQL( + dbawaremigrations.RunSQL( """ ALTER TABLE main_workflowjobnode RENAME survey_passwords TO survey_passwords_old; ALTER TABLE main_workflowjobnode ALTER COLUMN survey_passwords_old DROP NOT NULL; """, + sqlite_sql="ALTER TABLE main_workflowjobnode RENAME survey_passwords TO survey_passwords_old", state_operations=[ migrations.RemoveField( model_name='workflowjobnode', diff --git a/awx/main/migrations/0186_drop_django_taggit.py b/awx/main/migrations/0186_drop_django_taggit.py index c9c767b74c..99aa102fee 100644 --- a/awx/main/migrations/0186_drop_django_taggit.py +++ b/awx/main/migrations/0186_drop_django_taggit.py @@ -3,6 +3,8 @@ from __future__ import unicode_literals from django.db import migrations +from ._sqlite_helper import dbawaremigrations + def delete_taggit_contenttypes(apps, schema_editor): ContentType = apps.get_model('contenttypes', 'ContentType') @@ -20,8 +22,8 @@ class Migration(migrations.Migration): ] operations = [ - migrations.RunSQL("DROP TABLE IF EXISTS taggit_tag CASCADE;"), - migrations.RunSQL("DROP TABLE IF EXISTS taggit_taggeditem CASCADE;"), + dbawaremigrations.RunSQL("DROP TABLE IF EXISTS taggit_tag CASCADE;", sqlite_sql="DROP TABLE IF EXISTS taggit_tag;"), + dbawaremigrations.RunSQL("DROP TABLE IF EXISTS taggit_taggeditem CASCADE;", sqlite_sql="DROP TABLE IF EXISTS taggit_taggeditem;"), migrations.RunPython(delete_taggit_contenttypes), migrations.RunPython(delete_taggit_migration_records), ] diff --git a/awx/main/migrations/_sqlite_helper.py b/awx/main/migrations/_sqlite_helper.py new file mode 100644 index 0000000000..fcf0557c0f --- /dev/null +++ b/awx/main/migrations/_sqlite_helper.py @@ -0,0 +1,61 @@ +from django.db import migrations + + +class RunSQL(migrations.operations.special.RunSQL): + """ + Bit of a hack here. Django actually wants this decision made in the router + and we can pass **hints. + """ + + def __init__(self, *args, **kwargs): + if 'sqlite_sql' not in kwargs: + raise ValueError("sqlite_sql parameter required") + sqlite_sql = kwargs.pop('sqlite_sql') + + self.sqlite_sql = sqlite_sql + self.sqlite_reverse_sql = kwargs.pop('sqlite_reverse_sql', None) + super().__init__(*args, **kwargs) + + def database_forwards(self, app_label, schema_editor, from_state, to_state): + if not schema_editor.connection.vendor.startswith('postgres'): + self.sql = self.sqlite_sql or migrations.RunSQL.noop + super().database_forwards(app_label, schema_editor, from_state, to_state) + + def database_backwards(self, app_label, schema_editor, from_state, to_state): + if not schema_editor.connection.vendor.startswith('postgres'): + self.reverse_sql = self.sqlite_reverse_sql or migrations.RunSQL.noop + super().database_backwards(app_label, schema_editor, from_state, to_state) + + +class RunPython(migrations.operations.special.RunPython): + """ + Bit of a hack here. Django actually wants this decision made in the router + and we can pass **hints. + """ + + def __init__(self, *args, **kwargs): + if 'sqlite_code' not in kwargs: + raise ValueError("sqlite_code parameter required") + sqlite_code = kwargs.pop('sqlite_code') + + self.sqlite_code = sqlite_code + self.sqlite_reverse_code = kwargs.pop('sqlite_reverse_code', None) + super().__init__(*args, **kwargs) + + def database_forwards(self, app_label, schema_editor, from_state, to_state): + if not schema_editor.connection.vendor.startswith('postgres'): + self.code = self.sqlite_code or migrations.RunPython.noop + super().database_forwards(app_label, schema_editor, from_state, to_state) + + def database_backwards(self, app_label, schema_editor, from_state, to_state): + if not schema_editor.connection.vendor.startswith('postgres'): + self.reverse_code = self.sqlite_reverse_code or migrations.RunPython.noop + super().database_backwards(app_label, schema_editor, from_state, to_state) + + +class _sqlitemigrations: + RunPython = RunPython + RunSQL = RunSQL + + +dbawaremigrations = _sqlitemigrations() diff --git a/awx/main/tests/functional/test_migrations.py b/awx/main/tests/functional/test_migrations.py new file mode 100644 index 0000000000..cd0889c208 --- /dev/null +++ b/awx/main/tests/functional/test_migrations.py @@ -0,0 +1,44 @@ +import pytest + +from django_test_migrations.plan import all_migrations, nodes_to_tuples + +""" +Most tests that live in here can probably be deleted at some point. They are mainly +for a developer. When AWX versions that users upgrade from falls out of support that +is when migration tests can be deleted. This is also a good time to squash. Squashing +will likely mess with the tests that live here. + +The smoke test should be kept in here. The smoke test ensures that our migrations +continue to work when sqlite is the backing database (vs. the default DB of postgres). +""" + + +@pytest.mark.django_db +class TestMigrationSmoke: + def test_happy_path(self, migrator): + """ + This smoke test runs all the migrations. + + Example of how to use django-test-migration to invoke particular migration(s) + while weaving in object creation and assertions. + + Note that this is more than just an example. It is a smoke test because it runs ALL + the migrations. Our "normal" unit tests subvert the migrations running because it is slow. + """ + migration_nodes = all_migrations('default') + migration_tuples = nodes_to_tuples(migration_nodes) + final_migration = migration_tuples[-1] + + migrator.apply_initial_migration(('main', None)) + # I just picked a newish migration at the time of writing this. + # If someone from the future finds themselves here because the are squashing migrations + # it is fine to change the 0180_... below to some other newish migration + intermediate_state = migrator.apply_tested_migration(('main', '0180_add_hostmetric_fields')) + + Instance = intermediate_state.apps.get_model('main', 'Instance') + # Create any old object in the database + Instance.objects.create(hostname='foobar', node_type='control') + + final_state = migrator.apply_tested_migration(final_migration) + Instance = final_state.apps.get_model('main', 'Instance') + assert Instance.objects.filter(hostname='foobar').count() == 1 diff --git a/requirements/requirements_dev.txt b/requirements/requirements_dev.txt index a97c662079..ae9d81f4a7 100644 --- a/requirements/requirements_dev.txt +++ b/requirements/requirements_dev.txt @@ -1,6 +1,7 @@ build coreapi django-debug-toolbar==3.2.4 +django-test-migrations drf-yasg # pprofile - re-add once https://github.com/vpelletier/pprofile/issues/41 is addressed ipython>=7.31.1 # https://github.com/ansible/awx/security/dependabot/30 |