From efc6e28edceac26599eab7f274a40708d1889f8c Mon Sep 17 00:00:00 2001 From: Doug Szumski Date: Tue, 12 Nov 2019 10:38:15 +0000 Subject: [PATCH] Fix periodic notifications for webhooks - Removes the hard-coded magic number of 60 seconds allowing users to choose the period that they require. - Standardise on strings for DictOpt dict keys. When loaded from a config file, the DictOpt keys are parsed as strings, which was conflicting with the default integer dict keys. This caused the periodic engine to silently fail to load when configured via a config file. - Remove unused variable Story: 2006783 Task: 37313 Change-Id: Ibd61c45fc1ade37022150d34a5b00c56fdf69814 --- monasca_notification/conf/kafka.py | 2 +- monasca_notification/conf/zookeeper.py | 2 +- monasca_notification/main.py | 4 ++-- monasca_notification/notification.py | 4 ---- monasca_notification/notification_engine.py | 6 +++--- monasca_notification/periodic_engine.py | 4 ++-- monasca_notification/plugins/jiraformat.yml | 1 - .../notes/fix_periodic_notifications-d842bfb5ec665123.yaml | 6 ++++++ tests/test_notification.py | 2 -- 9 files changed, 15 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/fix_periodic_notifications-d842bfb5ec665123.yaml diff --git a/monasca_notification/conf/kafka.py b/monasca_notification/conf/kafka.py index 0fddbf0..641736c 100644 --- a/monasca_notification/conf/kafka.py +++ b/monasca_notification/conf/kafka.py @@ -22,7 +22,7 @@ _DEFAULT_ALARM_TOPIC = 'alarm-state-transitions' _DEFAULT_NOTIFICATION_TOPIC = 'alarm-notifications' _DEFAULT_RETRY_TOPIC = 'retry-notifications' _DEFAULT_PERIODIC_TOPICS = { - 60: '60-seconds-notifications' + '60': '60-seconds-notifications' } _DEFAULT_MAX_OFFSET_LAG = 600 diff --git a/monasca_notification/conf/zookeeper.py b/monasca_notification/conf/zookeeper.py index 60c4a9a..2d9e168 100644 --- a/monasca_notification/conf/zookeeper.py +++ b/monasca_notification/conf/zookeeper.py @@ -20,7 +20,7 @@ _DEFAULT_URL = '127.0.0.1:2181' _DEFAULT_NOTIFICATION_PATH = '/notification/alarms' _DEFAULT_RETRY_PATH = '/notification/retry' _DEFAULT_PERIODIC_PATH = { - 60: '/notification/60_seconds' + '60': '/notification/60_seconds' } zookeeper_group = cfg.OptGroup('zookeeper', diff --git a/monasca_notification/main.py b/monasca_notification/main.py index ce0ef32..8f0fe66 100644 --- a/monasca_notification/main.py +++ b/monasca_notification/main.py @@ -108,10 +108,10 @@ def main(argv=None): args=(retry_engine.RetryEngine,)) ) - if 60 in CONF.kafka.periodic: + for notification_period in CONF.kafka.periodic.keys(): processors.append(multiprocessing.Process( target=start_process, - args=(periodic_engine.PeriodicEngine, 60)) + args=(periodic_engine.PeriodicEngine, int(notification_period))) ) try: diff --git a/monasca_notification/notification.py b/monasca_notification/notification.py index 2ea5a4b..4bd1328 100644 --- a/monasca_notification/notification.py +++ b/monasca_notification/notification.py @@ -39,7 +39,6 @@ class Notification(object): 'retry_count', 'raw_alarm', 'period', - 'periodic_topic' ) def __init__(self, id, type, name, address, period, retry_count, alarm): @@ -77,8 +76,6 @@ class Notification(object): # to be updated on actual notification send time self.notification_timestamp = None - # set periodic topic - self.periodic_topic = period self.period = period def __eq__(self, other): @@ -115,7 +112,6 @@ class Notification(object): 'lifecycle_state', 'tenant_id', 'period', - 'periodic_topic' ] notification_data = {name: getattr(self, name) for name in notification_fields} diff --git a/monasca_notification/notification_engine.py b/monasca_notification/notification_engine.py index 41ef936..345d350 100644 --- a/monasca_notification/notification_engine.py +++ b/monasca_notification/notification_engine.py @@ -46,10 +46,10 @@ class NotificationEngine(object): def _add_periodic_notifications(self, notifications): for notification in notifications: - topic = notification.periodic_topic - if topic in CONF.kafka.periodic and notification.type == "webhook": + period = str(notification.period) + if period in CONF.kafka.periodic.keys() and notification.type == "webhook": notification.notification_timestamp = time.time() - self._producer.publish(CONF.kafka.periodic[topic], + self._producer.publish(CONF.kafka.periodic[period], [notification.to_json()]) def run(self): diff --git a/monasca_notification/periodic_engine.py b/monasca_notification/periodic_engine.py index 4e772dc..686ca36 100644 --- a/monasca_notification/periodic_engine.py +++ b/monasca_notification/periodic_engine.py @@ -33,7 +33,7 @@ CONF = cfg.CONF class PeriodicEngine(object): def __init__(self, period): - self._topic_name = CONF.kafka.periodic[period] + self._topic_name = CONF.kafka.periodic[str(period)] self._statsd = get_statsd_client() @@ -42,7 +42,7 @@ class PeriodicEngine(object): CONF.kafka.group, self._topic_name, CONF.zookeeper.url, - CONF.zookeeper.periodic_path[period], + CONF.zookeeper.periodic_path[str(period)], CONF.kafka.legacy_kafka_client_enabled) self._producer = client_factory.get_kafka_producer( CONF.kafka.url, diff --git a/monasca_notification/plugins/jiraformat.yml b/monasca_notification/plugins/jiraformat.yml index d11f3e0..279065e 100644 --- a/monasca_notification/plugins/jiraformat.yml +++ b/monasca_notification/plugins/jiraformat.yml @@ -22,7 +22,6 @@ jira_format: # notification.retry_count # notification.raw_alarm # notification.period - # notification.periodic_topic # Please include alarm_id in summary as it is used for searching the issues summary: Alaram created for {{ notification.alarm_name }} with severity {{ notification.state }} for {{ notification.alarm_id }} diff --git a/releasenotes/notes/fix_periodic_notifications-d842bfb5ec665123.yaml b/releasenotes/notes/fix_periodic_notifications-d842bfb5ec665123.yaml new file mode 100644 index 0000000..97fb188 --- /dev/null +++ b/releasenotes/notes/fix_periodic_notifications-d842bfb5ec665123.yaml @@ -0,0 +1,6 @@ +--- +features: + - Supports setting arbitrary notification periods. +fixes: + - Issue with periodic notification processor failing to load when using Oslo + config. diff --git a/tests/test_notification.py b/tests/test_notification.py index ad1a787..5d7928f 100644 --- a/tests/test_notification.py +++ b/tests/test_notification.py @@ -51,7 +51,6 @@ def test_json(): u'address': u'address', u'message': u'stateChangeReason', u'period': 0, - u'periodic_topic': 0, u'retry_count': 0, u'raw_alarm': { u'alarmId': u'alarmId', @@ -101,7 +100,6 @@ def test_json_non_zero_period(): u'address': u'address', u'message': u'stateChangeReason', u'period': 60, - u'periodic_topic': 60, u'retry_count': 0, u'raw_alarm': { u'alarmId': u'alarmId',