Merge "db: Add migration to resolve shadow table discrepancies"

nova/db/main/migrations/versions/16f1fbcab42b_resolve_shadow_table_diffs.py

@@ -0,0 +1,62 @@
+# 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.
+
+"""Resolve shadow table diffs
+
+Revision ID: 16f1fbcab42b
+Revises: 8f2f1571d55b
+Create Date: 2021-08-20 13:26:30.204633
+"""
+
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '16f1fbcab42b'
+down_revision = '8f2f1571d55b'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    bind = op.get_bind()
+
+    # 244_increase_user_id_length_volume_usage_cache; the length in the
+    # corresponding shadow table was not increased
+
+    with op.batch_alter_table('shadow_volume_usage_cache') as batch_op:
+        batch_op.alter_column(
+            'user_id',
+            type_=sa.String(64),
+            existing_type=sa.String(36),
+        )
+
+    # 252_add_instance_extra_table; we shouldn't have created an index for the
+    # shadow table
+
+    op.drop_index('shadow_instance_extra_idx', 'shadow_instance_extra')
+
+    # 373_migration_uuid; we shouldn't have created an index for the shadow
+    # table
+
+    op.drop_index('shadow_migrations_uuid', 'shadow_migrations')
+
+    # 298_mysql_extra_specs_binary_collation; we changed the collation on the
+    # main table but not the shadow table
+
+    if bind.engine.name == 'mysql':
+        op.execute(
+            'ALTER TABLE shadow_instance_type_extra_specs '
+            'CONVERT TO CHARACTER SET utf8 '
+            'COLLATE utf8_bin'
+        )

nova/db/main/migrations/versions/8f2f1571d55b_initial_version.py

@@ -89,17 +89,20 @@ def _create_shadow_tables(connection):
                 )
                 column_copy = sa.Column(column.name, enum)
 
-            # TODO(stephenfin): Fix these various bugs in a follow-up
+            # NOTE(stephenfin): There were some bugs in the squashed
+            # sqlalchemy-migrate migrations which we need to account for here
 
             # 244_increase_user_id_length_volume_usage_cache; this
             # alteration should apply to shadow tables also
+            # (fixed in migration 16f1fbcab42b)
 
             if table_name == 'volume_usage_cache' and column.name == 'user_id':
-                # nullable should be True
+                # column type should be String(64)
                 column_copy = sa.Column('user_id', sa.String(36))
 
-            # 247_nullable_mismatch; these alterations should apply to shadow
-            # tables also
+            # 247_nullable_mismatch; these alterations were not applied to
+            # shadow tables
+            # (wontfix since there could be null entries in the database still)
 
             if table_name == 'quota_usages' and column.name == 'resources':
                 # nullable should be False
@@ -124,8 +127,9 @@ def _create_shadow_tables(connection):
                     # nullable should be False
                     column_copy = sa.Column('dev_type', sa.String(8))
 
-            # 280_add_nullable_false_to_keypairs_name; this should apply to the
-            # shadow table also
+            # 280_add_nullable_false_to_keypairs_name; these alterations were
+            # not applied to the shadow tables
+            # (wontfix since there could be null entries in the database still)
 
             if table_name == 'key_pairs' and column.name == 'name':
                 # nullable should be False
@@ -163,10 +167,9 @@ def _create_shadow_tables(connection):
             'shadow_' + table_name, meta, *columns, mysql_engine='InnoDB',
         )
 
-    # TODO(stephenfin): Fix these various bugs in a follow-up
-
     # 252_add_instance_extra_table; we don't create indexes for shadow tables
     # in general and these should be removed
+    # (fixed in migration 16f1fbcab42b)
 
     op.create_index(
         'shadow_instance_extra_idx',
@@ -174,6 +177,7 @@ def _create_shadow_tables(connection):
         ['instance_uuid'])
 
     # 373_migration_uuid; we should't create indexes for shadow tables
+    # (fixed in migration 16f1fbcab42b)
 
     op.create_index(
         'shadow_migrations_uuid', 'shadow_migrations', ['uuid'], unique=True)
@@ -1603,10 +1607,12 @@ def upgrade():
 
     _create_shadow_tables(bind)
 
-    # TODO(stephenfin): Fix these various bugs in a follow-up
+    # NOTE(stephenfin): There were some bugs in the squashed sqlalchemy-migrate
+    # migrations which we need to account for here
 
     # 298_mysql_extra_specs_binary_collation; we should update the shadow table
     # also
+    # (fixed in migration 16f1fbcab42b)
 
     if bind.engine.name == 'mysql':
         # Use binary collation for extra specs table

nova/tests/unit/db/main/test_migrations.py

@@ -33,6 +33,7 @@ import mock
 from oslo_db.sqlalchemy import enginefacade
 from oslo_db.sqlalchemy import test_fixtures
 from oslo_db.sqlalchemy import test_migrations
+from oslo_db.sqlalchemy import utils as oslodbutils
 from oslo_log.fixture import logging_error as log_fixture
 from oslo_log import log as logging
 from oslotest import base
@@ -202,6 +203,18 @@ class NovaMigrationsWalk(
         self.config = migration._find_alembic_conf('main')
         self.init_version = migration.ALEMBIC_INIT_VERSION['main']
 
+    def assertIndexExists(self, connection, table_name, index):
+        self.assertTrue(
+            oslodbutils.index_exists(connection, table_name, index),
+            'Index %s on table %s should not exist' % (index, table_name),
+        )
+
+    def assertIndexNotExists(self, connection, table_name, index):
+        self.assertFalse(
+            oslodbutils.index_exists(connection, table_name, index),
+            'Index %s on table %s should not exist' % (index, table_name),
+        )
+
     def _migrate_up(self, connection, revision):
         if revision == self.init_version:  # no tests for the initial revision
             alembic_api.upgrade(self.config, revision)
@@ -224,6 +237,24 @@ class NovaMigrationsWalk(
         if post_upgrade:
             post_upgrade(connection)
 
+    def _pre_upgrade_16f1fbcab42b(self, connection):
+        self.assertIndexExists(
+            connection, 'shadow_instance_extra', 'shadow_instance_extra_idx',
+        )
+        self.assertIndexExists(
+            connection, 'shadow_migrations', 'shadow_migrations_uuid',
+        )
+
+    def _check_16f1fbcab42b(self, connection):
+        self.assertIndexNotExists(
+            connection, 'shadow_instance_extra', 'shadow_instance_extra_idx',
+        )
+        self.assertIndexNotExists(
+            connection, 'shadow_migrations', 'shadow_migrations_uuid',
+        )
+
+        # no check for the MySQL-specific change
+
     def test_single_base_revision(self):
         """Ensure we only have a single base revision.