Short circuit notifications when not enabled
Wasting time on generating the payload of the notifications is unnecessary if the notification will not be emitted due to configuration. This patch depends on an oslo messaging improvement that adds a way to check if a Notifier will emit the notification or not. Depends-On: Ib992f5c20fef85224fb00823e1d8d9c6cff19bec Depends-On: I979fb3385671aba6f4162ef991da8f0febe0068a Change-Id: I3e6741d59df49e1e58409314008c2ed609fdedc1
This commit is contained in:
parent
1e0e4080fb
commit
29cb8f1c45
|
@ -20,6 +20,7 @@ import nova.conf
|
|||
from nova.notifications.objects import base
|
||||
from nova.notifications.objects import exception
|
||||
from nova.objects import fields
|
||||
from nova import rpc
|
||||
from nova import safe_utils
|
||||
|
||||
CONF = nova.conf.CONF
|
||||
|
@ -32,6 +33,7 @@ def _emit_exception_notification(notifier, context, ex, function_name, args,
|
|||
_emit_versioned_exception_notification(context, ex, binary)
|
||||
|
||||
|
||||
@rpc.if_notifications_enabled
|
||||
def _emit_versioned_exception_notification(context, ex, binary):
|
||||
versioned_exception_payload = exception.ExceptionPayload.from_exception(ex)
|
||||
publisher = base.NotificationPublisher(context=context, host=CONF.host,
|
||||
|
|
|
@ -262,6 +262,7 @@ def _map_service_to_binary(service):
|
|||
return binary
|
||||
|
||||
|
||||
@rpc.if_notifications_enabled
|
||||
def _send_versioned_instance_update(context, instance, payload, host, service):
|
||||
|
||||
state_update = instance_notification.InstanceStateUpdatePayload(
|
||||
|
|
|
@ -93,6 +93,7 @@ class NotificationPayloadBase(NotificationObject):
|
|||
super(NotificationPayloadBase, self).__init__(**kwargs)
|
||||
self.populated = not self.SCHEMA
|
||||
|
||||
@rpc.if_notifications_enabled
|
||||
def populate_schema(self, **kwargs):
|
||||
"""Populate the object based on the SCHEMA and the source objects
|
||||
|
||||
|
@ -163,6 +164,7 @@ class NotificationBase(NotificationObject):
|
|||
notify = getattr(notifier, self.priority)
|
||||
notify(context, event_type=event_type, payload=payload)
|
||||
|
||||
@rpc.if_notifications_enabled
|
||||
def emit(self, context):
|
||||
"""Send the notification."""
|
||||
assert self.payload.populated
|
||||
|
|
13
nova/rpc.py
13
nova/rpc.py
|
@ -215,6 +215,19 @@ def get_versioned_notifier(publisher_id):
|
|||
return NOTIFIER.prepare(publisher_id=publisher_id)
|
||||
|
||||
|
||||
def if_notifications_enabled(f):
|
||||
"""Calls decorated method only if versioned notifications are enabled."""
|
||||
@functools.wraps(f)
|
||||
def wrapped(*args, **kwargs):
|
||||
if (NOTIFIER.is_enabled() and
|
||||
CONF.notifications.notification_format in ('both',
|
||||
'versioned')):
|
||||
return f(*args, **kwargs)
|
||||
else:
|
||||
return None
|
||||
return wrapped
|
||||
|
||||
|
||||
def create_transport(url):
|
||||
exmods = get_allowed_exmods()
|
||||
return messaging.get_transport(CONF,
|
||||
|
|
|
@ -223,6 +223,12 @@ class TestCase(testtools.TestCase):
|
|||
self.useFixture(conf_fixture.ConfFixture(CONF))
|
||||
self.useFixture(nova_fixtures.RPCFixture('nova.test'))
|
||||
|
||||
# we cannot set this in the ConfFixture as oslo only registers the
|
||||
# notification opts at the first instantiation of a Notifier that
|
||||
# happens only in the RPCFixture
|
||||
CONF.set_default('driver', ['test'],
|
||||
group='oslo_messaging_notifications')
|
||||
|
||||
# NOTE(danms): Make sure to reset us back to non-remote objects
|
||||
# for each test to avoid interactions. Also, backup the object
|
||||
# registry.
|
||||
|
|
|
@ -69,6 +69,9 @@ class FakeNotifier(object):
|
|||
payload, ctxt)
|
||||
NOTIFICATIONS.append(msg)
|
||||
|
||||
def is_enabled(self):
|
||||
return True
|
||||
|
||||
|
||||
class FakeVersionedNotifier(FakeNotifier):
|
||||
def _notify(self, priority, ctxt, event_type, payload):
|
||||
|
|
|
@ -296,6 +296,54 @@ class TestNotificationBase(test.NoDBTestCase):
|
|||
self.assertIn('test-update-1.json', self.TestNotification.samples)
|
||||
self.assertIn('test-update-2.json', self.TestNotification.samples)
|
||||
|
||||
@mock.patch('nova.notifications.objects.base.NotificationBase._emit')
|
||||
@mock.patch('nova.rpc.NOTIFIER')
|
||||
def test_payload_is_not_generated_if_notifier_is_not_enabled(
|
||||
self, mock_notifier, mock_emit):
|
||||
mock_notifier.is_enabled.return_value = False
|
||||
|
||||
payload = self.TestNotificationPayload(
|
||||
extra_field='test string')
|
||||
self.payload.populate_schema(source_field=self.my_obj)
|
||||
noti = self.TestNotification(
|
||||
event_type=notification.EventType(
|
||||
object='test_object',
|
||||
action=fields.NotificationAction.UPDATE),
|
||||
publisher=notification.NotificationPublisher.from_service_obj(
|
||||
self.service_obj),
|
||||
priority=fields.NotificationPriority.INFO,
|
||||
payload=payload)
|
||||
|
||||
mock_context = mock.Mock()
|
||||
|
||||
noti.emit(mock_context)
|
||||
|
||||
self.assertFalse(payload.populated)
|
||||
self.assertFalse(mock_emit.called)
|
||||
|
||||
@mock.patch('nova.notifications.objects.base.NotificationBase._emit')
|
||||
def test_payload_is_not_generated_if_notification_format_is_unversioned(
|
||||
self, mock_emit):
|
||||
self.flags(notification_format='unversioned', group='notifications')
|
||||
|
||||
payload = self.TestNotificationPayload(
|
||||
extra_field='test string')
|
||||
self.payload.populate_schema(source_field=self.my_obj)
|
||||
noti = self.TestNotification(
|
||||
event_type=notification.EventType(
|
||||
object='test_object',
|
||||
action=fields.NotificationAction.UPDATE),
|
||||
publisher=notification.NotificationPublisher.from_service_obj(
|
||||
self.service_obj),
|
||||
priority=fields.NotificationPriority.INFO,
|
||||
payload=payload)
|
||||
|
||||
mock_context = mock.Mock()
|
||||
|
||||
noti.emit(mock_context)
|
||||
|
||||
self.assertFalse(payload.populated)
|
||||
self.assertFalse(mock_emit.called)
|
||||
|
||||
notification_object_data = {
|
||||
'AggregateNotification': '1.0-a73147b93b520ff0061865849d3dfa56',
|
||||
|
@ -381,38 +429,79 @@ def get_extra_data(obj_class):
|
|||
|
||||
|
||||
class TestInstanceNotification(test.NoDBTestCase):
|
||||
@mock.patch('nova.notifications.objects.instance.'
|
||||
'InstanceUpdateNotification._emit')
|
||||
def test_send_version_instance_update_uses_flavor(self, mock_emit):
|
||||
# Make sure that the notification payload chooses the values in
|
||||
# instance.flavor.$value instead of instance.$value
|
||||
test_keys = ['memory_mb', 'vcpus', 'root_gb', 'ephemeral_gb', 'swap']
|
||||
flavor_values = {k: 123 for k in test_keys}
|
||||
instance_values = {k: 456 for k in test_keys}
|
||||
def setUp(self):
|
||||
super(TestInstanceNotification, self).setUp()
|
||||
self.test_keys = ['memory_mb', 'vcpus', 'root_gb', 'ephemeral_gb',
|
||||
'swap']
|
||||
self.flavor_values = {k: 123 for k in self.test_keys}
|
||||
instance_values = {k: 456 for k in self.test_keys}
|
||||
flavor = objects.Flavor(flavorid='test-flavor', name='test-flavor',
|
||||
disabled=False, projects=[], is_public=True,
|
||||
extra_specs = {}, **flavor_values)
|
||||
extra_specs={}, **self.flavor_values)
|
||||
info_cache = objects.InstanceInfoCache(
|
||||
network_info=network_model.NetworkInfo())
|
||||
instance = objects.Instance(
|
||||
self.instance = objects.Instance(
|
||||
flavor=flavor,
|
||||
info_cache=info_cache,
|
||||
metadata={},
|
||||
uuid=uuids.instance1,
|
||||
locked=False,
|
||||
**instance_values)
|
||||
payload = {
|
||||
self.payload = {
|
||||
'bandwidth': {},
|
||||
'audit_period_ending': timeutils.utcnow(),
|
||||
'audit_period_beginning': timeutils.utcnow(),
|
||||
}
|
||||
|
||||
@mock.patch('nova.notifications.objects.instance.'
|
||||
'InstanceUpdateNotification._emit')
|
||||
def test_send_version_instance_update_uses_flavor(self, mock_emit):
|
||||
# Make sure that the notification payload chooses the values in
|
||||
# instance.flavor.$value instead of instance.$value
|
||||
notification_base._send_versioned_instance_update(
|
||||
mock.MagicMock(),
|
||||
instance,
|
||||
payload,
|
||||
self.instance,
|
||||
self.payload,
|
||||
'host',
|
||||
'compute')
|
||||
payload = mock_emit.call_args_list[0][1]['payload']['nova_object.data']
|
||||
flavor_payload = payload['flavor']['nova_object.data']
|
||||
data = {k: flavor_payload[k] for k in test_keys}
|
||||
self.assertEqual(flavor_values, data)
|
||||
data = {k: flavor_payload[k] for k in self.test_keys}
|
||||
self.assertEqual(self.flavor_values, data)
|
||||
|
||||
@mock.patch('nova.rpc.NOTIFIER')
|
||||
@mock.patch('nova.notifications.objects.instance.'
|
||||
'InstanceUpdatePayload.__init__', return_value=None)
|
||||
@mock.patch('nova.notifications.objects.instance.'
|
||||
'InstanceUpdateNotification.__init__', return_value=None)
|
||||
def test_send_versioned_instance_notification_is_not_called_disabled(
|
||||
self, mock_notification, mock_payload, mock_notifier):
|
||||
mock_notifier.is_enabled.return_value = False
|
||||
|
||||
notification_base._send_versioned_instance_update(
|
||||
mock.MagicMock(),
|
||||
self.instance,
|
||||
self.payload,
|
||||
'host',
|
||||
'compute')
|
||||
|
||||
self.assertFalse(mock_payload.called)
|
||||
self.assertFalse(mock_notification.called)
|
||||
|
||||
@mock.patch('nova.notifications.objects.instance.'
|
||||
'InstanceUpdatePayload.__init__', return_value=None)
|
||||
@mock.patch('nova.notifications.objects.instance.'
|
||||
'InstanceUpdateNotification.__init__', return_value=None)
|
||||
def test_send_versioned_instance_notification_is_not_called_unversioned(
|
||||
self, mock_notification, mock_payload):
|
||||
self.flags(notification_format='unversioned', group='notifications')
|
||||
|
||||
notification_base._send_versioned_instance_update(
|
||||
mock.MagicMock(),
|
||||
self.instance,
|
||||
self.payload,
|
||||
'host',
|
||||
'compute')
|
||||
|
||||
self.assertFalse(mock_payload.called)
|
||||
self.assertFalse(mock_notification.called)
|
||||
|
|
|
@ -16,6 +16,7 @@
|
|||
|
||||
import inspect
|
||||
|
||||
import mock
|
||||
import six
|
||||
from webob.util import status_reasons
|
||||
|
||||
|
@ -110,6 +111,33 @@ class WrapExceptionTestCase(test.NoDBTestCase):
|
|||
self.assertEqual('nova.tests.unit.test_exception',
|
||||
payload['module_name'])
|
||||
|
||||
@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='fake-binary')
|
||||
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='fake-binary')
|
||||
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):
|
||||
def test_default_error_msg(self):
|
||||
|
|
|
@ -35,6 +35,7 @@ class RPCResetFixture(fixtures.Fixture):
|
|||
self.noti = copy.copy(rpc.NOTIFIER)
|
||||
self.all_mods = copy.copy(rpc.ALLOWED_EXMODS)
|
||||
self.ext_mods = copy.copy(rpc.EXTRA_EXMODS)
|
||||
self.conf = copy.copy(rpc.CONF)
|
||||
self.addCleanup(self._reset_everything)
|
||||
|
||||
def _reset_everything(self):
|
||||
|
@ -43,6 +44,7 @@ class RPCResetFixture(fixtures.Fixture):
|
|||
rpc.NOTIFIER = self.noti
|
||||
rpc.ALLOWED_EXMODS = self.all_mods
|
||||
rpc.EXTRA_EXMODS = self.ext_mods
|
||||
rpc.CONF = self.conf
|
||||
|
||||
|
||||
# We can't import nova.test.TestCase because that sets up an RPCFixture
|
||||
|
@ -525,3 +527,26 @@ class TestClientRouter(test.NoDBTestCase):
|
|||
|
||||
self.assertEqual(router.default_client, client)
|
||||
self.assertFalse(mock_rpcclient.called)
|
||||
|
||||
|
||||
class TestIsNotificationsEnabledDecorator(test.NoDBTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(TestIsNotificationsEnabledDecorator, self).setUp()
|
||||
self.f = mock.Mock()
|
||||
self.f.__name__ = 'f'
|
||||
self.decorated = rpc.if_notifications_enabled(self.f)
|
||||
|
||||
def test_call_func_if_needed(self):
|
||||
self.decorated()
|
||||
self.f.assert_called_once_with()
|
||||
|
||||
@mock.patch('nova.rpc.NOTIFIER.is_enabled', return_value=False)
|
||||
def test_not_call_func_if_notifier_disabled(self, mock_is_enabled):
|
||||
self.decorated()
|
||||
self.assertEqual(0, len(self.f.mock_calls))
|
||||
|
||||
def test_not_call_func_if_only_unversioned_notifications_requested(self):
|
||||
self.flags(notification_format='unversioned', group='notifications')
|
||||
self.decorated()
|
||||
self.assertEqual(0, len(self.f.mock_calls))
|
||||
|
|
Loading…
Reference in New Issue