File 0003-fix-notification-short-circuit.patch of Package openstack-cinder
From e7171fd74a683a1118c03506ef1600333193abb5 Mon Sep 17 00:00:00 2001
From: Gorka Eguileor <geguileo@redhat.com>
Date: Wed, 01 Feb 2017 12:17:39 +0100
Subject: [PATCH] Fix notification short-circuit
We recently introduced a mechanism to short-circuit notifications when
they were disabled, unfortunately the mechanism was not backward
compatible with deprecated oslo notification setup that defaults to use
the transport mechanism of RPC.
In Mitaka oslo messaging introduced the transport_url specific for
notifications under the oslo_messaging_notifications section, but some
projects still use the default transport_url defined in the DEFAULT
section.
This patch fixes the notification short-circuit, now we don't check the
transport_url but the driver option in the oslo_messaging_notifications
section, which is a more robust way of checking, since it is backward
compatible.
Change-Id: Iccf51756701759e3727d3d989bab08bc05cac9a7
Closes-Bug: #1660928
---
diff --git a/cinder/rpc.py b/cinder/rpc.py
index b59d232..d58ca1d 100644
--- a/cinder/rpc.py
+++ b/cinder/rpc.py
@@ -77,7 +77,7 @@
# get_notification_transport has loaded oslo_messaging_notifications config
# group, so we can now check if notifications are actually enabled.
- if conf.oslo_messaging_notifications.transport_url:
+ if utils.notifications_enabled(conf):
json_serializer = JsonPayloadSerializer()
serializer = RequestContextSerializer(json_serializer)
NOTIFIER = messaging.Notifier(NOTIFICATION_TRANSPORT,
diff --git a/cinder/test.py b/cinder/test.py
index eb98350..03b4a9e 100644
--- a/cinder/test.py
+++ b/cinder/test.py
@@ -32,6 +32,7 @@ from oslo_concurrency import lockutils
from oslo_config import cfg
from oslo_config import fixture as config_fixture
from oslo_log.fixture import logging_error as log_fixture
+import oslo_messaging
from oslo_messaging import conffixture as messaging_conffixture
from oslo_utils import strutils
from oslo_utils import timeutils
@@ -50,7 +51,6 @@ from cinder import service
from cinder.tests import fixtures as cinder_fixtures
from cinder.tests.unit import conf_fixture
from cinder.tests.unit import fake_notifier
-from cinder.tests.unit import utils as test_utils
CONF = cfg.CONF
@@ -154,6 +154,14 @@
self.messaging_conf.transport_driver = 'fake'
self.messaging_conf.response_timeout = 15
self.useFixture(self.messaging_conf)
+
+ # Load oslo_messaging_notifications config group so we can set an
+ # override to prevent notifications from being ignored due to the
+ # short-circuit mechanism.
+ oslo_messaging.get_notification_transport(CONF)
+ # We need to use a valid driver for the notifications, so we use test.
+ self.override_config('driver', ['test'],
+ group='oslo_messaging_notifications')
rpc.init(CONF)
# NOTE(geguileo): This is required because _determine_obj_version_cap
@@ -233,8 +241,6 @@
group='coordination')
coordination.COORDINATOR.start()
self.addCleanup(coordination.COORDINATOR.stop)
-
- test_utils.set_normal_rpc_notifier(self)
def _restore_obj_registry(self):
objects_base.CinderObjectRegistry._registry._obj_classes = \
diff --git a/cinder/tests/unit/test_rpc.py b/cinder/tests/unit/test_rpc.py
index 39ef198..87c4f4a 100644
--- a/cinder/tests/unit/test_rpc.py
+++ b/cinder/tests/unit/test_rpc.py
@@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations
# under the License.
+import ddt
import mock
from cinder.objects import base
@@ -25,6 +26,7 @@
BINARY = 'cinder-scheduler'
+@ddt.ddt
class RPCAPITestCase(test.TestCase):
"""Tests RPCAPI mixin aggregating stuff related to RPC compatibility."""
@@ -84,9 +86,11 @@
self.assertFalse(get_min_obj.called)
self.assertFalse(get_min_rpc.called)
+ @ddt.data([], ['noop'], ['noop', 'noop'])
@mock.patch('oslo_messaging.JsonPayloadSerializer', wraps=True)
- def test_init_no_notifications(self, serializer_mock):
- self.override_config('transport_url', '',
+ def test_init_no_notifications(self, driver, serializer_mock):
+ """Test short-circuiting notifications with default and noop driver."""
+ self.override_config('driver', driver,
group='oslo_messaging_notifications')
rpc.init(test.CONF)
self.assertEqual(rpc.utils.DO_NOTHING, rpc.NOTIFIER)
diff --git a/cinder/tests/unit/test_utils.py b/cinder/tests/unit/test_utils.py
index 788f1e7..3740089 100644
--- a/cinder/tests/unit/test_utils.py
+++ b/cinder/tests/unit/test_utils.py
@@ -1416,6 +1416,7 @@
value, 'limit', min_value=-1, max_value=(2 ** 31))
+@ddt.ddt
class TestNotificationShortCircuit(test.TestCase):
def test_do_nothing_getter(self):
"""Test any attribute will always return the same instance (self)."""
@@ -1441,9 +1442,10 @@
result = self._decorated_method()
self.assertEqual(mock.sentinel.success, result)
- def test_if_notification_enabled_when_disabled(self):
+ @ddt.data([], ['noop'], ['noop', 'noop'])
+ def test_if_notification_enabled_when_disabled(self, driver):
"""Test method is not called when notifications are disabled."""
- self.override_config('transport_url', '',
+ self.override_config('driver', driver,
group='oslo_messaging_notifications')
result = self._decorated_method()
self.assertEqual(utils.DO_NOTHING, result)
diff --git a/cinder/tests/unit/utils.py b/cinder/tests/unit/utils.py
index b96cfd7..b756071 100644
--- a/cinder/tests/unit/utils.py
+++ b/cinder/tests/unit/utils.py
@@ -27,7 +27,6 @@ from cinder import db
from cinder import exception
from cinder import objects
from cinder.objects import fields
-from cinder import rpc
from cinder.tests.unit import fake_constants as fake
@@ -437,13 +436,3 @@ def generate_timeout_series(timeout):
while True:
iteration += 1
yield (iteration * timeout) + iteration
-
-
-def set_normal_rpc_notifier(test_case):
- """Instead of shortcircuiting notifications, user Oslo's notifier."""
- test_case.override_config('transport_url', 'fake_transport',
- group='oslo_messaging_notifications')
- json_serializer = rpc.messaging.JsonPayloadSerializer()
- serializer = rpc.RequestContextSerializer(json_serializer)
- rpc.NOTIFIER = rpc.messaging.Notifier(rpc.NOTIFICATION_TRANSPORT,
- serializer=serializer)
diff --git a/cinder/utils.py b/cinder/utils.py
index 2d79f0b..8ef6f24 100644
--- a/cinder/utils.py
+++ b/cinder/utils.py
@@ -1089,11 +1089,17 @@
DO_NOTHING = DoNothing()
+def notifications_enabled(conf):
+ """Check if oslo notifications are enabled."""
+ notifications_driver = set(conf.oslo_messaging_notifications.driver)
+ return notifications_driver and notifications_driver != {'noop'}
+
+
def if_notifications_enabled(f):
"""Calls decorated method only if notifications are enabled."""
@functools.wraps(f)
def wrapped(*args, **kwargs):
- if CONF.oslo_messaging_notifications.transport_url:
+ if notifications_enabled(CONF):
return f(*args, **kwargs)
return DO_NOTHING
return wrapped
