openstack/aodh
Code Issues Proposed changes

Refactor ZaqarAlarmNotifier and fix tests

Browse Source 
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
This commit is contained in:
Zane Bitter 2016-12-13 19:59:19 -05:00
parent b2e1cc089b
commit b9242bf984
2 changed files with 52 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
aodh/notifier/zaqar.py
View File

@ -79,9 +79,9 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier):
" Keystone service catalog.")) " Keystone service catalog."))
return self._zendpoint return self._zendpoint
def get_zaqar_client(self): def _get_client_conf(self):
conf = self.conf.service_credentials conf = self.conf.service_credentials
params = { return {
'auth_opts': { 'auth_opts': {
'backend': 'keystone', 'backend': 'keystone',
'options': { 'options': {
@ -93,15 +93,17 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier):
} }
} }
} }
def get_zaqar_client(self, conf):
try: try:
from zaqarclient.queues import client as zaqar_client from zaqarclient.queues import client as zaqar_client
return zaqar_client.Client(self._get_endpoint(), return zaqar_client.Client(self._get_endpoint(),
version=2, conf=params) version=2, conf=conf)
except Exception: except Exception:
LOG.error(_LE("Failed to connect to Zaqar service "), LOG.error(_LE("Failed to connect to Zaqar service "),
exc_info=True) 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] queue_name = queue_info.get('queue_name', [''])[0]
if not queue_name: if not queue_name:
return None, None return None, None
@ -111,7 +113,7 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier):
paths = queue_info.get('paths', [''])[0].split(',') paths = queue_info.get('paths', [''])[0].split(',')
methods = queue_info.get('methods', [''])[0].split(',') methods = queue_info.get('methods', [''])[0].split(',')
project_id = queue_info.get('project_id', [''])[0] project_id = queue_info.get('project_id', [''])[0]
params = { conf = {
'auth_opts': { 'auth_opts': {
'backend': 'signed-url', 'backend': 'signed-url',
'options': { 'options': {
@ -123,14 +125,7 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier):
} }
} }
} }
try: return conf, queue_name
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)
def notify(self, action, alarm_id, alarm_name, severity, previous, def notify(self, action, alarm_id, alarm_name, severity, previous,
current, reason, reason_data, headers=None): current, reason, reason_data, headers=None):
@ -154,7 +149,7 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier):
@property @property
def client(self): def client(self):
if self._zclient is None: if self._zclient is None:
self._zclient = self.get_zaqar_client() self._zclient = self.get_zaqar_client(self._get_client_conf())
return self._zclient return self._zclient
def notify_zaqar(self, action, message): 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 # NOTE(flwang): Try to get build a pre-signed client if user has
# provide enough information about that. Otherwise, go to build # provide enough information about that. Otherwise, go to build
# a client with service account and queue name for this alarm. # 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 zaqar_client = self.client
# queue_name is a combination of <alarm-id>-<topic> # queue_name is a combination of <alarm-id>-<topic>
queue_name = "%s-%s" % (message['body']['alarm_id'], queue_name = "%s-%s" % (message['body']['alarm_id'],
@ -184,8 +181,8 @@ class ZaqarAlarmNotifier(notifier.AlarmNotifier):
# post the message to the queue # post the message to the queue
queue.post(message) queue.post(message)
except IndexError: except IndexError:
LOG.error(_LE("Required topic query option missing in action %s") LOG.error(_LE("Required query option missing in action %s"),
% action) action)
except Exception: except Exception:
LOG.error(_LE("Unknown error occurred; Failed to post message to" LOG.error(_LE("Unknown error occurred; Failed to post message to"
" Zaqar queue"), " Zaqar queue"),

59
aodh/tests/unit/test_notifier.py
View File

@ -378,55 +378,59 @@ class TestAlarmNotifier(tests_base.BaseTestCase):
self.assertEqual(DATA_JSON, jsonutils.loads(kwargs['data'])) self.assertEqual(DATA_JSON, jsonutils.loads(kwargs['data']))
def test_zaqar_notifier_action(self): def test_zaqar_notifier_action(self):
action = 'zaqar://?topic=critical&subscriber=http://example.com/data' \ with mock.patch.object(notifier.zaqar.ZaqarAlarmNotifier,
'&subscriber=mailto:foo@example.com&ttl=7200' '_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._msg_notifier.sample({}, 'alarm.update',
self._notification(action)) self._notification(action))
time.sleep(1) time.sleep(1)
get_conf.assert_called()
self.assertEqual(self.zaqar, self.assertEqual(self.zaqar,
self.service.notifiers['zaqar'].obj.client) 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): def test_presigned_zaqar_notifier_action(self):
with mock.patch.object(notifier.zaqar.ZaqarAlarmNotifier, action = ('zaqar://?'
'get_presigned_client') as zaqar_client: 'subscriber=http://example.com/data&ttl=7200'
zaqar_client.return_value = self.zaqar, 'foobar-critical' '&signature=mysignature&expires=2016-06-29T01:49:56'
action = 'zaqar://?topic=critical&' \ '&paths=/v2/queues/beijing/messages'
'subscriber=http://example.com/data' \ '&methods=GET,PATCH,POST,PUT&queue_name=foobar-critical'
'&subscriber=mailto:foo@example.com&ttl=7200' \ '&project_id=my_project_id')
'&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._msg_notifier.sample({}, 'alarm.update',
self._notification(action)) self._notification(action))
time.sleep(1) time.sleep(1)
self.assertEqual(zaqar_client.return_value[0], self.assertEqual(1, self.zaqar.subscriptions)
self.service.notifiers['zaqar'].obj.client) self.assertEqual(1, self.zaqar.posts)
queue_info = urlparse.parse_qs(urlparse.urlparse(action).query)
zaqar_client.assert_called_with(queue_info)
class FakeZaqarClient(object): class FakeZaqarClient(object):
def __init__(self, testcase): def __init__(self, testcase):
self.client = testcase self.testcase = testcase
self.subscriptions = 0
self.posts = 0
def queue(self, queue_name, **kwargs): def queue(self, queue_name, **kwargs):
self.client.assertEqual('foobar-critical', queue_name) self.testcase.assertEqual('foobar-critical', queue_name)
self.client.assertEqual(dict(force_create=True), kwargs) self.testcase.assertEqual({}, kwargs)
return FakeZaqarQueue(self.client) return FakeZaqarQueue(self)
def subscription(self, queue_name, **kwargs): 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'] subscribers = ['http://example.com/data', 'mailto:foo@example.com']
self.client.assertIn(kwargs['subscriber'], subscribers) self.testcase.assertIn(kwargs['subscriber'], subscribers)
self.client.assertEqual('7200', kwargs['ttl']) self.testcase.assertEqual(7200, kwargs['ttl'])
self.subscriptions += 1
class FakeZaqarQueue(object): class FakeZaqarQueue(object):
def __init__(self, testcase): def __init__(self, client):
self.queue = testcase self.client = client
self.testcase = client.testcase
def post(self, message): def post(self, message):
expected_message = {'body': {'alarm_name': 'testalarm', expected_message = {'body': {'alarm_name': 'testalarm',
@ -436,4 +440,5 @@ class FakeZaqarQueue(object):
'reason': 'what ?', 'reason': 'what ?',
'severity': 'critical', 'severity': 'critical',
'previous': 'OK'}} 'previous': 'OK'}}
self.queue.assertEqual(expected_message, message) self.testcase.assertEqual(expected_message, message)
self.client.posts += 1