From 6c291830232cf0b628bf55dd2ae3f0966327645b Mon Sep 17 00:00:00 2001 From: Neha Alhat Date: Mon, 18 Sep 2017 17:57:44 +0530 Subject: [PATCH] Fix soft deleting vm fails after "nova resize" vm Problem description: When trying to soft-delete an instance that is just resized but the resize action is not yet confirmed, the soft-delete call will first attempt to confirm_resize the instance, then an error will occur since confirm_resize method does not expect a SOFT_DELETING task_state currently. https://github.com/openstack/nova/blob/8e052c7/nova/compute/manager.py#L3911 Fix steps: 1 Add SOFT_DELETING to the expected_task_state of confirm_resize method. 2 Since confirm_resize method sets instance.task_state to None, set instance.task_state to back to SOFT_DELETING after confirm_resize is executed, so the rest workflow should finish as normal situations. Co-Authored-By: Chen Change-Id: Ia4592adc93960625148ffa6e9f7d1cfa0c6046aa Closes-Bug: #1712480 (cherry picked from commit 018522f4d0566bf9f22fe0264eaedffa12f245e9) --- nova/compute/api.py | 12 ++++++++++++ nova/compute/manager.py | 3 ++- nova/tests/unit/compute/test_compute_api.py | 21 +++++++++++++++++++++ nova/tests/unit/compute/test_compute_mgr.py | 3 +++ 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 3beb89d5d263..d4ff12503b8c 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1982,6 +1982,18 @@ class API(base.Base): if instance.vm_state == vm_states.RESIZED: self._confirm_resize_on_deleting(context, instance) + # NOTE(neha_alhat): After confirm resize vm_state will become + # 'active' and task_state will be set to 'None'. But for soft + # deleting a vm, the _do_soft_delete callback requires + # task_state in 'SOFT_DELETING' status. So, we need to set + # task_state as 'SOFT_DELETING' again for soft_delete case. + # After confirm resize and before saving the task_state to + # "SOFT_DELETING", during the short window, user can submit + # soft delete vm request again and system will accept and + # process it without any errors. + if delete_type == 'soft_delete': + instance.task_state = instance_attrs['task_state'] + instance.save() is_local_delete = True try: diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 0e31263a67a2..9bd0e1643afb 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -3908,7 +3908,8 @@ class ComputeManager(manager.Manager): instance.vm_state = vm_state instance.task_state = None - instance.save(expected_task_state=[None, task_states.DELETING]) + instance.save(expected_task_state=[None, task_states.DELETING, + task_states.SOFT_DELETING]) self._notify_about_instance_usage( context, instance, "resize.confirm.end", diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 3f86abff9b3b..26f74fa88a9b 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -1051,6 +1051,13 @@ class _ComputeAPIUnitTestMixIn(object): mock_confirm = self.useFixture( fixtures.MockPatchObject(rpcapi, 'confirm_resize')).mock + def _reset_task_state(context, instance, migration, src_host, + cast=False): + inst.update({'task_state': None}) + + # After confirm resize action, instance task_state is reset to None + mock_confirm.side_effect = _reset_task_state + is_shelved = inst.vm_state in (vm_states.SHELVED, vm_states.SHELVED_OFFLOADED) if is_shelved: @@ -1093,6 +1100,17 @@ class _ComputeAPIUnitTestMixIn(object): expected_record_calls.append( mock.call(self.context, inst, instance_actions.CONFIRM_RESIZE)) + + # After confirm resize action, instance task_state + # is reset to None, so is the expected value. But + # for soft delete, task_state will be again reset + # back to soft-deleting in the code to avoid status + # checking failure. + updates['task_state'] = None + if delete_type == 'soft_delete': + expected_save_calls.append(mock.call()) + updates['task_state'] = 'soft-deleting' + if inst.host is not None: mock_elevated.return_value = self.context expected_elevated_calls.append(mock.call()) @@ -1238,6 +1256,9 @@ class _ComputeAPIUnitTestMixIn(object): def test_delete_soft_with_down_host(self): self._test_delete('soft_delete', host='down-host') + def test_delete_soft_in_resized(self): + self._test_delete('soft_delete', vm_state=vm_states.RESIZED) + def test_delete_soft(self): self._test_delete('soft_delete') diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 72157a27dc65..a90a8a0aaa04 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -6947,6 +6947,9 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): self.migration, self.instance.old_flavor, self.migration.source_node) + mock_save.assert_called_with(expected_task_state= + [None, task_states.DELETING, + task_states.SOFT_DELETING]) do_confirm_resize()