From 4317166b72bb0aadd0321acdf9f2450c1a99d0a4 Mon Sep 17 00:00:00 2001
From: Matt Riedemann <mriedem@us.ibm.com>
Date: Tue, 14 Jun 2016 16:05:35 -0400
Subject: [PATCH] Handle keypair not found from metadata server

With commit e83842b80b73c451f78a4bb9e7bd5dfcebdefcab we
attempt to load keypairs for an instance from instance_extra,
but if that hasn't been migrated yet we fall back to loading
the keypair from the database by name.

If the keypair was deleted, the instance object will just set
an empty KeyPairList for instance.keypairs and we'll get an
IndexError when using self.instance.keypairs[0] in
_metadata_as_json.

This adds a check that instance.keypairs actually has
something in it. If not, we log a message and don't return
any key values in the metadata dict - same as if instance.key_name
wasn't set to begin with.

Change-Id: If823867d1df4bafa46978e62e05826d1f12c9269
Closes-Bug: #1592167
---
 nova/api/metadata/base.py        | 28 +++++++++++++++++++---------
 nova/tests/unit/test_metadata.py | 15 +++++++++++++++
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/nova/api/metadata/base.py b/nova/api/metadata/base.py
index 4d85e2270710..da5c7e4361fa 100644
--- a/nova/api/metadata/base.py
+++ b/nova/api/metadata/base.py
@@ -306,17 +306,27 @@ class InstanceMetadata(object):
                   context.get_admin_context(), self.instance.user_id,
                   self.instance.key_name)
             else:
-                keypair = self.instance.keypairs[0]
+                keypairs = self.instance.keypairs
+                # NOTE(mriedem): It's possible for the keypair to be deleted
+                # before it was migrated to the instance_extra table, in which
+                # case lazy-loading instance.keypairs will handle the 404 and
+                # just set an empty KeyPairList object on the instance.
+                keypair = keypairs[0] if keypairs else None
 
-            metadata['public_keys'] = {
-                keypair.name: keypair.public_key,
-            }
+            if keypair:
+                metadata['public_keys'] = {
+                    keypair.name: keypair.public_key,
+                }
 
-            metadata['keys'] = [
-                {'name': keypair.name,
-                 'type': keypair.type,
-                 'data': keypair.public_key}
-            ]
+                metadata['keys'] = [
+                    {'name': keypair.name,
+                     'type': keypair.type,
+                     'data': keypair.public_key}
+                ]
+            else:
+                LOG.debug("Unable to find keypair for instance with "
+                          "key name '%s'.", self.instance.key_name,
+                          instance=self.instance)
 
         metadata['hostname'] = self._get_hostname()
         metadata['name'] = self.instance.display_name
diff --git a/nova/tests/unit/test_metadata.py b/nova/tests/unit/test_metadata.py
index 8913d627218e..9d3f4f2ef1b0 100644
--- a/nova/tests/unit/test_metadata.py
+++ b/nova/tests/unit/test_metadata.py
@@ -458,6 +458,21 @@ class MetadataTestCase(test.TestCase):
             self._test_as_json_with_options(is_cells=True,
                                             os_version=os_version)
 
+    @mock.patch.object(objects.Instance, 'get_by_uuid')
+    def test_metadata_as_json_deleted_keypair(self, mock_inst_get_by_uuid):
+        """Tests that we handle missing instance keypairs.
+        """
+        instance = self.instance.obj_clone()
+        # we want to make sure that key_name is set but not keypairs so it has
+        # to be lazy-loaded from the database
+        delattr(instance, 'keypairs')
+        mock_inst_get_by_uuid.return_value = instance
+        md = fake_InstanceMetadata(self.stubs, instance)
+        meta = md._metadata_as_json(base.OPENSTACK_VERSIONS[-1], path=None)
+        meta = jsonutils.loads(meta)
+        self.assertNotIn('keys', meta)
+        self.assertNotIn('public_keys', meta)
+
 
 class OpenStackMetadataTestCase(test.TestCase):
     def setUp(self):