Merge "Fix dhcp agent rpc exception handling"

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:
+                if self.call_driver('enable', network):
-                    self.call_driver('enable', network)
                     self.cache.put(network)
                 break
 

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)
+            self.assertTrue(dhcp.call_driver('foo', network))
-            dev_mgr.assert_called()
+            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