From: Cedric Brandily Date: Thu, 5 Nov 2015 23:19:36 +0000 (+0100) Subject: Use get_interface_bridge instead of get_bridge_for_tap_device X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=d3da15d62d4270dfb5a20e8d4051c7e17c8c2c9b;p=openstack-build%2Fneutron-build.git 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 9568db251..0ad7e0904 100644 --- 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 dadd346b2..3cafcfbc4 100644 --- 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,