]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Improve performance of ensure_namespace
authorJohn Kasperski <jckasper@us.ibm.com>
Thu, 24 Sep 2015 23:16:18 +0000 (18:16 -0500)
committerJohn Kasperski <jckasper@linux.vnet.ibm.com>
Tue, 13 Oct 2015 20:29:59 +0000 (20:29 +0000)
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
neutron/tests/functional/agent/linux/test_ip_lib.py
neutron/tests/unit/agent/linux/test_ip_lib.py

index 17600812de9eb121b062fd4f4c059d7869cb8e9e..abbacb1412a681851b07303cd1368d55c6910084 100644 (file)
@@ -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
index 0aa39d113ae40acb9705b3653fb064793434b7bc..2359925d0617cc94122b3948bff03bdf30466354 100644 (file)
@@ -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()
 
index c49e44373cdba27d4ff18eaf1d07bb2024645563..2aa57ccc65e114206c6ef971306fa2e5a4e3b191 100644 (file)
@@ -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: