diff --git a/nova/objects/instance.py b/nova/objects/instance.py index 08b60593fd25..1884604eca8f 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 000000000000..8db10285f72b --- /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 934fdd2441a7..680b5148b0fe 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 14798e36decb..45d54f620184 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')