Warn the user about orphaned extra records during keypair migration

Operators who have manually deleted Instance records with FK constraints
disabled may have orphaned InstanceExtra records which will prevent the
keypair migration from running. Normally, this violation of the data
model would be something that earns no sympathy. However, that solution
was (incorrectly) offered up as a workaround in bug 1511466 and multiple
deployments have broken their data as a result. Since the experience
is a unhelpful error message and a blocked migration, this patch attempts
to at least highlight the problem, even though it is on the operator to
actually fix the problem.

Change-Id: I0c8bf2c495a98c412eb93e19f832948a779bca11
Related-Bug: #1684861
(cherry picked from commit 2cee5bb4d14a5ad6a037857bd3624833317cd931)

diff --git a/nova/objects/instance.py b/nova/objects/instance.py
index ca805b7d6ecbc99fe6d9cc9dbf85321e376e16ce..e0d79e26eed5cdfd4cd19c9cda1f34ec28a02daf 100644 (file)
--- a/nova/objects/instance.py
+++ b/nova/objects/instance.py
@@ -1384,6 +1384,12 @@ def _migrate_instance_keypairs(ctxt, count):
     count_all = len(db_extras)
     count_hit = 0
     for db_extra in db_extras:
+        if db_extra.instance is None:
+            LOG.error(
+                ('Instance %(uuid)s has been purged, but an instance_extra '
+                 'record remains for it. Unable to migrate.'),
+                {'uuid': db_extra.instance_uuid})
+            continue
         key_name = db_extra.instance.key_name
         keypairs = objects.KeyPairList(objects=[])
         if key_name:

diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py
index 13f59e9eac5b4126c7473c27014a8d08ea5d18a9..e5f7026c8dc2fc53cb453aa2fdbde2f562cd69cc 100644 (file)
--- a/nova/tests/unit/objects/test_instance.py
+++ b/nova/tests/unit/objects/test_instance.py
@@ -1973,8 +1973,20 @@ class TestInstanceObjectMisc(test.TestCase):
                                  key_name='missingkey')
         inst3.create()
 
+        inst4 = objects.Instance(context=ctxt,
+                                 user_id=ctxt.user_id,
+                                 project_id=ctxt.project_id,
+                                 key_name='missingkey')
+        inst4.create()
+        inst4.destroy()
+
+        # NOTE(danms): Add an orphaned instance_extra record for
+        # a totally invalid instance to make sure we don't explode.
+        # See bug 1684861 for more information.
+        db.instance_extra_update_by_uuid(ctxt, 'foo', {})
+
         hit, done = instance.migrate_instance_keypairs(ctxt, 10)
-        self.assertEqual(2, hit)
+        self.assertEqual(3, hit)
         self.assertEqual(2, done)
         db_extra = db.instance_extra_get_by_instance_uuid(
             ctxt, inst1.uuid, ['keypairs'])