From 891b8f9e987ba0a39ce4dc97b8e1d3602db15ba5 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Thu, 7 Nov 2019 15:33:50 -0500 Subject: [PATCH] Use named kwargs in compute.API.resize The only kwarg passed to resize() is the auto_disk_config kwarg during resize (not cold migrate). This expands it out to a named kwarg to resolve the TODO. The same is done with _check_auto_disk_config and as a result there is a hit to rebuild as well, but the same TODO exists on rebuild() to use named kwargs but that can be dealt with separately. Change-Id: Ide8eb9e09d22f20165474d499ef0524aefc67854 --- nova/api/openstack/compute/servers.py | 5 +++-- nova/compute/api.py | 16 +++++++--------- .../openstack/compute/test_server_actions.py | 7 ++++--- .../api/openstack/compute/test_serversV21.py | 6 ++++-- nova/tests/unit/compute/test_compute_api.py | 18 ++++++++++++------ 5 files changed, 30 insertions(+), 22 deletions(-) diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 67d2b421f2bc..7ff7cb6dd4cb 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -932,7 +932,7 @@ class ServersController(wsgi.Controller): common.raise_http_conflict_for_instance_invalid_state(state_error, 'reboot', id) - def _resize(self, req, instance_id, flavor_id, **kwargs): + def _resize(self, req, instance_id, flavor_id, auto_disk_config=None): """Begin the resize process with given instance/flavor.""" context = req.environ["nova.context"] instance = self._get_server(context, req, instance_id) @@ -953,7 +953,8 @@ class ServersController(wsgi.Controller): raise exc.HTTPConflict(explanation=msg) try: - self.compute_api.resize(context, instance, flavor_id, **kwargs) + self.compute_api.resize(context, instance, flavor_id, + auto_disk_config=auto_disk_config) except exception.QuotaError as error: raise exc.HTTPForbidden( explanation=error.format_message()) diff --git a/nova/compute/api.py b/nova/compute/api.py index c4e4a1d8805b..b40b668b2078 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1985,8 +1985,7 @@ class API(base.Base): requested_hypervisor_hostname=requested_hypervisor_hostname) def _check_auto_disk_config(self, instance=None, image=None, - **extra_instance_updates): - auto_disk_config = extra_instance_updates.get("auto_disk_config") + auto_disk_config=None): if auto_disk_config is None: return if not image and not instance: @@ -3360,7 +3359,8 @@ class API(base.Base): context, trusted_certs, rebuild=True) image_id, image = self._get_image(context, image_href) - self._check_auto_disk_config(image=image, **kwargs) + self._check_auto_disk_config(image=image, + auto_disk_config=auto_disk_config) flavor = instance.get_flavor() bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( @@ -3707,13 +3707,10 @@ class API(base.Base): return node - # TODO(mriedem): It looks like for resize (not cold migrate) the only - # possible kwarg here is auto_disk_config. Drop this dumb **kwargs and make - # it explicitly an auto_disk_config param @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED]) def resize(self, context, instance, flavor_id=None, clean_shutdown=True, - host_name=None, **extra_instance_updates): + host_name=None, auto_disk_config=None): """Resize (ie, migrate) a running instance. If flavor_id is None, the process is considered a migration, keeping @@ -3729,7 +3726,8 @@ class API(base.Base): node = self._validate_host_for_cold_migrate( context, instance, host_name, allow_cross_cell_resize) - self._check_auto_disk_config(instance, **extra_instance_updates) + self._check_auto_disk_config( + instance, auto_disk_config=auto_disk_config) current_instance_type = instance.get_flavor() @@ -3809,7 +3807,7 @@ class API(base.Base): instance.task_state = task_states.RESIZE_PREP instance.progress = 0 - instance.update(extra_instance_updates) + instance.auto_disk_config = auto_disk_config or False instance.save(expected_task_state=[None]) if not flavor_id: diff --git a/nova/tests/unit/api/openstack/compute/test_server_actions.py b/nova/tests/unit/api/openstack/compute/test_server_actions.py index 02ea7246e363..e84aae44aa3b 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_server_actions.py @@ -162,7 +162,7 @@ class ServerActionsControllerTestV21(test.TestCase): 'imageRef': self.image_uuid, 'adminPass': 'TNc53Dr8s7vw'}}} - args_map = {'_action_resize': (('2'), {}), + args_map = {'_action_resize': (('2'), {'auto_disk_config': None}), '_action_confirm_resize': ((), {}), '_action_reboot': (('HARD',), {}), '_action_rebuild': ((self.image_uuid, @@ -664,7 +664,7 @@ class ServerActionsControllerTestV21(test.TestCase): self.resize_called = False - def resize_mock(*args): + def resize_mock(*args, **kwargs): self.resize_called = True self.stub_out('nova.compute.api.API.resize', resize_mock) @@ -729,7 +729,8 @@ class ServerActionsControllerTestV21(test.TestCase): raised, expected = map(iter, zip(*exceptions)) - def _fake_resize(obj, context, instance, flavor_id): + def _fake_resize(obj, context, instance, flavor_id, + auto_disk_config=None): self.resize_called += 1 raise next(raised) diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index 2e16b9d3a05a..70f3e9a2f2ef 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -8502,7 +8502,8 @@ class ServersPolicyEnforcementV21(test.NoDBTestCase): body = {'resize': {'flavorRef': '1'}} self.controller._action_resize(self.req, fakes.FAKE_UUID, body=body) resize_mock.assert_called_once_with(self.req.environ['nova.context'], - instance, '1') + instance, '1', + auto_disk_config=None) @mock.patch('nova.api.openstack.common.get_instance') def test_resize_overridden_policy_failed_with_other_user_in_same_project( @@ -8535,7 +8536,8 @@ class ServersPolicyEnforcementV21(test.NoDBTestCase): body = {'resize': {'flavorRef': '1'}} self.controller._action_resize(self.req, fakes.FAKE_UUID, body=body) resize_mock.assert_called_once_with(self.req.environ['nova.context'], - instance, '1') + instance, '1', + auto_disk_config=None) @mock.patch('nova.api.openstack.common.get_instance') def test_rebuild_policy_failed_with_other_project(self, get_instance_mock): diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 2c3672b771d5..85ed941ec6ac 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -3579,7 +3579,8 @@ class _ComputeAPIUnitTestMixIn(object): preserve_ephemeral=False, host=instance.host, request_spec=fake_spec) - _check_auto_disk_config.assert_called_once_with(image=image) + _check_auto_disk_config.assert_called_once_with( + image=image, auto_disk_config=None) _checks_for_create_and_rebuild.assert_called_once_with(self.context, None, image, flavor, {}, [], None) self.assertNotEqual(orig_system_metadata, instance.system_metadata) @@ -3655,7 +3656,8 @@ class _ComputeAPIUnitTestMixIn(object): self.assertEqual('rebuild', fake_spec.scheduler_hints['_nova_check_type'][0]) - _check_auto_disk_config.assert_called_once_with(image=new_image) + _check_auto_disk_config.assert_called_once_with( + image=new_image, auto_disk_config=None) _checks_for_create_and_rebuild.assert_called_once_with(self.context, None, new_image, flavor, {}, [], None) self.assertEqual(fields_obj.VMMode.XEN, instance.vm_mode) @@ -3716,7 +3718,8 @@ class _ComputeAPIUnitTestMixIn(object): preserve_ephemeral=False, host=instance.host, request_spec=fake_spec) - _check_auto_disk_config.assert_called_once_with(image=image) + _check_auto_disk_config.assert_called_once_with( + image=image, auto_disk_config=None) _checks_for_create_and_rebuild.assert_called_once_with(self.context, None, image, flavor, {}, [], None) self.assertNotEqual(orig_key_name, instance.key_name) @@ -3774,7 +3777,8 @@ class _ComputeAPIUnitTestMixIn(object): preserve_ephemeral=False, host=instance.host, request_spec=fake_spec) - _check_auto_disk_config.assert_called_once_with(image=image) + _check_auto_disk_config.assert_called_once_with( + image=image, auto_disk_config=None) _checks_for_create_and_rebuild.assert_called_once_with( self.context, None, image, flavor, {}, [], None) self.assertEqual(new_trusted_certs, instance.trusted_certs.ids) @@ -3837,7 +3841,8 @@ class _ComputeAPIUnitTestMixIn(object): preserve_ephemeral=False, host=instance.host, request_spec=fake_spec) - _check_auto_disk_config.assert_called_once_with(image=image) + _check_auto_disk_config.assert_called_once_with( + image=image, auto_disk_config=None) _checks_for_create_and_rebuild.assert_called_once_with( self.context, None, image, flavor, {}, [], None) self.assertIsNone(instance.trusted_certs) @@ -3877,7 +3882,8 @@ class _ComputeAPIUnitTestMixIn(object): image_href, admin_pass, files_to_inject, trusted_certs=new_trusted_certs) - _check_auto_disk_config.assert_called_once_with(image=image) + _check_auto_disk_config.assert_called_once_with( + image=image, auto_disk_config=None) def _test_check_injected_file_quota_onset_file_limit_exceeded(self, side_effect):