From 1d680f173988dd02f86a72330d3ed2f3710ac7d3 Mon Sep 17 00:00:00 2001 From: Joe Keen Date: Thu, 16 Feb 2017 14:56:11 -0700 Subject: [PATCH] Make default notifiers pluggable Erasing distinction between plugins that are an inherent part of the notification engine and plugins that can be specified via the config file. Fixing broken tests. Story: 2003801 Task: 26532 Change-Id: I360cc2ad0782f209606706bf1869570fdae2260d --- monasca_notification/conf/__init__.py | 15 --- monasca_notification/conf/notifiers.py | 9 +- monasca_notification/types/notifiers.py | 11 +- tests/resources/notification.yaml | 3 + tests/test_notification_processor.py | 2 +- tests/test_notifiers.py | 159 +++++++++++++++++------- 6 files changed, 123 insertions(+), 76 deletions(-) diff --git a/monasca_notification/conf/__init__.py b/monasca_notification/conf/__init__.py index 9dc80aa..5dc861d 100644 --- a/monasca_notification/conf/__init__.py +++ b/monasca_notification/conf/__init__.py @@ -80,21 +80,6 @@ def load_from_yaml(yaml_config, conf=None): notifiers_cfg = {t.lower(): v for t, v in notifiers_cfg.items()} enabled_plugins = notifiers_cfg.pop('plugins', []) - # We still can have these 3 (email, pagerduty, webhook) - # that are considered as builtin, hence should be always enabled - conf_to_plugin = { - 'email': 'monasca_notification.plugins.' - 'email_notifier:EmailNotifier', - 'pagerduty': 'monasca_notification.plugins.' - 'pagerduty_notifier:PagerdutyNotifier', - 'webhook': 'monasca_notification.plugins.' - 'webhook_notifier:WebhookNotifier' - } - for ctp_key, ctp_clazz in conf_to_plugin.items(): - if ctp_key in notifiers_cfg and ctp_key not in enabled_plugins: - LOG.debug('%s enabled as builtin plugin', ctp_key) - enabled_plugins.append(ctp_clazz) - _plain_override(g='notification_types', enabled=enabled_plugins) if not enabled_plugins: return diff --git a/monasca_notification/conf/notifiers.py b/monasca_notification/conf/notifiers.py index 6f2787a..c37cf89 100644 --- a/monasca_notification/conf/notifiers.py +++ b/monasca_notification/conf/notifiers.py @@ -14,12 +14,11 @@ from oslo_config import cfg -# NOTE(kornicameister) Until https://review.openstack.org/#/c/435136/ -# is merged we only treat these below as plugins. -# WEBHOOK, EMAIL, PAGERDUTY are now treated as built-in & hardcoded -# user has no possibility of enabling/disabling them - _KEY_MAP = { + 'email': 'monasca_notification.plugins.email_notifier:EmailNotifier', + 'pagerduty': 'monasca_notification.plugins.' + 'pagerduty_notifier:PagerdutyNotifier', + 'webhook': 'monasca_notification.plugins.webhook_notifier:WebhookNotifier', 'hipchat': 'monasca_notification.plugins.hipchat_notifier.HipChatNotifier', 'slack': 'monasca_notification.plugins.slack_notifier.SlackNotifier', 'jira': 'monasca_notification.plugins.jira_notifier.JiraNotifier' diff --git a/monasca_notification/types/notifiers.py b/monasca_notification/types/notifiers.py index 89cebff..833a400 100644 --- a/monasca_notification/types/notifiers.py +++ b/monasca_notification/types/notifiers.py @@ -21,10 +21,6 @@ from oslo_config import cfg from oslo_log import log as logging from oslo_utils import importutils -from monasca_notification.plugins import email_notifier -from monasca_notification.plugins import pagerduty_notifier -from monasca_notification.plugins import webhook_notifier - log = logging.getLogger(__name__) CONF = cfg.CONF @@ -46,12 +42,7 @@ def init(statsd_obj): statsd_counter = {} configured_notifiers = {} - - possible_notifiers = [ - email_notifier.EmailNotifier(log), - webhook_notifier.WebhookNotifier(log), - pagerduty_notifier.PagerdutyNotifier(log) - ] + possible_notifiers = [] def load_plugins(): diff --git a/tests/resources/notification.yaml b/tests/resources/notification.yaml index cbc6a1f..a5b278f 100644 --- a/tests/resources/notification.yaml +++ b/tests/resources/notification.yaml @@ -29,6 +29,9 @@ postgresql: notification_types: plugins: + - monasca_notification.plugins.email_notifier:EmailNotifier + - monasca_notification.plugins.pagerduty_notifier:PagerdutyNotifier + - monasca_notification.plugins.webhook_notifier:WebhookNotifier - monasca_notification.plugins.hipchat_notifier:HipChatNotifier - monasca_notification.plugins.slack_notifier:SlackNotifier - monasca_notification.plugins.jira_notifier:JiraNotifier diff --git a/tests/test_notification_processor.py b/tests/test_notification_processor.py index 8765bc6..875d110 100644 --- a/tests/test_notification_processor.py +++ b/tests/test_notification_processor.py @@ -61,7 +61,7 @@ class TestNotificationProcessor(base.BaseTestCase): # ------------------------------------------------------------------------ @mock.patch('pymysql.connect') @mock.patch('monasca_notification.common.utils.monascastatsd') - @mock.patch('monasca_notification.types.notifiers.email_notifier.smtplib') + @mock.patch('monasca_notification.plugins.email_notifier.smtplib') @mock.patch('monasca_notification.processors.notification_processor.notifiers.log') def _start_processor(self, notifications, mock_log, mock_smtp, mock_statsd, mock_pymsql): """Start the processor with the proper mocks diff --git a/tests/test_notifiers.py b/tests/test_notifiers.py index ebd44d3..7367606 100644 --- a/tests/test_notifiers.py +++ b/tests/test_notifiers.py @@ -1,4 +1,4 @@ -# (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP +# (C) Copyright 2015-2017 Hewlett Packard Enterprise Development LP # Copyright 2017 Fujitsu LIMITED # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -149,55 +149,103 @@ class TestInterface(base.BaseTestCase): def _goodSendStub(self, log): return NotifyStub(self.trap, False, False, False) - @mock.patch('monasca_notification.types.notifiers.email_notifier.smtplib') - @mock.patch('monasca_notification.types.notifiers.log') - def test_enabled_notifications(self, mock_log, mock_smtp): + def test_enabled_notifications_none(self): + self.conf_override( + group='notification_types', + enabled=[] + ) + notifiers.init(self.statsd) + notifiers.load_plugins() notifiers.config() notifications = notifiers.enabled_notifications() - self.assertEqual(len(notifications), 3) - self.assertEqual(sorted(notifications), - ["EMAIL", "PAGERDUTY", "WEBHOOK"]) + self.assertEqual(len(notifications), 0) - @mock.patch('monasca_notification.types.notifiers.email_notifier') - @mock.patch('monasca_notification.types.notifiers.email_notifier.smtplib') + def test_enabled_notifications(self): + self.conf_override( + group='notification_types', + enabled=[ + 'monasca_notification.plugins.email_notifier:EmailNotifier', + 'monasca_notification.plugins.pagerduty_notifier:PagerdutyNotifier', + 'monasca_notification.plugins.webhook_notifier:WebhookNotifier', + 'monasca_notification.plugins.hipchat_notifier:HipChatNotifier', + 'monasca_notification.plugins.slack_notifier:SlackNotifier', + 'monasca_notification.plugins.jira_notifier:JiraNotifier' + ] + ) + notifiers.init(self.statsd) + notifiers.load_plugins() + notifiers.config() + + notifications = notifiers.enabled_notifications() + self.assertEqual(len(notifications), 6) + self.assertItemsEqual(notifications, + ['EMAIL', 'PAGERDUTY', 'WEBHOOK', + 'HIPCHAT', 'SLACK', 'JIRA']) + + @mock.patch('monasca_notification.plugins.email_notifier') + @mock.patch('monasca_notification.plugins.email_notifier.smtplib') @mock.patch('monasca_notification.types.notifiers.log') - def test_send_notification_exception(self, mock_log, mock_smtp, mock_email): + @mock.patch('monasca_notification.types.notifiers.importutils') + def test_send_notification_exception(self, mock_im, mock_log, + mock_smtp, mock_email): + self.conf_override( + group='notification_types', + enabled=[ + 'monasca_notification.plugins.email_notifier:EmailNotifier' + ] + ) + mock_log.warn = self.trap.append mock_log.error = self.trap.append mock_log.exception = self.trap.append mock_email.EmailNotifier = self._sendExceptionStub + mock_im.import_class.return_value = mock_email.EmailNotifier notifiers.init(self.statsd) + notifiers.load_plugins() notifiers.config() - notifications = [] - notifications.append(m_notification.Notification(0, 'email', 'email notification', - 'me@here.com', 0, 0, alarm({}))) + notifications = [ + m_notification.Notification(0, 'email', 'email notification', + 'me@here.com', 0, 0, alarm({})) + ] notifiers.send_notifications(notifications) self.assertIn("send_notification exception for email", self.trap) - @mock.patch('monasca_notification.types.notifiers.email_notifier') - @mock.patch('monasca_notification.types.notifiers.email_notifier.smtplib') + @mock.patch('monasca_notification.plugins.email_notifier') + @mock.patch('monasca_notification.plugins.email_notifier.smtplib') @mock.patch('monasca_notification.types.notifiers.log') - def test_send_notification_failure(self, mock_log, mock_smtp, mock_email): + @mock.patch('monasca_notification.types.notifiers.importutils') + def test_send_notification_failure(self, mock_im, mock_log, + mock_smtp, mock_email): + self.conf_override( + group='notification_types', + enabled=[ + 'monasca_notification.plugins.email_notifier:EmailNotifier' + ] + ) + mock_log.warn = self.trap.append mock_log.error = self.trap.append mock_log.exception = self.trap.append mock_email.EmailNotifier = self._sendFailureStub + mock_im.import_class.return_value = mock_email.EmailNotifier notifiers.init(self.statsd) + notifiers.load_plugins() notifiers.config() - notifications = [] - notifications.append(m_notification.Notification(0, 'email', 'email notification', - 'me@here.com', 0, 0, alarm({}))) + notifications = [ + m_notification.Notification(0, 'email', 'email notification', + 'me@here.com', 0, 0, alarm({})) + ] sent, failed, invalid = notifiers.send_notifications(notifications) @@ -206,28 +254,38 @@ class TestInterface(base.BaseTestCase): self.assertEqual(invalid, []) @mock.patch('monasca_notification.types.notifiers.time') - @mock.patch('monasca_notification.types.notifiers.email_notifier') - @mock.patch('monasca_notification.types.notifiers.email_notifier.smtplib') + @mock.patch('monasca_notification.plugins.email_notifier') + @mock.patch('monasca_notification.plugins.email_notifier.smtplib') @mock.patch('monasca_notification.types.notifiers.log') - def test_send_notification_correct(self, mock_log, mock_smtp, + @mock.patch('monasca_notification.types.notifiers.importutils') + def test_send_notification_correct(self, mock_im, mock_log, mock_smtp, mock_email, mock_time): + self.conf_override( + group='notification_types', + enabled=[ + 'monasca_notification.plugins.email_notifier:EmailNotifier' + ] + ) + mock_log.warn = self.trap.append mock_log.error = self.trap.append mock_email.EmailNotifier = self._goodSendStub - mock_time.time.return_value = 42 + mock_im.import_class.return_value = mock_email.EmailNotifier notifiers.init(self.statsd) + notifiers.load_plugins() notifiers.config() - notifications = [] - notifications.append(m_notification.Notification(0, 'email', 'email notification', - 'me@here.com', 0, 0, alarm({}))) - notifications.append(m_notification.Notification(1, 'email', 'email notification', - 'foo@here.com', 0, 0, alarm({}))) - notifications.append(m_notification.Notification(2, 'email', 'email notification', - 'bar@here.com', 0, 0, alarm({}))) + notifications = [ + m_notification.Notification(0, 'email', 'email notification', + 'me@here.com', 0, 0, alarm({})), + m_notification.Notification(1, 'email', 'email notification', + 'foo@here.com', 0, 0, alarm({})), + m_notification.Notification(2, 'email', 'email notification', + 'bar@here.com', 0, 0, alarm({})) + ] sent, failed, invalid = notifiers.send_notifications(notifications) @@ -238,25 +296,36 @@ class TestInterface(base.BaseTestCase): for n in sent: self.assertEqual(n.notification_timestamp, 42) - @mock.patch('monasca_notification.types.notifiers.email_notifier') - @mock.patch('monasca_notification.types.notifiers.email_notifier.smtplib') + @mock.patch('monasca_notification.plugins.email_notifier') + @mock.patch('monasca_notification.plugins.email_notifier.smtplib') @mock.patch('monasca_notification.types.notifiers.log') - def test_statsd(self, mock_log, mock_smtp, mock_email): + @mock.patch('monasca_notification.types.notifiers.importutils') + def test_statsd(self, mock_im, mock_log, mock_smtp, mock_email): + self.conf_override( + group='notification_types', + enabled=[ + 'monasca_notification.plugins.email_notifier:EmailNotifier' + ] + ) + mock_log.warn = self.trap.append mock_log.error = self.trap.append mock_email.EmailNotifier = self._goodSendStub + mock_im.import_class.return_value = mock_email.EmailNotifier notifiers.init(self.statsd) + notifiers.load_plugins() notifiers.config() - notifications = [] - notifications.append(m_notification.Notification(0, 'email', 'email notification', - 'me@here.com', 0, 0, alarm({}))) - notifications.append(m_notification.Notification(1, 'email', 'email notification', - 'foo@here.com', 0, 0, alarm({}))) - notifications.append(m_notification.Notification(2, 'email', 'email notification', - 'bar@here.com', 0, 0, alarm({}))) + notifications = [ + m_notification.Notification(0, 'email', 'email notification', + 'me@here.com', 0, 0, alarm({})), + m_notification.Notification(1, 'email', 'email notification', + 'foo@here.com', 0, 0, alarm({})), + m_notification.Notification(2, 'email', 'email notification', + 'bar@here.com', 0, 0, alarm({})) + ] notifiers.send_notifications(notifications) @@ -275,11 +344,11 @@ class TestInterface(base.BaseTestCase): notifiers.init(self.statsd) notifiers.load_plugins() - self.assertEqual(len(notifiers.possible_notifiers), 5) + notifiers.config() - configured_plugins = ["email", "webhook", "pagerduty", "hipchat", "slack"] - for plugin in notifiers.configured_notifiers: - self.asssertIn(plugin.type in configured_plugins) + self.assertEqual(len(notifiers.possible_notifiers), 2) + self.assertItemsEqual(['hipchat', 'slack'], + notifiers.configured_notifiers) @mock.patch('monasca_notification.types.notifiers.log') def test_invalid_plugin_load_exception_ignored(self, mock_log): @@ -294,7 +363,7 @@ class TestInterface(base.BaseTestCase): notifiers.init(self.statsd) notifiers.load_plugins() - self.assertEqual(len(notifiers.possible_notifiers), 4) + self.assertEqual(len(notifiers.possible_notifiers), 1) self.assertEqual(len(self.trap), 1) configured_plugins = ["email", "webhook", "pagerduty", "slack"]