From: Mark McClain <mark.mcclain@dreamhost.com>
Date: Fri, 7 Sep 2012 16:39:39 +0000 (-0400)
Subject: Fix dhcp agent rpc exception handling
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=64cd776da0c26f9324d3dc2918c321d4b6465d5e;p=openstack-build%2Fneutron-build.git

Fix dhcp agent rpc exception handling

fixes bug 1046904

This patch adds exception handling to log rpc exceptions.  Previously,
the agent would terminate due to uncaught errors during initialization.

Change-Id: I4835c1616e2ccfc9c42c591e8c7446db50721d01
---

diff --git a/quantum/agent/dhcp_agent.py b/quantum/agent/dhcp_agent.py
index a79f01b2f..50160d345 100644
--- a/quantum/agent/dhcp_agent.py
+++ b/quantum/agent/dhcp_agent.py
@@ -89,9 +89,10 @@ class DhcpAgent(object):
                                           self.device_manager,
                                           namespace)
             getattr(driver, action)()
+            return True
 
         except Exception, e:
-            LOG.warn('Unable to %s dhcp. Exception: %s' % (action, e))
+            LOG.exception('Unable to %s dhcp.' % action)
 
     def update_lease(self, network_id, ip_address, time_remaining):
         self.plugin_rpc.update_lease_expiration(network_id, ip_address,
@@ -99,11 +100,18 @@ class DhcpAgent(object):
 
     def enable_dhcp_helper(self, network_id):
         """Enable DHCP for a network that meets enabling criteria."""
-        network = self.plugin_rpc.get_network_info(network_id)
+        try:
+            network = self.plugin_rpc.get_network_info(network_id)
+        except:
+            LOG.exception(_('Network %s RPC info call failed.') % network_id)
+            return
+
+        if not network.admin_state_up:
+            return
+
         for subnet in network.subnets:
             if subnet.enable_dhcp:
-                if network.admin_state_up:
-                    self.call_driver('enable', network)
+                if self.call_driver('enable', network):
                     self.cache.put(network)
                 break
 
diff --git a/quantum/tests/unit/test_dhcp_agent.py b/quantum/tests/unit/test_dhcp_agent.py
index fe71ec518..f404e94aa 100644
--- a/quantum/tests/unit/test_dhcp_agent.py
+++ b/quantum/tests/unit/test_dhcp_agent.py
@@ -126,14 +126,30 @@ class TestDhcpAgent(unittest.TestCase):
         network.id = '1'
         with mock.patch('quantum.agent.dhcp_agent.DeviceManager') as dev_mgr:
             dhcp = dhcp_agent.DhcpAgent(cfg.CONF)
-            dhcp.call_driver('foo', network)
-            dev_mgr.assert_called()
+            self.assertTrue(dhcp.call_driver('foo', network))
+            self.assertTrue(dev_mgr.called)
             self.driver.assert_called_once_with(cfg.CONF,
                                                 mock.ANY,
                                                 'sudo',
                                                 mock.ANY,
                                                 'qdhcp-1')
 
+    def test_call_driver_failure(self):
+        network = mock.Mock()
+        network.id = '1'
+        self.driver.return_value.foo.side_effect = Exception
+        with mock.patch('quantum.agent.dhcp_agent.DeviceManager') as dev_mgr:
+            with mock.patch.object(dhcp_agent.LOG, 'exception') as log:
+                dhcp = dhcp_agent.DhcpAgent(cfg.CONF)
+                self.assertIsNone(dhcp.call_driver('foo', network))
+                self.assertTrue(dev_mgr.called)
+                self.driver.assert_called_once_with(cfg.CONF,
+                                                    mock.ANY,
+                                                    'sudo',
+                                                    mock.ANY,
+                                                    'qdhcp-1')
+                self.assertEqual(log.call_count, 1)
+
 
 class TestDhcpAgentEventHandler(unittest.TestCase):
     def setUp(self):
@@ -173,6 +189,7 @@ class TestDhcpAgentEventHandler(unittest.TestCase):
         self.plugin.assert_has_calls(
             [mock.call.get_network_info(fake_network.id)])
         self.call_driver.assert_called_once_with('enable', fake_network)
+        self.cache.assert_has_calls([mock.call.put(fake_network)])
 
     def test_enable_dhcp_helper_down_network(self):
         self.plugin.get_network_info.return_value = fake_down_network
@@ -180,6 +197,26 @@ class TestDhcpAgentEventHandler(unittest.TestCase):
         self.plugin.assert_has_calls(
             [mock.call.get_network_info(fake_down_network.id)])
         self.assertFalse(self.call_driver.called)
+        self.assertFalse(self.cache.called)
+
+    def test_enable_dhcp_helper_exception_during_rpc(self):
+        self.plugin.get_network_info.side_effect = Exception
+        with mock.patch.object(dhcp_agent.LOG, 'exception') as log:
+            self.dhcp.enable_dhcp_helper(fake_network.id)
+            self.plugin.assert_has_calls(
+                [mock.call.get_network_info(fake_network.id)])
+            self.assertFalse(self.call_driver.called)
+            self.assertTrue(log.called)
+            self.assertFalse(self.cache.called)
+
+    def test_enable_dhcp_helper_driver_failure(self):
+        self.plugin.get_network_info.return_value = fake_network
+        self.dhcp.enable_dhcp_helper(fake_network.id)
+        self.call_driver.enable.return_value = False
+        self.plugin.assert_has_calls(
+            [mock.call.get_network_info(fake_network.id)])
+        self.call_driver.assert_called_once_with('enable', fake_network)
+        self.assertFalse(self.cache.called)
 
     def test_disable_dhcp_helper_known_network(self):
         self.cache.get_network_by_id.return_value = fake_network