From 0e81ec00ceeef73c390fe51d62ccc54a4ee5b103 Mon Sep 17 00:00:00 2001 From: Daniel Abad Date: Wed, 9 Jan 2019 16:09:23 +0100 Subject: [PATCH] Make versioned notifications topics configurable Some services (such as telemetry) actually consume the notifications. If one deploys a service that listens on the same queue as telemetry, there will be race-conditions with these services and one will not get the notifications that are expected at points. To address this, one sets a different topic and consumes from there. This is not possible with versioned notifications at the moment. And, as services move towards using them, the same need will arise. This adds a configuration option to Ironic enabling the configuration of topics for this notifier. A similar change was introduced in nova: https://review.openstack.org/#/c/444947/ Change-Id: Ib75feac0979d0094cb137abb13b0fe0ff4576eee Story: 2004735 Task: 28788 --- doc/source/admin/notifications.rst | 5 ++++ ironic/common/rpc.py | 12 ++++------ ironic/conf/default.py | 24 +++++++++++++++---- ironic/tests/unit/common/test_rpc.py | 23 ++++++++++++++---- ...-topics-configurable-18d70d573c27809e.yaml | 6 +++++ 5 files changed, 55 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/make-versioned-notifications-topics-configurable-18d70d573c27809e.yaml diff --git a/doc/source/admin/notifications.rst b/doc/source/admin/notifications.rst index 7ffa583d44..1196b64af4 100644 --- a/doc/source/admin/notifications.rst +++ b/doc/source/admin/notifications.rst @@ -60,6 +60,11 @@ field of the payload. Consumers are guaranteed that microversion bumps will add new fields, while macroversion bumps are backwards-incompatible and may have fields removed. +Versioned notifications are emitted by default to the +`ironic_versioned_notifications` topic. This can be changed and it is +configurable in the ironic.conf with the `versioned_notifications_topics` +config option. + Available notifications ======================= .. TODO(mariojv) Add some form of tabular formatting below diff --git a/ironic/common/rpc.py b/ironic/common/rpc.py index ae2d21547a..95647a72a3 100644 --- a/ironic/common/rpc.py +++ b/ironic/common/rpc.py @@ -13,17 +13,15 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_config import cfg import oslo_messaging as messaging from oslo_messaging.rpc import dispatcher from osprofiler import profiler from ironic.common import context as ironic_context from ironic.common import exception +from ironic.conf import CONF -CONF = cfg.CONF - TRANSPORT = None NOTIFICATION_TRANSPORT = None SENSORS_NOTIFIER = None @@ -53,10 +51,10 @@ def init(conf): serializer=serializer, driver='noop') else: - VERSIONED_NOTIFIER = messaging.Notifier(NOTIFICATION_TRANSPORT, - serializer=serializer, - topics=['ironic_versioned_' - 'notifications']) + VERSIONED_NOTIFIER = messaging.Notifier( + NOTIFICATION_TRANSPORT, + serializer=serializer, + topics=CONF.versioned_notifications_topics) def cleanup(): diff --git a/ironic/conf/default.py b/ironic/conf/default.py index 46d1394b7b..86ec8c06df 100644 --- a/ironic/conf/default.py +++ b/ironic/conf/default.py @@ -238,10 +238,11 @@ netconf_opts = [ '"127.0.0.1".')), ] -# NOTE(mariojv) By default, accessing this option when it's unset will return -# None, indicating no notifications will be sent. oslo.config returns None by -# default for options without set defaults that aren't required. notification_opts = [ + # NOTE(mariojv) By default, accessing this option when it's unset will + # return None, indicating no notifications will be sent. oslo.config + # returns None by default for options without set defaults that aren't + # required. cfg.StrOpt('notification_level', choices=[('debug', _('"debug" level')), ('info', _('"info" level')), @@ -250,7 +251,22 @@ notification_opts = [ ('critical', _('"critical" level'))], help=_('Specifies the minimum level for which to send ' 'notifications. If not set, no notifications will ' - 'be sent. The default is for this option to be unset.')) + 'be sent. The default is for this option to be unset.')), + cfg.ListOpt( + 'versioned_notifications_topics', + default=['ironic_versioned_notifications'], + help=_(""" +Specifies the topics for the versioned notifications issued by Ironic. + +The default value is fine for most deployments and rarely needs to be changed. +However, if you have a third-party service that consumes versioned +notifications, it might be worth getting a topic for that service. +Ironic will send a message containing a versioned notification payload to each +topic queue in this list. + +The list of versioned notifications is visible in +https://docs.openstack.org/ironic/latest/admin/notifications.html +""")), ] path_opts = [ diff --git a/ironic/tests/unit/common/test_rpc.py b/ironic/tests/unit/common/test_rpc.py index 927b521c46..6cb25098da 100644 --- a/ironic/tests/unit/common/test_rpc.py +++ b/ironic/tests/unit/common/test_rpc.py @@ -48,9 +48,24 @@ class TestUtils(base.TestCase): mock_get_notification, mock_json_serializer, mock_notifier) - def _test_init_globals(self, notifications_enabled, mock_get_rpc_transport, - mock_get_notification, mock_json_serializer, - mock_notifier): + @mock.patch.object(messaging, 'Notifier', autospec=True) + @mock.patch.object(messaging, 'JsonPayloadSerializer', autospec=True) + @mock.patch.object(messaging, 'get_notification_transport', autospec=True) + @mock.patch.object(messaging, 'get_rpc_transport', autospec=True) + def test_init_globals_with_custom_topics(self, mock_get_rpc_transport, + mock_get_notification, + mock_json_serializer, + mock_notifier): + self._test_init_globals( + False, mock_get_rpc_transport, mock_get_notification, + mock_json_serializer, mock_notifier, + versioned_notifications_topics=['custom_topic1', 'custom_topic2']) + + def _test_init_globals( + self, notifications_enabled, mock_get_rpc_transport, + mock_get_notification, mock_json_serializer, mock_notifier, + versioned_notifications_topics=['ironic_versioned_notifications']): + rpc.TRANSPORT = None rpc.NOTIFICATION_TRANSPORT = None rpc.SENSORS_NOTIFIER = None @@ -89,7 +104,7 @@ class TestUtils(base.TestCase): mock.call( rpc.NOTIFICATION_TRANSPORT, serializer=mock_request_serializer.return_value, - topics=['ironic_versioned_notifications']) + topics=versioned_notifications_topics) ] mock_notifier.assert_has_calls(notifier_calls) diff --git a/releasenotes/notes/make-versioned-notifications-topics-configurable-18d70d573c27809e.yaml b/releasenotes/notes/make-versioned-notifications-topics-configurable-18d70d573c27809e.yaml new file mode 100644 index 0000000000..e6c80d8597 --- /dev/null +++ b/releasenotes/notes/make-versioned-notifications-topics-configurable-18d70d573c27809e.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Adds a ``[DEFAULT]/versioned_notifications_topics`` configuration option. + This enables operators to configure the topics used for versioned + notifications.