From 593e7081148b2b15f5923a8d7c609ae71dcbdbb6 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Thu, 20 Apr 2017 09:12:45 -0700 Subject: [PATCH] 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) --- nova/objects/instance.py | 6 ++++++ nova/tests/unit/objects/test_instance.py | 14 +++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/nova/objects/instance.py b/nova/objects/instance.py index ca805b7d6ecb..e0d79e26eed5 100644 --- 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 13f59e9eac5b..e5f7026c8dc2 100644 --- 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'])