Merge "Remove unnecessary call to get_dhcp_port from DeviceManager"

neutron/agent/linux/dhcp.py

@@ -646,26 +646,21 @@ class DeviceManager(object):
         host_uuid = uuid.uuid5(uuid.NAMESPACE_DNS, socket.gethostname())
         return 'dhcp%s-%s' % (host_uuid, network.id)
 
-    def _get_device(self, network):
+    def _get_device(self, network, port):
         """Return DHCP ip_lib device for this host on the network."""
-        device_id = self.get_device_id(network)
-        port = self.plugin.get_dhcp_port(network.id, device_id)
-        if port:
         interface_name = self.get_interface_name(network, port)
         return ip_lib.IPDevice(interface_name,
                                self.root_helper,
                                network.namespace)
-        else:
-            raise exceptions.NetworkNotFound(net_id=network.id)
 
-    def _set_default_route(self, network):
+    def _set_default_route(self, network, port):
         """Sets the default gateway for this dhcp namespace.
 
         This method is idempotent and will only adjust the route if adjusting
         it would change it from what it already is.  This makes it safe to call
         and avoids unnecessary perturbation of the system.
         """
-        device = self._get_device(network)
+        device = self._get_device(network, port)
         gateway = device.route.get_gateway()
         if gateway:
             gateway = gateway['gateway']
@@ -800,14 +795,18 @@ class DeviceManager(object):
             device.route.pullup_route(interface_name)
 
         if self.conf.use_namespaces:
-            self._set_default_route(network)
+            self._set_default_route(network, port)
 
         return interface_name
 
     def update(self, network):
         """Update device settings for the network's DHCP on this host."""
         if self.conf.use_namespaces:
-            self._set_default_route(network)
+            device_id = self.get_device_id(network)
+            port = self.plugin.get_dhcp_port(network.id, device_id)
+            if not port:
+                raise exceptions.NetworkNotFound(net_id=network.id)
+            self._set_default_route(network, port)
 
     def destroy(self, network, device_name):
         """Destroy the device used for the network's DHCP on this host."""

neutron/tests/unit/test_dhcp_agent.py

@@ -1165,7 +1165,7 @@ class TestDeviceManager(base.BaseTestCase):
                                            namespace=net.namespace))
         self.mock_driver.assert_has_calls(expected)
 
-        dh._set_default_route.assert_called_once_with(net)
+        dh._set_default_route.assert_called_once_with(net, port)
 
     def test_setup(self):
         cfg.CONF.set_override('enable_metadata_network', False)
@@ -1320,15 +1320,21 @@ class TestDeviceManager(base.BaseTestCase):
         dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.root_helper, None)
         dh._set_default_route = mock.Mock()
 
-        dh.update(True)
+        network = mock.Mock()
+        port = mock.Mock()
+        dh.plugin = mock.Mock()
+        dh.plugin.get_dhcp_port.return_value = port
+        dh.update(network)
 
-        dh._set_default_route.assert_called_once_with(True)
+        dh._set_default_route.assert_called_once_with(network, port)
 
         # No namespaces, shouldn't set default route.
         cfg.CONF.set_override('use_namespaces', False)
         cfg.CONF.set_override('enable_metadata_network', False)
         dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.root_helper, None)
         dh._set_default_route = mock.Mock()
+        dh.plugin = mock.Mock()
+        dh.plugin.get_dhcp_port.return_value = port
 
         dh.update(FakeV4Network())
 
@@ -1339,6 +1345,8 @@ class TestDeviceManager(base.BaseTestCase):
         cfg.CONF.set_override('enable_metadata_network', True)
         dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.root_helper, None)
         dh._set_default_route = mock.Mock()
+        dh.plugin = mock.Mock()
+        dh.plugin.get_dhcp_port.return_value = port
 
         dh.update(FakeV4Network())
 
@@ -1362,7 +1370,7 @@ class TestDeviceManager(base.BaseTestCase):
         dh = self._get_device_manager_with_mock_device(cfg.CONF, device)
         network = FakeV4Network()
 
-        dh._set_default_route(network)
+        dh._set_default_route(network, mock.Mock())
 
         device.route.get_gateway.assert_called_once()
         self.assertFalse(device.route.delete_gateway.called)
@@ -1376,7 +1384,7 @@ class TestDeviceManager(base.BaseTestCase):
         dh = self._get_device_manager_with_mock_device(cfg.CONF, device)
         network = FakeV4NetworkNoSubnet()
 
-        dh._set_default_route(network)
+        dh._set_default_route(network, mock.Mock())
 
         device.route.get_gateway.assert_called_once()
         self.assertFalse(device.route.delete_gateway.called)
@@ -1390,7 +1398,7 @@ class TestDeviceManager(base.BaseTestCase):
         dh = self._get_device_manager_with_mock_device(cfg.CONF, device)
         network = FakeV4NetworkNoSubnet()
 
-        dh._set_default_route(network)
+        dh._set_default_route(network, mock.Mock())
 
         device.route.get_gateway.assert_called_once()
         device.route.delete_gateway.assert_called_once_with('192.168.0.1')
@@ -1404,7 +1412,7 @@ class TestDeviceManager(base.BaseTestCase):
         dh = self._get_device_manager_with_mock_device(cfg.CONF, device)
         network = FakeV4NetworkNoGateway()
 
-        dh._set_default_route(network)
+        dh._set_default_route(network, mock.Mock())
 
         device.route.get_gateway.assert_called_once()
         device.route.delete_gateway.assert_called_once_with('192.168.0.1')
@@ -1418,7 +1426,7 @@ class TestDeviceManager(base.BaseTestCase):
         dh = self._get_device_manager_with_mock_device(cfg.CONF, device)
         network = FakeV4Network()
 
-        dh._set_default_route(network)
+        dh._set_default_route(network, mock.Mock())
 
         device.route.get_gateway.assert_called_once()
         self.assertFalse(device.route.delete_gateway.called)
@@ -1432,7 +1440,7 @@ class TestDeviceManager(base.BaseTestCase):
         dh = self._get_device_manager_with_mock_device(cfg.CONF, device)
         network = FakeV4Network()
 
-        dh._set_default_route(network)
+        dh._set_default_route(network, mock.Mock())
 
         device.route.get_gateway.assert_called_once()
         self.assertFalse(device.route.delete_gateway.called)
@@ -1449,12 +1457,24 @@ class TestDeviceManager(base.BaseTestCase):
         subnet2.gateway_ip = '192.168.1.1'
         network.subnets = [subnet2, FakeV4Subnet()]
 
-        dh._set_default_route(network)
+        dh._set_default_route(network, mock.Mock())
 
         device.route.get_gateway.assert_called_once()
         self.assertFalse(device.route.delete_gateway.called)
         device.route.add_gateway.assert_called_once_with('192.168.1.1')
 
+    def test_get_device_dont_call_get_dhcp_port(self):
+        dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.root_helper, None)
+        dh.get_interface_name = mock.Mock()
+        dh.plugin = mock.Mock()
+        network = mock.Mock()
+        port = mock.Mock()
+
+        dh._get_device(network, port)
+
+        self.assertFalse(dh.plugin.get_dhcp_port.called)
+        dh.get_interface_name.assert_called_once_with(network, port)
+
 
 class TestDictModel(base.BaseTestCase):
     def test_basic_dict(self):