Skip to content

Commit

Permalink
Remove BuildRequest when scheduling fails
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alaski committed Sep 1, 2016
1 parent dd44096 commit f577f65
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
6 changes: 6 additions & 0 deletions nova/conductor/manager.py
Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion nova/tests/unit/conductor/test_conductor.py
Expand Up @@ -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)]
Expand All @@ -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}
Expand All @@ -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)]
Expand Down

0 comments on commit f577f65

Please sign in to comment.