Catch any exception raised in call_monitor_plugin()

The call_monitor_plugin method is executed by polling monitor and
periodic healing. The two use oslo_service.threadgroup for its periodic
execution, but the threadgroup doesn't catch any exception. If
call_monitor_plugin() raises any exception, the thread automatically
terminates.

To keep the threadgroup running, this patch makes call_monitor_plugin()
catch any exception internally, preventing them from being raised to the
threadgroup.

Change-Id: Id26401c03e3e89d308a089097315996973cc1dfb
Closes-Bug: #1785945

blazar/monitor/base.py

@@ -53,18 +53,21 @@ class BaseMonitor(object):
 
     def call_monitor_plugin(self, callback, *args, **kwargs):
         """Call a callback and update lease/reservation flags."""
+        # This method has to handle any exception internally. It shouldn't
+        # raise an exception because the timer threads in the BaseMonitor class
+        # terminates its execution once the thread has received any exception.
         try:
             # The callback() has to return a dictionary of
             # {reservation id: flags to update}.
             # e.g. {'dummyid': {'missing_resources': True}}
             reservation_flags = callback(*args, **kwargs)
+
+            if reservation_flags:
+                self._update_flags(reservation_flags)
         except Exception as e:
             LOG.exception('Caught an exception while executing a callback. '
                           '%s', str(e))
 
-        if reservation_flags:
-            self._update_flags(reservation_flags)
-
     def _update_flags(self, reservation_flags):
         """Update lease/reservation flags."""
         lease_ids = set([])

blazar/tests/monitor/test_base.py

@@ -14,6 +14,7 @@ import mock
 from oslo_service import threadgroup
 
 from blazar.db import api as db_api
+from blazar import exceptions
 from blazar.monitor import base as base_monitor
 from blazar.plugins import base
 from blazar import tests
@@ -86,6 +87,13 @@ class BaseMonitorTestCase(tests.TestCase):
         update_flags.assert_called_once_with(
             {'dummy_id1': {'missing_resources': True}})
 
+    def test_error_in_callback(self):
+        callback = self.patch(DummyMonitorPlugin, 'poll')
+        callback.side_effect = exceptions.BlazarException('error')
+
+        # Testing that no exception is raised even if the callback raises one
+        self.monitor.call_monitor_plugin(callback)
+
     def test_call_update_flags(self):
         reservation_update = self.patch(db_api, 'reservation_update')
         reservation_get = self.patch(db_api, 'reservation_get')
@@ -100,3 +108,15 @@ class BaseMonitorTestCase(tests.TestCase):
         reservation_get.assert_called_once_with('dummy_id1')
         lease_update.assert_called_once_with('dummy_id2',
                                              {'degraded': True})
+
+    def test_error_in_update_flags(self):
+        callback = self.patch(DummyMonitorPlugin, 'poll')
+        callback.return_value = {
+            'dummy_id1': {'missing_resources': True}
+        }
+
+        update_flags = self.patch(self.monitor, '_update_flags')
+        update_flags.side_effect = exceptions.BlazarException('error')
+
+        # Testing that no exception is raised even if the callback raises one
+        self.monitor.call_monitor_plugin(callback)