diff --git a/nova/objects/instance.py b/nova/objects/instance.py index 6c7e8aff86cd..ba0ea158f934 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -734,8 +734,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 000000000000..74f391d63b6c --- /dev/null +++ b/nova/tests/functional/regressions/test_bug_1843708.py @@ -0,0 +1,93 @@ +# 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 fixtures as func_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(func_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 5de52d339f05..1740a9ebafc9 100644 --- a/nova/tests/unit/fake_instance.py +++ b/nova/tests/unit/fake_instance.py @@ -119,7 +119,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) @@ -140,7 +139,7 @@ def fake_instance_obj(context, obj_instance_class=None, **updates): inst.old_flavor = None inst.new_flavor = None inst.resources = 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 c108deb1db29..ec9ee2634ab9 100644 --- a/nova/tests/unit/objects/test_instance.py +++ b/nova/tests/unit/objects/test_instance.py @@ -683,15 +683,32 @@ 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(db, 'instance_get_by_uuid') def test_get_deleted(self, mock_get): fake_inst = dict(self.fake_instance, id=123, deleted=123)