Fix an error in archiving 'migrations' table

Add soft deleting 'migrations' table when the VM instance is deleted.
And add soft deleting 'migrations' table when archiving deleted rows
for the case to upgrade.

Change-Id: Ica35ce2628dfcf412eb097c2c61fdde8828e9d90
Closes-Bug: #1584702

nova/db/sqlalchemy/api.py

@@ -1847,6 +1847,9 @@ def instance_destroy(context, instance_uuid, constraint=None):
     model_query(context, models.BlockDeviceMapping).\
             filter_by(instance_uuid=instance_uuid).\
             soft_delete()
+    model_query(context, models.Migration).\
+            filter_by(instance_uuid=instance_uuid).\
+            soft_delete()
     # NOTE(snikitin): We can't use model_query here, because there is no
     # column 'deleted' in 'tags' table.
     context.session.query(models.Tag).filter_by(
@@ -6314,7 +6317,10 @@ def _archive_deleted_rows_for_table(tablename, max_rows):
     # NOTE(clecomte): Tables instance_actions and instances_actions_events
     # have to be manage differently so we soft-delete them here to let
     # the archive work the same for all tables
-    if tablename == "instance_actions":
+    # NOTE(takashin): The record in table migrations should be
+    # soft deleted when the instance is deleted.
+    # This is just for upgrading.
+    if tablename in ("instance_actions", "migrations"):
         instances = models.BASE.metadata.tables["instances"]
         deleted_instances = sql.select([instances.c.uuid]).\
             where(instances.c.deleted != instances.c.deleted.default.arg)

nova/tests/unit/db/test_db_api.py

@@ -2881,6 +2881,27 @@ class InstanceTestCase(test.TestCase, ModelsObjectComparatorMixin):
                          db.instance_group_members_get(ctxt,
                                                        group['uuid']))
 
+    def test_delete_migrations_on_instance_destroy(self):
+        ctxt = context.get_admin_context()
+        uuid = uuidsentinel.uuid1
+        db.instance_create(ctxt, {'uuid': uuid})
+
+        migrations_values = {'instance_uuid': uuid}
+        migration = db.migration_create(ctxt, migrations_values)
+
+        migrations = db.migration_get_all_by_filters(
+            ctxt, {'instance_uuid': uuid})
+
+        self.assertEqual(1, len(migrations))
+        self._assertEqualObjects(migration, migrations[0])
+
+        instance = db.instance_destroy(ctxt, uuid)
+        migrations = db.migration_get_all_by_filters(
+            ctxt, {'instance_uuid': uuid})
+
+        self.assertTrue(instance.deleted)
+        self.assertEqual(0, len(migrations))
+
     def test_instance_update_and_get_original(self):
         instance = self.create_instance_with_args(vm_state='building')
         (old_ref, new_ref) = db.instance_update_and_get_original(self.ctxt,
@@ -8842,6 +8863,9 @@ class ArchiveTestCase(test.TestCase, ModelsObjectComparatorMixin):
         self.instances = models.Instance.__table__
         self.shadow_instances = sqlalchemyutils.get_table(
             self.engine, "shadow_instances")
+        self.migrations = models.Migration.__table__
+        self.shadow_migrations = sqlalchemyutils.get_table(
+            self.engine, "shadow_migrations")
 
         self.uuidstrs = []
         for _ in range(6):
@@ -9044,8 +9068,7 @@ class ArchiveTestCase(test.TestCase, ModelsObjectComparatorMixin):
             'shadow_dns_domains',
         )
 
-    def test_archive_deleted_rows_fk_constraint(self):
-        # consoles.pool_id depends on console_pools.id
+    def _check_sqlite_version_less_than_3_7(self):
         # SQLite doesn't enforce foreign key constraints without a pragma.
         dialect = self.engine.url.get_dialect()
         if dialect == sqlite.dialect:
@@ -9059,6 +9082,10 @@ class ArchiveTestCase(test.TestCase, ModelsObjectComparatorMixin):
                 self.skipTest(
                     'sqlite version too old for reliable SQLA foreign_keys')
             self.conn.execute("PRAGMA foreign_keys = ON")
+
+    def test_archive_deleted_rows_fk_constraint(self):
+        # consoles.pool_id depends on console_pools.id
+        self._check_sqlite_version_less_than_3_7()
         ins_stmt = self.console_pools.insert().values(deleted=1)
         result = self.conn.execute(ins_stmt)
         id1 = result.inserted_primary_key[0]
@@ -9083,6 +9110,33 @@ class ArchiveTestCase(test.TestCase, ModelsObjectComparatorMixin):
             'shadow_consoles'
         )
 
+    def test_archive_deleted_rows_for_migrations(self):
+        # migrations.instance_uuid depends on instances.uuid
+        self._check_sqlite_version_less_than_3_7()
+        instance_uuid = uuidsentinel.instance
+        ins_stmt = self.instances.insert().values(uuid=instance_uuid,
+                                                  deleted=1)
+        self.conn.execute(ins_stmt)
+        ins_stmt = self.migrations.insert().values(instance_uuid=instance_uuid,
+                                                   deleted=0)
+        self.conn.execute(ins_stmt)
+        # The first try to archive instances should fail, due to FK.
+        num = sqlalchemy_api._archive_deleted_rows_for_table("instances",
+                                                             max_rows=None)
+        self.assertEqual(0, num)
+        # Then archiving migrations should work.
+        num = sqlalchemy_api._archive_deleted_rows_for_table("migrations",
+                                                             max_rows=None)
+        self.assertEqual(1, num)
+        # Then archiving instances should work.
+        num = sqlalchemy_api._archive_deleted_rows_for_table("instances",
+                                                             max_rows=None)
+        self.assertEqual(1, num)
+        self._assert_shadow_tables_empty_except(
+            'shadow_instances',
+            'shadow_migrations'
+        )
+
     def test_archive_deleted_rows_2_tables(self):
         # Add 6 rows to each table
         for uuidstr in self.uuidstrs: