openstack/aodh
Code Issues Proposed changes

Include previous state in alarm notification

Browse Source 
We now provide both the current and previous alarm states
in the notification.

This allows differential logic to be applied by the webhook
implementation, for example depending on whether the previous
state was known or insufficient data.

It also allows the initial and subsequent notifications to be
distinguished for repeat_actions alarms, without resorting to
fragile string comparisons between 'Transition to ...' and
'Remaining as ...'.

Change-Id: I61294e98ddf504b3ab22e9b16ab718d64c27486f
This commit is contained in:
Eoghan Glynn 2013-08-08 20:29:01 +00:00
parent 1e8b2b9b3a
commit 78def056db
10 changed files with 69 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
ceilometer/alarm/notifier/__init__.py
View File

@ -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.
"""

6
ceilometer/alarm/notifier/log.py
View File

@ -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)

9
ceilometer/alarm/notifier/rest.py
View File

@ -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':

4
ceilometer/alarm/notifier/test.py
View File

@ -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))

5
ceilometer/alarm/rpc.py
View File

@ -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)

10
ceilometer/alarm/service.py
View File

@ -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'))

5
ceilometer/alarm/threshold_evaluation.py
View File

@ -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)

88
tests/alarm/test_notifier.py
View File

@ -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

9
tests/alarm/test_rpc.py
View File

@ -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)

12
tests/alarm/test_threshold_evaluation.py
View File

@ -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)