From f5a034272ddeddf80dc858396cf31ab581f4ae5e Mon Sep 17 00:00:00 2001 From: Erik Olof Gunnar Andersson Date: Mon, 11 Dec 2023 05:23:36 -0800 Subject: [PATCH] Add upgrade test coverage Adding additional test upgrade coverage. This implementation tests things like the is_migration_needed flag. In the future we can also add support for MySQL functional coverage as well. Change-Id: I9386d1bfbfee1fe8ff41859520cdbe94381ee3fd --- designate/storage/sqlalchemy/alembic.ini | 2 +- designate/storage/sqlalchemy/alembic/env.py | 36 +++- .../tests/test_storage/test_migration.py | 175 ++++++++++++++++++ test-requirements.txt | 1 + 4 files changed, 203 insertions(+), 11 deletions(-) create mode 100644 designate/tests/test_storage/test_migration.py diff --git a/designate/storage/sqlalchemy/alembic.ini b/designate/storage/sqlalchemy/alembic.ini index 4cf877d2c..34be5f67a 100644 --- a/designate/storage/sqlalchemy/alembic.ini +++ b/designate/storage/sqlalchemy/alembic.ini @@ -2,7 +2,7 @@ [alembic] # path to migration scripts -script_location = alembic +script_location = %(here)s/alembic # template used to generate migration file names; The default value is %%(rev)s_%%(slug)s # Uncomment the line below if you want the files to be prepended with date and time diff --git a/designate/storage/sqlalchemy/alembic/env.py b/designate/storage/sqlalchemy/alembic/env.py index 0d23c33e1..fc48500b4 100644 --- a/designate/storage/sqlalchemy/alembic/env.py +++ b/designate/storage/sqlalchemy/alembic/env.py @@ -42,12 +42,12 @@ def run_migrations_offline() -> None: script output. """ - url = config.get_main_option("sqlalchemy.url") + url = config.get_main_option('sqlalchemy.url') context.configure( url=url, target_metadata=target_metadata, literal_binds=True, - dialect_opts={"paramstyle": "named"}, + dialect_opts={'paramstyle': 'named'}, ) with context.begin_transaction(): @@ -61,16 +61,32 @@ def run_migrations_online() -> None: and associate a connection with the context. """ - connectable = engine_from_config( - config.get_section(config.config_ini_section), - prefix="sqlalchemy.", - poolclass=pool.NullPool, - ) + connectable = config.attributes.get('connection', None) + if connectable is None: + # only create Engine if we don't have a Connection from the outside + connectable = engine_from_config( + config.get_section(config.config_ini_section), + prefix='sqlalchemy.', + poolclass=pool.NullPool, + ) + with connectable.connect() as connection: + context.configure( + connection=connection, + target_metadata=target_metadata, + render_as_batch=True, + transactional_ddl=True, + transaction_per_migration=True, + ) - with connectable.connect() as connection: + with context.begin_transaction(): + context.run_migrations() + else: context.configure( - connection=connection, target_metadata=target_metadata, - transactional_ddl=True, transaction_per_migration=True + connection=connectable, + target_metadata=target_metadata, + render_as_batch=True, + transactional_ddl=True, + transaction_per_migration=True, ) with context.begin_transaction(): diff --git a/designate/tests/test_storage/test_migration.py b/designate/tests/test_storage/test_migration.py new file mode 100644 index 000000000..c920d7684 --- /dev/null +++ b/designate/tests/test_storage/test_migration.py @@ -0,0 +1,175 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# Based on Nova's test_migrations.py + + +import os + +from unittest import mock + +from alembic import command as alembic_api +from alembic import config as alembic_config +from alembic import script as alembic_script +from oslo_db.sqlalchemy import enginefacade +from oslo_db.sqlalchemy import test_fixtures +from oslo_log import log as logging + +from designate.storage import sqlalchemy +from designate.storage.sqlalchemy.alembic import legacy_utils +from designate import tests + + +LOG = logging.getLogger(__name__) +ALEMBIC_PATH = os.path.join( + os.path.dirname(sqlalchemy.__file__), 'alembic.ini' +) + + +class DesignateMigrationsWalk( + test_fixtures.OpportunisticDBTestMixin, tests.TestCase, +): + # Migrations can take a long time, particularly on underpowered CI nodes. + # Give them some breathing room. + TIMEOUT_SCALING_FACTOR = 4 + + def setUp(self): + super().setUp() + self.engine = enginefacade.writer.get_engine() + self.config = alembic_config.Config(ALEMBIC_PATH) + self.init_version = 'c9f427f7180a' + + def _migrate_up(self, connection, revision): + if revision == self.init_version: # no tests for the initial revision + alembic_api.upgrade(self.config, revision) + return + + self.assertIsNotNone( + getattr(self, '_check_%s' % revision, None), + ( + 'DB Migration %s does not have a test; you must add one' + ) % revision, + ) + + pre_upgrade = getattr(self, '_pre_upgrade_%s' % revision, None) + if pre_upgrade: + pre_upgrade(connection) + + alembic_api.upgrade(self.config, revision) + + post_upgrade = getattr(self, '_check_%s' % revision, None) + if post_upgrade: + post_upgrade(connection) + + def _check_867a331ce1fc(self, connection): + pass + + def _check_d9a1883e93e9(self, connection): + pass + + def _check_bfcfc4a07487(self, connection): + pass + + def _check_f9f969f9d85e(self, connection): + pass + + def _check_a69b45715cc1(self, connection): + pass + + def _check_0bcf910ea823(self, connection): + pass + + def _check_d04819112169(self, connection): + pass + + def _check_304d41c3847a(self, connection): + pass + + def _check_15b34ff3ecb8(self, connection): + pass + + def _check_7977deaa5167(self, connection): + pass + + def _check_93a00a815f07(self, connection): + pass + + def _check_b8999fd10721(self, connection): + pass + + def _check_91eb1eb7c882(self, connection): + pass + + def _check_e5e2199ed76e(self, connection): + pass + + def _check_b20189fd288e(self, connection): + pass + + def _check_a005af3aa38e(self, connection): + pass + + def test_single_base_revision(self): + script = alembic_script.ScriptDirectory.from_config(self.config) + self.assertEqual(1, len(script.get_bases())) + + def test_walk_versions(self): + with self.engine.begin() as connection: + self.config.attributes['connection'] = connection + script = alembic_script.ScriptDirectory.from_config(self.config) + revisions = [x.revision for x in script.walk_revisions()] + + # for some reason, 'walk_revisions' gives us the revisions in + # reverse chronological order, so we have to invert this + revisions.reverse() + self.assertEqual(revisions[0], self.init_version) + + for revision in revisions: + LOG.info('Testing revision %s', revision) + self._migrate_up(connection, revision) + + def test_is_migration_needed(self): + with self.engine.begin() as connection: + self.config.attributes['connection'] = connection + script = alembic_script.ScriptDirectory.from_config(self.config) + revisions = [x.revision for x in script.walk_revisions()] + + # for some reason, 'walk_revisions' gives us the revisions in + # reverse chronological order, so we have to invert this + revisions.reverse() + self.assertEqual(revisions[0], self.init_version) + + for revision in revisions: + LOG.info('Testing revision %s', revision) + + # Let's stop after the first revision + # without is_migration_needed. + if revision == 'b20189fd288e': + break + self._migrate_up(connection, revision) + + # Reset alembic. + alembic_api.stamp(self.config, None) + + # We should only need the last few revisions. + with mock.patch.object(legacy_utils, + 'is_migration_needed', + return_value=False): + for revision in revisions: + LOG.info('Testing revision %s', revision) + self._migrate_up(connection, revision) + + +class TestMigrationsWalkSQLite( + DesignateMigrationsWalk, + test_fixtures.OpportunisticDBTestMixin, +): + pass diff --git a/test-requirements.txt b/test-requirements.txt index 673ad81b2..752dd7d4a 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -19,3 +19,4 @@ zake>=0.1.6 # Apache-2.0 doc8>=0.6.0 # Apache-2.0 Pygments>=2.2.0 # BSD license pymemcache!=1.3.0,>=1.2.9 # Apache 2.0 License +PyMySQL>=0.8.0 # MIT License