From f88f6863eaf70de74e63a11fbce1c23033141aea Mon Sep 17 00:00:00 2001
From: ZhiQiang Fan <zhiqiang.fan@huawei.com>
Date: Thu, 8 Jan 2015 20:50:54 +0800
Subject: [PATCH] Catch exception when evaluate single alarm

Currently, if we raise exception when evaluate alarms, the outside
method will not catch it, so the higher _evaluate_assigned_alarms
catches it, but this will stop evaluating the rest alarms. Alarm
should be evaluated no matter whether other alarms succeed or not.

This patch adds a try...except block in _evaluate_alarm, it will
log exception when it is raised, and the higher method can move
on next alarm.

Change-Id: Id95865c51dc4ffe2d7089a3ff4fa5b73bd93ae76
Closes-Bug: #1408620
---
 ceilometer/alarm/service.py                      |  5 ++++-
 ceilometer/tests/alarm/test_alarm_svc.py         | 16 ++++++++++++++++
 .../tests/alarm/test_singleton_alarm_svc.py      | 13 +++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/ceilometer/alarm/service.py b/ceilometer/alarm/service.py
index 9b2d3496c..bc560413d 100644
--- a/ceilometer/alarm/service.py
+++ b/ceilometer/alarm/service.py
@@ -113,7 +113,10 @@ class AlarmService(object):
             return
 
         LOG.debug(_('evaluating alarm %s') % alarm.alarm_id)
-        self.evaluators[alarm.type].obj.evaluate(alarm)
+        try:
+            self.evaluators[alarm.type].obj.evaluate(alarm)
+        except Exception:
+            LOG.exception(_('Failed to evaluate alarm %s'), alarm.alarm_id)
 
     @abc.abstractmethod
     def _assigned_alarms(self):
diff --git a/ceilometer/tests/alarm/test_alarm_svc.py b/ceilometer/tests/alarm/test_alarm_svc.py
index 5c06e997c..f2d80750b 100644
--- a/ceilometer/tests/alarm/test_alarm_svc.py
+++ b/ceilometer/tests/alarm/test_alarm_svc.py
@@ -107,6 +107,22 @@ class TestAlarmEvaluationService(tests_base.BaseTestCase):
                 self.svc.PARTITIONING_GROUP_NAME, [alarm])
             self.threshold_eval.evaluate.assert_called_once_with(alarm)
 
+    def test_evaluation_cycle_with_bad_alarm(self):
+        alarms = [
+            mock.Mock(type='threshold', name='bad'),
+            mock.Mock(type='threshold', name='good'),
+        ]
+        self.api_client.alarms.list.return_value = alarms
+        self.threshold_eval.evaluate.side_effect = [Exception('Boom!'), None]
+        with mock.patch('ceilometerclient.client.get_client',
+                        return_value=self.api_client):
+            p_coord_mock = self.svc.partition_coordinator
+            p_coord_mock.extract_my_subset.return_value = alarms
+
+            self.svc._evaluate_assigned_alarms()
+            self.assertEqual([mock.call(alarms[0]), mock.call(alarms[1])],
+                             self.threshold_eval.evaluate.call_args_list)
+
     def test_unknown_extension_skipped(self):
         alarms = [
             mock.Mock(type='not_existing_type'),
diff --git a/ceilometer/tests/alarm/test_singleton_alarm_svc.py b/ceilometer/tests/alarm/test_singleton_alarm_svc.py
index 58bb3d80c..f6bd2affd 100644
--- a/ceilometer/tests/alarm/test_singleton_alarm_svc.py
+++ b/ceilometer/tests/alarm/test_singleton_alarm_svc.py
@@ -71,6 +71,19 @@ class TestSingletonAlarmService(tests_base.BaseTestCase):
             self.singleton._evaluate_assigned_alarms()
             self.threshold_eval.evaluate.assert_called_once_with(alarm)
 
+    def test_evaluation_cycle_with_bad_alarm(self):
+        alarms = [
+            mock.Mock(type='threshold', name='bad'),
+            mock.Mock(type='threshold', name='good'),
+        ]
+        self.threshold_eval.evaluate.side_effect = [Exception('Boom!'), None]
+        self.api_client.alarms.list.return_value = alarms
+        with mock.patch('ceilometerclient.client.get_client',
+                        return_value=self.api_client):
+            self.singleton._evaluate_assigned_alarms()
+            self.assertEqual([mock.call(alarms[0]), mock.call(alarms[1])],
+                             self.threshold_eval.evaluate.call_args_list)
+
     def test_unknown_extension_skipped(self):
         alarms = [
             mock.Mock(type='not_existing_type'),