diff --git a/doc/notification_samples/compute-exception.json b/doc/notification_samples/compute-exception.json new file mode 100644 index 000000000000..77a03108bfec --- /dev/null +++ b/doc/notification_samples/compute-exception.json @@ -0,0 +1,16 @@ +{ + "event_type": "compute.exception", + "payload": { + "nova_object.data": { + "exception": "AggregateNameExists", + "exception_message": "Aggregate versioned_exc_aggregate already exists.", + "function_name": "aggregate_create", + "module_name": "nova.db.sqlalchemy.api" + }, + "nova_object.name": "ExceptionPayload", + "nova_object.namespace": "nova", + "nova_object.version": "1.0" + }, + "priority": "ERROR", + "publisher_id": "nova-api:fake-mini" +} diff --git a/nova/compute/api.py b/nova/compute/api.py index 2acee4620794..eab1d1eb0344 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -53,6 +53,7 @@ from nova.consoleauth import rpcapi as consoleauth_rpcapi from nova import crypto from nova.db import base from nova import exception +from nova import exception_wrapper from nova import hooks from nova.i18n import _ from nova.i18n import _LE @@ -85,9 +86,14 @@ from nova import volume LOG = logging.getLogger(__name__) get_notifier = functools.partial(rpc.get_notifier, service='compute') -wrap_exception = functools.partial(exception.wrap_exception, - get_notifier=get_notifier) - +# 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') CONF = nova.conf.CONF MAX_USERDATA_SIZE = 65535 @@ -3891,8 +3897,9 @@ 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.wrap_exception, - get_notifier=get_notifier) + wrap_exception = functools.partial(exception_wrapper.wrap_exception, + get_notifier=get_notifier, + binary='nova-api') def _notify(self, context, event_suffix, keypair_name): payload = { diff --git a/nova/compute/manager.py b/nova/compute/manager.py index dd4d27fc1ded..aa51a0654ecd 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -69,6 +69,7 @@ import nova.conf from nova import consoleauth import nova.context from nova import exception +from nova import exception_wrapper from nova import hooks from nova.i18n import _ from nova.i18n import _LE @@ -103,8 +104,9 @@ CONF = nova.conf.CONF LOG = logging.getLogger(__name__) get_notifier = functools.partial(rpc.get_notifier, service='compute') -wrap_exception = functools.partial(exception.wrap_exception, - get_notifier=get_notifier) +wrap_exception = functools.partial(exception_wrapper.wrap_exception, + get_notifier=get_notifier, + binary='nova-compute') @utils.expects_func_args('migration') diff --git a/nova/exception.py b/nova/exception.py index 6af10f2ef2de..631ebb53e5b3 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -22,19 +22,15 @@ SHOULD include dedicated exception logging. """ -import functools -import inspect import sys from oslo_log import log as logging -from oslo_utils import excutils import six import webob.exc from webob import util as woutil import nova.conf from nova.i18n import _, _LE -from nova import safe_utils LOG = logging.getLogger(__name__) @@ -65,48 +61,6 @@ class ConvertedException(webob.exc.WSGIHTTPException): super(ConvertedException, self).__init__() -def _cleanse_dict(original): - """Strip all admin_password, new_pass, rescue_pass keys from a dict.""" - return {k: v for k, v in six.iteritems(original) if "_pass" not in k} - - -def wrap_exception(notifier=None, get_notifier=None): - """This decorator wraps a method to catch any exceptions that may - get thrown. It also optionally sends the exception to the notification - system. - """ - def inner(f): - def wrapped(self, context, *args, **kw): - # Don't store self or context in the payload, it now seems to - # contain confidential information. - try: - return f(self, context, *args, **kw) - except Exception as e: - with excutils.save_and_reraise_exception(): - if notifier or get_notifier: - payload = dict(exception=e) - wrapped_func = safe_utils.get_wrapped_function(f) - call_dict = inspect.getcallargs(wrapped_func, self, - context, *args, **kw) - # self can't be serialized and shouldn't be in the - # payload - call_dict.pop('self', None) - cleansed = _cleanse_dict(call_dict) - payload.update({'args': cleansed}) - - # If f has multiple decorators, they must use - # functools.wraps to ensure the name is - # propagated. - event_type = f.__name__ - - (notifier or get_notifier()).error(context, - event_type, - payload) - - return functools.wraps(f)(wrapped) - return inner - - class NovaException(Exception): """Base Nova Exception diff --git a/nova/exception_wrapper.py b/nova/exception_wrapper.py new file mode 100644 index 000000000000..5b74c3b72ff4 --- /dev/null +++ b/nova/exception_wrapper.py @@ -0,0 +1,94 @@ +# 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 functools +import inspect + +from oslo_utils import excutils +import six + +import nova.conf +from nova.notifications.objects import base +from nova.notifications.objects import exception +from nova.objects import fields +from nova import safe_utils + +CONF = nova.conf.CONF + + +def _emit_exception_notification(notifier, context, ex, function_name, args, + binary): + _emit_legacy_exception_notification(notifier, context, ex, function_name, + args) + _emit_versioned_exception_notification(context, ex, binary) + + +def _emit_versioned_exception_notification(context, ex, binary): + versioned_exception_payload = exception.ExceptionPayload.from_exception(ex) + publisher = base.NotificationPublisher(context=context, host=CONF.host, + binary=binary) + event_type = base.EventType( + object='compute', + action=fields.NotificationAction.EXCEPTION) + notification = exception.ExceptionNotification( + publisher=publisher, + event_type=event_type, + priority=fields.NotificationPriority.ERROR, + payload=versioned_exception_payload) + notification.emit(context) + + +def _emit_legacy_exception_notification(notifier, context, ex, function_name, + args): + payload = dict(exception=ex, args=args) + notifier.error(context, function_name, payload) + + +def wrap_exception(notifier=None, get_notifier=None, binary=None): + """This decorator wraps a method to catch any exceptions that may + get thrown. It also optionally sends the exception to the notification + system. + """ + def inner(f): + def wrapped(self, context, *args, **kw): + # Don't store self or context in the payload, it now seems to + # contain confidential information. + try: + return f(self, context, *args, **kw) + except Exception as e: + 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) + + return functools.wraps(f)(wrapped) + return inner + + +def _get_call_dict(function, self, context, *args, **kw): + wrapped_func = safe_utils.get_wrapped_function(function) + + call_dict = inspect.getcallargs(wrapped_func, self, + context, *args, **kw) + # self can't be serialized and shouldn't be in the + # payload + call_dict.pop('self', None) + return _cleanse_dict(call_dict) + + +def _cleanse_dict(original): + """Strip all admin_password, new_pass, rescue_pass keys from a dict.""" + return {k: v for k, v in six.iteritems(original) if "_pass" not in k} diff --git a/nova/notifications/objects/base.py b/nova/notifications/objects/base.py index 6cd8b6cbfc92..54f710deae6e 100644 --- a/nova/notifications/objects/base.py +++ b/nova/notifications/objects/base.py @@ -27,7 +27,9 @@ class NotificationObject(base.NovaObject): @base.NovaObjectRegistry.register_notification class EventType(NotificationObject): # Version 1.0: Initial version - VERSION = '1.0' + # Version 1.1: New valid actions values are added to the + # NotificationActionField enum + VERSION = '1.1' fields = { 'object': fields.StringField(nullable=False), diff --git a/nova/notifications/objects/exception.py b/nova/notifications/objects/exception.py new file mode 100644 index 000000000000..b0ee28f7ceb6 --- /dev/null +++ b/nova/notifications/objects/exception.py @@ -0,0 +1,52 @@ +# 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 inspect + +import six + +from nova.notifications.objects import base +from nova.objects import base as nova_base +from nova.objects import fields + + +@nova_base.NovaObjectRegistry.register_notification +class ExceptionPayload(base.NotificationPayloadBase): + # Version 1.0: Initial version + VERSION = '1.0' + fields = { + 'module_name': fields.StringField(), + 'function_name': fields.StringField(), + 'exception': fields.StringField(), + 'exception_message': fields.StringField() + } + + @classmethod + def from_exception(cls, fault): + trace = inspect.trace()[-1] + # 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 + return cls( + function_name=trace[3], + module_name=inspect.getmodule(trace[0]).__name__, + exception=fault.__class__.__name__, + exception_message=six.text_type(fault)) + + +@base.notification_sample('compute-exception.json') +@nova_base.NovaObjectRegistry.register_notification +class ExceptionNotification(base.NotificationBase): + # Version 1.0: Initial version + VERSION = '1.0' + fields = { + 'payload': fields.ObjectField('ExceptionPayload') + } diff --git a/nova/objects/fields.py b/nova/objects/fields.py index 821d7559ce67..0ff55ee3d865 100644 --- a/nova/objects/fields.py +++ b/nova/objects/fields.py @@ -597,8 +597,9 @@ class NotificationPhase(Enum): class NotificationAction(Enum): UPDATE = 'update' + EXCEPTION = 'exception' - ALL = (UPDATE,) + ALL = (UPDATE, EXCEPTION) def __init__(self): super(NotificationAction, self).__init__( diff --git a/nova/rpc.py b/nova/rpc.py index 78c7189870bf..3f281d74e1fd 100644 --- a/nova/rpc.py +++ b/nova/rpc.py @@ -346,12 +346,12 @@ class LegacyValidatingNotifier(object): functools.partial(self._notify, priority)) def _is_wrap_exception_notification(self, payload): - # nova.exception.wrap_exception decorator emits notification where the - # event_type is the name of the decorated function. This is used in - # many places but it will be converted to versioned notification in one - # run by updating the decorator so it is pointless to white list all - # the function names here we white list the notification itself - # detected by the special payload keys. + # nova.exception_wrapper.wrap_exception decorator emits notification + # where the event_type is the name of the decorated function. This + # is used in many places but it will be converted to versioned + # notification in one run by updating the decorator so it is pointless + # to white list all the function names here we white list the + # notification itself detected by the special payload keys. return {'exception', 'args'} == set(payload.keys()) def _notify(self, priority, ctxt, event_type, payload): diff --git a/nova/tests/functional/notification_sample_tests/test_exception_notification.py b/nova/tests/functional/notification_sample_tests/test_exception_notification.py new file mode 100644 index 000000000000..10cf66365be1 --- /dev/null +++ b/nova/tests/functional/notification_sample_tests/test_exception_notification.py @@ -0,0 +1,37 @@ +# 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. +from nova.tests.functional.api import client as api_client +from nova.tests.functional.notification_sample_tests \ + import notification_sample_base +from nova.tests.unit import fake_notifier + + +class TestExceptionNotificationSample( + notification_sample_base.NotificationSampleTestBase): + + def test_versioned_exception_notification_with_correct_params( + self): + + post = { + "aggregate": { + "name": "versioned_exc_aggregate", + "availability_zone": "nova" + } + } + + self.admin_api.api_post('os-aggregates', post) + # recreating the aggregate raises exception + self.assertRaises(api_client.OpenStackApiException, + self.admin_api.api_post, 'os-aggregates', post) + + self.assertEqual(1, len(fake_notifier.VERSIONED_NOTIFICATIONS)) + self._verify_notification('compute-exception') diff --git a/nova/tests/unit/notifications/objects/test_notification.py b/nova/tests/unit/notifications/objects/test_notification.py index ae377b85f1b3..e6ce561113fa 100644 --- a/nova/tests/unit/notifications/objects/test_notification.py +++ b/nova/tests/unit/notifications/objects/test_notification.py @@ -248,7 +248,9 @@ class TestNotificationBase(test.NoDBTestCase): notification_object_data = { - 'EventType': '1.0-21dc35de314fc5fc0a7965211c0c00f7', + 'EventType': '1.1-8291570eed00192197c7fa02ac677cd4', + 'ExceptionNotification': '1.0-a73147b93b520ff0061865849d3dfa56', + 'ExceptionPayload': '1.0-4516ae282a55fe2fd5c754967ee6248b', 'NotificationPublisher': '1.0-bbbc1402fb0e443a3eb227cc52b61545', 'ServiceStatusNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'ServiceStatusPayload': '1.0-a5e7b4fd6cc5581be45b31ff1f3a3f7f', diff --git a/nova/tests/unit/test_exception.py b/nova/tests/unit/test_exception.py index 6a3b2b70a09a..a9bada1af6fd 100644 --- a/nova/tests/unit/test_exception.py +++ b/nova/tests/unit/test_exception.py @@ -21,21 +21,10 @@ 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 - - -class FakeNotifier(object): - """Acts like messaging.Notifier.""" - - def __init__(self): - self.provided_context = None - self.provided_event = None - self.provided_payload = None - - def error(self, context, event, payload): - self.provided_context = context - self.provided_event = event - self.provided_payload = payload +from nova.tests.unit import fake_notifier def good_function(self, context): @@ -43,25 +32,52 @@ def good_function(self, context): def bad_function_exception(self, context, extra, blah="a", boo="b", zoo=None): - raise test.TestingException() + raise test.TestingException('bad things happened') 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.wrap_exception('foo') + 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_with_notifier(self): - notifier = FakeNotifier() - wrapped = exception.wrap_exception(notifier) + wrapped = exception_wrapper.wrap_exception(rpc.get_notifier('fake'), + binary='fake-binary') ctxt = context.get_admin_context() self.assertRaises(test.TestingException, wrapped(bad_function_exception), 1, ctxt, 3, zoo=3) - self.assertEqual("bad_function_exception", notifier.provided_event) - self.assertEqual(notifier.provided_context, ctxt) - self.assertEqual(3, notifier.provided_payload['args']['extra']) + + 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, notifier.provided_payload.keys()) + self.assertIn(key, notification.payload.keys()) + + self.assertEqual(1, len(fake_notifier.VERSIONED_NOTIFICATIONS)) + notification = fake_notifier.VERSIONED_NOTIFICATIONS[0] + self.assertEqual('compute.exception', notification['event_type']) + self.assertEqual('fake-binary:fake-mini', notification['publisher_id']) + self.assertEqual('ERROR', notification['priority']) + + payload = notification['payload'] + self.assertEqual('ExceptionPayload', payload['nova_object.name']) + self.assertEqual('1.0', 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']) class NovaExceptionTestCase(test.NoDBTestCase): @@ -108,10 +124,10 @@ class NovaExceptionTestCase(test.NoDBTestCase): def test_cleanse_dict(self): kwargs = {'foo': 1, 'blah_pass': 2, 'zoo_password': 3, '_pass': 4} - self.assertEqual({'foo': 1}, exception._cleanse_dict(kwargs)) + self.assertEqual({'foo': 1}, exception_wrapper._cleanse_dict(kwargs)) kwargs = {} - self.assertEqual({}, exception._cleanse_dict(kwargs)) + self.assertEqual({}, exception_wrapper._cleanse_dict(kwargs)) def test_format_message_local(self): class FakeNovaException(exception.NovaException):