diff --git a/ceilometer/alarm/notifier/__init__.py b/ceilometer/alarm/notifier/__init__.py index 0a33914db..bd8ffd01b 100644 --- a/ceilometer/alarm/notifier/__init__.py +++ b/ceilometer/alarm/notifier/__init__.py @@ -25,11 +25,12 @@ class AlarmNotifier(object): __metaclass__ = abc.ABCMeta @abc.abstractmethod - def notify(self, action, alarm, state, reason): + def notify(self, action, alarm, previous, current, reason): """Notify that an alarm has been triggered. :param action: The action that is being attended, as a parsed URL. :param alarm: The triggered alarm. - :param state: The state the alarm is now in. + :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. """ diff --git a/ceilometer/alarm/notifier/log.py b/ceilometer/alarm/notifier/log.py index 754f308ee..d3bc59831 100644 --- a/ceilometer/alarm/notifier/log.py +++ b/ceilometer/alarm/notifier/log.py @@ -27,6 +27,6 @@ class LogAlarmNotifier(notifier.AlarmNotifier): "Log alarm notifier.""" @staticmethod - def notify(action, alarm, state, reason): - LOG.info("Notifying alarm %s in state %s with action %s because %s", - alarm, state, action, reason) + def notify(action, alarm, previous, current, reason): + LOG.info("Notifying alarm %s from %s to %s with action %s because %s", + alarm, previous, current, action, reason) diff --git a/ceilometer/alarm/notifier/rest.py b/ceilometer/alarm/notifier/rest.py index 23c0b5f12..487d50185 100644 --- a/ceilometer/alarm/notifier/rest.py +++ b/ceilometer/alarm/notifier/rest.py @@ -53,10 +53,11 @@ class RestAlarmNotifier(notifier.AlarmNotifier): """Rest alarm notifier.""" @staticmethod - def notify(action, alarm, state, reason): - LOG.info("Notifying alarm %s in state %s with action %s because %s", - alarm, state, action, reason) - body = {'alarm': alarm, 'state': state, 'reason': reason} + def notify(action, alarm, previous, current, reason): + LOG.info("Notifying alarm %s from %s to %s with action %s because %s", + alarm, previous, current, action, reason) + body = {'alarm': alarm, 'previous': previous, + 'current': current, 'reason': reason} kwargs = {'data': jsonutils.dumps(body)} if action.scheme == 'https': diff --git a/ceilometer/alarm/notifier/test.py b/ceilometer/alarm/notifier/test.py index 6d74a163e..b8a024ce3 100644 --- a/ceilometer/alarm/notifier/test.py +++ b/ceilometer/alarm/notifier/test.py @@ -26,5 +26,5 @@ class TestAlarmNotifier(notifier.AlarmNotifier): def __init__(self): self.notifications = [] - def notify(self, action, alarm, state, reason): - self.notifications.append((action, alarm, state, reason)) + def notify(self, action, alarm, previous, current, reason): + self.notifications.append((action, alarm, previous, current, reason)) diff --git a/ceilometer/alarm/rpc.py b/ceilometer/alarm/rpc.py index 6afb62967..0b3f119c3 100644 --- a/ceilometer/alarm/rpc.py +++ b/ceilometer/alarm/rpc.py @@ -37,11 +37,12 @@ class RPCAlarmNotifier(rpc_proxy.RpcProxy): default_version='1.0', topic=cfg.CONF.alarm.notifier_rpc_topic) - def notify(self, alarm, state, reason): + def notify(self, alarm, previous, reason): actions = getattr(alarm, Alarm.ALARM_ACTIONS_MAP[alarm.state]) msg = self.make_msg('notify_alarm', data={ 'actions': actions, 'alarm': alarm.alarm_id, - 'state': state, + 'previous': previous, + 'current': alarm.state, 'reason': reason}) self.cast(context.get_admin_context(), msg) diff --git a/ceilometer/alarm/service.py b/ceilometer/alarm/service.py index af8fbb5b9..792ddb3bf 100644 --- a/ceilometer/alarm/service.py +++ b/ceilometer/alarm/service.py @@ -128,7 +128,7 @@ class AlarmNotifierService(rpc_service.Service): 'ceilometer.alarm.' + cfg.CONF.alarm.notifier_rpc_topic, ) - def _handle_action(self, action, alarm, state, reason): + def _handle_action(self, action, alarm, previous, current, reason): try: action = network_utils.urlsplit(action) except Exception: @@ -150,7 +150,7 @@ class AlarmNotifierService(rpc_service.Service): try: LOG.debug("Notifying alarm %s with action %s", alarm, action) - notifier.notify(action, alarm, state, reason) + notifier.notify(action, alarm, previous, current, reason) except Exception: LOG.exception(_("Unable to notify alarm %s"), alarm) return @@ -162,7 +162,8 @@ class AlarmNotifierService(rpc_service.Service): - actions, the URL of the action to run; this is a mapped to extensions automatically - alarm, the alarm that has been triggered - - state, the new state the alarm transitionned to + - previous, the previous state of the alarm + - current, the new state the alarm has transitioned to - reason, the reason the alarm changed its state :param context: Request context. @@ -176,7 +177,8 @@ class AlarmNotifierService(rpc_service.Service): for action in actions: self._handle_action(action, data.get('alarm'), - data.get('state'), + data.get('previous'), + data.get('current'), data.get('reason')) diff --git a/ceilometer/alarm/threshold_evaluation.py b/ceilometer/alarm/threshold_evaluation.py index 4e87915fb..72d4a7713 100644 --- a/ceilometer/alarm/threshold_evaluation.py +++ b/ceilometer/alarm/threshold_evaluation.py @@ -127,7 +127,8 @@ class Evaluator(object): def _refresh(self, alarm, state, reason): """Refresh alarm state.""" try: - if alarm.state != state: + previous = alarm.state + if previous != state: LOG.info(_('alarm %(id)s transitioning to %(state)s because ' '%(reason)s') % {'id': alarm.alarm_id, 'state': state, @@ -136,7 +137,7 @@ class Evaluator(object): self._client.alarms.update(alarm.alarm_id, **dict(state=state)) alarm.state = state if self.notifier: - self.notifier.notify(alarm, state, reason) + self.notifier.notify(alarm, previous, reason) except Exception: # retry will occur naturally on the next evaluation # cycle (unless alarm state reverts in the meantime) diff --git a/tests/alarm/test_notifier.py b/tests/alarm/test_notifier.py index 468c1b2c5..911b9986d 100644 --- a/tests/alarm/test_notifier.py +++ b/tests/alarm/test_notifier.py @@ -26,6 +26,15 @@ from ceilometer.openstack.common import context from ceilometer.tests import base +DATA_JSON = ('{"current": "ALARM", "alarm": "foobar",' + ' "reason": "what ?", "previous": "OK"}') +NOTIFICATION = dict(alarm='foobar', + condition=dict(threshold=42), + reason='what ?', + previous='OK', + current='ALARM') + + class TestAlarmNotifier(base.TestCase): def setUp(self): @@ -44,7 +53,8 @@ class TestAlarmNotifier(base.TestCase): data = { 'actions': ['test://'], 'alarm': 'foobar', - 'state': 'ALARM', + 'previous': 'OK', + 'current': 'ALARM', 'reason': 'Everything is on fire', } self.service.notify_alarm(context.get_admin_context(), data) @@ -53,7 +63,8 @@ class TestAlarmNotifier(base.TestCase): self.assertEqual(notifications[0], ( urlparse.urlsplit(data['actions'][0]), data['alarm'], - data['state'], + data['previous'], + data['current'], data['reason'])) def test_notify_alarm_no_action(self): @@ -71,26 +82,25 @@ class TestAlarmNotifier(base.TestCase): def _fake_spawn_n(func, *args, **kwargs): func(*args, **kwargs) + @staticmethod + def _notification(action): + notification = {} + notification.update(NOTIFICATION) + notification['actions'] = [action] + return notification + def test_notify_alarm_rest_action_ok(self): action = 'http://host/action' - data_json = '{"reason": "what ?", "alarm": "foobar", "state": "ALARM"}' with mock.patch('eventlet.spawn_n', self._fake_spawn_n): with mock.patch.object(requests, 'post') as poster: self.service.notify_alarm(context.get_admin_context(), - { - 'actions': [action], - 'alarm': 'foobar', - 'condition': {'threshold': 42}, - 'reason': 'what ?', - 'state': 'ALARM', - }) - poster.assert_called_with(action, data=data_json) + self._notification(action)) + poster.assert_called_with(action, data=DATA_JSON) def test_notify_alarm_rest_action_with_ssl_client_cert(self): action = 'https://host/action' certificate = "/etc/ssl/cert/whatever.pem" - data_json = '{"reason": "what ?", "alarm": "foobar", "state": "ALARM"}' cfg.CONF.set_override("rest_notifier_certificate_file", certificate, group='alarm') @@ -98,21 +108,14 @@ class TestAlarmNotifier(base.TestCase): with mock.patch('eventlet.spawn_n', self._fake_spawn_n): with mock.patch.object(requests, 'post') as poster: self.service.notify_alarm(context.get_admin_context(), - { - 'actions': [action], - 'alarm': 'foobar', - 'condition': {'threshold': 42}, - 'reason': 'what ?', - 'state': 'ALARM', - }) - poster.assert_called_with(action, data=data_json, + self._notification(action)) + poster.assert_called_with(action, data=DATA_JSON, cert=certificate, verify=True) def test_notify_alarm_rest_action_with_ssl_client_cert_and_key(self): action = 'https://host/action' certificate = "/etc/ssl/cert/whatever.pem" key = "/etc/ssl/cert/whatever.key" - data_json = '{"reason": "what ?", "alarm": "foobar", "state": "ALARM"}' cfg.CONF.set_override("rest_notifier_certificate_file", certificate, group='alarm') @@ -122,19 +125,12 @@ class TestAlarmNotifier(base.TestCase): with mock.patch('eventlet.spawn_n', self._fake_spawn_n): with mock.patch.object(requests, 'post') as poster: self.service.notify_alarm(context.get_admin_context(), - { - 'actions': [action], - 'alarm': 'foobar', - 'condition': {'threshold': 42}, - 'reason': 'what ?', - 'state': 'ALARM', - }) - poster.assert_called_with(action, data=data_json, + self._notification(action)) + poster.assert_called_with(action, data=DATA_JSON, cert=(certificate, key), verify=True) def test_notify_alarm_rest_action_with_ssl_verify_disable_by_cfg(self): action = 'https://host/action' - data_json = '{"reason": "what ?", "alarm": "foobar", "state": "ALARM"}' cfg.CONF.set_override("rest_notifier_ssl_verify", False, group='alarm') @@ -142,36 +138,22 @@ class TestAlarmNotifier(base.TestCase): with mock.patch('eventlet.spawn_n', self._fake_spawn_n): with mock.patch.object(requests, 'post') as poster: self.service.notify_alarm(context.get_admin_context(), - { - 'actions': [action], - 'alarm': 'foobar', - 'condition': {'threshold': 42}, - 'reason': 'what ?', - 'state': 'ALARM', - }) - poster.assert_called_with(action, data=data_json, + self._notification(action)) + poster.assert_called_with(action, data=DATA_JSON, verify=False) def test_notify_alarm_rest_action_with_ssl_verify_disable(self): action = 'https://host/action?ceilometer-alarm-ssl-verify=0' - data_json = '{"reason": "what ?", "alarm": "foobar", "state": "ALARM"}' with mock.patch('eventlet.spawn_n', self._fake_spawn_n): with mock.patch.object(requests, 'post') as poster: self.service.notify_alarm(context.get_admin_context(), - { - 'actions': [action], - 'alarm': 'foobar', - 'condition': {'threshold': 42}, - 'reason': 'what ?', - 'state': 'ALARM', - }) - poster.assert_called_with(action, data=data_json, + self._notification(action)) + poster.assert_called_with(action, data=DATA_JSON, verify=False) def test_notify_alarm_rest_action_with_ssl_verify_enable_by_user(self): action = 'https://host/action?ceilometer-alarm-ssl-verify=1' - data_json = '{"reason": "what ?", "alarm": "foobar", "state": "ALARM"}' cfg.CONF.set_override("rest_notifier_ssl_verify", False, group='alarm') @@ -179,14 +161,8 @@ class TestAlarmNotifier(base.TestCase): with mock.patch('eventlet.spawn_n', self._fake_spawn_n): with mock.patch.object(requests, 'post') as poster: self.service.notify_alarm(context.get_admin_context(), - { - 'actions': [action], - 'alarm': 'foobar', - 'condition': {'threshold': 42}, - 'reason': 'what ?', - 'state': 'ALARM', - }) - poster.assert_called_with(action, data=data_json, + self._notification(action)) + poster.assert_called_with(action, data=DATA_JSON, verify=True) @staticmethod diff --git a/tests/alarm/test_rpc.py b/tests/alarm/test_rpc.py index f96ce5807..28dcffa75 100644 --- a/tests/alarm/test_rpc.py +++ b/tests/alarm/test_rpc.py @@ -73,8 +73,9 @@ class TestRPCAlarmNotifier(base.TestCase): ] def test_notify_alarm(self): + previous = ['alarm', 'ok'] for i, a in enumerate(self.alarms): - self.notifier.notify(a, "ok", "what? %d" % i) + self.notifier.notify(a, previous[i], "what? %d" % i) self.assertEqual(len(self.notified), 2) for i, a in enumerate(self.alarms): actions = getattr(a, AlarmModel.ALARM_ACTIONS_MAP[a.state]) @@ -84,7 +85,9 @@ class TestRPCAlarmNotifier(base.TestCase): self.alarms[i].alarm_id) self.assertEqual(self.notified[i][1]["args"]["data"]["actions"], actions) - self.assertEqual(self.notified[i][1]["args"]["data"]["state"], - "ok") + self.assertEqual(self.notified[i][1]["args"]["data"]["previous"], + previous[i]) + self.assertEqual(self.notified[i][1]["args"]["data"]["current"], + self.alarms[i].state) self.assertEqual(self.notified[i][1]["args"]["data"]["reason"], "what? %d" % i) diff --git a/tests/alarm/test_threshold_evaluation.py b/tests/alarm/test_threshold_evaluation.py index 3f70c3364..369661d22 100644 --- a/tests/alarm/test_threshold_evaluation.py +++ b/tests/alarm/test_threshold_evaluation.py @@ -102,7 +102,7 @@ class TestEvaluate(base.TestCase): update_calls = self.api_client.alarms.update.call_args_list self.assertEqual(update_calls, expected) expected = [mock.call(alarm, - 'insufficient data', + 'ok', ('%d datapoints are unknown' % alarm.evaluation_periods)) for alarm in self.alarms] @@ -123,7 +123,7 @@ class TestEvaluate(base.TestCase): ) self.notifier.notify.assert_called_once_with( self.alarms[0], - 'insufficient data', + 'ok', mock.ANY ) @@ -146,7 +146,7 @@ class TestEvaluate(base.TestCase): ' threshold, most recent: 85.0', 'Transition to alarm due to 4 samples outside' ' threshold, most recent: 7.0'] - expected = [mock.call(alarm, 'alarm', reason) + expected = [mock.call(alarm, 'ok', reason) for alarm, reason in zip(self.alarms, reasons)] self.assertEqual(self.notifier.notify.call_args_list, expected) @@ -169,7 +169,7 @@ class TestEvaluate(base.TestCase): ' threshold, most recent: 76.0', 'Transition to ok due to 4 samples inside' ' threshold, most recent: 14.0'] - expected = [mock.call(alarm, 'ok', reason) + expected = [mock.call(alarm, 'alarm', reason) for alarm, reason in zip(self.alarms, reasons)] self.assertEqual(self.notifier.notify.call_args_list, expected) @@ -247,7 +247,7 @@ class TestEvaluate(base.TestCase): ' threshold, most recent: 85.0', 'Transition to alarm due to 4 samples outside' ' threshold, most recent: 7.0'] - expected = [mock.call(alarm, 'alarm', reason) + expected = [mock.call(alarm, 'ok', reason) for alarm, reason in zip(self.alarms, reasons)] self.assertEqual(self.notifier.notify.call_args_list, expected) @@ -270,6 +270,6 @@ class TestEvaluate(base.TestCase): ' threshold, most recent: 85.0', 'Transition to alarm due to 4 samples outside' ' threshold, most recent: 7.0'] - expected = [mock.call(alarm, 'alarm', reason) + expected = [mock.call(alarm, 'insufficient data', reason) for alarm, reason in zip(self.alarms, reasons)] self.assertEqual(self.notifier.notify.call_args_list, expected)