Check if bridge exists and make sure it's UP in ensure_bridge

Introduce _bridge_exists_and_ensure_up that checks if the bridge
exists and if it's DOWN sets it UP

Change-Id: Ide9d2bb04016cadb347b00cadbee7007e61bd01e
Closes-bug: #1282741

diff --git a/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py b/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py
index 9fe433289236b711a0e9d0ce18d0d71b6a43f15f..962aba177342eb6bea9b2b0f65e7507881f209c1 100755 (executable)
--- a/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py
+++ b/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py
@@ -279,10 +279,28 @@ class LinuxBridgeManager:
                 src_device.addr.delete(ip_version=ip['ip_version'],
                                        cidr=ip['cidr'])
 
+    def _bridge_exists_and_ensure_up(self, bridge_name):
+        """Check if the bridge exists and make sure it is up."""
+        br = ip_lib.IPDevice(bridge_name, self.root_helper)
+        try:
+            # If the device doesn't exist this will throw a RuntimeError
+            br.link.set_up()
+        except RuntimeError:
+            return False
+        return True
+
     def ensure_bridge(self, bridge_name, interface=None, ips=None,
                       gateway=None):
         """Create a bridge unless it already exists."""
-        if not ip_lib.device_exists(bridge_name, root_helper=self.root_helper):
+        # _bridge_exists_and_ensure_up instead of device_exists is used here
+        # because there are cases where the bridge exists but it's not UP,
+        # for example:
+        # 1) A greenthread was executing this function and had not yet executed
+        # "ip link set bridge_name up" before eventlet switched to this
+        # thread running the same function
+        # 2) The Nova VIF driver was running concurrently and had just created
+        #    the bridge, but had not yet put it UP
+        if not self._bridge_exists_and_ensure_up(bridge_name):
             LOG.debug(_("Starting bridge %(bridge_name)s for subinterface "
                         "%(interface)s"),
                       {'bridge_name': bridge_name, 'interface': interface})

diff --git a/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py b/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py
index cc7cb334aa50e9a8ef457fc660f0eefd0ed2aa4d..643dd850b413888feb7c50d805b19a48d7e51b05 100644 (file)
--- a/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py
+++ b/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py
@@ -394,9 +394,19 @@ class TestLinuxBridgeManager(base.BaseTestCase):
             self.assertTrue(addgw_fn.called)
             self.assertTrue(delgw_fn.called)
 
+    def test_bridge_exists_and_ensure_up(self):
+        ip_lib_mock = mock.Mock()
+        with mock.patch.object(ip_lib, 'IPDevice', return_value=ip_lib_mock):
+            # device exists
+            self.assertTrue(self.lbm._bridge_exists_and_ensure_up("br0"))
+            self.assertTrue(ip_lib_mock.link.set_up.called)
+            # device doesn't exists
+            ip_lib_mock.link.set_up.side_effect = RuntimeError
+            self.assertFalse(self.lbm._bridge_exists_and_ensure_up("br0"))
+
     def test_ensure_bridge(self):
         with contextlib.nested(
-            mock.patch.object(ip_lib, 'device_exists'),
+            mock.patch.object(self.lbm, '_bridge_exists_and_ensure_up'),
             mock.patch.object(utils, 'execute'),
             mock.patch.object(self.lbm, 'update_interface_ip_details'),
             mock.patch.object(self.lbm, 'interface_exists_on_bridge'),