diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 81c7ee51b0ae..a7b2f483cd34 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -3335,7 +3335,12 @@ class ComputeManager(manager.Manager): allocs = self.reportclient.get_allocations_for_consumer( context, instance.uuid) - with self._error_out_instance_on_exception(context, instance): + # If the resource claim or group policy validation fails before we + # do anything to the guest or its networking/volumes we want to keep + # the current status rather than put the instance into ERROR status. + instance_state = instance.vm_state + with self._error_out_instance_on_exception( + context, instance, instance_state=instance_state): try: self._do_rebuild_instance_with_claim( context, instance, orig_image_ref, @@ -3369,8 +3374,13 @@ class ComputeManager(manager.Manager): self.rt.delete_allocation_for_evacuated_instance( context, instance, scheduled_node, node_type='destination') self._notify_instance_rebuild_error(context, instance, e, bdms) - raise exception.BuildAbortException( - instance_uuid=instance.uuid, reason=e.format_message()) + # Wrap this in InstanceFaultRollback so that the + # _error_out_instance_on_exception context manager keeps the + # vm_state unchanged. + raise exception.InstanceFaultRollback( + inner_exception=exception.BuildAbortException( + instance_uuid=instance.uuid, + reason=e.format_message())) except (exception.InstanceNotFound, exception.UnexpectedDeletingTaskStateError) as e: LOG.debug('Instance was deleted while rebuilding', diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index c6a9fdb59ece..54f7df2bd995 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -2503,8 +2503,10 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase): # evacuate the server self.api.post_server_action(server['id'], {'evacuate': {}}) # the migration will fail on the dest node and the instance will - # go into error state - server = self._wait_for_state_change(self.api, server, 'ERROR') + # stay ACTIVE and task_state will be set to None. + server = self._wait_for_server_parameter( + self.api, server, {'status': 'ACTIVE', + 'OS-EXT-STS:task_state': None}) # Run the periodics to show those don't modify allocations. self._run_periodics() @@ -7204,7 +7206,8 @@ class ServerMoveWithPortResourceRequestTest( server = self._wait_for_server_parameter( self.api, server, {'OS-EXT-SRV-ATTR:host': 'host1', - 'status': 'ERROR'}) + 'status': 'ACTIVE', + 'OS-EXT-STS:task_state': None}) self._wait_for_migration_status(server, ['failed']) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 348169e7bd43..18b2f736090d 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -5007,6 +5007,28 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, delete_alloc.assert_called_once_with(self.context, instance, 'foo', node_type='destination') + @mock.patch('nova.objects.Instance.save') + @mock.patch('nova.compute.utils.add_instance_fault_from_exc') + @mock.patch('nova.compute.resource_tracker.ResourceTracker.' + 'delete_allocation_for_evacuated_instance') + def test_rebuild_compute_resources_unavailable(self, mock_delete_alloc, + mock_add_fault, + mock_save): + """Tests that when the rebuild_claim fails with + ComputeResourcesUnavailable the vm_state on the instance remains + unchanged. + """ + instance = fake_instance.fake_instance_obj(self.context) + instance.vm_state = vm_states.ACTIVE + ex = exception.ComputeResourcesUnavailable(reason='out of foo') + self.assertRaises(exception.BuildAbortException, + self._test_rebuild_ex, instance, ex) + # Make sure the instance vm_state did not change. + self.assertEqual(vm_states.ACTIVE, instance.vm_state) + mock_delete_alloc.assert_called_once() + mock_save.assert_called() + mock_add_fault.assert_called_once() + @mock.patch('nova.context.RequestContext.elevated') @mock.patch('nova.objects.instance.Instance.drop_migration_context') @mock.patch('nova.objects.instance.Instance.apply_migration_context') @@ -5071,6 +5093,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, instance = fake_instance.fake_instance_obj(self.context) instance.info_cache = None instance.system_metadata = {} + instance.vm_state = vm_states.ACTIVE elevated_context = mock.Mock() mock_context_elevated.return_value = elevated_context request_spec = objects.RequestSpec() @@ -5094,6 +5117,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, mock_notify.assert_called_once_with( elevated_context, instance, 'fake-mini', bdms=None, exception=exc, phase='error', tb=mock.ANY) + # Make sure the instance vm_state did not change. + self.assertEqual(vm_states.ACTIVE, instance.vm_state) def test_rebuild_node_not_updated_if_not_recreate(self): node = uuidutils.generate_uuid() # ironic node uuid