From 125df26bf9d6b4cfbfb68770fa47f2055f29b8dc Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 4 Jun 2020 12:13:56 +0100 Subject: [PATCH] Use 'Exception.__traceback__' for versioned notifications The 'inspect.trace()' function is expected to be called within the context of an exception handler. The 'from_exc_and_traceback' class method of the 'nova.notification.objects.exception.ExceptionPayload' class uses this to get information about a provided exception, however, there are cases where this is called from outside of an exception handler. In these cases, we see an 'IndexError' since we can't get the last frame of a non-existent stacktrace. The solution to this is to fallback to using the traceback embedded in the exception. This is a bit lossy when decorators are involved but for all other cases this will give us the same information. This also allows us to avoid passing a traceback argument to the function since we have it to hand already. Change-Id: I404ca316b1bf2a963106cd34e927934befbd9b12 Signed-off-by: Stephen Finucane Closes-Bug: #1881455 --- nova/compute/manager.py | 44 +++------ nova/compute/utils.py | 53 +++++----- nova/exception_wrapper.py | 37 ++++--- nova/notifications/objects/exception.py | 41 +++++--- nova/scheduler/utils.py | 4 +- nova/tests/unit/compute/test_compute.py | 96 +++++++++---------- nova/tests/unit/compute/test_compute_mgr.py | 8 +- nova/tests/unit/compute/test_compute_utils.py | 4 +- nova/tests/unit/conductor/test_conductor.py | 28 ++---- .../notifications/objects/test_exception.py | 20 ++-- .../unit/scheduler/test_scheduler_utils.py | 2 +- nova/tests/unit/virt/libvirt/test_driver.py | 4 +- nova/virt/libvirt/host.py | 3 +- 13 files changed, 159 insertions(+), 185 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 18b730a1bef2..0985c63100b0 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2384,20 +2384,18 @@ class ComputeManager(manager.Manager): with excutils.save_and_reraise_exception(): self._notify_about_instance_usage(context, instance, 'create.error', fault=e) - tb = traceback.format_exc() compute_utils.notify_about_instance_create( context, instance, self.host, phase=fields.NotificationPhase.ERROR, exception=e, - bdms=block_device_mapping, tb=tb) + bdms=block_device_mapping) except exception.ComputeResourcesUnavailable as e: LOG.debug(e.format_message(), instance=instance) self._notify_about_instance_usage(context, instance, 'create.error', fault=e) - tb = traceback.format_exc() compute_utils.notify_about_instance_create( context, instance, self.host, phase=fields.NotificationPhase.ERROR, exception=e, - bdms=block_device_mapping, tb=tb) + bdms=block_device_mapping) raise exception.RescheduledException( instance_uuid=instance.uuid, reason=e.format_message()) except exception.BuildAbortException as e: @@ -2405,21 +2403,19 @@ class ComputeManager(manager.Manager): LOG.debug(e.format_message(), instance=instance) self._notify_about_instance_usage(context, instance, 'create.error', fault=e) - tb = traceback.format_exc() compute_utils.notify_about_instance_create( context, instance, self.host, phase=fields.NotificationPhase.ERROR, exception=e, - bdms=block_device_mapping, tb=tb) + bdms=block_device_mapping) except exception.NoMoreFixedIps as e: LOG.warning('No more fixed IP to be allocated', instance=instance) self._notify_about_instance_usage(context, instance, 'create.error', fault=e) - tb = traceback.format_exc() compute_utils.notify_about_instance_create( context, instance, self.host, phase=fields.NotificationPhase.ERROR, exception=e, - bdms=block_device_mapping, tb=tb) + bdms=block_device_mapping) msg = _('Failed to allocate the network(s) with error %s, ' 'not rescheduling.') % e.format_message() raise exception.BuildAbortException(instance_uuid=instance.uuid, @@ -2434,11 +2430,10 @@ class ComputeManager(manager.Manager): instance=instance) self._notify_about_instance_usage(context, instance, 'create.error', fault=e) - tb = traceback.format_exc() compute_utils.notify_about_instance_create( context, instance, self.host, phase=fields.NotificationPhase.ERROR, exception=e, - bdms=block_device_mapping, tb=tb) + bdms=block_device_mapping) msg = _('Failed to allocate the network(s), not rescheduling.') raise exception.BuildAbortException(instance_uuid=instance.uuid, reason=msg) @@ -2457,11 +2452,10 @@ class ComputeManager(manager.Manager): exception.RequestedVRamTooHigh) as e: self._notify_about_instance_usage(context, instance, 'create.error', fault=e) - tb = traceback.format_exc() compute_utils.notify_about_instance_create( context, instance, self.host, phase=fields.NotificationPhase.ERROR, exception=e, - bdms=block_device_mapping, tb=tb) + bdms=block_device_mapping) raise exception.BuildAbortException(instance_uuid=instance.uuid, reason=e.format_message()) except Exception as e: @@ -2469,11 +2463,10 @@ class ComputeManager(manager.Manager): instance=instance) self._notify_about_instance_usage(context, instance, 'create.error', fault=e) - tb = traceback.format_exc() compute_utils.notify_about_instance_create( context, instance, self.host, phase=fields.NotificationPhase.ERROR, exception=e, - bdms=block_device_mapping, tb=tb) + bdms=block_device_mapping) raise exception.RescheduledException( instance_uuid=instance.uuid, reason=six.text_type(e)) @@ -2505,11 +2498,10 @@ class ComputeManager(manager.Manager): with excutils.save_and_reraise_exception(): self._notify_about_instance_usage(context, instance, 'create.error', fault=e) - tb = traceback.format_exc() compute_utils.notify_about_instance_create( context, instance, self.host, phase=fields.NotificationPhase.ERROR, exception=e, - bdms=block_device_mapping, tb=tb) + bdms=block_device_mapping) self._update_scheduler_instance_info(context, instance) self._notify_about_instance_usage(context, instance, 'create.end', @@ -3280,13 +3272,11 @@ class ComputeManager(manager.Manager): block_device_info=new_block_device_info) def _notify_instance_rebuild_error(self, context, instance, error, bdms): - tb = traceback.format_exc() self._notify_about_instance_usage(context, instance, 'rebuild.error', fault=error) compute_utils.notify_about_instance_rebuild( context, instance, self.host, - phase=fields.NotificationPhase.ERROR, exception=error, bdms=bdms, - tb=tb) + phase=fields.NotificationPhase.ERROR, exception=error, bdms=bdms) @messaging.expected_exceptions(exception.PreserveEphemeralNotSupported) @wrap_exception() @@ -3793,12 +3783,11 @@ class ComputeManager(manager.Manager): instance, error, exc_info) self._notify_about_instance_usage(context, instance, 'reboot.error', fault=error) - tb = traceback.format_exc() compute_utils.notify_about_instance_action( context, instance, self.host, action=fields.NotificationAction.REBOOT, phase=fields.NotificationPhase.ERROR, - exception=error, bdms=bdms, tb=tb + exception=error, bdms=bdms ) ctxt.reraise = False else: @@ -5329,7 +5318,7 @@ class ComputeManager(manager.Manager): action=fields.NotificationAction.RESIZE, phase=fields.NotificationPhase.ERROR, exception=error, - tb=','.join(traceback.format_exception(*exc_info))) + ) if rescheduled: self._log_original_error(exc_info, instance_uuid) @@ -5342,7 +5331,7 @@ class ComputeManager(manager.Manager): action=fields.NotificationAction.RESIZE, phase=fields.NotificationPhase.ERROR, exception=exc_info[1], - tb=','.join(traceback.format_exception(*exc_info))) + ) else: # not re-scheduling six.reraise(*exc_info) @@ -7043,13 +7032,12 @@ class ComputeManager(manager.Manager): exc, instance=instance) else: self.volume_api.unreserve_volume(context, bdm.volume_id) - tb = traceback.format_exc() compute_utils.notify_about_volume_attach_detach( context, instance, self.host, action=fields.NotificationAction.VOLUME_ATTACH, phase=fields.NotificationPhase.ERROR, exception=e, - volume_id=bdm.volume_id, tb=tb) + volume_id=bdm.volume_id) info = {'volume_id': bdm.volume_id} self._notify_about_instance_usage( @@ -7250,11 +7238,10 @@ class ComputeManager(manager.Manager): except Exception as ex: failed = True with excutils.save_and_reraise_exception(): - tb = traceback.format_exc() compute_utils.notify_about_volume_swap( context, instance, self.host, fields.NotificationPhase.ERROR, - old_volume_id, new_volume_id, ex, tb) + old_volume_id, new_volume_id, ex) if new_cinfo: msg = ("Failed to swap volume %(old_volume_id)s " "for %(new_volume_id)s") @@ -7563,12 +7550,11 @@ class ComputeManager(manager.Manager): instance=instance) self._deallocate_port_for_instance(context, instance, port_id) - tb = traceback.format_exc() compute_utils.notify_about_instance_action( context, instance, self.host, action=fields.NotificationAction.INTERFACE_ATTACH, phase=fields.NotificationPhase.ERROR, - exception=ex, tb=tb) + exception=ex) raise exception.InterfaceAttachFailed( instance_uuid=instance.uuid) diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 767050c123cf..d13b9487a3d3 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -452,14 +452,15 @@ def notify_about_instance_usage(notifier, context, instance, event_suffix, method(context, 'compute.instance.%s' % event_suffix, usage_info) -def _get_fault_and_priority_from_exc_and_tb(exception, tb): +def _get_fault_and_priority_from_exception(exception: Exception): fault = None priority = fields.NotificationPriority.INFO - if exception: - priority = fields.NotificationPriority.ERROR - fault = notification_exception.ExceptionPayload.from_exc_and_traceback( - exception, tb) + if not exception: + return fault, priority + + fault = notification_exception.ExceptionPayload.from_exception(exception) + priority = fields.NotificationPriority.ERROR return fault, priority @@ -467,7 +468,7 @@ def _get_fault_and_priority_from_exc_and_tb(exception, tb): @rpc.if_notifications_enabled def notify_about_instance_action(context, instance, host, action, phase=None, source=fields.NotificationSource.COMPUTE, - exception=None, bdms=None, tb=None): + exception=None, bdms=None): """Send versioned notification about the action made on the instance :param instance: the instance which the action performed on :param host: the host emitting the notification @@ -476,10 +477,9 @@ def notify_about_instance_action(context, instance, host, action, phase=None, :param source: the source of the notification :param exception: the thrown exception (used in error notifications) :param bdms: BlockDeviceMappingList object for the instance. If it is not - provided then we will load it from the db if so configured - :param tb: the traceback (used in error notifications) + provided then we will load it from the db if so configured """ - fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb) + fault, priority = _get_fault_and_priority_from_exception(exception) payload = instance_notification.InstanceActionPayload( context=context, instance=instance, @@ -500,7 +500,7 @@ def notify_about_instance_action(context, instance, host, action, phase=None, @rpc.if_notifications_enabled def notify_about_instance_create(context, instance, host, phase=None, - exception=None, bdms=None, tb=None): + exception=None, bdms=None): """Send versioned notification about instance creation :param context: the request context @@ -510,9 +510,8 @@ def notify_about_instance_create(context, instance, host, phase=None, :param exception: the thrown exception (used in error notifications) :param bdms: BlockDeviceMappingList object for the instance. If it is not provided then we will load it from the db if so configured - :param tb: the traceback (used in error notifications) """ - fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb) + fault, priority = _get_fault_and_priority_from_exception(exception) payload = instance_notification.InstanceCreatePayload( context=context, instance=instance, @@ -558,7 +557,7 @@ def notify_about_scheduler_action(context, request_spec, action, phase=None, @rpc.if_notifications_enabled def notify_about_volume_attach_detach(context, instance, host, action, phase, - volume_id=None, exception=None, tb=None): + volume_id=None, exception=None): """Send versioned notification about the action made on the instance :param instance: the instance which the action performed on :param host: the host emitting the notification @@ -566,9 +565,8 @@ def notify_about_volume_attach_detach(context, instance, host, action, phase, :param phase: the phase of the action :param volume_id: id of the volume will be attached :param exception: the thrown exception (used in error notifications) - :param tb: the traceback (used in error notifications) """ - fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb) + fault, priority = _get_fault_and_priority_from_exception(exception) payload = instance_notification.InstanceActionVolumePayload( context=context, instance=instance, @@ -590,7 +588,7 @@ def notify_about_volume_attach_detach(context, instance, host, action, phase, @rpc.if_notifications_enabled def notify_about_instance_rescue_action(context, instance, host, rescue_image_ref, phase=None, - exception=None, tb=None): + exception=None): """Send versioned notification about the action made on the instance :param instance: the instance which the action performed on @@ -598,9 +596,8 @@ def notify_about_instance_rescue_action(context, instance, host, :param rescue_image_ref: the rescue image ref :param phase: the phase of the action :param exception: the thrown exception (used in error notifications) - :param tb: the traceback (used in error notifications) """ - fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb) + fault, priority = _get_fault_and_priority_from_exception(exception) payload = instance_notification.InstanceActionRescuePayload( context=context, instance=instance, @@ -644,8 +641,7 @@ def notify_about_keypair_action(context, keypair, action, phase): @rpc.if_notifications_enabled def notify_about_volume_swap(context, instance, host, phase, - old_volume_id, new_volume_id, exception=None, - tb=None): + old_volume_id, new_volume_id, exception=None): """Send versioned notification about the volume swap action on the instance @@ -656,9 +652,8 @@ def notify_about_volume_swap(context, instance, host, phase, :param old_volume_id: the ID of the volume that is copied from and detached :param new_volume_id: the ID of the volume that is copied to and attached :param exception: an exception - :param tb: the traceback (used in error notifications) """ - fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb) + fault, priority = _get_fault_and_priority_from_exception(exception) payload = instance_notification.InstanceActionVolumeSwapPayload( context=context, instance=instance, @@ -877,7 +872,7 @@ def notify_about_instance_rebuild(context, instance, host, action=fields.NotificationAction.REBUILD, phase=None, source=fields.NotificationSource.COMPUTE, - exception=None, bdms=None, tb=None): + exception=None, bdms=None): """Send versioned notification about instance rebuild :param instance: the instance which the action performed on @@ -888,9 +883,8 @@ def notify_about_instance_rebuild(context, instance, host, :param exception: the thrown exception (used in error notifications) :param bdms: BlockDeviceMappingList object for the instance. If it is not provided then we will load it from the db if so configured - :param tb: the traceback (used in error notifications) """ - fault, priority = _get_fault_and_priority_from_exc_and_tb(exception, tb) + fault, priority = _get_fault_and_priority_from_exception(exception) payload = instance_notification.InstanceActionRebuildPayload( context=context, instance=instance, @@ -938,15 +932,14 @@ def notify_about_metrics_update(context, host, host_ip, nodename, @rpc.if_notifications_enabled -def notify_about_libvirt_connect_error(context, ip, exception, tb): +def notify_about_libvirt_connect_error(context, ip, exception): """Send a versioned notification about libvirt connect error. :param context: the request context :param ip: the IP address of the host :param exception: the thrown exception - :param tb: the traceback """ - fault, _ = _get_fault_and_priority_from_exc_and_tb(exception, tb) + fault, _ = _get_fault_and_priority_from_exception(exception) payload = libvirt_notification.LibvirtErrorPayload(ip=ip, reason=fault) notification = libvirt_notification.LibvirtErrorNotification( priority=fields.NotificationPriority.ERROR, @@ -984,7 +977,7 @@ def notify_about_volume_usage(context, vol_usage, host): @rpc.if_notifications_enabled def notify_about_compute_task_error(context, action, instance_uuid, - request_spec, state, exception, tb): + request_spec, state, exception): """Send a versioned notification about compute task error. :param context: the request context @@ -1001,7 +994,7 @@ def notify_about_compute_task_error(context, action, instance_uuid, request_spec = objects.RequestSpec.from_primitives( context, request_spec, {}) - fault, _ = _get_fault_and_priority_from_exc_and_tb(exception, tb) + fault, _ = _get_fault_and_priority_from_exception(exception) payload = task_notification.ComputeTaskPayload( instance_uuid=instance_uuid, request_spec=request_spec, state=state, reason=fault) diff --git a/nova/exception_wrapper.py b/nova/exception_wrapper.py index bc4c8717456c..7a1a5df1e4a8 100644 --- a/nova/exception_wrapper.py +++ b/nova/exception_wrapper.py @@ -12,14 +12,12 @@ import functools import inspect -import traceback from oslo_utils import excutils - import nova.conf from nova.notifications.objects import base -from nova.notifications.objects import exception +from nova.notifications.objects import exception as exception_obj from nova.objects import fields from nova import rpc from nova import safe_utils @@ -27,26 +25,28 @@ from nova import safe_utils CONF = nova.conf.CONF -def _emit_exception_notification(notifier, context, ex, function_name, args, - source, trace_back): - _emit_legacy_exception_notification(notifier, context, ex, function_name, - args) - _emit_versioned_exception_notification(context, ex, source, trace_back) +def _emit_exception_notification( + notifier, context, exception, function_name, args, source, +): + _emit_legacy_exception_notification( + notifier, context, exception, function_name, args) + _emit_versioned_exception_notification(context, exception, source) @rpc.if_notifications_enabled -def _emit_versioned_exception_notification(context, ex, source, trace_back): - versioned_exception_payload = \ - exception.ExceptionPayload.from_exc_and_traceback(ex, trace_back) +def _emit_versioned_exception_notification(context, exception, source): + payload = exception_obj.ExceptionPayload.from_exception(exception) publisher = base.NotificationPublisher(host=CONF.host, source=source) event_type = base.EventType( - object='compute', - action=fields.NotificationAction.EXCEPTION) - notification = exception.ExceptionNotification( + object='compute', + action=fields.NotificationAction.EXCEPTION, + ) + notification = exception_obj.ExceptionNotification( publisher=publisher, event_type=event_type, priority=fields.NotificationPriority.ERROR, - payload=versioned_exception_payload) + payload=payload, + ) notification.emit(context) @@ -67,16 +67,15 @@ def wrap_exception(notifier=None, get_notifier=None, binary=None): # contain confidential information. try: return f(self, context, *args, **kw) - except Exception as e: - tb = traceback.format_exc() + except Exception as exc: with excutils.save_and_reraise_exception(): if notifier or get_notifier: call_dict = _get_call_dict( f, self, context, *args, **kw) function_name = f.__name__ _emit_exception_notification( - notifier or get_notifier(), context, e, - function_name, call_dict, binary, tb) + notifier or get_notifier(), context, exc, + function_name, call_dict, binary) return functools.wraps(f)(wrapped) return inner diff --git a/nova/notifications/objects/exception.py b/nova/notifications/objects/exception.py index 6ebb927b279e..5e502211307b 100644 --- a/nova/notifications/objects/exception.py +++ b/nova/notifications/objects/exception.py @@ -9,9 +9,9 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -import inspect -import six +import inspect +import traceback as tb from nova.notifications.objects import base from nova.objects import base as nova_base @@ -41,19 +41,38 @@ class ExceptionPayload(base.NotificationPayloadBase): self.traceback = traceback @classmethod - def from_exc_and_traceback(cls, fault, traceback): - trace = inspect.trace()[-1] + def from_exception(cls, fault: Exception): + traceback = fault.__traceback__ + + # NOTE(stephenfin): inspect.trace() will only return something if we're + # inside the scope of an exception handler. If we are not, we fallback + # to extracting information from the traceback. This is lossy, since + # the stack stops at the exception handler, not the exception raise. + # Check the inspect docs for more information. + # + # https://docs.python.org/3/library/inspect.html#types-and-members + trace = inspect.trace() + if trace: + module = inspect.getmodule(trace[-1][0]) + function_name = trace[-1][3] + else: + module = inspect.getmodule(traceback) + function_name = traceback.tb_frame.f_code.co_name + + module_name = module.__name__ if module else 'unknown' + # TODO(gibi): apply strutils.mask_password on exception_message and # consider emitting the exception_message only if the safe flag is # true in the exception like in the REST API - module = inspect.getmodule(trace[0]) - module_name = module.__name__ if module else 'unknown' return cls( - function_name=trace[3], - module_name=module_name, - exception=fault.__class__.__name__, - exception_message=six.text_type(fault), - traceback=traceback) + function_name=function_name, + module_name=module_name, + exception=fault.__class__.__name__, + exception_message=str(fault), + # NOTE(stephenfin): the first argument to format_exception is + # ignored since Python 3.5 + traceback=','.join(tb.format_exception(None, fault, traceback)), + ) @base.notification_sample('compute-exception.json') diff --git a/nova/scheduler/utils.py b/nova/scheduler/utils.py index e1ea3f7acf9e..ef3a6cc87fc9 100644 --- a/nova/scheduler/utils.py +++ b/nova/scheduler/utils.py @@ -17,7 +17,6 @@ import collections import re import sys -import traceback import os_resource_classes as orc import os_traits @@ -840,8 +839,7 @@ def set_vm_state_and_notify(context, instance_uuid, service, method, updates, event_type = '%s.%s' % (service, method) notifier.error(context, event_type, payload) compute_utils.notify_about_compute_task_error( - context, method, instance_uuid, request_spec, vm_state, ex, - traceback.format_exc()) + context, method, instance_uuid, request_spec, vm_state, ex) def build_filter_properties(scheduler_hints, forced_host, diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 1db1bff1054f..2a37b3d720ad 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -424,8 +424,7 @@ class ComputeVolumeTestCase(BaseTestCase): mock.call(self.context, instance, 'fake-mini', action='volume_attach', phase='error', volume_id=uuids.volume_id, - exception=expected_exception, - tb=mock.ANY), + exception=expected_exception), ]) mock_event.assert_called_once_with( self.context, 'compute_attach_volume', CONF.host, @@ -465,8 +464,7 @@ class ComputeVolumeTestCase(BaseTestCase): mock.call(self.context, instance, 'fake-mini', action='volume_attach', phase='error', volume_id=uuids.volume_id, - exception=expected_exception, - tb=mock.ANY), + exception=expected_exception), ]) @mock.patch.object(compute_manager.LOG, 'debug') @@ -514,8 +512,7 @@ class ComputeVolumeTestCase(BaseTestCase): mock.call(self.context, instance, 'fake-mini', action='volume_attach', phase='error', volume_id=uuids.volume_id, - exception=expected_exception, - tb=mock.ANY), + exception=expected_exception), ]) mock_event.assert_called_once_with( self.context, 'compute_attach_volume', CONF.host, @@ -3153,7 +3150,7 @@ class ComputeTestCase(BaseTestCase, notify_action_call_list.append( mock.call(econtext, instance, 'fake-mini', action='reboot', phase='error', exception=fault, - bdms=bdms, tb=mock.ANY)) + bdms=bdms)) notify_call_list.append(mock.call(econtext, instance, 'reboot.end')) notify_action_call_list.append( @@ -10450,7 +10447,7 @@ class ComputeAPITestCase(BaseTestCase): mock.call(self.context, instance, self.compute.host, action='interface_attach', exception=mock_attach.side_effect, - phase='error', tb=mock.ANY)]) + phase='error')]) @mock.patch.object(compute_utils, 'notify_about_instance_action') def test_detach_interface(self, mock_notify): @@ -12583,11 +12580,12 @@ class ComputeRescheduleResizeOrReraiseTestCase(BaseTestCase): raise test.TestingException("Original") except Exception: exc_info = sys.exc_info() - # because we're not retrying, we should re-raise the exception - self.assertRaises(test.TestingException, - self.compute._reschedule_resize_or_reraise, self.context, - self.instance, exc_info, self.instance_type, - self.request_spec, filter_properties, None) + + # because we're not retrying, we should re-raise the exception + self.assertRaises(test.TestingException, + self.compute._reschedule_resize_or_reraise, self.context, + self.instance, exc_info, self.instance_type, + self.request_spec, filter_properties, None) def test_reschedule_resize_or_reraise_no_retry_info(self): """Test behavior when ``filter_properties`` doesn't contain 'retry'. @@ -12601,11 +12599,12 @@ class ComputeRescheduleResizeOrReraiseTestCase(BaseTestCase): raise test.TestingException("Original") except Exception: exc_info = sys.exc_info() - # because we're not retrying, we should re-raise the exception - self.assertRaises(test.TestingException, - self.compute._reschedule_resize_or_reraise, self.context, - self.instance, exc_info, self.instance_type, - self.request_spec, filter_properties, None) + + # because we're not retrying, we should re-raise the exception + self.assertRaises(test.TestingException, + self.compute._reschedule_resize_or_reraise, self.context, + self.instance, exc_info, self.instance_type, + self.request_spec, filter_properties, None) @mock.patch.object(compute_manager.ComputeManager, '_instance_update') @mock.patch('nova.conductor.api.ComputeTaskAPI.resize_instance', @@ -12623,23 +12622,24 @@ class ComputeRescheduleResizeOrReraiseTestCase(BaseTestCase): raise test.TestingException('Original') except Exception: exc_info = sys.exc_info() - self.assertRaises(test.TestingException, - self.compute._reschedule_resize_or_reraise, self.context, - self.instance, exc_info, self.instance_type, - self.request_spec, filter_properties, None) - mock_update.assert_called_once_with( - self.context, mock.ANY, task_state=task_states.RESIZE_PREP) - mock_resize.assert_called_once_with( - self.context, mock.ANY, - {'filter_properties': filter_properties}, self.instance_type, - request_spec=self.request_spec, host_list=None) - mock_notify.assert_called_once_with( - self.context, self.instance, 'fake-mini', action='resize', - phase='error', exception=mock_resize.side_effect, tb=mock.ANY) - # If not rescheduled, the original resize exception should not be - # logged. - mock_log.assert_not_called() + self.assertRaises(test.TestingException, + self.compute._reschedule_resize_or_reraise, self.context, + self.instance, exc_info, self.instance_type, + self.request_spec, filter_properties, None) + + mock_update.assert_called_once_with( + self.context, mock.ANY, task_state=task_states.RESIZE_PREP) + mock_resize.assert_called_once_with( + self.context, mock.ANY, + {'filter_properties': filter_properties}, self.instance_type, + request_spec=self.request_spec, host_list=None) + mock_notify.assert_called_once_with( + self.context, self.instance, 'fake-mini', action='resize', + phase='error', exception=mock_resize.side_effect) + # If not rescheduled, the original resize exception should not be + # logged. + mock_log.assert_not_called() @mock.patch.object(compute_manager.ComputeManager, '_instance_update') @mock.patch('nova.conductor.api.ComputeTaskAPI.resize_instance') @@ -12654,21 +12654,21 @@ class ComputeRescheduleResizeOrReraiseTestCase(BaseTestCase): except Exception: exc_info = sys.exc_info() - self.compute._reschedule_resize_or_reraise( - self.context, self.instance, exc_info, self.instance_type, - self.request_spec, filter_properties, None) + self.compute._reschedule_resize_or_reraise( + self.context, self.instance, exc_info, self.instance_type, + self.request_spec, filter_properties, None) - mock_update.assert_called_once_with( - self.context, mock.ANY, task_state=task_states.RESIZE_PREP) - mock_resize.assert_called_once_with( - self.context, mock.ANY, - {'filter_properties': filter_properties}, self.instance_type, - request_spec=self.request_spec, host_list=None) - mock_notify.assert_called_once_with( - self.context, self.instance, 'fake-mini', action='resize', - phase='error', exception=exc_info[1], tb=mock.ANY) - # If rescheduled, the original resize exception should be logged. - mock_log.assert_called_once_with(exc_info, self.instance.uuid) + mock_update.assert_called_once_with( + self.context, mock.ANY, task_state=task_states.RESIZE_PREP) + mock_resize.assert_called_once_with( + self.context, mock.ANY, + {'filter_properties': filter_properties}, self.instance_type, + request_spec=self.request_spec, host_list=None) + mock_notify.assert_called_once_with( + self.context, self.instance, 'fake-mini', action='resize', + phase='error', exception=exc_info[1]) + # If rescheduled, the original resize exception should be logged. + mock_log.assert_called_once_with(exc_info, self.instance.uuid) class ComputeInactiveImageTestCase(BaseTestCase): diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 20e13cdfff57..3b47da94ab5c 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -2595,7 +2595,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, self.compute.host, fields.NotificationPhase.ERROR, uuids.old_volume, uuids.new_volume, - test.MatchType(expected_exception), mock.ANY) + test.MatchType(expected_exception)) else: self.compute.swap_volume(self.context, uuids.old_volume, uuids.new_volume, instance1, None) @@ -5017,7 +5017,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, ) mock_notify.assert_called_once_with( mock.ANY, instance, 'fake-mini', phase='error', exception=exc, - bdms=None, tb=mock.ANY) + bdms=None) def test_rebuild_deleting(self): instance = fake_instance.fake_instance_obj(self.context) @@ -5168,7 +5168,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, elevated_context, instance, 'fake-node', node_type='destination') mock_notify.assert_called_once_with( elevated_context, instance, 'fake-mini', bdms=None, exception=exc, - phase='error', tb=mock.ANY) + phase='error') # Make sure the instance vm_state did not change. self.assertEqual(vm_states.ACTIVE, instance.vm_state) @@ -7152,7 +7152,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): mock.call(self.context, self.instance, 'fake-mini', phase='start', bdms=[]), mock.call(self.context, self.instance, 'fake-mini', - phase='error', exception=exc, bdms=[], tb=mock.ANY)]) + phase='error', exception=exc, bdms=[])]) save.assert_has_calls([ mock.call(), diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py index 0f84b3e51361..50c6bb756506 100644 --- a/nova/tests/unit/compute/test_compute_utils.py +++ b/nova/tests/unit/compute/test_compute_utils.py @@ -19,7 +19,6 @@ import copy import datetime import string -import traceback import mock from oslo_serialization import jsonutils @@ -733,11 +732,10 @@ class UsageInfoTestCase(test.TestCase): # To get exception trace, raise and catch an exception raise test.TestingException('Volume swap error.') except Exception as ex: - tb = traceback.format_exc() compute_utils.notify_about_volume_swap( self.context, instance, 'fake-compute', fields.NotificationPhase.ERROR, - uuids.old_volume_id, uuids.new_volume_id, ex, tb) + uuids.old_volume_id, uuids.new_volume_id, ex) self.assertEqual(len(fake_notifier.VERSIONED_NOTIFICATIONS), 1) notification = fake_notifier.VERSIONED_NOTIFICATIONS[0] diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index c2c12f048b65..79e08f2d3250 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -826,7 +826,7 @@ class _BaseTaskTestCase(object): mock_notify.assert_called_once_with( self.context, 'build_instances', instance.uuid, test.MatchType(dict), 'error', - test.MatchType(exc.MaxRetriesExceeded), test.MatchType(str)) + test.MatchType(exc.MaxRetriesExceeded)) @mock.patch.object(conductor_manager.ComputeTaskManager, '_destroy_build_request') @@ -2228,13 +2228,11 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): mock_notify.assert_called_once_with( test.MatchType(context.RequestContext), 'build_instances', instance.uuid, test.MatchType(dict), 'error', - test.MatchType(Exception), test.MatchType(str)) + test.MatchType(Exception)) request_spec_dict = mock_notify.call_args_list[0][0][3] for key in ('instance_type', 'num_instances', 'instance_properties', 'image'): self.assertIn(key, request_spec_dict) - tb = mock_notify.call_args_list[0][0][6] - self.assertIn('Traceback (most recent call last):', tb) @mock.patch('nova.objects.TagList.destroy') @mock.patch('nova.objects.TagList.create') @@ -2465,13 +2463,11 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): mock_notify.assert_called_once_with( test.MatchType(context.RequestContext), 'build_instances', instance.uuid, test.MatchType(dict), 'error', - test.MatchType(exc.TooManyInstances), test.MatchType(str)) + test.MatchType(exc.TooManyInstances)) request_spec_dict = mock_notify.call_args_list[0][0][3] for key in ('instance_type', 'num_instances', 'instance_properties', 'image'): self.assertIn(key, request_spec_dict) - tb = mock_notify.call_args_list[0][0][6] - self.assertIn('Traceback (most recent call last):', tb) @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance') @mock.patch('nova.objects.quotas.Quotas.check_deltas') @@ -2712,19 +2708,19 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): mock.call( test.MatchType(context.RequestContext), 'build_instances', bare_br.instance_uuid, test.MatchType(dict), 'error', - test.MatchType(Exception), test.MatchType(str)), + test.MatchType(Exception)), mock.call( test.MatchType(context.RequestContext), 'build_instances', inst_br.instance_uuid, test.MatchType(dict), 'error', - test.MatchType(Exception), test.MatchType(str)), + test.MatchType(Exception)), mock.call( test.MatchType(context.RequestContext), 'build_instances', deleted_br.instance_uuid, test.MatchType(dict), 'error', - test.MatchType(Exception), test.MatchType(str)), + test.MatchType(Exception)), mock.call( test.MatchType(context.RequestContext), 'build_instances', fast_deleted_br.instance_uuid, test.MatchType(dict), 'error', - test.MatchType(Exception), test.MatchType(str))], + test.MatchType(Exception))], any_order=True) for i in range(0, 3): @@ -2760,7 +2756,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): mock_notify.assert_called_once_with( test.MatchType(context.RequestContext), 'build_instances', inst.uuid, test.MatchType(dict), 'error', - test.MatchType(Exception), test.MatchType(str)) + test.MatchType(Exception)) # traceback.format_exc() returns 'NoneType' # because an exception is not raised in this test. # So the argument for traceback is not checked. @@ -3480,13 +3476,11 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): mock_notify.assert_called_once_with( self.context, 'build_instances', instance.uuid, test.MatchType(dict), 'error', - test.MatchType(exc.MaxRetriesExceeded), test.MatchType(str)) + test.MatchType(exc.MaxRetriesExceeded)) request_spec_dict = mock_notify.call_args_list[0][0][3] for key in ('instance_type', 'num_instances', 'instance_properties', 'image'): self.assertIn(key, request_spec_dict) - tb = mock_notify.call_args_list[0][0][6] - self.assertIn('Traceback (most recent call last):', tb) @mock.patch('nova.compute.utils.notify_about_compute_task_error') @mock.patch('nova.objects.Instance.save') @@ -3518,13 +3512,11 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): mock_notify.assert_called_once_with( self.context, 'build_instances', instance.uuid, test.MatchType(dict), 'error', - test.MatchType(exc.NoValidHost), test.MatchType(str)) + test.MatchType(exc.NoValidHost)) request_spec_dict = mock_notify.call_args_list[0][0][3] for key in ('instance_type', 'num_instances', 'instance_properties', 'image'): self.assertIn(key, request_spec_dict) - tb = mock_notify.call_args_list[0][0][6] - self.assertIn('Traceback (most recent call last):', tb) @mock.patch('nova.scheduler.utils.claim_resources', return_value=True) @mock.patch('nova.scheduler.utils.fill_provider_mapping') diff --git a/nova/tests/unit/notifications/objects/test_exception.py b/nova/tests/unit/notifications/objects/test_exception.py index ae6f4485e487..4da7a92666a9 100644 --- a/nova/tests/unit/notifications/objects/test_exception.py +++ b/nova/tests/unit/notifications/objects/test_exception.py @@ -13,8 +13,6 @@ # under the License. import sys -import traceback -import unittest from nova.notifications.objects import exception from nova import test @@ -22,40 +20,34 @@ from nova import test class TestExceptionPayload(test.NoDBTestCase): - # Failing due to bug #1881455 - @unittest.expectedFailure - def test_from_exc_and_traceback(self): + def test_from_exception(self): try: raise Exception('foo') except Exception: exc_info = sys.exc_info() - tb = traceback.format_exc() - payload = exception.ExceptionPayload.from_exc_and_traceback( - exc_info[1], tb) + payload = exception.ExceptionPayload.from_exception(exc_info[1]) self.assertEqual( 'nova.tests.unit.notifications.objects.test_exception', payload.module_name, ) self.assertEqual( - 'test_from_exc_and_traceback', payload.function_name) + 'test_from_exception', payload.function_name) self.assertEqual('foo', payload.exception_message) - def test_from_exc_and_traceback_nested(self): + def test_from_exception_nested(self): try: raise Exception('foo') except Exception: exc_info = sys.exc_info() - tb = traceback.format_exc() - payload = exception.ExceptionPayload.from_exc_and_traceback( - exc_info[1], tb) + payload = exception.ExceptionPayload.from_exception(exc_info[1]) self.assertEqual( 'nova.tests.unit.notifications.objects.test_exception', payload.module_name, ) self.assertEqual( - 'test_from_exc_and_traceback_nested', payload.function_name) + 'test_from_exception_nested', payload.function_name) self.assertEqual('foo', payload.exception_message) diff --git a/nova/tests/unit/scheduler/test_scheduler_utils.py b/nova/tests/unit/scheduler/test_scheduler_utils.py index 651acfbf2983..d63529c2954d 100644 --- a/nova/tests/unit/scheduler/test_scheduler_utils.py +++ b/nova/tests/unit/scheduler/test_scheduler_utils.py @@ -99,7 +99,7 @@ class SchedulerUtilsTestCase(test.NoDBTestCase): mock_notify_task.assert_called_once_with( self.context, method, expected_uuid, payload_request_spec, updates['vm_state'], - exc_info, test.MatchType(str)) + exc_info) def test_set_vm_state_and_notify_request_spec_dict(self): """Tests passing a legacy dict format request spec to diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 8203f2a8f787..37adff1ed08c 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -2009,9 +2009,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, drvr._host.get_connection) mock_get.assert_called_once_with() mock_notify.assert_called_once_with(self.context, ip=CONF.my_ip, - exception=fake_error, tb=mock.ANY) - _, kwargs = mock_notify.call_args - self.assertIn('Traceback (most recent call last):', kwargs['tb']) + exception=fake_error) @mock.patch.object(fakelibvirt.virConnect, "nodeDeviceLookupByName") @mock.patch.object(fakelibvirt.virNodeDevice, "dettach") diff --git a/nova/virt/libvirt/host.py b/nova/virt/libvirt/host.py index b27babc74f66..e3314d41d6e0 100644 --- a/nova/virt/libvirt/host.py +++ b/nova/virt/libvirt/host.py @@ -34,7 +34,6 @@ import os import socket import sys import threading -import traceback from eventlet import greenio from eventlet import greenthread @@ -510,7 +509,7 @@ class Host(object): 'compute.libvirt.error', payload) compute_utils.notify_about_libvirt_connect_error( - ctxt, ip=CONF.my_ip, exception=ex, tb=traceback.format_exc()) + ctxt, ip=CONF.my_ip, exception=ex) raise exception.HypervisorUnavailable(host=CONF.host) return conn