From 49502246ef319cf8981b8a867ce2a3b6a163fc9c Mon Sep 17 00:00:00 2001 From: rossella Date: Fri, 4 Apr 2014 11:55:11 +0200 Subject: [PATCH] Remove device_exists in LinuxBridgeManager LinuxBridgeManager is adding a device_exists method instead of using the one in ip_lib as the rest of the codebase Change-Id: Ibeb95be76a1e1b20c69841e708c2a2cbf1cbe05a Closes-Bug: #1302463 --- .../agent/linuxbridge_neutron_agent.py | 33 +++++++---------- .../unit/linuxbridge/test_lb_neutron_agent.py | 37 ++++++++----------- 2 files changed, 29 insertions(+), 41 deletions(-) diff --git a/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py b/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py index ec3b22d16..9fe433289 100755 --- a/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py +++ b/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py @@ -86,15 +86,6 @@ class LinuxBridgeManager: # Store network mapping to segments self.network_map = {} - def device_exists(self, device): - """Check if ethernet device exists.""" - try: - utils.execute(['ip', 'link', 'show', 'dev', device], - root_helper=self.root_helper) - except RuntimeError: - return False - return True - def interface_exists_on_bridge(self, bridge, interface): directory = '/sys/class/net/%s/brif' % bridge for filename in os.listdir(directory): @@ -139,7 +130,7 @@ class LinuxBridgeManager: return neutron_bridge_list def get_interfaces_on_bridge(self, bridge_name): - if self.device_exists(bridge_name): + if ip_lib.device_exists(bridge_name, root_helper=self.root_helper): bridge_interface_path = BRIDGE_INTERFACES_FS.replace( BRIDGE_NAME_PLACEHOLDER, bridge_name) return os.listdir(bridge_interface_path) @@ -221,7 +212,7 @@ class LinuxBridgeManager: def ensure_vlan(self, physical_interface, vlan_id): """Create a vlan unless it already exists.""" interface = self.get_subinterface_name(physical_interface, vlan_id) - if not self.device_exists(interface): + if not ip_lib.device_exists(interface, root_helper=self.root_helper): LOG.debug(_("Creating subinterface %(interface)s for " "VLAN %(vlan_id)s on interface " "%(physical_interface)s"), @@ -241,7 +232,7 @@ class LinuxBridgeManager: def ensure_vxlan(self, segmentation_id): """Create a vxlan unless it already exists.""" interface = self.get_vxlan_device_name(segmentation_id) - if not self.device_exists(interface): + if not ip_lib.device_exists(interface, root_helper=self.root_helper): LOG.debug(_("Creating vxlan interface %(interface)s for " "VNI %(segmentation_id)s"), {'interface': interface, @@ -291,7 +282,7 @@ class LinuxBridgeManager: def ensure_bridge(self, bridge_name, interface=None, ips=None, gateway=None): """Create a bridge unless it already exists.""" - if not self.device_exists(bridge_name): + if not ip_lib.device_exists(bridge_name, root_helper=self.root_helper): LOG.debug(_("Starting bridge %(bridge_name)s for subinterface " "%(interface)s"), {'bridge_name': bridge_name, 'interface': interface}) @@ -369,7 +360,8 @@ class LinuxBridgeManager: If a VIF has been plugged into a network, this function will add the corresponding tap device to the relevant bridge. """ - if not self.device_exists(tap_device_name): + if not ip_lib.device_exists(tap_device_name, + root_helper=self.root_helper): LOG.debug(_("Tap device: %s does not exist on " "this host, skipped"), tap_device_name) return False @@ -413,7 +405,7 @@ class LinuxBridgeManager: tap_device_name) def delete_vlan_bridge(self, bridge_name): - if self.device_exists(bridge_name): + if ip_lib.device_exists(bridge_name, root_helper=self.root_helper): interfaces_on_bridge = self.get_interfaces_on_bridge(bridge_name) for interface in interfaces_on_bridge: self.remove_interface(bridge_name, interface) @@ -456,7 +448,7 @@ class LinuxBridgeManager: del self.network_map[network_id] def remove_interface(self, bridge_name, interface_name): - if self.device_exists(bridge_name): + if ip_lib.device_exists(bridge_name, root_helper=self.root_helper): if not self.is_device_on_bridge(interface_name): return True LOG.debug(_("Removing device %(interface_name)s from bridge " @@ -479,7 +471,7 @@ class LinuxBridgeManager: return False def delete_vlan(self, interface): - if self.device_exists(interface): + if ip_lib.device_exists(interface, root_helper=self.root_helper): LOG.debug(_("Deleting subinterface %s for vlan"), interface) if utils.execute(['ip', 'link', 'set', interface, 'down'], root_helper=self.root_helper): @@ -490,7 +482,7 @@ class LinuxBridgeManager: LOG.debug(_("Done deleting subinterface %s"), interface) def delete_vxlan(self, interface): - if self.device_exists(interface): + if ip_lib.device_exists(interface, root_helper=self.root_helper): LOG.debug(_("Deleting vxlan interface %s for vlan"), interface) int_vxlan = self.ip.device(interface) @@ -527,8 +519,9 @@ class LinuxBridgeManager: 'mode': 'VXLAN UCAST'}) return False for segmentation_id in range(1, constants.MAX_VXLAN_VNI + 1): - if not self.device_exists( - self.get_vxlan_device_name(segmentation_id)): + if not ip_lib.device_exists( + self.get_vxlan_device_name(segmentation_id), + root_helper=self.root_helper): break else: LOG.error(_('No valid Segmentation ID to perform UCAST test.')) diff --git a/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py b/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py index 2477d163c..cc7cb334a 100644 --- a/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py +++ b/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py @@ -168,12 +168,6 @@ class TestLinuxBridgeManager(base.BaseTestCase): self.lbm = linuxbridge_neutron_agent.LinuxBridgeManager( self.interface_mappings, self.root_helper) - def test_device_exists(self): - with mock.patch.object(utils, 'execute') as execute_fn: - self.assertTrue(self.lbm.device_exists("eth0")) - execute_fn.side_effect = RuntimeError() - self.assertFalse(self.lbm.device_exists("eth0")) - def test_interface_exists_on_bridge(self): with mock.patch.object(os, 'listdir') as listdir_fn: listdir_fn.return_value = ["abc"] @@ -223,15 +217,16 @@ class TestLinuxBridgeManager(base.BaseTestCase): def test_get_interfaces_on_bridge(self): with contextlib.nested( mock.patch.object(utils, 'execute'), - mock.patch.object(os, 'listdir') - ) as (exec_fn, listdir_fn): + mock.patch.object(os, 'listdir'), + mock.patch.object(ip_lib, 'device_exists', return_value=True) + ) as (exec_fn, listdir_fn, dev_exists_fn): listdir_fn.return_value = ["qbr1"] self.assertEqual(self.lbm.get_interfaces_on_bridge("br0"), ["qbr1"]) def test_get_interfaces_on_bridge_not_existing(self): - self.lbm.device_exists = mock.Mock(return_value=False) - self.assertEqual([], self.lbm.get_interfaces_on_bridge("br0")) + with mock.patch.object(ip_lib, 'device_exists', return_value=False): + self.assertEqual([], self.lbm.get_interfaces_on_bridge("br0")) def test_get_tap_devices_count(self): with mock.patch.object(os, 'listdir') as listdir_fn: @@ -337,7 +332,7 @@ class TestLinuxBridgeManager(base.BaseTestCase): ens_fn.assert_called_once_with("brq54321") def test_ensure_vlan(self): - with mock.patch.object(self.lbm, 'device_exists') as de_fn: + with mock.patch.object(ip_lib, 'device_exists') as de_fn: de_fn.return_value = True self.assertEqual(self.lbm.ensure_vlan("eth0", "1"), "eth0.1") de_fn.return_value = False @@ -353,7 +348,7 @@ class TestLinuxBridgeManager(base.BaseTestCase): seg_id = "12345678" self.lbm.local_int = 'eth0' self.lbm.vxlan_mode = lconst.VXLAN_MCAST - with mock.patch.object(self.lbm, 'device_exists') as de_fn: + with mock.patch.object(ip_lib, 'device_exists') as de_fn: de_fn.return_value = True self.assertEqual(self.lbm.ensure_vxlan(seg_id), "vxlan-" + seg_id) de_fn.return_value = False @@ -401,7 +396,7 @@ class TestLinuxBridgeManager(base.BaseTestCase): def test_ensure_bridge(self): with contextlib.nested( - mock.patch.object(self.lbm, 'device_exists'), + mock.patch.object(ip_lib, 'device_exists'), mock.patch.object(utils, 'execute'), mock.patch.object(self.lbm, 'update_interface_ip_details'), mock.patch.object(self.lbm, 'interface_exists_on_bridge'), @@ -466,7 +461,7 @@ class TestLinuxBridgeManager(base.BaseTestCase): self.assertTrue(vlbr_fn.called) def test_add_tap_interface(self): - with mock.patch.object(self.lbm, "device_exists") as de_fn: + with mock.patch.object(ip_lib, "device_exists") as de_fn: de_fn.return_value = False self.assertFalse( self.lbm.add_tap_interface("123", p_const.TYPE_VLAN, @@ -511,7 +506,7 @@ class TestLinuxBridgeManager(base.BaseTestCase): def test_delete_vlan_bridge(self): with contextlib.nested( - mock.patch.object(self.lbm, "device_exists"), + mock.patch.object(ip_lib, "device_exists"), mock.patch.object(self.lbm, "get_interfaces_on_bridge"), mock.patch.object(self.lbm, "remove_interface"), mock.patch.object(self.lbm, "get_interface_details"), @@ -534,7 +529,7 @@ class TestLinuxBridgeManager(base.BaseTestCase): def test_delete_vlan_bridge_with_ip(self): with contextlib.nested( - mock.patch.object(self.lbm, "device_exists"), + mock.patch.object(ip_lib, "device_exists"), mock.patch.object(self.lbm, "get_interfaces_on_bridge"), mock.patch.object(self.lbm, "remove_interface"), mock.patch.object(self.lbm, "get_interface_details"), @@ -553,7 +548,7 @@ class TestLinuxBridgeManager(base.BaseTestCase): def test_delete_vlan_bridge_no_ip(self): with contextlib.nested( - mock.patch.object(self.lbm, "device_exists"), + mock.patch.object(ip_lib, "device_exists"), mock.patch.object(self.lbm, "get_interfaces_on_bridge"), mock.patch.object(self.lbm, "remove_interface"), mock.patch.object(self.lbm, "get_interface_details"), @@ -576,7 +571,7 @@ class TestLinuxBridgeManager(base.BaseTestCase): interface_mappings, self.root_helper) with contextlib.nested( - mock.patch.object(lbm, "device_exists"), + mock.patch.object(ip_lib, "device_exists"), mock.patch.object(lbm, "get_interfaces_on_bridge"), mock.patch.object(lbm, "remove_interface"), mock.patch.object(lbm, "delete_vxlan"), @@ -608,7 +603,7 @@ class TestLinuxBridgeManager(base.BaseTestCase): def test_remove_interface(self): with contextlib.nested( - mock.patch.object(self.lbm, "device_exists"), + mock.patch.object(ip_lib, "device_exists"), mock.patch.object(self.lbm, "is_device_on_bridge"), mock.patch.object(utils, "execute") ) as (de_fn, isdev_fn, exec_fn): @@ -629,7 +624,7 @@ class TestLinuxBridgeManager(base.BaseTestCase): def test_delete_vlan(self): with contextlib.nested( - mock.patch.object(self.lbm, "device_exists"), + mock.patch.object(ip_lib, "device_exists"), mock.patch.object(utils, "execute") ) as (de_fn, exec_fn): de_fn.return_value = False @@ -708,7 +703,7 @@ class TestLinuxBridgeManager(base.BaseTestCase): cfg.CONF.set_override('l2_population', l2_population, 'VXLAN') with contextlib.nested( mock.patch.object( - self.lbm, 'device_exists', return_value=False), + ip_lib, 'device_exists', return_value=False), mock.patch.object(self.lbm, 'delete_vxlan', return_value=None), mock.patch.object(self.lbm, 'ensure_vxlan', return_value=None), mock.patch.object( -- 2.45.2