diff options
author | Henry Nash <henryn@linux.vnet.ibm.com> | 2014-09-28 11:16:26 +0100 |
---|---|---|
committer | David Stanek <dstanek@dstanek.com> | 2014-10-06 16:14:52 +0000 |
commit | ff970f4e6215ff11dead4ad1b8abada644b50b31 (patch) | |
tree | 364864cebac9cc26c954b290a3e91624376c540d | |
parent | e2589177d9d8b2b31609e77619afd6ae462d13e3 (diff) | |
download | keystone-ff970f4e6215ff11dead4ad1b8abada644b50b31.tar.gz |
Ensure sql upgrade tests can run with non-sqlite databases.
This patch fixes the issues that were preventing the running of
live sql upgrade tests (either by running test_sql_upgrade directly
or via test_sql_livetest), namely:
- Dropping the tables that were in existence before the current
scope of migration in an order that is FK friendly
- Fixing an issue where the tables were being dropped in the
wrong order in the downgrade of federation
- Ensuring we don't hold sessions open over upgrade/downgrade
steps in our test methods
Limitations:
- This patch has not been tested with DB2
Closes-Bug: 1363047
Closes-Bug: 1375937
Change-Id: Ied4741a9646b57bc6f2ddcdc8a380ea55b2a9634
-rw-r--r-- | keystone/contrib/federation/migrate_repo/versions/001_add_identity_provider_table.py | 2 | ||||
-rw-r--r-- | keystone/tests/test_sql_upgrade.py | 82 |
2 files changed, 65 insertions, 19 deletions
diff --git a/keystone/contrib/federation/migrate_repo/versions/001_add_identity_provider_table.py b/keystone/contrib/federation/migrate_repo/versions/001_add_identity_provider_table.py index 1a522c206..8e05c9764 100644 --- a/keystone/contrib/federation/migrate_repo/versions/001_add_identity_provider_table.py +++ b/keystone/contrib/federation/migrate_repo/versions/001_add_identity_provider_table.py @@ -41,7 +41,7 @@ def upgrade(migrate_engine): def downgrade(migrate_engine): meta = sql.MetaData() meta.bind = migrate_engine - tables = ['identity_provider', 'federation_protocol'] + tables = ['federation_protocol', 'identity_provider'] for table_name in tables: table = sql.Table(table_name, meta, autoload=True) table.drop() diff --git a/keystone/tests/test_sql_upgrade.py b/keystone/tests/test_sql_upgrade.py index 474895332..c8208b9e5 100644 --- a/keystone/tests/test_sql_upgrade.py +++ b/keystone/tests/test_sql_upgrade.py @@ -38,7 +38,9 @@ from oslo.db import exception as db_exception from oslo.db.sqlalchemy import migration from oslo.db.sqlalchemy import session as db_session import six +from sqlalchemy.engine import reflection import sqlalchemy.exc +from sqlalchemy import schema from keystone.assignment.backends import sql as assignment_sql from keystone.common import sql @@ -179,9 +181,30 @@ class SqlMigrateBase(tests.SQLDriverOverrides, tests.TestCase): meta = sqlalchemy.MetaData() meta.bind = self.engine meta.reflect(self.engine) - for table in list(meta.tables.keys()): - table = sqlalchemy.Table(table, meta, autoload=True) - table.drop(self.engine, checkfirst=True) + + with self.engine.begin() as conn: + inspector = reflection.Inspector.from_engine(self.engine) + metadata = schema.MetaData() + tbs = [] + all_fks = [] + + for table_name in inspector.get_table_names(): + fks = [] + for fk in inspector.get_foreign_keys(table_name): + if not fk['name']: + continue + fks.append( + schema.ForeignKeyConstraint((), (), name=fk['name'])) + table = schema.Table(table_name, metadata, *fks) + tbs.append(table) + all_fks.extend(fks) + + for fkc in all_fks: + conn.execute(schema.DropConstraint(fkc)) + + for table in tbs: + conn.execute(schema.DropTable(table)) + sql.cleanup() super(SqlMigrateBase, self).tearDown() @@ -237,7 +260,10 @@ class SqlMigrateBase(tests.SQLDriverOverrides, tests.TestCase): self.initialize_sql() table = self.select_table(table_name) actual_cols = [col.name for col in table.columns] - self.assertEqual(expected_cols, actual_cols, '%s table' % table_name) + # Check if the columns are equal, but allow for a different order, + # which might occur after an upgrade followed by a downgrade + self.assertEqual(expected_cols.sort(), actual_cols.sort(), + '%s table' % table_name) @property def initial_db_version(self): @@ -758,6 +784,7 @@ class SqlUpgradeTests(SqlMigrateBase): # we should have 3 trusts in base self.assertEqual(3, session.query(trust_table).count()) + session.close() self.downgrade(40) session = self.Session() trust_table = sqlalchemy.Table( @@ -795,8 +822,6 @@ class SqlUpgradeTests(SqlMigrateBase): def test_upgrade_service_enabled_data(self): """Migration 44 has to migrate data from `extra` to `enabled`.""" - session = self.Session() - def add_service(**extra_data): service_id = uuid.uuid4().hex @@ -811,6 +836,7 @@ class SqlUpgradeTests(SqlMigrateBase): return service_id self.upgrade(43) + session = self.Session() # Different services with expected enabled and extra values, and a # description. @@ -839,7 +865,9 @@ class SqlUpgradeTests(SqlMigrateBase): (False, random_attr), random_attr_enabled_false_str), ] + session.close() self.upgrade(44) + session = self.Session() # Verify that the services have the expected values. @@ -859,7 +887,7 @@ class SqlUpgradeTests(SqlMigrateBase): enabled, extra = fetch_service(service_id) - self.assertIs(exp_enabled, enabled, msg) + self.assertEqual(exp_enabled, enabled, msg) self.assertEqual(exp_extra, extra, msg) def test_downgrade_service_enabled_data(self): @@ -870,8 +898,6 @@ class SqlUpgradeTests(SqlMigrateBase): """ - session = self.Session() - def add_service(enabled=True, **extra_data): service_id = uuid.uuid4().hex @@ -887,6 +913,7 @@ class SqlUpgradeTests(SqlMigrateBase): return service_id self.upgrade(44) + session = self.Session() # Insert some services using the new format. @@ -912,7 +939,9 @@ class SqlUpgradeTests(SqlMigrateBase): "enabled=False, something='whatever'"), ] + session.close() self.downgrade(43) + session = self.Session() # Verify that the services have the expected values. @@ -959,8 +988,6 @@ class SqlUpgradeTests(SqlMigrateBase): def test_upgrade_endpoint_enabled_data(self): """Migration 42 has to migrate data from `extra` to `enabled`.""" - session = self.Session() - def add_service(): service_id = uuid.uuid4().hex @@ -988,6 +1015,7 @@ class SqlUpgradeTests(SqlMigrateBase): return endpoint_id self.upgrade(41) + session = self.Session() # Insert some endpoints using the old format where `enabled` is in # `extra` JSON. @@ -1024,7 +1052,9 @@ class SqlUpgradeTests(SqlMigrateBase): (False, random_attr), random_attr_enabled_false_str), ] + session.close() self.upgrade(42) + session = self.Session() # Verify that the endpoints have the expected values. @@ -1044,7 +1074,9 @@ class SqlUpgradeTests(SqlMigrateBase): enabled, extra = fetch_endpoint(endpoint_id) - self.assertIs(exp_enabled, enabled, msg) + # NOTE(henry-nash): Different databases may return enabled as a + # real boolean of 0/1 - so we use assertEqual not assertIs here. + self.assertEqual(exp_enabled, enabled, msg) self.assertEqual(exp_extra, extra, msg) def test_downgrade_endpoint_enabled_data(self): @@ -1055,8 +1087,6 @@ class SqlUpgradeTests(SqlMigrateBase): """ - session = self.Session() - def add_service(): service_id = uuid.uuid4().hex @@ -1085,6 +1115,7 @@ class SqlUpgradeTests(SqlMigrateBase): return endpoint_id self.upgrade(42) + session = self.Session() # Insert some endpoints using the new format. @@ -1109,7 +1140,9 @@ class SqlUpgradeTests(SqlMigrateBase): "enabled=False, something='whatever'"), ] + session.close() self.downgrade(41) + session = self.Session() # Verify that the endpoints have the expected values. @@ -1137,7 +1170,6 @@ class SqlUpgradeTests(SqlMigrateBase): Create two regions with the same description. """ - session = self.Session() def add_region(): region_uuid = uuid.uuid4().hex @@ -1151,6 +1183,7 @@ class SqlUpgradeTests(SqlMigrateBase): return region_uuid self.upgrade(43) + session = self.Session() # Write one region to the database add_region() # Write another region to the database with the same description @@ -1165,7 +1198,6 @@ class SqlUpgradeTests(SqlMigrateBase): Create two regions with the same description. """ - session = self.Session() def add_region(table): region_uuid = uuid.uuid4().hex @@ -1185,6 +1217,7 @@ class SqlUpgradeTests(SqlMigrateBase): # Migrate to version 42 self.upgrade(42) + session = self.Session() region_table = sqlalchemy.Table('region', get_metadata(), autoload=True) @@ -1205,16 +1238,23 @@ class SqlUpgradeTests(SqlMigrateBase): # into more specific exception objects, we should catch both of # sqlalchemy and oslo.db exceptions. If an old oslo.db version # is installed, IntegrityError is raised. If >=0.4.0 version of - # oslo.db is installed, DBDuplicateEntry is raised. + # oslo.db is installed, DBError is raised. # When the global requirements is updated with # the version fixes exceptions wrapping, IntegrityError must be # removed from the tuple. - (sqlalchemy.exc.IntegrityError, db_exception.DBDuplicateEntry), + + # NOTE(henry-nash): The above re-creation of the (now erased from + # history) unique constraint doesn't appear to work well with the + # Postgresql SQA driver, leading to it throwing a ValueError, so + # we also catch that here. + (sqlalchemy.exc.IntegrityError, db_exception.DBError, ValueError), add_region, table=region_unique_table) # migrate to 43, unique constraint should be dropped + session.close() self.upgrade(43) + session = self.Session() # reload the region table from the schema region_nonunique = sqlalchemy.Table('region', @@ -1267,7 +1307,9 @@ class SqlUpgradeTests(SqlMigrateBase): self.insert_dict(session, 'region', acme) region_table = sqlalchemy.Table('region', self.metadata, autoload=True) self.assertEqual(2, session.query(region_table).count()) + session.close() self.downgrade(51) + session = self.Session() self.metadata.clear() region_table = sqlalchemy.Table('region', self.metadata, autoload=True) self.assertEqual(2, session.query(region_table).count()) @@ -1344,7 +1386,9 @@ class SqlUpgradeTests(SqlMigrateBase): add_endpoint(_service_id_, region=None) # upgrade to 53 + session.close() self.upgrade(53) + session = self.Session() self.metadata.clear() region_table = sqlalchemy.Table('region', self.metadata, autoload=True) @@ -1367,7 +1411,9 @@ class SqlUpgradeTests(SqlMigrateBase): filter_by(region_id=_small_region_name).count()) # downgrade to 52 + session.close() self.downgrade(52) + session = self.Session() self.metadata.clear() region_table = sqlalchemy.Table('region', self.metadata, autoload=True) |