Merge "objects: Update keypairs when saving an instance" into stable/queens

4 files changed, 116 insertions, 8 deletions

diff --git a/nova/objects/instance.py b/nova/objects/instance.py
index 08b60593fd..1884604eca 100644
--- a/nova/objects/instance.py
+++ b/nova/objects/instance.py
@@ -695,8 +695,9 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
         pass
 
     def _save_keypairs(self, context):
-        # NOTE(danms): Read-only so no need to save this.
-        pass
+        if 'keypairs' in self.obj_what_changed():
+            self._save_extra_generic('keypairs')
+            self.obj_reset_changes(['keypairs'], recursive=True)
 
     def _save_extra_generic(self, field):
         if field in self.obj_what_changed():
diff --git a/nova/tests/functional/regressions/test_bug_1843708.py b/nova/tests/functional/regressions/test_bug_1843708.py
new file mode 100644
index 0000000000..8db10285f7
--- /dev/null
+++ b/nova/tests/functional/regressions/test_bug_1843708.py
@@ -0,0 +1,92 @@
+# Copyright 2019 NTT Corporation
+#
+# 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.
+
+from nova import context
+from nova import objects
+from nova import test
+from nova.tests import fixtures as nova_fixtures
+from nova.tests.functional import integrated_helpers
+from nova.tests.unit import fake_notifier
+from nova.tests.unit.image import fake as fake_image
+
+
+class RebuildWithKeypairTestCase(
+    test.TestCase, integrated_helpers.InstanceHelperMixin,
+):
+    """Regression test for bug 1843708.
+
+    This tests a rebuild scenario with new key pairs.
+    """
+
+    def setUp(self):
+        super(RebuildWithKeypairTestCase, self).setUp()
+        # Start standard fixtures.
+        self.useFixture(nova_fixtures.PlacementFixture())
+        self.useFixture(nova_fixtures.NeutronFixture(self))
+        fake_image.stub_out_image_service(self)
+        self.addCleanup(fake_image.FakeImageService_reset)
+        fake_notifier.stub_notifier(self)
+        self.addCleanup(fake_notifier.reset)
+        self.api = self.useFixture(nova_fixtures.OSAPIFixture(
+            api_version='v2.1')).admin_api
+        self.api.microversion = 'latest'
+        # Start nova services.
+        self.start_service('conductor')
+        self.start_service('scheduler')
+        self.start_service('compute')
+
+    def test_rebuild_with_keypair(self):
+        keypair_req = {
+            'keypair': {
+                'name': 'test-key1',
+                'type': 'ssh',
+            },
+        }
+        keypair1 = self.api.post_keypair(keypair_req)
+        keypair_req['keypair']['name'] = 'test-key2'
+        keypair2 = self.api.post_keypair(keypair_req)
+
+        server = self._build_minimal_create_server_request(
+            self.api, 'test-rebuild-with-keypair',
+            image_uuid=fake_image.get_valid_image_id(),
+            networks='none')
+        server.update({'key_name': 'test-key1'})
+
+        # Create a server with keypair 'test-key1'
+        server = self.api.post_server({'server': server})
+        self._wait_for_state_change(self.api, server, 'ACTIVE')
+
+        # Check keypairs
+        ctxt = context.get_admin_context()
+        instance = objects.Instance.get_by_uuid(
+            ctxt, server['id'], expected_attrs=['keypairs'])
+        self.assertEqual(
+            keypair1['public_key'], instance.keypairs[0].public_key)
+
+        # Rebuild a server with keypair 'test-key2'
+        body = {
+            'rebuild': {
+                'imageRef': fake_image.get_valid_image_id(),
+                'key_name': 'test-key2',
+            },
+        }
+        self.api.api_post('servers/%s/action' % server['id'], body)
+        fake_notifier.wait_for_versioned_notifications('instance.rebuild.end')
+        self._wait_for_state_change(self.api, server, 'ACTIVE')
+
+        # Check keypairs changed
+        instance = objects.Instance.get_by_uuid(
+            ctxt, server['id'], expected_attrs=['keypairs'])
+        self.assertEqual(
+            keypair2['public_key'], instance.keypairs[0].public_key)
diff --git a/nova/tests/unit/fake_instance.py b/nova/tests/unit/fake_instance.py
index 934fdd2441..680b5148b0 100644
--- a/nova/tests/unit/fake_instance.py
+++ b/nova/tests/unit/fake_instance.py
@@ -117,7 +117,6 @@ def fake_instance_obj(context, obj_instance_class=None, **updates):
                                 is_public=True,
                                 extra_specs={},
                                 projects=[])
-        flavor.obj_reset_changes()
     inst = obj_instance_class._from_db_object(context,
                obj_instance_class(), fake_db_instance(**updates),
                expected_attrs=expected_attrs)
@@ -133,7 +132,7 @@ def fake_instance_obj(context, obj_instance_class=None, **updates):
             inst.memory_mb = flavor.memory_mb
     inst.old_flavor = None
     inst.new_flavor = None
-    inst.obj_reset_changes()
+    inst.obj_reset_changes(recursive=True)
     return inst
 
 
diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py
index 14798e36de..45d54f6201 100644
--- a/nova/tests/unit/objects/test_instance.py
+++ b/nova/tests/unit/objects/test_instance.py
@@ -683,14 +683,30 @@ class _TestInstanceObject(object):
         inst.numa_topology = None
         inst.migration_context = None
         inst.vcpu_model = test_vcpu_model.fake_vcpumodel
-        inst.save()
+        inst.keypairs = objects.KeyPairList(
+            objects=[objects.KeyPair(name='foo')])
+
         json_vcpu_model = jsonutils.dumps(
             test_vcpu_model.fake_vcpumodel.obj_to_primitive())
-        expected_vals = {'numa_topology': None,
-                         'migration_context': None,
-                         'vcpu_model': json_vcpu_model}
+        json_keypairs = jsonutils.dumps(inst.keypairs.obj_to_primitive())
+
+        # Check changed fields in the instance object
+        self.assertIn('keypairs', inst.obj_what_changed())
+        self.assertEqual({'objects'}, inst.keypairs.obj_what_changed())
+
+        inst.save()
+
+        expected_vals = {
+            'numa_topology': None,
+            'migration_context': None,
+            'vcpu_model': json_vcpu_model,
+            'keypairs': json_keypairs,
+        }
         mock_update.assert_called_once_with(self.context, inst.uuid,
                                             expected_vals)
+        # Verify that the record of changed fields has been cleared
+        self.assertNotIn('keypairs', inst.obj_what_changed())
+        self.assertEqual(set(), inst.keypairs.obj_what_changed())
 
     @mock.patch.object(notifications, 'send_update')
     @mock.patch.object(cells_rpcapi.CellsAPI, 'instance_update_from_api')