From 7fa5f826d1ff79f30589a0ba38f37a4d9aa081d8 Mon Sep 17 00:00:00 2001 From: Nejc Saje Date: Thu, 30 Jan 2014 09:09:37 +0100 Subject: [PATCH] Adds additional details to alarm notifications Previously, the alarm notification contained only a stringified reason. This change adds a JSON formatted reason why the alarm changed its state to the notification. New notification: { "alarm_id": "0ca2845e-c142-4d4b-a346-e495553628ce", "current": "alarm", "previous": "ok", "reason": "Transition to alarm due to 1 samples outside threshold, most recent: 99.4" "reason_data": { "type": "threshold", "disposition": "outside", "count": 1, "most_recent": 99.4 } } It also improves the reporting of OR-combination alarms. Previously the reason said something like "At least one alarm in xx,yy,zz in state alarm". Now only the offending alarms are listed. Change-Id: I258c7bfc0c093c07e518418fea1bb1f044fe98eb Implements: blueprint alarm-notification-details --- ceilometer/alarm/evaluator/__init__.py | 4 +- ceilometer/alarm/evaluator/combination.py | 72 ++++++----- ceilometer/alarm/evaluator/threshold.py | 36 ++++-- ceilometer/alarm/notifier/__init__.py | 3 +- ceilometer/alarm/notifier/log.py | 2 +- ceilometer/alarm/notifier/rest.py | 5 +- ceilometer/alarm/notifier/test.py | 5 +- ceilometer/alarm/rpc.py | 5 +- ceilometer/alarm/service.py | 10 +- ceilometer/tests/alarm/evaluator/test_base.py | 5 +- .../tests/alarm/evaluator/test_combination.py | 122 +++++++++++------- .../tests/alarm/evaluator/test_threshold.py | 69 +++++++--- ceilometer/tests/alarm/test_notifier.py | 8 +- ceilometer/tests/alarm/test_rpc.py | 10 +- 14 files changed, 222 insertions(+), 134 deletions(-) diff --git a/ceilometer/alarm/evaluator/__init__.py b/ceilometer/alarm/evaluator/__init__.py index 3eb2ede44..856b16fb3 100644 --- a/ceilometer/alarm/evaluator/__init__.py +++ b/ceilometer/alarm/evaluator/__init__.py @@ -58,7 +58,7 @@ class Evaluator(object): self.api_client = ceiloclient.get_client(2, **creds) return self.api_client - def _refresh(self, alarm, state, reason): + def _refresh(self, alarm, state, reason, reason_data): """Refresh alarm state.""" try: previous = alarm.state @@ -71,7 +71,7 @@ class Evaluator(object): self._client.alarms.set_state(alarm.alarm_id, state=state) alarm.state = state if self.notifier: - self.notifier.notify(alarm, previous, reason) + self.notifier.notify(alarm, previous, reason, reason_data) except Exception: # retry will occur naturally on the next evaluation # cycle (unless alarm state reverts in the meantime) diff --git a/ceilometer/alarm/evaluator/combination.py b/ceilometer/alarm/evaluator/combination.py index 904e4e47e..7f0a18881 100644 --- a/ceilometer/alarm/evaluator/combination.py +++ b/ceilometer/alarm/evaluator/combination.py @@ -17,6 +17,8 @@ # under the License. +import itertools + from ceilometer.alarm import evaluator from ceilometer.openstack.common.gettextutils import _ # noqa from ceilometer.openstack.common import log @@ -40,63 +42,63 @@ class CombinationEvaluator(evaluator.Evaluator): """Ensure there is sufficient data for evaluation, transitioning to unknown otherwise. """ - missing_states = len(alarm.rule['alarm_ids']) - len(states) - sufficient = missing_states == 0 + #note(sileht): alarm can be evaluated only with + #stable state of other alarm + alarms_missing_states = [alarm_id for alarm_id, state in states + if not state or state == evaluator.UNKNOWN] + sufficient = len(alarms_missing_states) == 0 if not sufficient and alarm.state != evaluator.UNKNOWN: - reason = _('%(missing_states)d alarms in %(alarm_ids)s' + reason = _('Alarms %(alarm_ids)s' ' are in unknown state') % \ - {'missing_states': missing_states, - 'alarm_ids': ",".join(alarm.rule['alarm_ids'])} - self._refresh(alarm, evaluator.UNKNOWN, reason) + {'alarm_ids': ",".join(alarms_missing_states)} + reason_data = self._reason_data(alarms_missing_states) + self._refresh(alarm, evaluator.UNKNOWN, reason, reason_data) return sufficient @staticmethod - def _reason(alarm, state): + def _reason_data(alarm_ids): + """Create a reason data dictionary for this evaluator type. + """ + return {'type': 'combination', 'alarm_ids': alarm_ids} + + @classmethod + def _reason(cls, alarm, state, underlying_states): """Fabricate reason string.""" transition = alarm.state != state - if alarm.rule['operator'] == 'or': - if transition: - return (_('Transition to %(state)s due at least to one alarm' - ' in %(alarm_ids)s in state %(state)s') % - {'state': state, - 'alarm_ids': ",".join(alarm.rule['alarm_ids'])}) - return (_('Remaining as %(state)s due at least to one alarm in' + + alarms_to_report = [alarm_id for alarm_id, alarm_state + in underlying_states + if alarm_state == state] + reason_data = cls._reason_data(alarms_to_report) + if transition: + return (_('Transition to %(state)s due to alarms' ' %(alarm_ids)s in state %(state)s') % {'state': state, - 'alarm_ids': ",".join(alarm.rule['alarm_ids'])}) - elif alarm.rule['operator'] == 'and': - if transition: - return (_('Transition to %(state)s due to all alarms' - ' (%(alarm_ids)s) in state %(state)s') % - {'state': state, - 'alarm_ids': ",".join(alarm.rule['alarm_ids'])}) - return (_('Remaining as %(state)s due to all alarms' - ' (%(alarm_ids)s) in state %(state)s') % - {'state': state, - 'alarm_ids': ",".join(alarm.rule['alarm_ids'])}) + 'alarm_ids': ",".join(alarms_to_report)}), reason_data + return (_('Remaining as %(state)s due to alarms' + ' %(alarm_ids)s in state %(state)s') % + {'state': state, + 'alarm_ids': ",".join(alarms_to_report)}), reason_data def _transition(self, alarm, underlying_states): """Transition alarm state if necessary. """ op = alarm.rule['operator'] - if COMPARATORS[op](s == evaluator.ALARM for s in underlying_states): + if COMPARATORS[op](s == evaluator.ALARM + for __, s in underlying_states): state = evaluator.ALARM else: state = evaluator.OK continuous = alarm.repeat_actions - reason = self._reason(alarm, state) + reason, reason_data = self._reason(alarm, state, underlying_states) if alarm.state != state or continuous: - self._refresh(alarm, state, reason) + self._refresh(alarm, state, reason, reason_data) def evaluate(self, alarm): - states = [] - for _id in alarm.rule['alarm_ids']: - state = self._get_alarm_state(_id) - #note(sileht): alarm can be evaluated only with - #stable state of other alarm - if state and state != evaluator.UNKNOWN: - states.append(state) + states = zip(alarm.rule['alarm_ids'], + itertools.imap(self._get_alarm_state, + alarm.rule['alarm_ids'])) if self._sufficient_states(alarm, states): self._transition(alarm, states) diff --git a/ceilometer/alarm/evaluator/threshold.py b/ceilometer/alarm/evaluator/threshold.py index 0e13f9d89..978dd9fbb 100644 --- a/ceilometer/alarm/evaluator/threshold.py +++ b/ceilometer/alarm/evaluator/threshold.py @@ -111,25 +111,35 @@ class ThresholdEvaluator(evaluator.Evaluator): if not sufficient and alarm.state != evaluator.UNKNOWN: reason = _('%d datapoints are unknown') % alarm.rule[ 'evaluation_periods'] - self._refresh(alarm, evaluator.UNKNOWN, reason) + reason_data = self._reason_data('unknown', + alarm.rule['evaluation_periods'], + None) + self._refresh(alarm, evaluator.UNKNOWN, reason, reason_data) return sufficient @staticmethod - def _reason(alarm, statistics, distilled, state): + def _reason_data(disposition, count, most_recent): + """Create a reason data dictionary for this evaluator type. + """ + return {'type': 'threshold', 'disposition': disposition, + 'count': count, 'most_recent': most_recent} + + @classmethod + def _reason(cls, alarm, statistics, distilled, state): """Fabricate reason string.""" count = len(statistics) disposition = 'inside' if state == evaluator.OK else 'outside' last = getattr(statistics[-1], alarm.rule['statistic']) transition = alarm.state != state + reason_data = cls._reason_data(disposition, count, last) if transition: return (_('Transition to %(state)s due to %(count)d samples' - ' %(disposition)s threshold, most recent: %(last)s') % - {'state': state, 'count': count, - 'disposition': disposition, 'last': last}) + ' %(disposition)s threshold, most recent:' + ' %(most_recent)s') + % dict(reason_data, state=state)), reason_data return (_('Remaining as %(state)s due to %(count)d samples' - ' %(disposition)s threshold, most recent: %(last)s') % - {'state': state, 'count': count, - 'disposition': disposition, 'last': last}) + ' %(disposition)s threshold, most recent: %(most_recent)s') + % dict(reason_data, state=state)), reason_data def _transition(self, alarm, statistics, compared): """Transition alarm state if necessary. @@ -151,14 +161,16 @@ class ThresholdEvaluator(evaluator.Evaluator): if unequivocal: state = evaluator.ALARM if distilled else evaluator.OK - reason = self._reason(alarm, statistics, distilled, state) + reason, reason_data = self._reason(alarm, statistics, + distilled, state) if alarm.state != state or continuous: - self._refresh(alarm, state, reason) + self._refresh(alarm, state, reason, reason_data) elif unknown or continuous: trending_state = evaluator.ALARM if compared[-1] else evaluator.OK state = trending_state if unknown else alarm.state - reason = self._reason(alarm, statistics, distilled, state) - self._refresh(alarm, state, reason) + reason, reason_data = self._reason(alarm, statistics, + distilled, state) + self._refresh(alarm, state, reason, reason_data) def evaluate(self, alarm): query = self._bound_duration( diff --git a/ceilometer/alarm/notifier/__init__.py b/ceilometer/alarm/notifier/__init__.py index ce155820e..3d252d9e5 100644 --- a/ceilometer/alarm/notifier/__init__.py +++ b/ceilometer/alarm/notifier/__init__.py @@ -25,7 +25,7 @@ class AlarmNotifier(object): """Base class for alarm notifier plugins.""" @abc.abstractmethod - def notify(self, action, alarm_id, previous, current, reason): + def notify(self, action, alarm_id, previous, current, reason, reason_data): """Notify that an alarm has been triggered. :param action: The action that is being attended, as a parsed URL. @@ -33,4 +33,5 @@ class AlarmNotifier(object): :param previous: The previous state of the alarm. :param current: The current state of the alarm. :param reason: The reason the alarm changed its state. + :param reason_data: A dict representation of the reason. """ diff --git a/ceilometer/alarm/notifier/log.py b/ceilometer/alarm/notifier/log.py index 0f1a2fb10..e3baabeaf 100644 --- a/ceilometer/alarm/notifier/log.py +++ b/ceilometer/alarm/notifier/log.py @@ -28,7 +28,7 @@ class LogAlarmNotifier(notifier.AlarmNotifier): "Log alarm notifier.""" @staticmethod - def notify(action, alarm_id, previous, current, reason): + def notify(action, alarm_id, previous, current, reason, reason_data): LOG.info(_( "Notifying alarm %(alarm_id)s from %(previous)s " "to %(current)s with action %(action)s because " diff --git a/ceilometer/alarm/notifier/rest.py b/ceilometer/alarm/notifier/rest.py index b9e35e009..edda64134 100644 --- a/ceilometer/alarm/notifier/rest.py +++ b/ceilometer/alarm/notifier/rest.py @@ -54,7 +54,7 @@ class RestAlarmNotifier(notifier.AlarmNotifier): """Rest alarm notifier.""" @staticmethod - def notify(action, alarm_id, previous, current, reason): + def notify(action, alarm_id, previous, current, reason, reason_data): LOG.info(_( "Notifying alarm %(alarm_id)s from %(previous)s " "to %(current)s with action %(action)s because " @@ -62,7 +62,8 @@ class RestAlarmNotifier(notifier.AlarmNotifier): 'current': current, 'action': action, 'reason': reason})) body = {'alarm_id': alarm_id, 'previous': previous, - 'current': current, 'reason': reason} + 'current': current, 'reason': reason, + 'reason_data': reason_data} kwargs = {'data': jsonutils.dumps(body)} if action.scheme == 'https': diff --git a/ceilometer/alarm/notifier/test.py b/ceilometer/alarm/notifier/test.py index 8f1daa04d..3b233dcb2 100644 --- a/ceilometer/alarm/notifier/test.py +++ b/ceilometer/alarm/notifier/test.py @@ -26,9 +26,10 @@ class TestAlarmNotifier(notifier.AlarmNotifier): def __init__(self): self.notifications = [] - def notify(self, action, alarm_id, previous, current, reason): + def notify(self, action, alarm_id, previous, current, reason, reason_data): self.notifications.append((action, alarm_id, previous, current, - reason)) + reason, + reason_data)) diff --git a/ceilometer/alarm/rpc.py b/ceilometer/alarm/rpc.py index 47acf2805..dd66a4460 100644 --- a/ceilometer/alarm/rpc.py +++ b/ceilometer/alarm/rpc.py @@ -46,7 +46,7 @@ class RPCAlarmNotifier(rpc_proxy.RpcProxy): default_version='1.0', topic=cfg.CONF.alarm.notifier_rpc_topic) - def notify(self, alarm, previous, reason): + def notify(self, alarm, previous, reason, reason_data): actions = getattr(alarm, models.Alarm.ALARM_ACTIONS_MAP[alarm.state]) if not actions: LOG.debug(_('alarm %(alarm_id)s has no action configured ' @@ -61,7 +61,8 @@ class RPCAlarmNotifier(rpc_proxy.RpcProxy): 'alarm_id': alarm.alarm_id, 'previous': previous, 'current': alarm.state, - 'reason': unicode(reason)}) + 'reason': unicode(reason), + 'reason_data': reason_data}) self.cast(context.get_admin_context(), msg) diff --git a/ceilometer/alarm/service.py b/ceilometer/alarm/service.py index 7c60bdf87..38fb493ab 100644 --- a/ceilometer/alarm/service.py +++ b/ceilometer/alarm/service.py @@ -225,7 +225,8 @@ class AlarmNotifierService(rpc_service.Service): 'ceilometer.alarm.' + cfg.CONF.alarm.notifier_rpc_topic, ) - def _handle_action(self, action, alarm_id, previous, current, reason): + def _handle_action(self, action, alarm_id, previous, + current, reason, reason_data): try: action = network_utils.urlsplit(action) except Exception: @@ -247,7 +248,8 @@ class AlarmNotifierService(rpc_service.Service): try: LOG.debug(_("Notifying alarm %(id)s with action %(act)s") % ( {'id': alarm_id, 'act': action})) - notifier.notify(action, alarm_id, previous, current, reason) + notifier.notify(action, alarm_id, previous, + current, reason, reason_data) except Exception: LOG.exception(_("Unable to notify alarm %s"), alarm_id) return @@ -262,6 +264,7 @@ class AlarmNotifierService(rpc_service.Service): - previous, the previous state of the alarm - current, the new state the alarm has transitioned to - reason, the reason the alarm changed its state + - reason_data, a dict representation of the reason :param context: Request context. :param data: A dict as described above. @@ -276,7 +279,8 @@ class AlarmNotifierService(rpc_service.Service): data.get('alarm_id'), data.get('previous'), data.get('current'), - data.get('reason')) + data.get('reason'), + data.get('reason_data')) def alarm_notifier(): diff --git a/ceilometer/tests/alarm/evaluator/test_base.py b/ceilometer/tests/alarm/evaluator/test_base.py index b06fd55a2..925eb7f8b 100644 --- a/ceilometer/tests/alarm/evaluator/test_base.py +++ b/ceilometer/tests/alarm/evaluator/test_base.py @@ -28,7 +28,7 @@ class TestEvaluatorBaseClass(test.BaseTestCase): super(TestEvaluatorBaseClass, self).setUp() self.called = False - def _notify(self, alarm, previous, reason): + def _notify(self, alarm, previous, reason, details): self.called = True raise Exception('Boom!') @@ -42,5 +42,6 @@ class TestEvaluatorBaseClass(test.BaseTestCase): ev = EvaluatorSub(notifier) ev.api_client = mock.MagicMock() - ev._refresh(mock.MagicMock(), mock.MagicMock(), mock.MagicMock()) + ev._refresh(mock.MagicMock(), mock.MagicMock(), + mock.MagicMock(), mock.MagicMock()) self.assertTrue(self.called) diff --git a/ceilometer/tests/alarm/evaluator/test_combination.py b/ceilometer/tests/alarm/evaluator/test_combination.py index d1492f39f..8565ec172 100644 --- a/ceilometer/tests/alarm/evaluator/test_combination.py +++ b/ceilometer/tests/alarm/evaluator/test_combination.py @@ -78,25 +78,27 @@ class TestEvaluate(base.TestEvaluatorBase): def _get_alarm(state): return alarms.Alarm(None, {'state': state}) - def _combination_transition_reason(self, state): - return ['Transition to %(state)s due at least to one alarm in' - ' 9cfc3e51-2ff1-4b1d-ac01-c1bd4c6d0d1e,' - '1d441595-d069-4e05-95ab-8693ba6a8302' - ' in state %(state)s' % {'state': state}, - 'Transition to %(state)s due to all alarms' - ' (b82734f4-9d06-48f3-8a86-fa59a0c99dc8,' - '15a700e5-2fe8-4b3d-8c55-9e92831f6a2b)' - ' in state %(state)s' % {'state': state}] + @staticmethod + def _reason_data(alarm_ids): + return {'type': 'combination', 'alarm_ids': alarm_ids} - def _combination_remaining_reason(self, state): - return ['Remaining as %(state)s due at least to one alarm in' - ' 9cfc3e51-2ff1-4b1d-ac01-c1bd4c6d0d1e,' - '1d441595-d069-4e05-95ab-8693ba6a8302' - ' in state %(state)s' % {'state': state}, - 'Remaining as %(state)s due to all alarms' - ' (b82734f4-9d06-48f3-8a86-fa59a0c99dc8,' - '15a700e5-2fe8-4b3d-8c55-9e92831f6a2b)' - ' in state %(state)s' % {'state': state}] + def _combination_transition_reason(self, state, alarm_ids1, alarm_ids2): + return ([('Transition to %(state)s due to alarms %(alarm_ids)s' + ' in state %(state)s') + % {'state': state, 'alarm_ids': ",".join(alarm_ids1)}, + ('Transition to %(state)s due to alarms %(alarm_ids)s' + ' in state %(state)s') + % {'state': state, 'alarm_ids': ",".join(alarm_ids2)}], + [self._reason_data(alarm_ids1), self._reason_data(alarm_ids2)]) + + def _combination_remaining_reason(self, state, alarm_ids1, alarm_ids2): + return ([('Remaining as %(state)s due to alarms %(alarm_ids)s' + ' in state %(state)s') + % {'state': state, 'alarm_ids': ",".join(alarm_ids1)}, + ('Remaining as %(state)s due to alarms %(alarm_ids)s' + ' in state %(state)s') + % {'state': state, 'alarm_ids': ",".join(alarm_ids2)}], + [self._reason_data(alarm_ids1), self._reason_data(alarm_ids2)]) def test_retry_transient_api_failure(self): with mock.patch('ceilometerclient.client.get_client', @@ -129,11 +131,13 @@ class TestEvaluate(base.TestEvaluatorBase): for alarm in self.alarms] update_calls = self.api_client.alarms.set_state.call_args_list self.assertEqual(update_calls, expected) - expected = [mock.call(alarm, - 'ok', - ('%d alarms in %s are in unknown state' % - (2, ",".join(alarm.rule['alarm_ids'])))) - for alarm in self.alarms] + expected = [mock.call( + alarm, + 'ok', + ('Alarms %s are in unknown state' % + (",".join(alarm.rule['alarm_ids']))), + self._reason_data(alarm.rule['alarm_ids'])) + for alarm in self.alarms] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_to_ok_with_all_ok(self): @@ -151,9 +155,14 @@ class TestEvaluate(base.TestEvaluatorBase): for alarm in self.alarms] update_calls = self.api_client.alarms.set_state.call_args_list self.assertEqual(update_calls, expected) - reasons = self._combination_transition_reason('ok') - expected = [mock.call(alarm, 'insufficient data', reason) - for alarm, reason in zip(self.alarms, reasons)] + reasons, reason_datas = self._combination_transition_reason( + 'ok', + self.alarms[0].rule['alarm_ids'], + self.alarms[1].rule['alarm_ids']) + expected = [mock.call(alarm, 'insufficient data', + reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_to_ok_with_one_alarm(self): @@ -171,9 +180,13 @@ class TestEvaluate(base.TestEvaluatorBase): for alarm in self.alarms] update_calls = self.api_client.alarms.set_state.call_args_list self.assertEqual(update_calls, expected) - reasons = self._combination_transition_reason('ok') - expected = [mock.call(alarm, 'alarm', reason) - for alarm, reason in zip(self.alarms, reasons)] + reasons, reason_datas = self._combination_transition_reason( + 'ok', + self.alarms[0].rule['alarm_ids'], + [self.alarms[1].rule['alarm_ids'][1]]) + expected = [mock.call(alarm, 'alarm', reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_to_alarm_with_all_alarm(self): @@ -191,9 +204,13 @@ class TestEvaluate(base.TestEvaluatorBase): for alarm in self.alarms] update_calls = self.api_client.alarms.set_state.call_args_list self.assertEqual(update_calls, expected) - reasons = self._combination_transition_reason('alarm') - expected = [mock.call(alarm, 'ok', reason) - for alarm, reason in zip(self.alarms, reasons)] + reasons, reason_datas = self._combination_transition_reason( + 'alarm', + self.alarms[0].rule['alarm_ids'], + self.alarms[1].rule['alarm_ids']) + expected = [mock.call(alarm, 'ok', reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_to_alarm_with_one_ok(self): @@ -211,9 +228,13 @@ class TestEvaluate(base.TestEvaluatorBase): for alarm in self.alarms] update_calls = self.api_client.alarms.set_state.call_args_list self.assertEqual(update_calls, expected) - reasons = self._combination_transition_reason('alarm') - expected = [mock.call(alarm, 'ok', reason) - for alarm, reason in zip(self.alarms, reasons)] + reasons, reason_datas = self._combination_transition_reason( + 'alarm', + [self.alarms[0].rule['alarm_ids'][1]], + self.alarms[1].rule['alarm_ids']) + expected = [mock.call(alarm, 'ok', reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_to_unknown(self): @@ -232,16 +253,16 @@ class TestEvaluate(base.TestEvaluatorBase): for alarm in self.alarms] update_calls = self.api_client.alarms.set_state.call_args_list self.assertEqual(update_calls, expected) - reasons = ['1 alarms in' - ' 9cfc3e51-2ff1-4b1d-ac01-c1bd4c6d0d1e,' - '1d441595-d069-4e05-95ab-8693ba6a8302' - ' are in unknown state', - '1 alarms in' - ' b82734f4-9d06-48f3-8a86-fa59a0c99dc8,' - '15a700e5-2fe8-4b3d-8c55-9e92831f6a2b' - ' are in unknown state'] - expected = [mock.call(alarm, 'ok', reason) - for alarm, reason in zip(self.alarms, reasons)] + reasons = ['Alarms %s are in unknown state' + % self.alarms[0].rule['alarm_ids'][0], + 'Alarms %s are in unknown state' + % self.alarms[1].rule['alarm_ids'][0]] + reason_datas = [ + self._reason_data([self.alarms[0].rule['alarm_ids'][0]]), + self._reason_data([self.alarms[1].rule['alarm_ids'][0]])] + expected = [mock.call(alarm, 'ok', reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_no_state_change(self): @@ -274,7 +295,12 @@ class TestEvaluate(base.TestEvaluatorBase): self._evaluate_all_alarms() update_calls = self.api_client.alarms.set_state.call_args_list self.assertEqual(update_calls, []) - reasons = self._combination_remaining_reason('ok') - expected = [mock.call(alarm, 'ok', reason) - for alarm, reason in zip(self.alarms, reasons)] + reasons, reason_datas = self._combination_remaining_reason( + 'ok', + self.alarms[0].rule['alarm_ids'], + self.alarms[1].rule['alarm_ids']) + expected = [mock.call(alarm, 'ok', reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] + self.assertEqual(self.notifier.notify.call_args_list, expected) diff --git a/ceilometer/tests/alarm/evaluator/test_threshold.py b/ceilometer/tests/alarm/evaluator/test_threshold.py index 54fcede2a..6abaa2395 100644 --- a/ceilometer/tests/alarm/evaluator/test_threshold.py +++ b/ceilometer/tests/alarm/evaluator/test_threshold.py @@ -99,6 +99,11 @@ class TestEvaluate(base.TestEvaluatorBase): def _get_stat(attr, value, count=1): return statistics.Statistics(None, {attr: value, 'count': count}) + @staticmethod + def _reason_data(disposition, count, most_recent): + return {'type': 'threshold', 'disposition': disposition, + 'count': count, 'most_recent': most_recent} + def _set_all_rules(self, field, value): for alarm in self.alarms: alarm.rule[field] = value @@ -131,11 +136,15 @@ class TestEvaluate(base.TestEvaluatorBase): for alarm in self.alarms] update_calls = self.api_client.alarms.set_state.call_args_list self.assertEqual(update_calls, expected) - expected = [mock.call(alarm, - 'ok', - ('%d datapoints are unknown' % - alarm.rule['evaluation_periods'])) - for alarm in self.alarms] + expected = [mock.call( + alarm, + 'ok', + ('%d datapoints are unknown' + % alarm.rule['evaluation_periods']), + self._reason_data('unknown', + alarm.rule['evaluation_periods'], + None)) + for alarm in self.alarms] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_simple_alarm_trip(self): @@ -157,8 +166,11 @@ class TestEvaluate(base.TestEvaluatorBase): ' threshold, most recent: %s' % avgs[-1].avg, 'Transition to alarm due to 4 samples outside' ' threshold, most recent: %s' % maxs[-1].max] - expected = [mock.call(alarm, 'ok', reason) - for alarm, reason in zip(self.alarms, reasons)] + reason_datas = [self._reason_data('outside', 5, avgs[-1].avg), + self._reason_data('outside', 4, maxs[-1].max)] + expected = [mock.call(alarm, 'ok', reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_simple_alarm_clear(self): @@ -180,8 +192,11 @@ class TestEvaluate(base.TestEvaluatorBase): ' threshold, most recent: %s' % avgs[-1].avg, 'Transition to ok due to 4 samples inside' ' threshold, most recent: %s' % maxs[-1].max] - expected = [mock.call(alarm, 'alarm', reason) - for alarm, reason in zip(self.alarms, reasons)] + reason_datas = [self._reason_data('inside', 5, avgs[-1].avg), + self._reason_data('inside', 4, maxs[-1].max)] + expected = [mock.call(alarm, 'alarm', reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_equivocal_from_known_state(self): @@ -215,7 +230,8 @@ class TestEvaluate(base.TestEvaluatorBase): []) reason = 'Remaining as ok due to 4 samples inside' \ ' threshold, most recent: 8.0' - expected = [mock.call(self.alarms[1], 'ok', reason)] + reason_datas = self._reason_data('inside', 4, 8.0) + expected = [mock.call(self.alarms[1], 'ok', reason, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_unequivocal_from_known_state_and_repeat_actions(self): @@ -234,7 +250,9 @@ class TestEvaluate(base.TestEvaluatorBase): []) reason = 'Remaining as alarm due to 4 samples outside' \ ' threshold, most recent: 7.0' - expected = [mock.call(self.alarms[1], 'alarm', reason)] + reason_datas = self._reason_data('outside', 4, 7.0) + expected = [mock.call(self.alarms[1], 'alarm', + reason, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_state_change_and_repeat_actions(self): @@ -258,8 +276,11 @@ class TestEvaluate(base.TestEvaluatorBase): ' threshold, most recent: %s' % avgs[-1].avg, 'Transition to alarm due to 4 samples outside' ' threshold, most recent: %s' % maxs[-1].max] - expected = [mock.call(alarm, 'ok', reason) - for alarm, reason in zip(self.alarms, reasons)] + reason_datas = [self._reason_data('outside', 5, avgs[-1].avg), + self._reason_data('outside', 4, maxs[-1].max)] + expected = [mock.call(alarm, 'ok', reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_equivocal_from_unknown(self): @@ -281,8 +302,12 @@ class TestEvaluate(base.TestEvaluatorBase): ' threshold, most recent: %s' % avgs[-1].avg, 'Transition to alarm due to 4 samples outside' ' threshold, most recent: %s' % maxs[-1].max] - expected = [mock.call(alarm, 'insufficient data', reason) - for alarm, reason in zip(self.alarms, reasons)] + reason_datas = [self._reason_data('outside', 5, avgs[-1].avg), + self._reason_data('outside', 4, maxs[-1].max)] + expected = [mock.call(alarm, 'insufficient data', + reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def _do_test_bound_duration(self, start, exclude_outliers=None): @@ -359,8 +384,11 @@ class TestEvaluate(base.TestEvaluatorBase): ' threshold, most recent: %s' % avgs[-2].avg, 'Transition to alarm due to 4 samples outside' ' threshold, most recent: %s' % maxs[-2].max] - expected = [mock.call(alarm, 'ok', reason) - for alarm, reason in zip(self.alarms, reasons)] + reason_datas = [self._reason_data('outside', 5, avgs[-2].avg), + self._reason_data('outside', 4, maxs[-2].max)] + expected = [mock.call(alarm, 'ok', reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_simple_alarm_trip_with_outlier_exclusion(self): @@ -398,8 +426,11 @@ class TestEvaluate(base.TestEvaluatorBase): ' threshold, most recent: %s' % avgs[-2].avg, 'Transition to ok due to 4 samples inside' ' threshold, most recent: %s' % maxs[-2].max] - expected = [mock.call(alarm, 'alarm', reason) - for alarm, reason in zip(self.alarms, reasons)] + reason_datas = [self._reason_data('inside', 5, avgs[-2].avg), + self._reason_data('inside', 4, maxs[-2].max)] + expected = [mock.call(alarm, 'alarm', reason, reason_data) + for alarm, reason, reason_data + in zip(self.alarms, reasons, reason_datas)] self.assertEqual(self.notifier.notify.call_args_list, expected) def test_simple_alarm_clear_with_outlier_exclusion(self): diff --git a/ceilometer/tests/alarm/test_notifier.py b/ceilometer/tests/alarm/test_notifier.py index f7f4653ef..608163a20 100644 --- a/ceilometer/tests/alarm/test_notifier.py +++ b/ceilometer/tests/alarm/test_notifier.py @@ -27,10 +27,12 @@ from ceilometer.openstack.common import test DATA_JSON = ('{"current": "ALARM", "alarm_id": "foobar",' - ' "reason": "what ?", "previous": "OK"}') + ' "reason": "what ?", "reason_data": {"test": "test"},' + ' "previous": "OK"}') NOTIFICATION = dict(alarm_id='foobar', condition=dict(threshold=42), reason='what ?', + reason_data={'test': 'test'}, previous='OK', current='ALARM') @@ -57,6 +59,7 @@ class TestAlarmNotifier(test.BaseTestCase): 'previous': 'OK', 'current': 'ALARM', 'reason': 'Everything is on fire', + 'reason_data': {'fire': 'everywhere'} } self.service.notify_alarm(context.get_admin_context(), data) notifications = self.service.notifiers['test'].obj.notifications @@ -66,7 +69,8 @@ class TestAlarmNotifier(test.BaseTestCase): data['alarm_id'], data['previous'], data['current'], - data['reason'])) + data['reason'], + data['reason_data'])) def test_notify_alarm_no_action(self): self.service.notify_alarm(context.get_admin_context(), {}) diff --git a/ceilometer/tests/alarm/test_rpc.py b/ceilometer/tests/alarm/test_rpc.py index 7d2bdc1cd..63eb34f37 100644 --- a/ceilometer/tests/alarm/test_rpc.py +++ b/ceilometer/tests/alarm/test_rpc.py @@ -80,7 +80,8 @@ class TestRPCAlarmNotifier(test.BaseTestCase): def test_notify_alarm(self): previous = ['alarm', 'ok'] for i, a in enumerate(self.alarms): - self.notifier.notify(a, previous[i], "what? %d" % i) + self.notifier.notify(a, previous[i], "what? %d" % i, + {'fire': '%d' % i}) self.assertEqual(len(self.notified), 2) for i, a in enumerate(self.alarms): actions = getattr(a, models.Alarm.ALARM_ACTIONS_MAP[a.state]) @@ -96,9 +97,12 @@ class TestRPCAlarmNotifier(test.BaseTestCase): self.alarms[i].state) self.assertEqual(self.notified[i][1]["args"]["data"]["reason"], "what? %d" % i) + self.assertEqual( + self.notified[i][1]["args"]["data"]["reason_data"], + {'fire': '%d' % i}) def test_notify_non_string_reason(self): - self.notifier.notify(self.alarms[0], 'ok', 42) + self.notifier.notify(self.alarms[0], 'ok', 42, {}) reason = self.notified[0][1]['args']['data']['reason'] self.assertIsInstance(reason, basestring) @@ -119,7 +123,7 @@ class TestRPCAlarmNotifier(test.BaseTestCase): 'matching_metadata': {'resource_id': 'my_instance'} }) - self.notifier.notify(alarm, 'alarm', "what?") + self.notifier.notify(alarm, 'alarm', "what?", {}) self.assertEqual(len(self.notified), 0)