Merge "Don't reraise Exceptions from agent driver"

2 files changed, 59 insertions, 2 deletions

diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py
index 6bb0332f4..7261aeff5 100644
--- a/ironic/drivers/modules/agent.py
+++ b/ironic/drivers/modules/agent.py
@@ -377,11 +377,18 @@ class AgentVendorInterface(base.VendorInterface):
         func = self.vendor_routes[method]
         try:
             return func(task, **kwargs)
-        except Exception:
-            # catch-all in case something bubbles up here
+        except exception.IronicException as e:
             with excutils.save_and_reraise_exception():
+                # log this because even though the exception is being
+                # reraised, it won't be handled if it is an async. call.
                 LOG.exception(_LE('vendor_passthru failed with method %s'),
                               method)
+        except Exception as e:
+            # catch-all in case something bubbles up here
+            # log this because even though the exception is being
+            # reraised, it won't be handled if it is an async. call.
+            LOG.exception(_LE('vendor_passthru failed with method %s'), method)
+            raise exception.VendorPassthruException(message=e)
 
     def _heartbeat(self, task, **kwargs):
         """Method for agent to periodically check in.
@@ -402,7 +409,10 @@ class AgentVendorInterface(base.VendorInterface):
             {'node': node.uuid,
              'heartbeat': driver_info.get('agent_last_heartbeat')})
         driver_info['agent_last_heartbeat'] = int(_time())
+        # FIXME(rloo): This could raise KeyError exception if 'agent_url'
+        #              wasn't specified. Instead, raise MissingParameterValue.
         driver_info['agent_url'] = kwargs['agent_url']
+
         node.driver_info = driver_info
         node.save()
 
diff --git a/ironic/tests/drivers/test_agent.py b/ironic/tests/drivers/test_agent.py
index 274c2554e..fb315e360 100644
--- a/ironic/tests/drivers/test_agent.py
+++ b/ironic/tests/drivers/test_agent.py
@@ -319,3 +319,50 @@ class TestAgentVendor(db_base.DbTestCase):
         with task_manager.acquire(
                 self.context, self.node['uuid'], shared=True) as task:
             self.passthru._heartbeat(task, **kwargs)
+
+    def test_heartbeat_bad(self):
+        kwargs = {}
+        with task_manager.acquire(
+                self.context, self.node['uuid'], shared=True) as task:
+            self.assertRaises(KeyError,
+                              self.passthru._heartbeat, task, **kwargs)
+
+    @mock.patch('ironic.drivers.modules.agent.AgentVendorInterface'
+                '._heartbeat')
+    def test_vendor_passthru_heartbeat(self, mock_heartbeat):
+        kwargs = {
+            'method': 'heartbeat',
+        }
+        self.passthru.vendor_routes['heartbeat'] = mock_heartbeat
+        with task_manager.acquire(
+                self.context, self.node['uuid'], shared=True) as task:
+            self.passthru.vendor_passthru(task, **kwargs)
+            mock_heartbeat.assert_called_once_with(task, **kwargs)
+
+    @mock.patch('ironic.drivers.modules.agent.AgentVendorInterface'
+                '._heartbeat')
+    def test_vendor_passthru_heartbeat_ironic_exc(self, mock_heartbeat):
+        mock_heartbeat.side_effect = exception.IronicException()
+        kwargs = {
+            'method': 'heartbeat',
+        }
+        self.passthru.vendor_routes['heartbeat'] = mock_heartbeat
+        with task_manager.acquire(
+                self.context, self.node['uuid'], shared=True) as task:
+            self.assertRaises(exception.IronicException,
+                              self.passthru.vendor_passthru, task, **kwargs)
+            mock_heartbeat.assert_called_once_with(task, **kwargs)
+
+    @mock.patch('ironic.drivers.modules.agent.AgentVendorInterface'
+                '._heartbeat')
+    def test_vendor_passthru_heartbeat_exception(self, mock_heartbeat):
+        mock_heartbeat.side_effect = KeyError()
+        kwargs = {
+            'method': 'heartbeat',
+        }
+        self.passthru.vendor_routes['heartbeat'] = mock_heartbeat
+        with task_manager.acquire(
+                self.context, self.node['uuid'], shared=True) as task:
+            self.assertRaises(exception.VendorPassthruException,
+                              self.passthru.vendor_passthru, task, **kwargs)
+            mock_heartbeat.assert_called_once_with(task, **kwargs)