diff --git a/nova/compute/api.py b/nova/compute/api.py index 89be9a4b5366..7bc72ad3931d 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -85,15 +85,13 @@ from nova.volume import cinder LOG = logging.getLogger(__name__) -get_notifier = functools.partial(rpc.get_notifier, service='compute') # NOTE(gibi): legacy notification used compute as a service but these # calls still run on the client side of the compute service which is # nova-api. By setting the binary to nova-api below, we can make sure # that the new versioned notifications has the right publisher_id but the # legacy notifications does not change. -wrap_exception = functools.partial(exception_wrapper.wrap_exception, - get_notifier=get_notifier, - binary='nova-api') +wrap_exception = functools.partial( + exception_wrapper.wrap_exception, service='compute', binary='nova-api') CONF = nova.conf.CONF AGGREGATE_ACTION_UPDATE = 'Update' @@ -328,7 +326,7 @@ class API(base.Base): self.compute_task_api = conductor.ComputeTaskAPI() self.servicegroup_api = servicegroup.API() self.host_api = HostAPI(self.compute_rpcapi, self.servicegroup_api) - self.notifier = rpc.get_notifier('compute', CONF.host) + self.notifier = rpc.get_notifier('compute') if CONF.ephemeral_storage_encryption.enabled: self.key_manager = key_manager.API() # Help us to record host in EventReporter @@ -6326,10 +6324,12 @@ class AggregateAPI(base.Base): class KeypairAPI(base.Base): """Subset of the Compute Manager API for managing key pairs.""" - get_notifier = functools.partial(rpc.get_notifier, service='api') - wrap_exception = functools.partial(exception_wrapper.wrap_exception, - get_notifier=get_notifier, - binary='nova-api') + wrap_exception = functools.partial( + exception_wrapper.wrap_exception, service='api', binary='nova-api') + + def __init__(self): + super().__init__() + self.notifier = rpc.get_notifier('api') def _notify(self, context, event_suffix, keypair_name): payload = { @@ -6337,8 +6337,7 @@ class KeypairAPI(base.Base): 'user_id': context.user_id, 'key_name': keypair_name, } - notify = self.get_notifier() - notify.info(context, 'keypair.%s' % event_suffix, payload) + self.notifier.info(context, 'keypair.%s' % event_suffix, payload) def _validate_new_key_pair(self, context, user_id, key_name, key_type): safe_chars = "_- " + string.digits + string.ascii_letters diff --git a/nova/compute/manager.py b/nova/compute/manager.py index a78bbdfc450c..4e6a712bbc6a 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -85,7 +85,6 @@ from nova.objects import instance as obj_instance from nova.objects import migrate_data as migrate_data_obj from nova.pci import request as pci_req_module from nova.pci import whitelist -from nova import rpc from nova import safe_utils from nova.scheduler.client import query from nova.scheduler.client import report @@ -104,10 +103,8 @@ CONF = nova.conf.CONF LOG = logging.getLogger(__name__) -get_notifier = functools.partial(rpc.get_notifier, service='compute') -wrap_exception = functools.partial(exception_wrapper.wrap_exception, - get_notifier=get_notifier, - binary='nova-compute') +wrap_exception = functools.partial( + exception_wrapper.wrap_exception, service='compute', binary='nova-compute') @contextlib.contextmanager diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index c74f0b67406c..63e7624e11ef 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -244,7 +244,7 @@ class ComputeTaskManager(base.Base): self.servicegroup_api = servicegroup.API() self.query_client = query.SchedulerQueryClient() self.report_client = report.SchedulerReportClient() - self.notifier = rpc.get_notifier('compute', CONF.host) + self.notifier = rpc.get_notifier('compute') # Help us to record host in EventReporter self.host = CONF.host diff --git a/nova/exception_wrapper.py b/nova/exception_wrapper.py index 7a1a5df1e4a8..2a8bb236159d 100644 --- a/nova/exception_wrapper.py +++ b/nova/exception_wrapper.py @@ -25,14 +25,6 @@ from nova import safe_utils CONF = nova.conf.CONF -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, exception, source): payload = exception_obj.ExceptionPayload.from_exception(exception) @@ -50,13 +42,15 @@ def _emit_versioned_exception_notification(context, exception, source): notification.emit(context) -def _emit_legacy_exception_notification(notifier, context, ex, function_name, - args): - payload = dict(exception=ex, args=args) +def _emit_legacy_exception_notification( + context, exception, service, function_name, args, +): + notifier = rpc.get_notifier(service) + payload = {'exception': exception, 'args': args} notifier.error(context, function_name, payload) -def wrap_exception(notifier=None, get_notifier=None, binary=None): +def wrap_exception(service, binary): """This decorator wraps a method to catch any exceptions that may get thrown. It also optionally sends the exception to the notification system. @@ -69,14 +63,13 @@ def wrap_exception(notifier=None, get_notifier=None, binary=None): return f(self, context, *args, **kw) 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, exc, - function_name, call_dict, binary) + call_dict = _get_call_dict(f, self, context, *args, **kw) + function_name = f.__name__ + _emit_legacy_exception_notification( + context, exc, service, function_name, call_dict) + _emit_versioned_exception_notification( + context, exc, binary) return functools.wraps(f)(wrapped) return inner diff --git a/nova/rpc.py b/nova/rpc.py index 2c3950a26faa..a32b920e06a2 100644 --- a/nova/rpc.py +++ b/nova/rpc.py @@ -227,12 +227,11 @@ def get_server(target, endpoints, serializer=None): access_policy=access_policy) -def get_notifier(service, host=None, publisher_id=None): +def get_notifier(service, host=None): assert LEGACY_NOTIFIER is not None - if not publisher_id: - publisher_id = "%s.%s" % (service, host or CONF.host) + publisher_id = '%s.%s' % (service, host or CONF.host) return LegacyValidatingNotifier( - LEGACY_NOTIFIER.prepare(publisher_id=publisher_id)) + LEGACY_NOTIFIER.prepare(publisher_id=publisher_id)) def get_versioned_notifier(publisher_id): diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 1e3e528a5ff3..619e7864cbbe 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -7131,14 +7131,19 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): mock_succeeded.assert_not_called() @mock.patch.object(manager.ComputeManager, '_do_build_and_run_instance') - @mock.patch('nova.exception_wrapper._emit_exception_notification') - @mock.patch('nova.compute.utils.add_instance_fault_from_exc') + @mock.patch( + 'nova.exception_wrapper._emit_versioned_exception_notification', + new=mock.Mock()) + @mock.patch( + 'nova.exception_wrapper._emit_legacy_exception_notification', + new=mock.Mock()) + @mock.patch( + 'nova.compute.utils.add_instance_fault_from_exc', new=mock.Mock()) @mock.patch.object(manager.ComputeManager, '_build_failed') @mock.patch.object(manager.ComputeManager, '_build_succeeded') - def test_build_exceptions_reported(self, mock_succeeded, - mock_failed, - mock_if, mock_notify, - mock_dbari): + def test_build_exceptions_reported( + self, mock_succeeded, mock_failed, mock_dbari, + ): mock_dbari.side_effect = test.TestingException() instance = objects.Instance(uuid=uuids.instance, task_state=None) diff --git a/nova/tests/unit/test_exception.py b/nova/tests/unit/test_exception.py index ec8546ab7dfd..aef248b57542 100644 --- a/nova/tests/unit/test_exception.py +++ b/nova/tests/unit/test_exception.py @@ -17,128 +17,10 @@ import inspect import fixtures -import mock from webob.util import status_reasons -from nova import context from nova import exception -from nova import exception_wrapper -from nova import rpc from nova import test -from nova.tests.unit import fake_notifier - - -def good_function(self, context): - return 99 - - -def bad_function_exception(self, context, extra, blah="a", boo="b", zoo=None): - raise test.TestingException('bad things happened') - - -def bad_function_unknown_module(self, context): - """Example traceback that points to a module that getmodule() can't find. - - Traceback (most recent call last): - File "", line 1, in - File "src/lxml/lxml.etree.pyx", line 2402, in - lxml.etree._Attrib.__setitem__ (src/lxml/lxml.etree.c:67548) - File "src/lxml/apihelpers.pxi", line 570, in - lxml.etree._setAttributeValue (src/lxml/lxml.etree.c:21551) - File "src/lxml/apihelpers.pxi", line 1437, in - lxml.etree._utf8 (src/lxml/lxml.etree.c:30194) - TypeError: Argument must be bytes or unicode, got 'NoneType' - - """ - from lxml import etree - x = etree.fromstring('') - x.attrib['foo'] = None - - -class WrapExceptionTestCase(test.NoDBTestCase): - def setUp(self): - super(WrapExceptionTestCase, self).setUp() - fake_notifier.stub_notifier(self) - self.addCleanup(fake_notifier.reset) - - def test_wrap_exception_good_return(self): - wrapped = exception_wrapper.wrap_exception(rpc.get_notifier('fake')) - self.assertEqual(99, wrapped(good_function)(1, 2)) - self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) - self.assertEqual(0, len(fake_notifier.VERSIONED_NOTIFICATIONS)) - - def test_wrap_exception_unknown_module(self): - ctxt = context.get_admin_context() - wrapped = exception_wrapper.wrap_exception( - rpc.get_notifier('fake'), binary='nova-compute') - self.assertRaises( - TypeError, wrapped(bad_function_unknown_module), None, ctxt) - self.assertEqual(1, len(fake_notifier.VERSIONED_NOTIFICATIONS)) - notification = fake_notifier.VERSIONED_NOTIFICATIONS[0] - payload = notification['payload']['nova_object.data'] - self.assertEqual('unknown', payload['module_name']) - - def test_wrap_exception_with_notifier(self): - wrapped = exception_wrapper.wrap_exception(rpc.get_notifier('fake'), - binary='nova-compute') - ctxt = context.get_admin_context() - self.assertRaises(test.TestingException, - wrapped(bad_function_exception), 1, ctxt, 3, zoo=3) - - self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) - notification = fake_notifier.NOTIFICATIONS[0] - self.assertEqual('bad_function_exception', notification.event_type) - self.assertEqual(ctxt, notification.context) - self.assertEqual(3, notification.payload['args']['extra']) - for key in ['exception', 'args']: - self.assertIn(key, notification.payload.keys()) - self.assertNotIn('context', notification.payload['args'].keys()) - - self.assertEqual(1, len(fake_notifier.VERSIONED_NOTIFICATIONS)) - notification = fake_notifier.VERSIONED_NOTIFICATIONS[0] - self.assertEqual('compute.exception', notification['event_type']) - self.assertEqual('nova-compute:fake-mini', - notification['publisher_id']) - self.assertEqual('ERROR', notification['priority']) - - payload = notification['payload'] - self.assertEqual('ExceptionPayload', payload['nova_object.name']) - self.assertEqual('1.1', payload['nova_object.version']) - - payload = payload['nova_object.data'] - self.assertEqual('TestingException', payload['exception']) - self.assertEqual('bad things happened', payload['exception_message']) - self.assertEqual('bad_function_exception', payload['function_name']) - self.assertEqual('nova.tests.unit.test_exception', - payload['module_name']) - self.assertIn('bad_function_exception', payload['traceback']) - - @mock.patch('nova.rpc.NOTIFIER') - @mock.patch('nova.notifications.objects.exception.' - 'ExceptionNotification.__init__') - def test_wrap_exception_notification_not_emitted_if_disabled( - self, mock_notification, mock_notifier): - mock_notifier.is_enabled.return_value = False - - wrapped = exception_wrapper.wrap_exception(rpc.get_notifier('fake'), - binary='nova-compute') - ctxt = context.get_admin_context() - self.assertRaises(test.TestingException, - wrapped(bad_function_exception), 1, ctxt, 3, zoo=3) - self.assertFalse(mock_notification.called) - - @mock.patch('nova.notifications.objects.exception.' - 'ExceptionNotification.__init__') - def test_wrap_exception_notification_not_emitted_if_unversioned( - self, mock_notifier): - self.flags(notification_format='unversioned', group='notifications') - - wrapped = exception_wrapper.wrap_exception(rpc.get_notifier('fake'), - binary='nova-compute') - ctxt = context.get_admin_context() - self.assertRaises(test.TestingException, - wrapped(bad_function_exception), 1, ctxt, 3, zoo=3) - self.assertFalse(mock_notifier.called) class NovaExceptionTestCase(test.NoDBTestCase): @@ -184,13 +66,6 @@ class NovaExceptionTestCase(test.NoDBTestCase): exc = FakeNovaException(code=404) self.assertEqual(exc.kwargs['code'], 404) - def test_cleanse_dict(self): - kwargs = {'foo': 1, 'blah_pass': 2, 'zoo_password': 3, '_pass': 4} - self.assertEqual({'foo': 1}, exception_wrapper._cleanse_dict(kwargs)) - - kwargs = {} - self.assertEqual({}, exception_wrapper._cleanse_dict(kwargs)) - def test_format_message_local(self): class FakeNovaException(exception.NovaException): msg_fmt = "some message" diff --git a/nova/tests/unit/test_exception_wrapper.py b/nova/tests/unit/test_exception_wrapper.py new file mode 100644 index 000000000000..7da86d7f748c --- /dev/null +++ b/nova/tests/unit/test_exception_wrapper.py @@ -0,0 +1,143 @@ +# Copyright 2010 United States Government as represented by the +# Administrator of the National Aeronautics and Space Administration. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# 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 mock + +from nova import context as nova_context +from nova import exception_wrapper +from nova import test +from nova.tests.unit import fake_notifier + + +def bad_function_exception(self, context, extra, blah="a", boo="b", zoo=None): + raise test.TestingException('bad things happened') + + +def bad_function_unknown_module(self, context): + """Example traceback that points to a module that getmodule() can't find. + + Traceback (most recent call last): + File "", line 1, in + File "src/lxml/lxml.etree.pyx", line 2402, in + lxml.etree._Attrib.__setitem__ (src/lxml/lxml.etree.c:67548) + File "src/lxml/apihelpers.pxi", line 570, in + lxml.etree._setAttributeValue (src/lxml/lxml.etree.c:21551) + File "src/lxml/apihelpers.pxi", line 1437, in + lxml.etree._utf8 (src/lxml/lxml.etree.c:30194) + TypeError: Argument must be bytes or unicode, got 'NoneType' + + """ + from lxml import etree + x = etree.fromstring('') + x.attrib['foo'] = None + + +def good_function(self, context): + return 99 + + +class WrapExceptionTestCase(test.NoDBTestCase): + def setUp(self): + super(WrapExceptionTestCase, self).setUp() + fake_notifier.stub_notifier(self) + self.addCleanup(fake_notifier.reset) + + def test_cleanse_dict(self): + kwargs = {'foo': 1, 'blah_pass': 2, 'zoo_password': 3, '_pass': 4} + self.assertEqual({'foo': 1}, exception_wrapper._cleanse_dict(kwargs)) + + kwargs = {} + self.assertEqual({}, exception_wrapper._cleanse_dict(kwargs)) + + def test_wrap_exception_good_return(self): + wrapped = exception_wrapper.wrap_exception( + service='compute', binary='nova-compute') + self.assertEqual(99, wrapped(good_function)(1, 2)) + self.assertEqual(0, len(fake_notifier.NOTIFICATIONS)) + self.assertEqual(0, len(fake_notifier.VERSIONED_NOTIFICATIONS)) + + def test_wrap_exception_unknown_module(self): + ctxt = nova_context.get_admin_context() + wrapped = exception_wrapper.wrap_exception( + service='compute', binary='nova-compute') + self.assertRaises( + TypeError, wrapped(bad_function_unknown_module), None, ctxt) + self.assertEqual(1, len(fake_notifier.VERSIONED_NOTIFICATIONS)) + notification = fake_notifier.VERSIONED_NOTIFICATIONS[0] + payload = notification['payload']['nova_object.data'] + self.assertEqual('unknown', payload['module_name']) + + def test_wrap_exception_with_notifier(self): + wrapped = exception_wrapper.wrap_exception( + service='compute', binary='nova-compute') + ctxt = nova_context.get_admin_context() + self.assertRaises(test.TestingException, + wrapped(bad_function_exception), 1, ctxt, 3, zoo=3) + + self.assertEqual(1, len(fake_notifier.NOTIFICATIONS)) + notification = fake_notifier.NOTIFICATIONS[0] + self.assertEqual('bad_function_exception', notification.event_type) + self.assertEqual(ctxt, notification.context) + self.assertEqual(3, notification.payload['args']['extra']) + for key in ['exception', 'args']: + self.assertIn(key, notification.payload.keys()) + self.assertNotIn('context', notification.payload['args'].keys()) + + self.assertEqual(1, len(fake_notifier.VERSIONED_NOTIFICATIONS)) + notification = fake_notifier.VERSIONED_NOTIFICATIONS[0] + self.assertEqual('compute.exception', notification['event_type']) + self.assertEqual('nova-compute:fake-mini', + notification['publisher_id']) + self.assertEqual('ERROR', notification['priority']) + + payload = notification['payload'] + self.assertEqual('ExceptionPayload', payload['nova_object.name']) + self.assertEqual('1.1', payload['nova_object.version']) + + payload = payload['nova_object.data'] + self.assertEqual('TestingException', payload['exception']) + self.assertEqual('bad things happened', payload['exception_message']) + self.assertEqual('bad_function_exception', payload['function_name']) + self.assertEqual('nova.tests.unit.test_exception_wrapper', + payload['module_name']) + self.assertIn('bad_function_exception', payload['traceback']) + + @mock.patch('nova.rpc.NOTIFIER') + @mock.patch('nova.notifications.objects.exception.' + 'ExceptionNotification.__init__') + def test_wrap_exception_notification_not_emitted_if_disabled( + self, mock_notification, mock_notifier): + mock_notifier.is_enabled.return_value = False + + wrapped = exception_wrapper.wrap_exception( + service='compute', binary='nova-compute') + ctxt = nova_context.get_admin_context() + self.assertRaises(test.TestingException, + wrapped(bad_function_exception), 1, ctxt, 3, zoo=3) + self.assertFalse(mock_notification.called) + + @mock.patch('nova.notifications.objects.exception.' + 'ExceptionNotification.__init__') + def test_wrap_exception_notification_not_emitted_if_unversioned( + self, mock_notifier): + self.flags(notification_format='unversioned', group='notifications') + + wrapped = exception_wrapper.wrap_exception( + service='compute', binary='nova-compute') + ctxt = nova_context.get_admin_context() + self.assertRaises(test.TestingException, + wrapped(bad_function_exception), 1, ctxt, 3, zoo=3) + self.assertFalse(mock_notifier.called) diff --git a/nova/tests/unit/test_fake_notifier.py b/nova/tests/unit/test_fake_notifier.py index c34b39b94f57..6e51a1e1c337 100644 --- a/nova/tests/unit/test_fake_notifier.py +++ b/nova/tests/unit/test_fake_notifier.py @@ -12,11 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. -import functools - from nova import context from nova import exception_wrapper -from nova import rpc from nova import test from nova.tests.unit import fake_notifier @@ -30,10 +27,7 @@ class FakeVersionedNotifierTestCase(test.NoDBTestCase): self.context = context.RequestContext() - _get_notifier = functools.partial(rpc.get_notifier, 'compute') - - @exception_wrapper.wrap_exception(get_notifier=_get_notifier, - binary='nova-compute') + @exception_wrapper.wrap_exception(service='compute', binary='nova-compute') def _raise_exception(self, context): raise test.TestingException diff --git a/nova/tests/unit/test_rpc.py b/nova/tests/unit/test_rpc.py index 1c55fabe2b7c..eece75af96cf 100644 --- a/nova/tests/unit/test_rpc.py +++ b/nova/tests/unit/test_rpc.py @@ -297,18 +297,6 @@ class TestRPC(test.NoDBTestCase): mock_prep.return_value = 'notifier' mock_LEGACY_NOTIFIER.prepare = mock_prep - notifier = rpc.get_notifier('service', publisher_id='foo') - - mock_prep.assert_called_once_with(publisher_id='foo') - self.assertIsInstance(notifier, rpc.LegacyValidatingNotifier) - self.assertEqual('notifier', notifier.notifier) - - @mock.patch.object(rpc, 'LEGACY_NOTIFIER') - def test_get_notifier_null_publisher(self, mock_LEGACY_NOTIFIER): - mock_prep = mock.Mock() - mock_prep.return_value = 'notifier' - mock_LEGACY_NOTIFIER.prepare = mock_prep - notifier = rpc.get_notifier('service', host='bar') mock_prep.assert_called_once_with(publisher_id='service.bar')