openstack/nova
Code Issues Proposed changes

Report actual request_spec when MaxRetriesExceeded raised

Browse Source 
If a MaxRetriesExceeded exception is raised by
scheduler_utils.populate_retry then request_spec will be empty in the
exception handler[1], then _set_vm_state_and_notify method will just
put a empty dict as request_spec into the payload of notification[2].
It would make more sense if we report the actual value of request_spec
in the notification.

[1]https://github.com/openstack/nova/blob/13.0.0.0rc3/nova/conductor/manager.py#L382
[2]https://github.com/openstack/nova/blob/13.0.0.0rc3/nova/scheduler/utils.py#L109

Simply moving the initialization of request_spec up one line before the
call to populate_retry should fix the issue.

Change-Id: I7c51f635d52f368c8df549f62024cbdf64a032b3
Closes-Bug: #1575998
This commit is contained in:
Wenzhi Yu 2016-04-28 10:32:44 +08:00
parent 33dd4d827b
commit 1a80c8899d
2 changed files with 5 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
nova/conductor/manager.py
View File

@ -488,10 +488,10 @@ class ComputeTaskManager(base.Base):
# check retry policy. Rather ugly use of instances[0]...
# but if we've exceeded max retries... then we really only
# have a single instance.
request_spec = scheduler_utils.build_request_spec(
context, image, instances)
scheduler_utils.populate_retry(
filter_properties, instances[0].uuid)
request_spec = scheduler_utils.build_request_spec(
context, image, instances)
hosts = self._schedule_instances(
context, request_spec, filter_properties)
except Exception as exc:

5
nova/tests/unit/conductor/test_conductor.py
View File

@ -573,8 +573,9 @@ class _BaseTaskTestCase(object):
@mock.patch.object(conductor_manager.ComputeTaskManager,
'_cleanup_allocated_networks')
@mock.patch.object(scheduler_utils, 'set_vm_state_and_notify')
@mock.patch.object(scheduler_utils, 'build_request_spec')
@mock.patch.object(scheduler_utils, 'populate_retry')
def _test(populate_retry,
def _test(populate_retry, build_spec,
set_vm_state_and_notify, cleanup_mock):
# build_instances() is a cast, we need to wait for it to
# complete
@ -599,7 +600,7 @@ class _BaseTaskTestCase(object):
filter_properties, instances[0].uuid)
set_vm_state_and_notify.assert_called_once_with(
self.context, instances[0].uuid, 'compute_task',
'build_instances', updates, mock.ANY, {})
'build_instances', updates, mock.ANY, build_spec.return_value)
cleanup_mock.assert_called_once_with(self.context, mock.ANY, None)
_test()