diff --git a/manila/db/migrations/alembic/versions/579c267fbb4d_add_share_instances_access_map.py b/manila/db/migrations/alembic/versions/579c267fbb4d_add_share_instances_access_map.py index 7feb27f7a4..0b49bfd53e 100644 --- a/manila/db/migrations/alembic/versions/579c267fbb4d_add_share_instances_access_map.py +++ b/manila/db/migrations/alembic/versions/579c267fbb4d_add_share_instances_access_map.py @@ -89,10 +89,12 @@ def downgrade(): instance_access_table = utils.load_table('share_instance_access_map', connection) - for access_rule in connection.execute(access_table.select()): + share_access_rules = connection.execute( + access_table.select().where(access_table.c.deleted == "False")) + + for access_rule in share_access_rules: access_mapping = connection.execute( instance_access_table.select().where( - instance_access_table.c.deleted == "False").where( instance_access_table.c.access_id == access_rule['id']) ).first() diff --git a/manila/tests/db/migrations/alembic/migrations_data_checks.py b/manila/tests/db/migrations/alembic/migrations_data_checks.py index f72ddc8c25..ee92ac43db 100644 --- a/manila/tests/db/migrations/alembic/migrations_data_checks.py +++ b/manila/tests/db/migrations/alembic/migrations_data_checks.py @@ -586,6 +586,9 @@ class AccessRulesStatusMigrationChecks(BaseMigrationChecks): def check_downgrade(self, engine): share_instances_rules_table = utils.load_table( 'share_instance_access_map', engine) + share_instance_rules_to_check = engine.execute( + share_instances_rules_table.select().where( + share_instances_rules_table.c.id.in_(('1', '2', '3', '4')))) valid_statuses = { '1': 'active', @@ -594,7 +597,7 @@ class AccessRulesStatusMigrationChecks(BaseMigrationChecks): '4': None, } - for rule in engine.execute(share_instances_rules_table.select()): + for rule in share_instance_rules_to_check: valid_state = valid_statuses[rule['share_instance_id']] self.test_case.assertEqual(valid_state, rule['state']) @@ -2562,3 +2565,104 @@ class BackenInfoTableChecks(BaseMigrationChecks): def check_downgrade(self, engine): self.test_case.assertRaises(sa_exc.NoSuchTableError, utils.load_table, self.new_table_name, engine) + + +@map_to_migration('579c267fbb4d') +class ShareInstanceAccessMapTableChecks(BaseMigrationChecks): + share_access_table = 'share_access_map' + share_instance_access_table = 'share_instance_access_map' + + @staticmethod + def generate_share_instance(share_id, **kwargs): + share_instance_data = { + 'id': uuidutils.generate_uuid(), + 'deleted': 'False', + 'host': 'fake', + 'share_id': share_id, + 'status': constants.STATUS_AVAILABLE, + } + share_instance_data.update(**kwargs) + return share_instance_data + + @staticmethod + def generate_share_access_map(share_id, **kwargs): + share_access_data = { + 'id': uuidutils.generate_uuid(), + 'share_id': share_id, + 'deleted': 'False', + 'access_type': 'ip', + 'access_to': '192.0.2.10', + } + share_access_data.update(**kwargs) + return share_access_data + + def setup_upgrade_data(self, engine): + share = { + 'id': uuidutils.generate_uuid(), + 'share_proto': 'fake', + 'size': 1, + 'snapshot_id': None, + 'user_id': 'fake', + 'project_id': 'fake' + } + share_table = utils.load_table('shares', engine) + engine.execute(share_table.insert(share)) + + share_instances = [ + self.generate_share_instance(share['id']), + self.generate_share_instance(share['id']), + ] + + share_instance_table = utils.load_table('share_instances', engine) + for share_instance in share_instances: + engine.execute(share_instance_table.insert(share_instance)) + + share_accesses = [ + self.generate_share_access_map( + share['id'], state=constants.ACCESS_STATE_ACTIVE), + self.generate_share_access_map( + share['id'], state=constants.ACCESS_STATE_ERROR), + ] + self.active_share_access = share_accesses[0] + self.error_share_access = share_accesses[1] + share_access_table = utils.load_table('share_access_map', engine) + engine.execute(share_access_table.insert(share_accesses)) + + def check_upgrade(self, engine, data): + share_access_table = utils.load_table( + self.share_access_table, engine) + share_instance_access_table = utils.load_table( + self.share_instance_access_table, engine) + share_accesses = engine.execute(share_access_table.select()) + share_instance_accesses = engine.execute( + share_instance_access_table.select()) + + for share_access in share_accesses: + self.test_case.assertFalse(hasattr(share_access, 'state')) + + for si_access in share_instance_accesses: + if si_access['access_id'] in (self.active_share_access['id'], + self.error_share_access['id']): + self.test_case.assertIn(si_access['state'], + (self.active_share_access['state'], + self.error_share_access['state'])) + + def check_downgrade(self, engine): + self.test_case.assertRaises( + sa_exc.NoSuchTableError, utils.load_table, + self.share_instance_access_table, engine) + + share_access_table = utils.load_table( + self.share_access_table, engine) + share_accesses = engine.execute(share_access_table.select().where( + share_access_table.c.id.in_((self.active_share_access['id'], + self.error_share_access['id'])))) + + for share_access in share_accesses: + self.test_case.assertTrue(hasattr(share_access, 'state')) + if share_access['id'] == self.active_share_access['id']: + self.test_case.assertEqual( + constants.ACCESS_STATE_ACTIVE, share_access['state']) + elif share_access['id'] == self.error_share_access['id']: + self.test_case.assertEqual( + constants.ACCESS_STATE_ERROR, share_access['state']) diff --git a/releasenotes/notes/bug-1717392-fix-downgrade-share-access-map-bbd5fe9cc7002f2d.yaml b/releasenotes/notes/bug-1717392-fix-downgrade-share-access-map-bbd5fe9cc7002f2d.yaml new file mode 100644 index 0000000000..c75f4b3e1a --- /dev/null +++ b/releasenotes/notes/bug-1717392-fix-downgrade-share-access-map-bbd5fe9cc7002f2d.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - The `Launchpad bug 1717392 + `_ + has been fixed and database downgrades do not fail if the database + contains deleted access rules. Database downgrades are not recommended + in production environments.