From dfa63a5e4eb0886f09c386a174eae3f2f16e9abb Mon Sep 17 00:00:00 2001 From: Mehdi Abaakouk Date: Sun, 4 Jun 2017 18:30:32 +0200 Subject: [PATCH] Expose alarm state reason to API We currently show no reason from the API point of view of why we don't have enough data to evaluate alarms or why we change the state of the alarm. This change exposes the reason we known to the API. Change-Id: Ic1fe95090339d39ad9638654db815aee41a7921e --- aodh/api/controllers/v2/alarms.py | 14 +++- aodh/evaluator/__init__.py | 1 + aodh/storage/impl_sqlalchemy.py | 1 + aodh/storage/models.py | 7 +- .../6ae0d05d9451_add_reason_column.py | 37 +++++++++ aodh/storage/sqlalchemy/models.py | 1 + .../functional/api/v2/test_alarm_scenarios.py | 77 ++++++++++++++++++- .../api/v2/test_complex_query_scenarios.py | 1 + .../storage/test_storage_scenarios.py | 4 + aodh/tests/unit/evaluator/test_composite.py | 5 ++ aodh/tests/unit/evaluator/test_event.py | 1 + aodh/tests/unit/evaluator/test_gnocchi.py | 3 + aodh/tests/unit/evaluator/test_threshold.py | 3 + ...te-reason-to-the-API-7bc5a9465466db2b.yaml | 4 + 14 files changed, 154 insertions(+), 5 deletions(-) create mode 100644 aodh/storage/sqlalchemy/alembic/versions/6ae0d05d9451_add_reason_column.py create mode 100644 releasenotes/notes/Add-state-reason-to-the-API-7bc5a9465466db2b.yaml diff --git a/aodh/api/controllers/v2/alarms.py b/aodh/api/controllers/v2/alarms.py index 5e81f39b4..823edbf1d 100644 --- a/aodh/api/controllers/v2/alarms.py +++ b/aodh/api/controllers/v2/alarms.py @@ -74,6 +74,9 @@ state_kind_enum = wtypes.Enum(str, *state_kind) severity_kind = ["low", "moderate", "critical"] severity_kind_enum = wtypes.Enum(str, *severity_kind) +ALARM_REASON_DEFAULT = "Not evaluated yet" +ALARM_REASON_MANUAL = "Manually set via API" + class OverQuota(base.ClientSideError): def __init__(self, data): @@ -250,6 +253,9 @@ class Alarm(base.Base): state_timestamp = datetime.datetime "The date of the last alarm state changed" + state_reason = wsme.wsattr(wtypes.text, default=ALARM_REASON_DEFAULT) + "The reason of the current state" + severity = base.AdvEnum('severity', str, *severity_kind, default='low') "The severity of the alarm" @@ -359,6 +365,7 @@ class Alarm(base.Base): timestamp=datetime.datetime(2015, 1, 1, 12, 0, 0, 0), state="ok", severity="moderate", + state_reason="threshold over 90%", state_timestamp=datetime.datetime(2015, 1, 1, 12, 0, 0, 0), ok_actions=["http://site:8000/ok"], alarm_actions=["http://site:8000/alarm"], @@ -620,8 +627,10 @@ class AlarmController(rest.RestController): data.timestamp = now if alarm_in.state != data.state: data.state_timestamp = now + data.state_reason = ALARM_REASON_MANUAL else: data.state_timestamp = alarm_in.state_timestamp + data.state_reason = alarm_in.state_reason ALARMS_RULES[data.type].plugin.update_hook(data) @@ -699,8 +708,10 @@ class AlarmController(rest.RestController): now = timeutils.utcnow() alarm.state = state alarm.state_timestamp = now + alarm.state_reason = ALARM_REASON_MANUAL alarm = pecan.request.storage.update_alarm(alarm) - change = {'state': alarm.state} + change = {'state': alarm.state, + 'state_reason': alarm.state_reason} self._record_change(change, now, on_behalf_of=alarm.project_id, type=models.AlarmChange.STATE_TRANSITION) return alarm.state @@ -785,6 +796,7 @@ class AlarmsController(rest.RestController): data.timestamp = now data.state_timestamp = now + data.state_reason = ALARM_REASON_DEFAULT ALARMS_RULES[data.type].plugin.create_hook(data) diff --git a/aodh/evaluator/__init__.py b/aodh/evaluator/__init__.py index 661a66b27..f8f7798d1 100644 --- a/aodh/evaluator/__init__.py +++ b/aodh/evaluator/__init__.py @@ -116,6 +116,7 @@ class Evaluator(object): try: previous = alarm.state alarm.state = state + alarm.state_reason = reason if previous != state or always_record: LOG.info('alarm %(id)s transitioning to %(state)s because ' '%(reason)s', {'id': alarm.alarm_id, diff --git a/aodh/storage/impl_sqlalchemy.py b/aodh/storage/impl_sqlalchemy.py index df1141db1..b9c465b68 100644 --- a/aodh/storage/impl_sqlalchemy.py +++ b/aodh/storage/impl_sqlalchemy.py @@ -147,6 +147,7 @@ class Connection(base.Connection): project_id=row.project_id, state=row.state, state_timestamp=row.state_timestamp, + state_reason=row.state_reason, ok_actions=row.ok_actions, alarm_actions=row.alarm_actions, insufficient_data_actions=( diff --git a/aodh/storage/models.py b/aodh/storage/models.py index becce7634..43358054f 100644 --- a/aodh/storage/models.py +++ b/aodh/storage/models.py @@ -51,6 +51,7 @@ class Alarm(base.Model): :param description: User friendly description of the alarm :param enabled: Is the alarm enabled :param state: Alarm state (ok/alarm/insufficient data) + :param state_reason: Alarm state reason :param rule: A rule that defines when the alarm fires :param user_id: the owner/creator of the alarm :param project_id: the project_id of the creator @@ -70,8 +71,9 @@ class Alarm(base.Model): """ def __init__(self, alarm_id, type, enabled, name, description, timestamp, user_id, project_id, state, state_timestamp, - ok_actions, alarm_actions, insufficient_data_actions, - repeat_actions, rule, time_constraints, severity=None): + state_reason, ok_actions, alarm_actions, + insufficient_data_actions, repeat_actions, rule, + time_constraints, severity=None): if not isinstance(timestamp, datetime.datetime): raise TypeError(_("timestamp should be datetime object")) if not isinstance(state_timestamp, datetime.datetime): @@ -88,6 +90,7 @@ class Alarm(base.Model): project_id=project_id, state=state, state_timestamp=state_timestamp, + state_reason=state_reason, ok_actions=ok_actions, alarm_actions=alarm_actions, insufficient_data_actions=insufficient_data_actions, diff --git a/aodh/storage/sqlalchemy/alembic/versions/6ae0d05d9451_add_reason_column.py b/aodh/storage/sqlalchemy/alembic/versions/6ae0d05d9451_add_reason_column.py new file mode 100644 index 000000000..e95873ac1 --- /dev/null +++ b/aodh/storage/sqlalchemy/alembic/versions/6ae0d05d9451_add_reason_column.py @@ -0,0 +1,37 @@ +# -*- encoding: utf-8 -*- +# +# Copyright 2017 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""add_reason_column + +Revision ID: 6ae0d05d9451 +Revises: 367aadf5485f +Create Date: 2017-06-05 16:42:42.379029 + +""" + +# revision identifiers, used by Alembic. +revision = '6ae0d05d9451' +down_revision = '367aadf5485f' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('alarm', sa.Column('state_reason', sa.Text, nullable=True)) diff --git a/aodh/storage/sqlalchemy/models.py b/aodh/storage/sqlalchemy/models.py index 829b1d8a3..f8d5eb2ba 100644 --- a/aodh/storage/sqlalchemy/models.py +++ b/aodh/storage/sqlalchemy/models.py @@ -94,6 +94,7 @@ class Alarm(Base): project_id = Column(String(128)) state = Column(String(255)) + state_reason = Column(Text) state_timestamp = Column(TimestampUTC, default=lambda: timeutils.utcnow()) diff --git a/aodh/tests/functional/api/v2/test_alarm_scenarios.py b/aodh/tests/functional/api/v2/test_alarm_scenarios.py index 39546daf3..5699fb9ce 100644 --- a/aodh/tests/functional/api/v2/test_alarm_scenarios.py +++ b/aodh/tests/functional/api/v2/test_alarm_scenarios.py @@ -38,6 +38,7 @@ def default_alarms(auth_headers): alarm_id='a', description='a', state='insufficient data', + state_reason='Not evaluated', severity='critical', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, @@ -67,6 +68,7 @@ def default_alarms(auth_headers): alarm_id='b', description='b', state='insufficient data', + state_reason='Not evaluated', severity='critical', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, @@ -94,6 +96,7 @@ def default_alarms(auth_headers): alarm_id='c', description='c', state='insufficient data', + state_reason='Not evaluated', severity='moderate', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, @@ -221,6 +224,7 @@ class TestAlarms(TestAlarmsBase): alarm_id='c', description='c', state='ok', + state_reason='Not evaluated', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, ok_actions=[], @@ -298,6 +302,7 @@ class TestAlarms(TestAlarmsBase): alarm_id='c', description='c', state='insufficient data', + state_reason='Not evaluated', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, ok_actions=[], @@ -809,6 +814,7 @@ class TestAlarms(TestAlarmsBase): 'enabled': False, 'name': 'added_alarm', 'state': 'ok', + 'state_reason': 'ignored', 'type': 'threshold', 'severity': 'low', 'ok_actions': ['http://something/ok'], @@ -841,6 +847,8 @@ class TestAlarms(TestAlarmsBase): # to check to IntegerType type conversion json['threshold_rule']['evaluation_periods'] = 3 json['threshold_rule']['period'] = 180 + # to check it's read only + json['state_reason'] = "Not evaluated yet" self._verify_alarm(json, alarms[0], 'added_alarm') def test_post_alarm_outlier_exclusion_set(self): @@ -1289,6 +1297,58 @@ class TestAlarms(TestAlarmsBase): self.assertEqual(['test://', 'log://'], alarms[0].alarm_actions) + def test_exercise_state_reason(self): + body = { + 'name': 'nostate', + 'type': 'threshold', + 'threshold_rule': { + 'meter_name': 'ameter', + 'query': [{'field': 'metadata.field', + 'op': 'eq', + 'value': '5', + 'type': 'string'}], + 'comparison_operator': 'le', + 'statistic': 'count', + 'threshold': 50, + 'evaluation_periods': '3', + 'period': '180', + }, + } + headers = self.auth_headers + headers['X-Roles'] = 'admin' + + self.post_json('/alarms', params=body, status=201, + headers=headers) + alarms = list(self.alarm_conn.get_alarms(name='nostate')) + self.assertEqual(1, len(alarms)) + alarm_id = alarms[0].alarm_id + + alarm = self._get_alarm(alarm_id) + self.assertEqual("insufficient data", alarm['state']) + self.assertEqual("Not evaluated yet", alarm['state_reason']) + + # Ensure state reason is updated + alarm = self._get_alarm('a') + alarm['state'] = 'ok' + self.put_json('/alarms/%s' % alarm_id, + params=alarm, + headers=self.auth_headers) + alarm = self._get_alarm(alarm_id) + self.assertEqual("ok", alarm['state']) + self.assertEqual("Manually set via API", alarm['state_reason']) + + # Ensure state reason read only + alarm = self._get_alarm('a') + alarm['state'] = 'alarm' + alarm['state_reason'] = 'oh no!' + self.put_json('/alarms/%s' % alarm_id, + params=alarm, + headers=self.auth_headers) + + alarm = self._get_alarm(alarm_id) + self.assertEqual("alarm", alarm['state']) + self.assertEqual("Manually set via API", alarm['state_reason']) + def test_post_alarm_without_actions(self): body = { 'name': 'alarm_actions_none', @@ -1641,6 +1701,8 @@ class TestAlarms(TestAlarmsBase): alarms = list(self.alarm_conn.get_alarms(alarm_id=data[0]['alarm_id'])) self.assertEqual(1, len(alarms)) self.assertEqual('alarm', alarms[0].state) + self.assertEqual('Manually set via API', + alarms[0].state_reason) self.assertEqual('alarm', resp.json) def test_set_invalid_state_alarm(self): @@ -1726,6 +1788,7 @@ class TestAlarmsHistory(TestAlarmsBase): alarm_id='a', description='a', state='insufficient data', + state_reason='insufficient data', severity='critical', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, @@ -1764,7 +1827,10 @@ class TestAlarmsHistory(TestAlarmsBase): def _assert_is_subset(self, expected, actual): for k, v in six.iteritems(expected): - self.assertEqual(v, actual.get(k), 'mismatched field: %s' % k) + current = actual.get(k) + if k == 'detail' and isinstance(v, dict): + current = jsonutils.loads(current) + self.assertEqual(v, current, 'mismatched field: %s' % k) self.assertIsNotNone(actual['event_id']) def _assert_in_json(self, expected, actual): @@ -1955,7 +2021,9 @@ class TestAlarmsHistory(TestAlarmsBase): auth_headers=auth) self.assertEqual(2, len(history), 'hist: %s' % history) self._assert_is_subset(dict(alarm_id=alarm['alarm_id'], - detail='{"state": "alarm"}', + detail={"state": "alarm", + "state_reason": + "Manually set via API"}, on_behalf_of=alarm['project_id'], project_id=admin_project, type='rule change', @@ -2405,6 +2473,7 @@ class TestAlarmsRuleGnocchi(TestAlarmsBase): alarm_id='e', description='e', state='insufficient data', + state_reason='Not evaluated', severity='critical', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, @@ -2432,6 +2501,7 @@ class TestAlarmsRuleGnocchi(TestAlarmsBase): alarm_id='f', description='f', state='insufficient data', + state_reason='Not evaluated', severity='critical', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, @@ -2458,6 +2528,7 @@ class TestAlarmsRuleGnocchi(TestAlarmsBase): alarm_id='g', description='f', state='insufficient data', + state_reason='Not evaluated', severity='critical', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, @@ -2671,6 +2742,7 @@ class TestAlarmsEvent(TestAlarmsBase): alarm_id='h', description='h', state='insufficient data', + state_reason='insufficient data', severity='moderate', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, @@ -2796,6 +2868,7 @@ class TestAlarmsCompositeRule(TestAlarmsBase): alarm_id='composite', description='composite', state='insufficient data', + state_reason='insufficient data', severity='moderate', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, diff --git a/aodh/tests/functional/api/v2/test_complex_query_scenarios.py b/aodh/tests/functional/api/v2/test_complex_query_scenarios.py index 38eaa0f5f..0cb8ed980 100644 --- a/aodh/tests/functional/api/v2/test_complex_query_scenarios.py +++ b/aodh/tests/functional/api/v2/test_complex_query_scenarios.py @@ -49,6 +49,7 @@ class TestQueryAlarmsController(tests_api.FunctionalTest): alarm_id=alarm_id, description='a', state=state, + state_reason="state_reason", state_timestamp=date, timestamp=date, ok_actions=[], diff --git a/aodh/tests/functional/storage/test_storage_scenarios.py b/aodh/tests/functional/storage/test_storage_scenarios.py index 6d3776ca5..2679f8bea 100644 --- a/aodh/tests/functional/storage/test_storage_scenarios.py +++ b/aodh/tests/functional/storage/test_storage_scenarios.py @@ -56,6 +56,7 @@ class AlarmTestBase(DBTestBase): user_id='me', project_id='and-da-boys', state="insufficient data", + state_reason="insufficient data", state_timestamp=constants.MIN_DATETIME, ok_actions=[], alarm_actions=['http://nowhere/alarms'], @@ -85,6 +86,7 @@ class AlarmTestBase(DBTestBase): user_id='me', project_id='and-da-boys', state="insufficient data", + state_reason="insufficient data", state_timestamp=constants.MIN_DATETIME, ok_actions=[], alarm_actions=['http://nowhere/alarms'], @@ -112,6 +114,7 @@ class AlarmTestBase(DBTestBase): user_id='me', project_id='and-da-boys', state="insufficient data", + state_reason="insufficient data", state_timestamp=constants.MIN_DATETIME, ok_actions=[], alarm_actions=['http://nowhere/alarms'], @@ -219,6 +222,7 @@ class AlarmTest(AlarmTestBase): user_id='bla', project_id='ffo', state="insufficient data", + state_reason="insufficient data", state_timestamp=constants.MIN_DATETIME, ok_actions=[], alarm_actions=[], diff --git a/aodh/tests/unit/evaluator/test_composite.py b/aodh/tests/unit/evaluator/test_composite.py index dcba2e6b2..ee67dc2b1 100644 --- a/aodh/tests/unit/evaluator/test_composite.py +++ b/aodh/tests/unit/evaluator/test_composite.py @@ -170,6 +170,7 @@ class CompositeTest(BaseCompositeEvaluate): project_id='fake_project', alarm_id=uuidutils.generate_uuid(), state='insufficient data', + state_reason='insufficient data', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, insufficient_data_actions=[], @@ -190,6 +191,7 @@ class CompositeTest(BaseCompositeEvaluate): user_id='fake_user', project_id='fake_project', state='insufficient data', + state_reason='insufficient data', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, insufficient_data_actions=[], @@ -211,6 +213,7 @@ class CompositeTest(BaseCompositeEvaluate): user_id='fake_user', project_id='fake_project', state='insufficient data', + state_reason='insufficient data', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, insufficient_data_actions=[], @@ -233,6 +236,7 @@ class CompositeTest(BaseCompositeEvaluate): project_id='fake_project', alarm_id=uuidutils.generate_uuid(), state='insufficient data', + state_reason='insufficient data', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, insufficient_data_actions=[], @@ -456,6 +460,7 @@ class OtherCompositeTest(BaseCompositeEvaluate): user_id='fake_user', project_id='fake_project', state='insufficient data', + state_reason='insufficient data', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, insufficient_data_actions=['log://'], diff --git a/aodh/tests/unit/evaluator/test_event.py b/aodh/tests/unit/evaluator/test_event.py index ac171aaa5..df51f6a91 100644 --- a/aodh/tests/unit/evaluator/test_event.py +++ b/aodh/tests/unit/evaluator/test_event.py @@ -41,6 +41,7 @@ class TestEventAlarmEvaluate(base.TestEvaluatorBase): alarm_id=alarm_id, description='desc', state=kwargs.get('state', 'insufficient data'), + state_reason='reason', severity='critical', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, diff --git a/aodh/tests/unit/evaluator/test_gnocchi.py b/aodh/tests/unit/evaluator/test_gnocchi.py index 32182b73c..c1d78409a 100644 --- a/aodh/tests/unit/evaluator/test_gnocchi.py +++ b/aodh/tests/unit/evaluator/test_gnocchi.py @@ -45,6 +45,7 @@ class TestGnocchiEvaluatorBase(base.TestEvaluatorBase): project_id='snafu', alarm_id=uuidutils.generate_uuid(), state='insufficient data', + state_reason='insufficient data', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, insufficient_data_actions=[], @@ -69,6 +70,7 @@ class TestGnocchiEvaluatorBase(base.TestEvaluatorBase): user_id='foobar', project_id='snafu', state='insufficient data', + state_reason='insufficient data', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, insufficient_data_actions=[], @@ -94,6 +96,7 @@ class TestGnocchiEvaluatorBase(base.TestEvaluatorBase): project_id='snafu', alarm_id=uuidutils.generate_uuid(), state='insufficient data', + state_reason='insufficient data', state_timestamp=constants.MIN_DATETIME, timestamp=constants.MIN_DATETIME, insufficient_data_actions=[], diff --git a/aodh/tests/unit/evaluator/test_threshold.py b/aodh/tests/unit/evaluator/test_threshold.py index 1d487100c..090dfce43 100644 --- a/aodh/tests/unit/evaluator/test_threshold.py +++ b/aodh/tests/unit/evaluator/test_threshold.py @@ -47,6 +47,7 @@ class TestEvaluate(base.TestEvaluatorBase): alarm_id=uuidutils.generate_uuid(), state='insufficient data', state_timestamp=constants.MIN_DATETIME, + state_reason='Not evaluated', timestamp=constants.MIN_DATETIME, insufficient_data_actions=[], ok_actions=[], @@ -76,6 +77,7 @@ class TestEvaluate(base.TestEvaluatorBase): project_id='snafu', state='insufficient data', state_timestamp=constants.MIN_DATETIME, + state_reason='Not evaluated', timestamp=constants.MIN_DATETIME, insufficient_data_actions=[], ok_actions=[], @@ -419,6 +421,7 @@ class TestEvaluate(base.TestEvaluatorBase): primitive_alarms = [a.as_dict() for a in self.alarms] for alarm in original_alarms: alarm.state = 'alarm' + alarm.state_reason = mock.ANY primitive_original_alarms = [a.as_dict() for a in original_alarms] self.assertEqual(primitive_original_alarms, primitive_alarms) diff --git a/releasenotes/notes/Add-state-reason-to-the-API-7bc5a9465466db2b.yaml b/releasenotes/notes/Add-state-reason-to-the-API-7bc5a9465466db2b.yaml new file mode 100644 index 000000000..bda07b7cf --- /dev/null +++ b/releasenotes/notes/Add-state-reason-to-the-API-7bc5a9465466db2b.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + The reason of the state change is now part of the API as "state_reason" field of the alarm object.