From 1ae91ce9be56fd6952d53ce3f8b094a6958b2709 Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Mon, 10 Apr 2017 12:21:41 -0700 Subject: [PATCH] ip_lib: ignore gre and lo devices in get_devices by default This is the most common use pattern for the method, so it makes sense to make it default. (Actually, it may be that there are no usage for the arguments whatsoever, but better safe than sorry.) NeutronLibImpact this change potentially breaks callers of get_devices that may want to get the automatic devices by default. Those imaginary callers may need to set exclude_gre_devices and/or exclude_loopback to True from now on. Change-Id: Ic32b8abc7f8502b8907ae21c996e13cb8fd5401d Related-Bug: #1604115 --- neutron/agent/l3/dvr_edge_router.py | 3 +-- neutron/agent/l3/dvr_fip_ns.py | 3 +-- neutron/agent/l3/dvr_snat_ns.py | 3 +-- neutron/agent/l3/namespaces.py | 3 +-- neutron/agent/l3/router_info.py | 3 +-- neutron/agent/linux/dhcp.py | 3 +-- neutron/agent/linux/ip_lib.py | 5 ++--- neutron/cmd/netns_cleanup.py | 3 +-- neutron/tests/functional/agent/linux/test_dhcp.py | 11 +++-------- neutron/tests/unit/agent/linux/test_ip_lib.py | 4 ++-- neutron/tests/unit/cmd/test_netns_cleanup.py | 4 +--- 11 files changed, 15 insertions(+), 30 deletions(-) diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py index 11f31bc5372..514a83a8699 100644 --- a/neutron/agent/l3/dvr_edge_router.py +++ b/neutron/agent/l3/dvr_edge_router.py @@ -251,8 +251,7 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter): return ns_ip = ip_lib.IPWrapper(namespace=self.snat_namespace.name) - for d in ns_ip.get_devices(exclude_loopback=True, - exclude_gre_devices=True): + for d in ns_ip.get_devices(): if (d.name.startswith(router.EXTERNAL_DEV_PREFIX) and d.name != interface_name): LOG.debug('Deleting stale external router device: %s', d.name) diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py index 09c12817ed8..f86ab4c2cf2 100644 --- a/neutron/agent/l3/dvr_fip_ns.py +++ b/neutron/agent/l3/dvr_fip_ns.py @@ -207,8 +207,7 @@ class FipNamespace(namespaces.Namespace): @namespaces.check_ns_existence def _delete(self): ip_wrapper = ip_lib.IPWrapper(namespace=self.name) - for d in ip_wrapper.get_devices(exclude_loopback=True, - exclude_gre_devices=True): + for d in ip_wrapper.get_devices(): if d.name.startswith(FIP_2_ROUTER_DEV_PREFIX): # internal link between IRs and FIP NS ip_wrapper.del_veth(d.name) diff --git a/neutron/agent/l3/dvr_snat_ns.py b/neutron/agent/l3/dvr_snat_ns.py index 085dc5cbc47..f5f76fe56ed 100644 --- a/neutron/agent/l3/dvr_snat_ns.py +++ b/neutron/agent/l3/dvr_snat_ns.py @@ -41,8 +41,7 @@ class SnatNamespace(namespaces.Namespace): @namespaces.check_ns_existence def delete(self): ns_ip = ip_lib.IPWrapper(namespace=self.name) - for d in ns_ip.get_devices(exclude_loopback=True, - exclude_gre_devices=True): + for d in ns_ip.get_devices(): if d.name.startswith(constants.SNAT_INT_DEV_PREFIX): LOG.debug('Unplugging DVR device %s', d.name) self.driver.unplug(d.name, namespace=self.name, diff --git a/neutron/agent/l3/namespaces.py b/neutron/agent/l3/namespaces.py index f65c706c7b4..6db124e805b 100644 --- a/neutron/agent/l3/namespaces.py +++ b/neutron/agent/l3/namespaces.py @@ -123,8 +123,7 @@ class RouterNamespace(Namespace): @check_ns_existence def delete(self): ns_ip = ip_lib.IPWrapper(namespace=self.name) - for d in ns_ip.get_devices(exclude_loopback=True, - exclude_gre_devices=True): + for d in ns_ip.get_devices(): if d.name.startswith(INTERNAL_DEV_PREFIX): # device is on default bridge self.driver.unplug(d.name, namespace=self.name, diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index 459a882af8e..45ce625103d 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -468,8 +468,7 @@ class RouterInfo(object): def _get_existing_devices(self): ip_wrapper = ip_lib.IPWrapper(namespace=self.ns_name) - ip_devs = ip_wrapper.get_devices(exclude_loopback=True, - exclude_gre_devices=True) + ip_devs = ip_wrapper.get_devices() return [ip_dev.name for ip_dev in ip_devs] @staticmethod diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 810f01f5c5f..6628a8d858f 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -1337,8 +1337,7 @@ class DeviceManager(object): ns_ip = ip_lib.IPWrapper(namespace=network.namespace) if not ns_ip.netns.exists(network.namespace): return - for d in ns_ip.get_devices(exclude_loopback=True, - exclude_gre_devices=True): + for d in ns_ip.get_devices(): # delete all devices except current active DHCP port device if d.name != skip_dev_name: LOG.debug("Found stale device %s, deleting", d.name) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index ff931d50fae..c715588a888 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -122,7 +122,7 @@ class IPWrapper(SubProcessBase): def device(self, name): return IPDevice(name, namespace=self.namespace) - def get_devices(self, exclude_loopback=False, exclude_gre_devices=False): + def get_devices(self, exclude_loopback=True, exclude_gre_devices=True): retval = [] if self.namespace: # we call out manually because in order to avoid screen scraping @@ -215,8 +215,7 @@ class IPWrapper(SubProcessBase): return ip def namespace_is_empty(self): - return not self.get_devices(exclude_loopback=True, - exclude_gre_devices=True) + return not self.get_devices() def garbage_collect_namespace(self): """Conditionally destroy the namespace if it is empty.""" diff --git a/neutron/cmd/netns_cleanup.py b/neutron/cmd/netns_cleanup.py index 3729560928e..b16f82b2a06 100644 --- a/neutron/cmd/netns_cleanup.py +++ b/neutron/cmd/netns_cleanup.py @@ -248,8 +248,7 @@ def destroy_namespace(conf, namespace, force=False): # the error and continue with the cleanup LOG.error(_LE('Not all processes were killed in %s'), namespace) - for device in ip.get_devices(exclude_loopback=True, - exclude_gre_devices=True): + for device in ip.get_devices(): unplug_device(conf, device) ip.garbage_collect_namespace() diff --git a/neutron/tests/functional/agent/linux/test_dhcp.py b/neutron/tests/functional/agent/linux/test_dhcp.py index 6ac3bf88c85..f66fbc3f6a4 100644 --- a/neutron/tests/functional/agent/linux/test_dhcp.py +++ b/neutron/tests/functional/agent/linux/test_dhcp.py @@ -12,8 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -import functools - import mock from oslo_config import cfg @@ -77,19 +75,16 @@ class TestDhcp(functional_base.BaseSudoTestCase): "10:22:33:44:55:69", namespace="qdhcp-foo_id") ipw = ip_lib.IPWrapper(namespace="qdhcp-foo_id") - get_devices = functools.partial( - ipw.get_devices, - exclude_loopback=True, exclude_gre_devices=True) - devices = get_devices() + devices = ipw.get_devices() self.addCleanup(ipw.netns.delete, 'qdhcp-foo_id') self.assertEqual(sorted(["tapfoo_id2", "tapfoo_id3"]), sorted(map(str, devices))) # setting up dhcp for the network dev_mgr.setup(tests_base.AttributeDict(network)) common_utils.wait_until_true( - lambda: 1 == len(get_devices()), + lambda: 1 == len(ipw.get_devices()), timeout=5, sleep=0.1, exception=RuntimeError("only one non-loopback device must remain")) - devices = get_devices() + devices = ipw.get_devices() self.assertEqual("tapfoo_port_id", devices[0].name) diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index 3c0a4bcceef..c287b0920c8 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -275,7 +275,7 @@ class TestIpWrapper(base.BaseTestCase): def test_get_devices(self, mocked_listdir, mocked_islink): retval = ip_lib.IPWrapper().get_devices() mocked_islink.assert_called_once_with('/sys/class/net/lo') - self.assertEqual(retval, [ip_lib.IPDevice('lo')]) + self.assertEqual([], retval) @mock.patch('neutron.agent.common.utils.execute') def test_get_devices_namespaces(self, mocked_execute): @@ -288,7 +288,7 @@ class TestIpWrapper(base.BaseTestCase): '-maxdepth', '1', '-type', 'l', '-printf', '%f '], run_as_root=True, log_fail_as_error=True) self.assertTrue(fake_str.split.called) - self.assertEqual(retval, [ip_lib.IPDevice('lo', namespace='foo')]) + self.assertEqual([], retval) @mock.patch('neutron.agent.common.utils.execute') def test_get_devices_namespaces_ns_not_exists(self, mocked_execute): diff --git a/neutron/tests/unit/cmd/test_netns_cleanup.py b/neutron/tests/unit/cmd/test_netns_cleanup.py index cbf030d677b..5ce69fb381d 100644 --- a/neutron/tests/unit/cmd/test_netns_cleanup.py +++ b/neutron/tests/unit/cmd/test_netns_cleanup.py @@ -335,9 +335,7 @@ class TestNetnsCleanup(base.BaseTestCase): if force: expected.extend([ mock.call().netns.exists(ns), - mock.call().get_devices( - exclude_loopback=True, - exclude_gre_devices=True)]) + mock.call().get_devices()]) self.assertTrue(kill_dhcp.called) unplug.assert_has_calls( [mock.call(conf, d) for d in