diff --git a/aodh/api/controllers/v2/alarms.py b/aodh/api/controllers/v2/alarms.py index a2cd01ed..1fc41569 100644 --- a/aodh/api/controllers/v2/alarms.py +++ b/aodh/api/controllers/v2/alarms.py @@ -615,10 +615,6 @@ class AlarmController(rest.RestController): self.conn.delete_alarm(alarm.alarm_id) alarm_object = Alarm.from_db_model(alarm) alarm_object.delete_actions() - change = alarm_object.as_dict(models.Alarm) - self._record_change(change, - timeutils.utcnow(), - type=models.AlarmChange.DELETION) @wsme_pecan.wsexpose([AlarmChange], [base.Query]) def history(self, q=None): diff --git a/aodh/storage/base.py b/aodh/storage/base.py index 3d652a50..477d4f3a 100644 --- a/aodh/storage/base.py +++ b/aodh/storage/base.py @@ -105,7 +105,7 @@ class Connection(object): @staticmethod def delete_alarm(alarm_id): - """Delete an alarm.""" + """Delete an alarm and its history data.""" raise aodh.NotImplementedError('Alarms not implemented') @staticmethod diff --git a/aodh/storage/impl_hbase.py b/aodh/storage/impl_hbase.py index 4ef1544f..af444149 100644 --- a/aodh/storage/impl_hbase.py +++ b/aodh/storage/impl_hbase.py @@ -112,9 +112,14 @@ class Connection(hbase_base.Connection, base.Connection): create_alarm = update_alarm def delete_alarm(self, alarm_id): + """Delete an alarm and its history data.""" with self.conn_pool.connection() as conn: alarm_table = conn.table(self.ALARM_TABLE) alarm_table.delete(alarm_id) + q = hbase_utils.make_query(alarm_id=alarm_id) + alarm_history_table = conn.table(self.ALARM_HISTORY_TABLE) + for alarm_id, ignored in alarm_history_table.scan(filter=q): + alarm_history_table.delete(alarm_id) def get_alarms(self, name=None, user=None, state=None, meter=None, project=None, enabled=None, alarm_id=None, diff --git a/aodh/storage/impl_log.py b/aodh/storage/impl_log.py index 3f697f70..1f1ed431 100644 --- a/aodh/storage/impl_log.py +++ b/aodh/storage/impl_log.py @@ -53,7 +53,7 @@ class Connection(base.Connection): @staticmethod def delete_alarm(alarm_id): - """Delete an alarm.""" + """Delete an alarm and its history data.""" @staticmethod def clear_expired_alarm_history_data(alarm_history_ttl): diff --git a/aodh/storage/impl_sqlalchemy.py b/aodh/storage/impl_sqlalchemy.py index d045e5a8..f73f195f 100644 --- a/aodh/storage/impl_sqlalchemy.py +++ b/aodh/storage/impl_sqlalchemy.py @@ -185,7 +185,7 @@ class Connection(base.Connection): return self._row_to_alarm_model(alarm_row) def delete_alarm(self, alarm_id): - """Delete an alarm + """Delete an alarm and its history data. :param alarm_id: ID of the alarm to delete """ @@ -193,6 +193,9 @@ class Connection(base.Connection): with session.begin(): session.query(models.Alarm).filter( models.Alarm.alarm_id == alarm_id).delete() + # FIXME(liusheng): we should use delete cascade + session.query(models.AlarmChange).filter( + models.AlarmChange.alarm_id == alarm_id).delete() @staticmethod def _row_to_alarm_change_model(row): diff --git a/aodh/storage/models.py b/aodh/storage/models.py index 567b4149..dba99b58 100644 --- a/aodh/storage/models.py +++ b/aodh/storage/models.py @@ -109,7 +109,6 @@ class AlarmChange(base.Model): CREATION = 'creation' RULE_CHANGE = 'rule change' STATE_TRANSITION = 'state transition' - DELETION = 'deletion' def __init__(self, event_id, diff --git a/aodh/storage/pymongo_base.py b/aodh/storage/pymongo_base.py index e30a11be..f172434e 100644 --- a/aodh/storage/pymongo_base.py +++ b/aodh/storage/pymongo_base.py @@ -77,8 +77,9 @@ class Connection(base.Connection): create_alarm = update_alarm def delete_alarm(self, alarm_id): - """Delete an alarm.""" + """Delete an alarm and its history data.""" self.db.alarm.remove({'alarm_id': alarm_id}) + self.db.alarm_history.remove({'alarm_id': alarm_id}) def record_alarm_change(self, alarm_change): """Record alarm change event.""" diff --git a/aodh/tests/api/v2/test_alarm_scenarios.py b/aodh/tests/api/v2/test_alarm_scenarios.py index f287ee23..e95a01c7 100644 --- a/aodh/tests/api/v2/test_alarm_scenarios.py +++ b/aodh/tests/api/v2/test_alarm_scenarios.py @@ -2290,26 +2290,18 @@ class TestAlarms(TestAlarmsBase): PayloadMatcher(), mock.ANY) def test_alarm_sends_notification(self): - # Hit the AlarmController (with alarm_id supplied) ... - data = self.get_json('/alarms') - del_alarm_name = "name1" - for d in data: - if d['name'] == del_alarm_name: - del_alarm_id = d['alarm_id'] - + alarm = self._get_alarm('a') with mock.patch.object(messaging, 'get_notifier') as get_notifier: notifier = get_notifier.return_value - - self.delete('/alarms/%s' % del_alarm_id, - headers=self.auth_headers, status=204) + self._update_alarm(alarm, dict(name='new_name')) get_notifier.assert_called_once_with(mock.ANY, publisher_id='aodh.api') calls = notifier.info.call_args_list self.assertEqual(1, len(calls)) args, _ = calls[0] context, event_type, payload = args - self.assertEqual('alarm.deletion', event_type) - self.assertEqual(del_alarm_name, payload['detail']['name']) + self.assertEqual('alarm.rule_change', event_type) + self.assertEqual('new_name', payload['detail']['name']) self.assertTrue(set(['alarm_id', 'detail', 'event_id', 'on_behalf_of', 'project_id', 'timestamp', 'type', 'user_id']).issubset(payload.keys())) @@ -2710,7 +2702,7 @@ class TestAlarmsHistory(TestAlarmsBase): history = self._get_alarm_history(self._get_alarm('a'), auth) self.assertEqual([], history) - def test_get_recorded_alarm_history_preserved_after_deletion(self): + def test_delete_alarm_history_after_deletion(self): alarm = self._get_alarm('a') history = self._get_alarm_history(alarm) self.assertEqual([], history) @@ -2722,45 +2714,24 @@ class TestAlarmsHistory(TestAlarmsBase): headers=self.auth_headers, status=204) history = self._get_alarm_history(alarm) - self.assertEqual(2, len(history)) - self._assert_is_subset(dict(alarm_id=alarm['alarm_id'], - on_behalf_of=alarm['project_id'], - project_id=alarm['project_id'], - type='deletion', - user_id=alarm['user_id']), - history[0]) - alarm['rule'] = alarm['threshold_rule'] - del alarm['threshold_rule'] - self._assert_in_json(alarm, history[0]['detail']) - detail = '{"name": "renamed"}' - self._assert_is_subset(dict(alarm_id=alarm['alarm_id'], - detail=detail, - on_behalf_of=alarm['project_id'], - project_id=alarm['project_id'], - type='rule change', - user_id=alarm['user_id']), - history[1]) + self.assertEqual(0, len(history)) def test_get_alarm_history_ordered_by_recentness(self): alarm = self._get_alarm('a') for i in moves.xrange(10): self._update_alarm(alarm, dict(name='%s' % i)) alarm = self._get_alarm('a') - self._delete_alarm(alarm) history = self._get_alarm_history(alarm) - self.assertEqual(11, len(history), 'hist: %s' % history) + self.assertEqual(10, len(history), 'hist: %s' % history) self._assert_is_subset(dict(alarm_id=alarm['alarm_id'], - type='deletion'), + type='rule change'), history[0]) - alarm['rule'] = alarm['threshold_rule'] - del alarm['threshold_rule'] - self._assert_in_json(alarm, history[0]['detail']) - for i in moves.xrange(1, 10): + for i in moves.xrange(1, 11): detail = '{"name": "%s"}' % (10 - i) self._assert_is_subset(dict(alarm_id=alarm['alarm_id'], detail=detail, type='rule change'), - history[i]) + history[i - 1]) def test_get_alarm_history_constrained_by_timestamp(self): alarm = self._get_alarm('a') @@ -2783,19 +2754,18 @@ class TestAlarmsHistory(TestAlarmsBase): def test_get_alarm_history_constrained_by_type(self): alarm = self._get_alarm('a') - self._delete_alarm(alarm) - query = dict(field='type', op='eq', value='deletion') + self._update_alarm(alarm, dict(name='renamed2')) + query = dict(field='type', op='eq', value='rule change') history = self._get_alarm_history(alarm, query=query) self.assertEqual(1, len(history)) + detail = '{"name": "renamed2"}' self._assert_is_subset(dict(alarm_id=alarm['alarm_id'], + detail=detail, on_behalf_of=alarm['project_id'], project_id=alarm['project_id'], - type='deletion', + type='rule change', user_id=alarm['user_id']), history[0]) - alarm['rule'] = alarm['threshold_rule'] - del alarm['threshold_rule'] - self._assert_in_json(alarm, history[0]['detail']) def test_get_alarm_history_constrained_by_alarm_id_failed(self): alarm = self._get_alarm('a') diff --git a/aodh/tests/storage/test_storage_scenarios.py b/aodh/tests/storage/test_storage_scenarios.py index 3d13331f..40419ac8 100644 --- a/aodh/tests/storage/test_storage_scenarios.py +++ b/aodh/tests/storage/test_storage_scenarios.py @@ -288,6 +288,17 @@ class AlarmHistoryTest(AlarmTestBase, utcnow = datetime.datetime(2014, 4, 7, 7, 45) self._clear_alarm_history(utcnow, 3 * 60, 0) + def test_delete_history_when_delete_alarm(self): + alarms = list(self.alarm_conn.get_alarms()) + self.assertEqual(3, len(alarms)) + history = list(self.alarm_conn.query_alarm_history()) + self.assertEqual(3, len(history)) + for alarm in alarms: + self.alarm_conn.delete_alarm(alarm.alarm_id) + self.assertEqual(3, len(alarms)) + history = list(self.alarm_conn.query_alarm_history()) + self.assertEqual(0, len(history)) + class ComplexAlarmQueryTest(AlarmTestBase, tests_db.MixinTestsWithBackendScenarios): @@ -416,24 +427,9 @@ class ComplexAlarmHistoryQueryTest(AlarmTestBase, self.alarm_conn.record_alarm_change(alarm_change=alarm_change3) - if alarm.name in ["red-alert", "yellow-alert"]: - alarm_change4 = dict(event_id=( - "16fd2706-8baf-433b-82eb-8c7fada847f%s" - % i), - alarm_id=alarm.alarm_id, - type=alarm_models.AlarmChange.DELETION, - detail="detail %s" % (i + 2), - user_id=alarm.user_id, - project_id=alarm.project_id, - on_behalf_of=alarm.project_id, - timestamp=datetime.datetime(2012, 9, 27, - 10 + i, - 30 + i)) - self.alarm_conn.record_alarm_change(alarm_change=alarm_change4) - def test_alarm_history_with_no_filter(self): history = list(self.alarm_conn.query_alarm_history()) - self.assertEqual(11, len(history)) + self.assertEqual(9, len(history)) def test_alarm_history_with_no_filter_and_limit(self): history = list(self.alarm_conn.query_alarm_history(limit=3)) @@ -481,9 +477,8 @@ class ComplexAlarmHistoryQueryTest(AlarmTestBase, filter_expr = {"=": {"alarm_id": "r3d"}} history = list(self.alarm_conn.query_alarm_history( filter_expr=filter_expr, orderby=[{"timestamp": "asc"}])) - self.assertEqual(4, len(history)) + self.assertEqual(3, len(history)) self.assertEqual([alarm_models.AlarmChange.CREATION, alarm_models.AlarmChange.RULE_CHANGE, - alarm_models.AlarmChange.STATE_TRANSITION, - alarm_models.AlarmChange.DELETION], + alarm_models.AlarmChange.STATE_TRANSITION], [h.type for h in history])