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
This commit is contained in:
Ihar Hrachyshka 2017-04-10 12:21:41 -07:00
parent 91c15edf54
commit 1ae91ce9be
11 changed files with 15 additions and 30 deletions

View File

@ -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)

View File

@ -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)

View File

@ -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,

View File

@ -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,

View File

@ -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

View File

@ -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)

View File

@ -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."""

View File

@ -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()

View File

@ -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)

View File

@ -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):

View File

@ -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