diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 3447654ebd71..7a16df3118a0 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -749,9 +749,12 @@ class ComputeManager(manager.Manager): instance.host = None instance.node = None - def _set_instance_obj_error_state(self, context, instance): + def _set_instance_obj_error_state(self, context, instance, + clean_task_state=False): try: instance.vm_state = vm_states.ERROR + if clean_task_state: + instance.task_state = None instance.save() except exception.InstanceNotFound: LOG.debug('Instance has been destroyed from under us while ' @@ -1911,7 +1914,8 @@ class ComputeManager(manager.Manager): compute_utils.add_instance_fault_from_exc(context, instance, e, sys.exc_info()) self._nil_out_instance_obj_host_and_node(instance) - self._set_instance_obj_error_state(context, instance) + self._set_instance_obj_error_state(context, instance, + clean_task_state=True) return build_results.FAILED LOG.debug(e.format_message(), instance=instance) retry['exc'] = traceback.format_exception(*sys.exc_info()) @@ -1953,7 +1957,8 @@ class ComputeManager(manager.Manager): compute_utils.add_instance_fault_from_exc(context, instance, e, sys.exc_info()) self._nil_out_instance_obj_host_and_node(instance) - self._set_instance_obj_error_state(context, instance) + self._set_instance_obj_error_state(context, instance, + clean_task_state=True) return build_results.FAILED except Exception as e: # Should not reach here. @@ -1966,7 +1971,8 @@ class ComputeManager(manager.Manager): compute_utils.add_instance_fault_from_exc(context, instance, e, sys.exc_info()) self._nil_out_instance_obj_host_and_node(instance) - self._set_instance_obj_error_state(context, instance) + self._set_instance_obj_error_state(context, instance, + clean_task_state=True) return build_results.FAILED def _build_and_run_instance(self, context, instance, image, injected_files, diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index a4069a994c50..bb861eb3ba1c 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -1685,7 +1685,7 @@ class ComputeTestCase(BaseTestCase): """block device mapping failure test. Make sure that when there is a block device mapping problem, - the instance goes to ERROR state, keeping the task state + the instance goes to ERROR state, cleaning the task state """ def fake(*args, **kwargs): raise exception.InvalidBDM() @@ -1700,10 +1700,10 @@ class ComputeTestCase(BaseTestCase): node=None) # check state is failed even after the periodic poll self._assert_state({'vm_state': vm_states.ERROR, - 'task_state': task_states.BLOCK_DEVICE_MAPPING}) + 'task_state': None}) self.compute.periodic_tasks(context.get_admin_context()) self._assert_state({'vm_state': vm_states.ERROR, - 'task_state': task_states.BLOCK_DEVICE_MAPPING}) + 'task_state': None}) @mock.patch('nova.compute.manager.ComputeManager._prep_block_device', side_effect=exception.OverQuota(overs='volumes')) @@ -1711,8 +1711,8 @@ class ComputeTestCase(BaseTestCase): """block device mapping over quota failure test. Make sure when we're over volume quota according to Cinder client, the - appropriate exception is raised and the instances to ERROR state, keep - the task state. + appropriate exception is raised and the instances to ERROR state, + cleaning the task state. """ instance = self._create_fake_instance_obj() self.compute.build_and_run_instance( @@ -1722,17 +1722,17 @@ class ComputeTestCase(BaseTestCase): node=None, block_device_mapping=[], image={}) # check state is failed even after the periodic poll self._assert_state({'vm_state': vm_states.ERROR, - 'task_state': task_states.BLOCK_DEVICE_MAPPING}) + 'task_state': None}) self.compute.periodic_tasks(context.get_admin_context()) self._assert_state({'vm_state': vm_states.ERROR, - 'task_state': task_states.BLOCK_DEVICE_MAPPING}) + 'task_state': None}) self.assertTrue(mock_prep_block_dev.called) def test_run_instance_spawn_fail(self): """spawn failure test. Make sure that when there is a spawning problem, - the instance goes to ERROR state, keeping the task state. + the instance goes to ERROR state, cleaning the task state. """ def fake(*args, **kwargs): raise test.TestingException() @@ -1745,10 +1745,10 @@ class ComputeTestCase(BaseTestCase): block_device_mapping=[], image={}, node=None) # check state is failed even after the periodic poll self._assert_state({'vm_state': vm_states.ERROR, - 'task_state': task_states.SPAWNING}) + 'task_state': None}) self.compute.periodic_tasks(context.get_admin_context()) self._assert_state({'vm_state': vm_states.ERROR, - 'task_state': task_states.SPAWNING}) + 'task_state': None}) def test_run_instance_dealloc_network_instance_not_found(self): """spawn network deallocate test. diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 42ba5a015650..3d2ed3314a2d 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -2663,6 +2663,23 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): self.assertIsInstance(mock_r.call_args_list[0][0][0], objects.Instance) + def test_set_instance_obj_error_state_with_clean_task_state(self): + instance = fake_instance.fake_instance_obj(self.context, + vm_state=vm_states.BUILDING, task_state=task_states.SPAWNING) + with mock.patch.object(instance, 'save'): + self.compute._set_instance_obj_error_state(self.context, instance, + clean_task_state=True) + self.assertEqual(vm_states.ERROR, instance.vm_state) + self.assertIsNone(instance.task_state) + + def test_set_instance_obj_error_state_by_default(self): + instance = fake_instance.fake_instance_obj(self.context, + vm_state=vm_states.BUILDING, task_state=task_states.SPAWNING) + with mock.patch.object(instance, 'save'): + self.compute._set_instance_obj_error_state(self.context, instance) + self.assertEqual(vm_states.ERROR, instance.vm_state) + self.assertEqual(task_states.SPAWNING, instance.task_state) + class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): def setUp(self): @@ -2835,7 +2852,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): compute_utils.add_instance_fault_from_exc(self.context, self.instance, mox.IgnoreArg(), mox.IgnoreArg()) self.compute._nil_out_instance_obj_host_and_node(self.instance) - self.compute._set_instance_obj_error_state(self.context, self.instance) + self.compute._set_instance_obj_error_state(self.context, self.instance, + clean_task_state=True) self._instance_action_events() self.mox.ReplayAll() @@ -2996,8 +3014,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): compute_utils.add_instance_fault_from_exc(self.context, self.instance, mox.IgnoreArg(), mox.IgnoreArg()) self.compute._nil_out_instance_obj_host_and_node(self.instance) - self.compute._set_instance_obj_error_state(self.context, - self.instance) + self.compute._set_instance_obj_error_state(self.context, self.instance, + clean_task_state=True) self._instance_action_events() self.mox.ReplayAll() @@ -3135,7 +3153,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): compute_utils.add_instance_fault_from_exc(self.context, self.instance, mox.IgnoreArg(), mox.IgnoreArg()) self.compute._set_instance_obj_error_state(self.context, - self.instance) + self.instance, clean_task_state=True) self._instance_action_events() self.mox.ReplayAll()