From 81823e86328e62850a89aef9f0b609bfc0a6dacd Mon Sep 17 00:00:00 2001 From: John Kasperski Date: Thu, 24 Sep 2015 18:16:18 -0500 Subject: [PATCH] Improve performance of ensure_namespace The ensure_namespace method calls IpNetnsCommand.exists to determine if the specified namespace exists or not. This is accomplished by listing all namespaces with "ip netns list" and then looping through the output to determine if the specified namespace was included in the output. Research of various Linux operating systems has indicated that namespaces are represented as files in /var/run/netns and root authority is "typically" not required in order to look at the files in this subdirectory. The existing configuration option "use_helper_for_ns_read" will be used to determine if the root-helper should be used to to retrieve the list of namespaces. If this configuraton option is set to False, the native python os.listdir(/var/run/netns) will be used. Related-Bug: #1311804 Closes-Bug: #1497396 Change-Id: I9da627d07d6cbb6e5ef1a921a5f22963317a04e2 --- neutron/agent/linux/ip_lib.py | 12 +++-- .../functional/agent/linux/test_ip_lib.py | 6 +++ neutron/tests/unit/agent/linux/test_ip_lib.py | 45 +++++++------------ 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 17600812d..abbacb141 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -38,6 +38,7 @@ OPTS = [ LOOPBACK_DEVNAME = 'lo' +IP_NETNS_PATH = '/var/run/netns' SYS_NET_PATH = '/sys/class/net' DEFAULT_GW_PATTERN = re.compile(r"via (\S+)") METRIC_PATTERN = re.compile(r"metric (\S+)") @@ -210,7 +211,10 @@ class IPWrapper(SubProcessBase): @classmethod def get_namespaces(cls): - output = cls._execute([], 'netns', ('list',)) + if not cfg.CONF.AGENT.use_helper_for_ns_read: + return os.listdir(IP_NETNS_PATH) + + output = cls._execute([], 'netns', ['list'], run_as_root=True) return [l.split()[0] for l in output.splitlines()] @@ -833,9 +837,11 @@ class IpNetnsCommand(IpCommandBase): extra_ok_codes=extra_ok_codes, **kwargs) def exists(self, name): + if not cfg.CONF.AGENT.use_helper_for_ns_read: + return name in os.listdir(IP_NETNS_PATH) + output = self._parent._execute( - ['o'], 'netns', ['list'], - run_as_root=cfg.CONF.AGENT.use_helper_for_ns_read) + ['o'], 'netns', ['list'], run_as_root=True) for line in [l.split()[0] for l in output.splitlines()]: if name == line: return True diff --git a/neutron/tests/functional/agent/linux/test_ip_lib.py b/neutron/tests/functional/agent/linux/test_ip_lib.py index 0aa39d113..2359925d0 100644 --- a/neutron/tests/functional/agent/linux/test_ip_lib.py +++ b/neutron/tests/functional/agent/linux/test_ip_lib.py @@ -86,6 +86,12 @@ class IpLibTestFramework(functional_base.BaseSudoTestCase): class IpLibTestCase(IpLibTestFramework): + def test_namespace_exists(self): + namespace = self.useFixture(net_helpers.NamespaceFixture()) + self.assertTrue(namespace.ip_wrapper.netns.exists(namespace.name)) + namespace.destroy() + self.assertFalse(namespace.ip_wrapper.netns.exists(namespace.name)) + def test_device_exists(self): attr = self.generate_device_details() diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index c49e44373..2aa57ccc6 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -15,6 +15,7 @@ import mock import netaddr +from oslo_config import cfg import testtools from neutron.agent.common import utils # noqa @@ -276,23 +277,37 @@ class TestIpWrapper(base.BaseTestCase): def test_get_namespaces(self): self.execute.return_value = '\n'.join(NETNS_SAMPLE) + cfg.CONF.AGENT.use_helper_for_ns_read = True retval = ip_lib.IPWrapper.get_namespaces() self.assertEqual(retval, ['12345678-1234-5678-abcd-1234567890ab', 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb', 'cccccccc-cccc-cccc-cccc-cccccccccccc']) - self.execute.assert_called_once_with([], 'netns', ('list',)) + self.execute.assert_called_once_with([], 'netns', ['list'], + run_as_root=True) def test_get_namespaces_iproute2_4(self): self.execute.return_value = '\n'.join(NETNS_SAMPLE_IPROUTE2_4) + cfg.CONF.AGENT.use_helper_for_ns_read = True retval = ip_lib.IPWrapper.get_namespaces() self.assertEqual(retval, ['12345678-1234-5678-abcd-1234567890ab', 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb', 'cccccccc-cccc-cccc-cccc-cccccccccccc']) - self.execute.assert_called_once_with([], 'netns', ('list',)) + self.execute.assert_called_once_with([], 'netns', ['list'], + run_as_root=True) + + @mock.patch('os.listdir', return_value=NETNS_SAMPLE) + def test_get_namespaces_listdir(self, mocked_listdir): + cfg.CONF.AGENT.use_helper_for_ns_read = False + retval = ip_lib.IPWrapper.get_namespaces() + self.assertEqual(retval, + ['12345678-1234-5678-abcd-1234567890ab', + 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb', + 'cccccccc-cccc-cccc-cccc-cccccccccccc']) + mocked_listdir.assert_called_once_with(ip_lib.IP_NETNS_PATH) def test_add_tuntap(self): ip_lib.IPWrapper().add_tuntap('tap0') @@ -1173,32 +1188,6 @@ class TestIpNetnsCommand(TestIPCmdBase): self.netns_cmd.delete('ns') self._assert_sudo([], ('delete', 'ns'), use_root_namespace=True) - def test_namespace_exists_use_helper(self): - self.config(group='AGENT', use_helper_for_ns_read=True) - retval = '\n'.join(NETNS_SAMPLE) - # need another instance to avoid mocking - netns_cmd = ip_lib.IpNetnsCommand(ip_lib.SubProcessBase()) - with mock.patch('neutron.agent.common.utils.execute') as execute: - execute.return_value = retval - self.assertTrue( - netns_cmd.exists('bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb')) - execute.assert_called_once_with(['ip', '-o', 'netns', 'list'], - run_as_root=True, - log_fail_as_error=True) - - def test_namespace_doest_not_exist_no_helper(self): - self.config(group='AGENT', use_helper_for_ns_read=False) - retval = '\n'.join(NETNS_SAMPLE) - # need another instance to avoid mocking - netns_cmd = ip_lib.IpNetnsCommand(ip_lib.SubProcessBase()) - with mock.patch('neutron.agent.common.utils.execute') as execute: - execute.return_value = retval - self.assertFalse( - netns_cmd.exists('bbbbbbbb-1111-2222-3333-bbbbbbbbbbbb')) - execute.assert_called_once_with(['ip', '-o', 'netns', 'list'], - run_as_root=False, - log_fail_as_error=True) - def test_execute(self): self.parent.namespace = 'ns' with mock.patch('neutron.agent.common.utils.execute') as execute: -- 2.45.2