Merge "DHCP agent should check interface is UP before adding route"

neutron/agent/linux/dhcp.py

@@ -815,9 +815,9 @@ class DeviceManager(object):
         port = self.setup_dhcp_port(network)
         interface_name = self.get_interface_name(network, port)
 
-        if ip_lib.device_exists(interface_name,
+        if ip_lib.ensure_device_is_ready(interface_name,
-                                self.root_helper,
+                                         self.root_helper,
-                                network.namespace):
+                                         network.namespace):
             LOG.debug(_('Reusing existing device: %s.'), interface_name)
         else:
             self.driver.plug(network.id,

neutron/agent/linux/ip_lib.py

@@ -482,6 +482,17 @@ def device_exists(device_name, root_helper=None, namespace=None):
     return bool(address)
 
 
+def ensure_device_is_ready(device_name, root_helper=None, namespace=None):
+    dev = IPDevice(device_name, root_helper, namespace)
+    try:
+        # Ensure the device is up, even if it is already up. If the device
+        # doesn't exist, a RuntimeError will be raised.
+        dev.link.set_up()
+    except RuntimeError:
+        return False
+    return True
+
+
 def iproute_arg_supported(command, arg, root_helper=None):
     command += ['help']
     stdout, stderr = utils.execute(command, root_helper=root_helper,

neutron/tests/unit/test_dhcp_agent.py

@@ -1088,9 +1088,9 @@ class TestDeviceManager(base.BaseTestCase):
         cfg.CONF.set_override('use_namespaces', True)
         cfg.CONF.set_override('enable_isolated_metadata', True)
 
-        self.device_exists_p = mock.patch(
+        self.ensure_device_is_ready_p = mock.patch(
-            'neutron.agent.linux.ip_lib.device_exists')
+            'neutron.agent.linux.ip_lib.ensure_device_is_ready')
-        self.device_exists = self.device_exists_p.start()
+        self.ensure_device_is_ready = (self.ensure_device_is_ready_p.start())
 
         self.dvr_cls_p = mock.patch('neutron.agent.linux.interface.NullDriver')
         self.iproute_cls_p = mock.patch('neutron.agent.linux.'
@@ -1104,13 +1104,13 @@ class TestDeviceManager(base.BaseTestCase):
         driver_cls.return_value = self.mock_driver
         iproute_cls.return_value = self.mock_iproute
 
-    def _test_setup_helper(self, device_exists, net=None, port=None):
+    def _test_setup_helper(self, device_is_ready, net=None, port=None):
         net = net or fake_network
         port = port or fake_port1
         plugin = mock.Mock()
         plugin.create_dhcp_port.return_value = port or fake_port1
         plugin.get_dhcp_port.return_value = port or fake_port1
-        self.device_exists.return_value = device_exists
+        self.ensure_device_is_ready.return_value = device_is_ready
         self.mock_driver.get_device_name.return_value = 'tap12345678-12'
 
         dh = dhcp.DeviceManager(cfg.CONF, cfg.CONF.root_helper, plugin)
@@ -1135,7 +1135,7 @@ class TestDeviceManager(base.BaseTestCase):
                 expected_ips,
                 namespace=net.namespace)]
 
-        if not device_exists:
+        if not device_is_ready:
             expected.insert(1,
                             mock.call.plug(net.id,
                                            port.id,
@@ -1152,7 +1152,7 @@ class TestDeviceManager(base.BaseTestCase):
         cfg.CONF.set_override('enable_metadata_network', True)
         self._test_setup_helper(False)
 
-    def test_setup_device_exists(self):
+    def test_setup_device_is_ready(self):
         self._test_setup_helper(True)
 
     def test_create_dhcp_port_raise_conflict(self):

neutron/tests/unit/test_linux_ip_lib.py

@@ -789,3 +789,13 @@ class TestDeviceExists(base.BaseTestCase):
             _execute.return_value = ''
             _execute.side_effect = RuntimeError
             self.assertFalse(ip_lib.device_exists('eth0'))
+
+    def test_ensure_device_is_ready(self):
+        ip_lib_mock = mock.Mock()
+        with mock.patch.object(ip_lib, 'IPDevice', return_value=ip_lib_mock):
+            self.assertTrue(ip_lib.ensure_device_is_ready("eth0"))
+            self.assertTrue(ip_lib_mock.link.set_up.called)
+            ip_lib_mock.reset_mock()
+            # device doesn't exists
+            ip_lib_mock.link.set_up.side_effect = RuntimeError
+            self.assertFalse(ip_lib.ensure_device_is_ready("eth0"))