From fa1537814007a7b9492bb73eb629558c06d629da Mon Sep 17 00:00:00 2001 From: Igor Malinovskiy Date: Tue, 1 Sep 2015 12:31:01 +0300 Subject: [PATCH] Fix data copying issue in DB migration 1f0bd302c1a6 Fix data copying issue and add data checks in unit tests for this migration. Change-Id: I50429e07d108dae75026b8370f7df056362f4b4e Closes-Bug: #1489827 --- ...0bd302c1a6_add_availability_zones_table.py | 15 +- .../alembic/migrations_data_checks.py | 174 ++++++++++++++++++ .../db/migrations/alembic/test_migration.py | 19 +- 3 files changed, 192 insertions(+), 16 deletions(-) create mode 100644 manila/tests/db/migrations/alembic/migrations_data_checks.py diff --git a/manila/db/migrations/alembic/versions/1f0bd302c1a6_add_availability_zones_table.py b/manila/db/migrations/alembic/versions/1f0bd302c1a6_add_availability_zones_table.py index afc93960dc..506ae73d6a 100644 --- a/manila/db/migrations/alembic/versions/1f0bd302c1a6_add_availability_zones_table.py +++ b/manila/db/migrations/alembic/versions/1f0bd302c1a6_add_availability_zones_table.py @@ -33,17 +33,20 @@ from manila.db.migrations import utils def collect_existing_az_from_services_table(connection, services_table, az_table): az_name_to_id_mapping = dict() - existing_services = [] + existing_az = [] for service in connection.execute(services_table.select()): - service_id = uuidutils.generate_uuid() - az_name_to_id_mapping[service.availability_zone] = service_id - existing_services.append({ + if service.availability_zone in az_name_to_id_mapping: + continue + + az_id = uuidutils.generate_uuid() + az_name_to_id_mapping[service.availability_zone] = az_id + existing_az.append({ 'created_at': timeutils.utcnow(), - 'id': service_id, + 'id': az_id, 'name': service.availability_zone }) - op.bulk_insert(az_table, existing_services) + op.bulk_insert(az_table, existing_az) return az_name_to_id_mapping diff --git a/manila/tests/db/migrations/alembic/migrations_data_checks.py b/manila/tests/db/migrations/alembic/migrations_data_checks.py new file mode 100644 index 0000000000..f04aa677a3 --- /dev/null +++ b/manila/tests/db/migrations/alembic/migrations_data_checks.py @@ -0,0 +1,174 @@ +# Copyright 2015 Mirantis inc. +# All Rights Reserved. +# +# 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. + +""" +Tests data for database migrations. + +All database migrations with data manipulation +(like moving data from column to the table) should have data check class: + +@map_to_migration('1f0bd302c1a6') # Revision of checked db migration +class FooMigrationChecks(BaseMigrationChecks): + def setup_upgrade_data(self, engine): + ... + + def check_upgrade(self, engine, data): + ... + + def check_downgrade(self, engine): + ... + +See BaseMigrationChecks class for more information. +""" + +import abc + +from oslo_utils import uuidutils +import six + +from manila.db.migrations import utils + + +class DbMigrationsData(object): + + migration_mappings = {} + + methods_mapping = { + 'pre': 'setup_upgrade_data', + 'check': 'check_upgrade', + 'post': 'check_downgrade', + } + + def __getattr__(self, item): + parts = item.split('_') + + is_mapping_method = ( + len(parts) > 2 and parts[0] == '' + and parts[1] in self.methods_mapping + ) + + if not is_mapping_method: + return super(DbMigrationsData, self).__getattribute__(item) + + check_obj = self.migration_mappings.get(parts[-1], None) + + if check_obj is None: + raise AttributeError + + check_obj.set_test_case(self) + + return getattr(check_obj, self.methods_mapping.get(parts[1])) + + +def map_to_migration(revision): + def decorator(cls): + DbMigrationsData.migration_mappings[revision] = cls() + return cls + return decorator + + +class BaseMigrationChecks(object): + + six.add_metaclass(abc.ABCMeta) + + def __init__(self): + self.test_case = None + + def set_test_case(self, test_case): + self.test_case = test_case + + @abc.abstractmethod + def setup_upgrade_data(self, engine): + """This method should be used to insert test data for migration. + + :param engine: SQLAlchemy engine + :return: any data which will be passed to 'check_upgrade' as 'data' arg + """ + + @abc.abstractmethod + def check_upgrade(self, engine, data): + """This method should be used to do assertions after upgrade method. + + To perform assertions use 'self.test_case' instance property: + self.test_case.assertTrue(True) + + :param engine: SQLAlchemy engine + :param data: data returned by 'setup_upgrade_data' + """ + + @abc.abstractmethod + def check_downgrade(self, engine): + """This method should be used to do assertions after downgrade method. + + To perform assertions use 'self.test_case' instance property: + self.test_case.assertTrue(True) + + :param engine: SQLAlchemy engine + """ + + +@map_to_migration('1f0bd302c1a6') +class AvailabilityZoneMigrationChecks(BaseMigrationChecks): + + valid_az_names = ('az1', 'az2') + + def _get_service_data(self, options): + base_dict = { + 'binary': 'manila-share', + 'topic': 'share', + 'disabled': '0', + 'report_count': '100', + } + base_dict.update(options) + return base_dict + + def setup_upgrade_data(self, engine): + service_fixture = [ + self._get_service_data( + {'deleted': 0, 'host': 'fake1', 'availability_zone': 'az1'} + ), + self._get_service_data( + {'deleted': 0, 'host': 'fake2', 'availability_zone': 'az1'} + ), + self._get_service_data( + {'deleted': 1, 'host': 'fake3', 'availability_zone': 'az2'} + ), + ] + + services_table = utils.load_table('services', engine) + + for fixture in service_fixture: + engine.execute(services_table.insert(fixture)) + + def check_upgrade(self, engine, _): + az_table = utils.load_table('availability_zones', engine) + + for az in engine.execute(az_table.select()): + self.test_case.assertTrue(uuidutils.is_uuid_like(az.id)) + self.test_case.assertTrue(az.name in self.valid_az_names) + self.test_case.assertEqual('False', az.deleted) + + services_table = utils.load_table('services', engine) + for service in engine.execute(services_table.select()): + self.test_case.assertTrue( + uuidutils.is_uuid_like(service.availability_zone_id) + ) + + def check_downgrade(self, engine): + services_table = utils.load_table('services', engine) + for service in engine.execute(services_table.select()): + self.test_case.assertIn( + service.availability_zone, self.valid_az_names + ) diff --git a/manila/tests/db/migrations/alembic/test_migration.py b/manila/tests/db/migrations/alembic/test_migration.py index b0903d1706..4e94a06ac8 100644 --- a/manila/tests/db/migrations/alembic/test_migration.py +++ b/manila/tests/db/migrations/alembic/test_migration.py @@ -25,11 +25,13 @@ from oslo_log import log from sqlalchemy.sql import text from manila.db.migrations.alembic import migration +from manila.tests.db.migrations.alembic import migrations_data_checks LOG = log.getLogger('manila.tests.test_migrations') -class ManilaMigrationsCheckers(test_migrations.WalkVersionsMixin): +class ManilaMigrationsCheckers(test_migrations.WalkVersionsMixin, + migrations_data_checks.DbMigrationsData): """Test alembic migrations.""" @property @@ -76,33 +78,30 @@ class ManilaMigrationsCheckers(test_migrations.WalkVersionsMixin): self._migrate_up(version.revision, with_data=True) if snake_walk: downgraded = self._migrate_down( - version.down_revision, with_data=True) + version, with_data=True) if downgraded: self._migrate_up(version.revision) if downgrade: for version in versions: - downgraded = self._migrate_down(version.down_revision) + downgraded = self._migrate_down(version) if snake_walk and downgraded: self._migrate_up(version.revision) - self._migrate_down(version.down_revision) + self._migrate_down(version) def _migrate_down(self, version, with_data=False): try: - self.migration_api.downgrade(version) + self.migration_api.downgrade(version.down_revision) except NotImplementedError: # NOTE(sirp): some migrations, namely release-level # migrations, don't support a downgrade. return False - self.assertEqual(version, self.migration_api.version()) + self.assertEqual(version.down_revision, self.migration_api.version()) - # NOTE(sirp): `version` is what we're downgrading to (i.e. the 'target' - # version). So if we have any downgrade checks, they need to be run for - # the previous (higher numbered) migration. if with_data: post_downgrade = getattr( - self, "_post_downgrade_%s" % (version), None) + self, "_post_downgrade_%s" % version.revision, None) if post_downgrade: post_downgrade(self.engine)