From b9242bf984b399868ad785db4fbfb449e554816d Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Tue, 13 Dec 2016 19:59:19 -0500 Subject: [PATCH] Refactor ZaqarAlarmNotifier and fix tests Avoid duplicating the code for getting a Zaqar client. Also fix the tests, where assertion failures in the FakeZaqarClient code were being suppressed because of indiscriminate exception catching. We now actually check that a message was sent, so if there is an error we'll see the failure. Change-Id: Ibbd7bcec15eead9ef2afda4e1c98b576162bfe8a --- aodh/notifier/zaqar.py | 33 +++++++-------- aodh/tests/unit/test_notifier.py | 69 +++++++++++++++++--------------- 2 files changed, 52 insertions(+), 50 deletions(-) diff --git a/aodh/notifier/zaqar.py b/aodh/notifier/zaqar.py index f4b36631e..d759f5590 100644 --- a/aodh/notifier/zaqar.py +++ b/aodh/notifier/zaqar.py @@ -79,9 +79,9 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier): " Keystone service catalog.")) return self._zendpoint - def get_zaqar_client(self): + def _get_client_conf(self): conf = self.conf.service_credentials - params = { + return { 'auth_opts': { 'backend': 'keystone', 'options': { @@ -93,15 +93,17 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier): } } } + + def get_zaqar_client(self, conf): try: from zaqarclient.queues import client as zaqar_client return zaqar_client.Client(self._get_endpoint(), - version=2, conf=params) + version=2, conf=conf) except Exception: LOG.error(_LE("Failed to connect to Zaqar service "), exc_info=True) - def get_presigned_client(self, queue_info): + def _get_presigned_client_conf(self, queue_info): queue_name = queue_info.get('queue_name', [''])[0] if not queue_name: return None, None @@ -111,7 +113,7 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier): paths = queue_info.get('paths', [''])[0].split(',') methods = queue_info.get('methods', [''])[0].split(',') project_id = queue_info.get('project_id', [''])[0] - params = { + conf = { 'auth_opts': { 'backend': 'signed-url', 'options': { @@ -123,14 +125,7 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier): } } } - try: - from zaqarclient.queues import client as zaqar_client - return (zaqar_client.Client(self._get_endpoint(), - version=2, conf=params), - queue_name) - except Exception: - LOG.error(_LE("Failed to connect to Zaqar service "), - exc_info=True) + return conf, queue_name def notify(self, action, alarm_id, alarm_name, severity, previous, current, reason, reason_data, headers=None): @@ -154,7 +149,7 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier): @property def client(self): if self._zclient is None: - self._zclient = self.get_zaqar_client() + self._zclient = self.get_zaqar_client(self._get_client_conf()) return self._zclient def notify_zaqar(self, action, message): @@ -163,9 +158,11 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier): # NOTE(flwang): Try to get build a pre-signed client if user has # provide enough information about that. Otherwise, go to build # a client with service account and queue name for this alarm. - zaqar_client, queue_name = self.get_presigned_client(queue_info) + conf, queue_name = self._get_presigned_client_conf(queue_info) + if conf is not None: + zaqar_client = self.get_zaqar_client(conf) - if not zaqar_client or not queue_name: + if conf is None or queue_name is None or zaqar_client is None: zaqar_client = self.client # queue_name is a combination of - queue_name = "%s-%s" % (message['body']['alarm_id'], @@ -184,8 +181,8 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier): # post the message to the queue queue.post(message) except IndexError: - LOG.error(_LE("Required topic query option missing in action %s") - % action) + LOG.error(_LE("Required query option missing in action %s"), + action) except Exception: LOG.error(_LE("Unknown error occurred; Failed to post message to" " Zaqar queue"), diff --git a/aodh/tests/unit/test_notifier.py b/aodh/tests/unit/test_notifier.py index f33687586..2b5682120 100644 --- a/aodh/tests/unit/test_notifier.py +++ b/aodh/tests/unit/test_notifier.py @@ -378,55 +378,59 @@ class TestAlarmNotifier(tests_base.BaseTestCase): self.assertEqual(DATA_JSON, jsonutils.loads(kwargs['data'])) def test_zaqar_notifier_action(self): - action = 'zaqar://?topic=critical&subscriber=http://example.com/data' \ - '&subscriber=mailto:foo@example.com&ttl=7200' - self._msg_notifier.sample({}, 'alarm.update', - self._notification(action)) - time.sleep(1) - self.assertEqual(self.zaqar, - self.service.notifiers['zaqar'].obj.client) - - def test_presigned_zaqar_notifier_action(self): with mock.patch.object(notifier.zaqar.ZaqarAlarmNotifier, - 'get_presigned_client') as zaqar_client: - zaqar_client.return_value = self.zaqar, 'foobar-critical' - action = 'zaqar://?topic=critical&' \ - 'subscriber=http://example.com/data' \ - '&subscriber=mailto:foo@example.com&ttl=7200' \ - '&signature=mysignature&expires=2016-06-29T01:49:56' \ - '&paths=/v2/queues/beijing/messages' \ - '&methods=GET,PATCH,POST,PUT&queue_name=foobar-critical' \ - '&project_id=my_project_id' + '_get_client_conf') as get_conf: + action = ('zaqar://?topic=critical' + '&subscriber=http://example.com/data' + '&subscriber=mailto:foo@example.com&ttl=7200') self._msg_notifier.sample({}, 'alarm.update', self._notification(action)) time.sleep(1) - self.assertEqual(zaqar_client.return_value[0], - self.service.notifiers['zaqar'].obj.client) - queue_info = urlparse.parse_qs(urlparse.urlparse(action).query) - zaqar_client.assert_called_with(queue_info) + get_conf.assert_called() + self.assertEqual(self.zaqar, + self.service.notifiers['zaqar'].obj._zclient) + self.assertEqual(2, self.zaqar.subscriptions) + self.assertEqual(1, self.zaqar.posts) + + def test_presigned_zaqar_notifier_action(self): + action = ('zaqar://?' + 'subscriber=http://example.com/data&ttl=7200' + '&signature=mysignature&expires=2016-06-29T01:49:56' + '&paths=/v2/queues/beijing/messages' + '&methods=GET,PATCH,POST,PUT&queue_name=foobar-critical' + '&project_id=my_project_id') + self._msg_notifier.sample({}, 'alarm.update', + self._notification(action)) + time.sleep(1) + self.assertEqual(1, self.zaqar.subscriptions) + self.assertEqual(1, self.zaqar.posts) class FakeZaqarClient(object): def __init__(self, testcase): - self.client = testcase + self.testcase = testcase + self.subscriptions = 0 + self.posts = 0 def queue(self, queue_name, **kwargs): - self.client.assertEqual('foobar-critical', queue_name) - self.client.assertEqual(dict(force_create=True), kwargs) - return FakeZaqarQueue(self.client) + self.testcase.assertEqual('foobar-critical', queue_name) + self.testcase.assertEqual({}, kwargs) + return FakeZaqarQueue(self) def subscription(self, queue_name, **kwargs): - self.client.assertEqual('foobar-critical', queue_name) + self.testcase.assertEqual('foobar-critical', queue_name) subscribers = ['http://example.com/data', 'mailto:foo@example.com'] - self.client.assertIn(kwargs['subscriber'], subscribers) - self.client.assertEqual('7200', kwargs['ttl']) + self.testcase.assertIn(kwargs['subscriber'], subscribers) + self.testcase.assertEqual(7200, kwargs['ttl']) + self.subscriptions += 1 class FakeZaqarQueue(object): - def __init__(self, testcase): - self.queue = testcase + def __init__(self, client): + self.client = client + self.testcase = client.testcase def post(self, message): expected_message = {'body': {'alarm_name': 'testalarm', @@ -436,4 +440,5 @@ class FakeZaqarQueue(object): 'reason': 'what ?', 'severity': 'critical', 'previous': 'OK'}} - self.queue.assertEqual(expected_message, message) + self.testcase.assertEqual(expected_message, message) + self.client.posts += 1