Use get_interface_bridge instead of get_bridge_for_tap_device

bridge_lib defines get_interface_bridge which returns the bridge on
which an interface is connected (or None) where LB L2-agent[1] returns
it only if the bridge is a bridge managed by LB L2-agent.

According to get_bridge_for_tap_device 1st usage:

  if bridge_lib.is_bridged_interface(interface):
     bridge = self.get_bridge_for_tap_device(interface)
     bridge.delif(interface)

The code crashs if interface is owned by a non-LB-agent bridge because
the condition is verified but bridge is None.

[1] neutron.plugins.ml2.drivers.linuxbridge.agent.linuxbridge_neutron_agent

Partial-Bug: #1514548
Change-Id: If6dba1197b3f08ecc0ac01fcf085c8268f81169f

diff --git a/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py b/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py
index 9568db2519a75652c10c822b6ea8ed4963ac3fef..0ad7e0904bc9d4a5f1a8957956e1365ab56a8d7b 100644 (file)
--- a/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py
+++ b/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py
@@ -181,13 +181,6 @@ class LinuxBridgeManager(object):
         return len([interface for interface in if_list if
                     interface.startswith(constants.TAP_DEVICE_PREFIX)])
 
-    def get_bridge_for_tap_device(self, tap_device_name):
-        bridge = bridge_lib.BridgeDevice.get_interface_bridge(tap_device_name)
-        if (bridge and (bridge.name.startswith(BRIDGE_NAME_PREFIX)
-                        or bridge.name in self.bridge_mappings.values())):
-            return bridge
-        return None
-
     def ensure_vlan_bridge(self, network_id, phy_bridge_name,
                            physical_interface, vlan_id):
         """Create a vlan and bridge unless they already exist."""
@@ -368,8 +361,9 @@ class LinuxBridgeManager(object):
         if not bridge_device.owns_interface(interface):
             try:
                 # Check if the interface is not enslaved in another bridge
-                if bridge_lib.is_bridged_interface(interface):
-                    bridge = self.get_bridge_for_tap_device(interface)
+                bridge = bridge_lib.BridgeDevice.get_interface_bridge(
+                    interface)
+                if bridge:
                     bridge.delif(interface)
 
                 bridge_device.addif(interface)
@@ -440,7 +434,8 @@ class LinuxBridgeManager(object):
             self.ensure_tap_mtu(tap_device_name, phy_dev_name)
 
         # Check if device needs to be added to bridge
-        tap_device_in_bridge = self.get_bridge_for_tap_device(tap_device_name)
+        tap_device_in_bridge = bridge_lib.BridgeDevice.get_interface_bridge(
+            tap_device_name)
         if not tap_device_in_bridge:
             data = {'tap_device_name': tap_device_name,
                     'bridge_name': bridge_name}

diff --git a/neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_linuxbridge_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_linuxbridge_neutron_agent.py
index dadd346b2b04987cf89899cb478bbf4bec362d9c..3cafcfbc4a88134254bef5520596701b1eec28cf 100644 (file)
--- a/neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_linuxbridge_neutron_agent.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_linuxbridge_neutron_agent.py
@@ -462,24 +462,6 @@ class TestLinuxBridgeManager(base.BaseTestCase):
             get_ifs_fn.return_value = ['tap2101', 'eth0.100', 'vxlan-1000']
             self.assertEqual(self.lbm.get_tap_devices_count('br0'), 1)
 
-    def test_get_bridge_for_tap_device(self):
-
-        with mock.patch.object(
-                bridge_lib.BridgeDevice, 'get_interface_bridge') as get_br:
-            get_br.return_value = bridge_lib.BridgeDevice("brq-fake")
-            self.assertEqual(get_br.return_value,
-                             self.lbm.get_bridge_for_tap_device("tap1"))
-
-            get_br.return_value = bridge_lib.BridgeDevice(BRIDGE_MAPPING_VALUE)
-            self.assertEqual(get_br.return_value,
-                             self.lbm.get_bridge_for_tap_device("tap2"))
-
-            get_br.return_value = bridge_lib.BridgeDevice('notneutronbridge')
-            self.assertIsNone(self.lbm.get_bridge_for_tap_device("tap3"))
-
-            get_br.return_value = None
-            self.assertIsNone(self.lbm.get_bridge_for_tap_device("tap4"))
-
     def test_get_interface_details(self):
         with mock.patch.object(ip_lib.IpAddrCommand, 'list') as list_fn,\
                 mock.patch.object(ip_lib.IpRouteCommand,
@@ -656,8 +638,8 @@ class TestLinuxBridgeManager(base.BaseTestCase):
                 mock.patch.object(self.lbm,
                                   'update_interface_ip_details') as upd_fn,\
                 mock.patch.object(bridge_lib, 'is_bridged_interface'),\
-                mock.patch.object(self.lbm,
-                                  'get_bridge_for_tap_device') as get_if_br_fn:
+                mock.patch.object(bridge_lib.BridgeDevice,
+                                  'get_interface_bridge') as get_if_br_fn:
             de_fn.return_value = False
             br_fn.addbr.return_value = bridge_device
             bridge_device.setfd.return_value = False
@@ -732,8 +714,8 @@ class TestLinuxBridgeManager(base.BaseTestCase):
             with mock.patch.object(self.lbm, "ensure_local_bridge") as en_fn,\
                     mock.patch.object(bridge_lib, "BridgeDevice",
                                       return_value=bridge_device), \
-                    mock.patch.object(self.lbm,
-                                      "get_bridge_for_tap_device") as get_br:
+                    mock.patch.object(bridge_lib.BridgeDevice,
+                                      "get_interface_bridge") as get_br:
                 bridge_device.addif.retun_value = False
                 get_br.return_value = True
                 self.assertTrue(self.lbm.add_tap_interface("123",
@@ -760,8 +742,8 @@ class TestLinuxBridgeManager(base.BaseTestCase):
                                    "ensure_physical_in_bridge") as ens_fn,\
                     mock.patch.object(self.lbm,
                                       "ensure_tap_mtu") as en_mtu_fn,\
-                    mock.patch.object(self.lbm,
-                                      "get_bridge_for_tap_device") as get_br:
+                    mock.patch.object(bridge_lib.BridgeDevice,
+                                      "get_interface_bridge") as get_br:
                 ens_fn.return_value = False
                 self.assertFalse(self.lbm.add_tap_interface("123",
                                                             p_const.TYPE_VLAN,