From f577f650c7ca9d8dd66eaec919e4805c09d16f6d Mon Sep 17 00:00:00 2001 From: Andrew Laski Date: Wed, 31 Aug 2016 20:41:09 -0400 Subject: [PATCH] Remove BuildRequest when scheduling fails If there's a scheduler failure the instance is put into an error status and that's what should be returned by an instance show/list. However because the BuildRequest is not deleted that's what actually gets returned. Delete the BuildRequest on scheduler failures. Change-Id: Ia6c87aecc1484a2366d50822b669291c9f954545 Partially-implements: bp add-buildrequest-obj --- nova/conductor/manager.py | 6 ++++++ nova/tests/unit/conductor/test_conductor.py | 11 ++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 93ef979cf96..9b4442cc3da 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -488,6 +488,12 @@ def build_instances(self, context, instances, image, filter_properties, self._set_vm_state_and_notify( context, instance.uuid, 'build_instances', updates, exc, request_spec) + try: + # If the BuildRequest stays around then instance show/lists + # will pull from it rather than the errored instance. + self._destroy_build_request(context, instance) + except exception.BuildRequestNotFound: + pass self._cleanup_allocated_networks( context, instance, requested_networks) return diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 26950ffaa5a..21793c098df 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -465,8 +465,10 @@ def test_build_instances(self, mock_refresh): 'select_destinations') @mock.patch.object(conductor_manager.ComputeTaskManager, '_cleanup_allocated_networks') + @mock.patch.object(conductor_manager.ComputeTaskManager, + '_destroy_build_request') def test_build_instances_scheduler_failure( - self, cleanup_mock, sd_mock, state_mock, + self, dest_build_req_mock, cleanup_mock, sd_mock, state_mock, sig_mock, bs_mock): instances = [fake_instance.fake_instance_obj(self.context) for i in range(2)] @@ -475,6 +477,9 @@ def test_build_instances_scheduler_failure( 'instance_properties': instances[0]} exception = exc.NoValidHost(reason='fake-reason') + dest_build_req_mock.side_effect = ( + exc.BuildRequestNotFound(uuid='fake'), + None) bs_mock.return_value = spec sd_mock.side_effect = exception updates = {'vm_state': vm_states.ERROR, 'task_state': None} @@ -496,14 +501,18 @@ def test_build_instances_scheduler_failure( set_state_calls = [] cleanup_network_calls = [] + dest_build_req_calls = [] for instance in instances: set_state_calls.append(mock.call( self.context, instance.uuid, 'compute_task', 'build_instances', updates, exception, spec)) cleanup_network_calls.append(mock.call( self.context, mock.ANY, None)) + dest_build_req_calls.append( + mock.call(self.context, test.MatchType(type(instance)))) state_mock.assert_has_calls(set_state_calls) cleanup_mock.assert_has_calls(cleanup_network_calls) + dest_build_req_mock.assert_has_calls(dest_build_req_calls) def test_build_instances_retry_exceeded(self): instances = [fake_instance.fake_instance_obj(self.context)]