From aaf11f45ecf8f832301491017a8009f1897e8d43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C5=82awek=20Kap=C5=82o=C5=84ski?= Date: Fri, 16 Mar 2018 15:58:10 +0100 Subject: [PATCH] Switch IPDevice.exists() method to use pyroute2 Check if network device exists is now done by checking interface index with pyroute2 interface instead of checking if MAC address for device is set. Change-Id: I2d5b95ec109fb19fc2a46c1017959f74011b9a22 Related-Bug: #1492714 --- neutron/agent/linux/ip_lib.py | 10 +-------- neutron/cmd/linuxbridge_cleanup.py | 3 ++- neutron/privileged/agent/linux/ip_lib.py | 13 +++++++++++ neutron/tests/base.py | 13 ++++++++--- .../cmd/test_linuxbridge_cleanup.py | 11 ++++++++++ neutron/tests/unit/agent/linux/test_ip_lib.py | 22 ------------------- 6 files changed, 37 insertions(+), 35 deletions(-) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index caecae48387..14e97f31200 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -294,15 +294,7 @@ class IPDevice(SubProcessBase): def exists(self): """Return True if the device exists in the namespace.""" - # we must save and restore this before returning - orig_log_fail_as_error = self.get_log_fail_as_error() - self.set_log_fail_as_error(False) - try: - return bool(self.link.address) - except RuntimeError: - return False - finally: - self.set_log_fail_as_error(orig_log_fail_as_error) + return privileged.interface_exists(self.name, self.namespace) def delete_addr_and_conntrack_state(self, cidr): """Delete an address along with its conntrack state diff --git a/neutron/cmd/linuxbridge_cleanup.py b/neutron/cmd/linuxbridge_cleanup.py index 9853c6e2b7b..34e22ce3847 100644 --- a/neutron/cmd/linuxbridge_cleanup.py +++ b/neutron/cmd/linuxbridge_cleanup.py @@ -16,7 +16,7 @@ from neutron_lib.utils import helpers from oslo_config import cfg from oslo_log import log as logging -from neutron.common import config +from neutron.conf.agent import common as config from neutron.plugins.ml2.drivers.linuxbridge.agent \ import linuxbridge_neutron_agent @@ -69,4 +69,5 @@ def main(): """ cfg.CONF(sys.argv[1:]) config.setup_logging() + config.setup_privsep() remove_empty_bridges() diff --git a/neutron/privileged/agent/linux/ip_lib.py b/neutron/privileged/agent/linux/ip_lib.py index 7e3750f3be5..858d71b0332 100644 --- a/neutron/privileged/agent/linux/ip_lib.py +++ b/neutron/privileged/agent/linux/ip_lib.py @@ -194,6 +194,19 @@ def delete_interface(ifname, namespace, **kwargs): _run_iproute_link("del", ifname, namespace, **kwargs) +@privileged.default.entrypoint +def interface_exists(ifname, namespace): + try: + idx = _get_link_id(ifname, namespace) + return bool(idx) + except NetworkInterfaceNotFound: + return False + except OSError as e: + if e.errno == errno.ENOENT: + return False + raise + + @privileged.default.entrypoint def add_neigh_entry(ip_version, ip_address, mac_address, device, namespace, **kwargs): diff --git a/neutron/tests/base.py b/neutron/tests/base.py index d57050c29a4..3abfc4b7158 100644 --- a/neutron/tests/base.py +++ b/neutron/tests/base.py @@ -112,6 +112,14 @@ def unstable_test(reason): return decor +def get_rootwrap_cmd(): + return os.environ.get('OS_ROOTWRAP_CMD', SUDO_CMD) + + +def get_rootwrap_daemon_cmd(): + return os.environ.get('OS_ROOTWRAP_DAEMON_CMD') + + class AttributeDict(dict): """ @@ -424,10 +432,9 @@ class BaseTestCase(DietTestCase): def setup_rootwrap(self): agent_config.register_root_helper(cfg.CONF) self.config(group='AGENT', - root_helper=os.environ.get('OS_ROOTWRAP_CMD', SUDO_CMD)) + root_helper=get_rootwrap_cmd()) self.config(group='AGENT', - root_helper_daemon=os.environ.get( - 'OS_ROOTWRAP_DAEMON_CMD')) + root_helper_daemon=get_rootwrap_daemon_cmd()) class PluginFixture(fixtures.Fixture): diff --git a/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py b/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py index 4d7c9b4b2d2..06d195c1d45 100644 --- a/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py +++ b/neutron/tests/functional/cmd/test_linuxbridge_cleanup.py @@ -20,6 +20,7 @@ from neutron_lib import constants from neutron.agent.linux import ip_lib from neutron.plugins.ml2.drivers.linuxbridge.agent import \ linuxbridge_neutron_agent as lb_agent +from neutron.tests import base as tests_base from neutron.tests.common import config_fixtures from neutron.tests.common import net_helpers from neutron.tests.functional import base @@ -35,6 +36,16 @@ class LinuxbridgeCleanupTest(base.BaseSudoTestCase): prefix=lb_agent.BRIDGE_NAME_PREFIX))).fixture config = callback(br_fixture) + # NOTE(slaweq): use of oslo.privsep inside neutron-linuxbridge-cleanup + # script requires rootwrap helper to be configured in this script's + # config + config.update({ + 'AGENT': { + 'root_helper': tests_base.get_rootwrap_cmd(), + 'root_helper_daemon': tests_base.get_rootwrap_daemon_cmd() + } + }) + config.update({'VXLAN': {'enable_vxlan': 'False'}}) temp_dir = self.useFixture(fixtures.TempDir()).path diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index b23a3dce132..97730415eaa 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -1383,28 +1383,6 @@ class TestIpNetnsCommand(TestIPCmdBase): class TestDeviceExists(base.BaseTestCase): - def test_device_exists(self): - with mock.patch('neutron.agent.common.utils.execute') as execute: - execute.return_value = LINK_SAMPLE[1] - self.assertTrue(ip_lib.device_exists('eth0')) - execute.assert_called_once_with( - ['ip', '-o', 'link', 'show', 'eth0'], - run_as_root=False, log_fail_as_error=False) - - def test_device_exists_reset_fail(self): - device = ip_lib.IPDevice('eth0') - device.set_log_fail_as_error(True) - with mock.patch.object(ip_lib.IPDevice, '_execute') as _execute: - _execute.return_value = LINK_SAMPLE[1] - self.assertTrue(device.exists()) - self.assertTrue(device.get_log_fail_as_error()) - - def test_device_does_not_exist(self): - with mock.patch.object(ip_lib.IPDevice, '_execute') as _execute: - _execute.return_value = '' - _execute.side_effect = RuntimeError - self.assertFalse(ip_lib.device_exists('eth0')) - def test_ensure_device_is_ready(self): ip_lib_mock = mock.Mock() with mock.patch.object(ip_lib, 'IPDevice', return_value=ip_lib_mock):