From 1a80c8899d541fe451afb030d5011cf8c7543a3c Mon Sep 17 00:00:00 2001 From: Wenzhi Yu Date: Thu, 28 Apr 2016 10:32:44 +0800 Subject: [PATCH] Report actual request_spec when MaxRetriesExceeded raised 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 --- nova/conductor/manager.py | 4 ++-- nova/tests/unit/conductor/test_conductor.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 2e8b707e5fa9..1e8998d17fdc 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -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: diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 398db5942571..9d0ac5875056 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -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()