From 9cdd3310363b445e5751fc848e1354f7f52a0547 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 21 Oct 2021 08:49:41 +0000 Subject: [PATCH] Check a namespace existence by checking only its own directory To check the existance of a namespace, instead of listing the namespaces directory (by default "/var/run/netns"), this patch directly checks the existence of the namespace directory, using "os.path.exists". This check is faster than listing the whole directory and avoids timeout problems as reported in the related bug. Conflicts: neutron/privileged/agent/linux/utils.py Closes-Bug: #1947974 Change-Id: I558d50d28378beb3710d98a2113ff9549c82ae17 (cherry picked from commit 8127221479834350465251839ab88056f4f64fb4) (cherry picked from commit 6a9c05a92498630b85102eb6af1eccb415fd605a) --- neutron/agent/linux/ip_lib.py | 13 ++++++++-- neutron/privileged/agent/linux/utils.py | 10 ++++++-- neutron/tests/common/net_helpers.py | 4 +-- .../functional/agent/linux/test_ip_lib.py | 10 ++++++-- neutron/tests/unit/agent/l3/test_agent.py | 3 +++ neutron/tests/unit/agent/linux/test_ip_lib.py | 25 ------------------- 6 files changed, 32 insertions(+), 33 deletions(-) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 289065a620f..c0ce718348a 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -14,6 +14,7 @@ # under the License. import errno +from os import path import re import threading import time @@ -35,6 +36,7 @@ from neutron.agent.common import utils from neutron.common import ipv6_utils from neutron.common import utils as common_utils from neutron.privileged.agent.linux import ip_lib as privileged +from neutron.privileged.agent.linux import utils as priv_utils LOG = logging.getLogger(__name__) @@ -931,9 +933,16 @@ def network_namespace_exists(namespace, try_is_ready=False, **kwargs): is ready to be operated. :param kwargs: Callers add any filters they use as kwargs """ + if not namespace: + return False + if not try_is_ready: - output = list_network_namespaces(**kwargs) - return namespace in output + nspath = kwargs.get('nspath') or netns.NETNS_RUN_DIR + nspath += '/' + namespace + if cfg.CONF.AGENT.use_helper_for_ns_read: + return priv_utils.path_exists(nspath) + else: + return path.exists(nspath) try: privileged.open_namespace(namespace) diff --git a/neutron/privileged/agent/linux/utils.py b/neutron/privileged/agent/linux/utils.py index edceb643d61..ecbfc2c518a 100644 --- a/neutron/privileged/agent/linux/utils.py +++ b/neutron/privileged/agent/linux/utils.py @@ -13,6 +13,7 @@ # under the License. import os +from os import path import re from oslo_concurrency import processutils @@ -45,5 +46,10 @@ def _find_listen_pids_namespace(namespace): @privileged.default.entrypoint -def delete_if_exists(path, remove=os.unlink): - fileutils.delete_if_exists(path, remove=remove) +def delete_if_exists(_path, remove=os.unlink): + fileutils.delete_if_exists(_path, remove=remove) + + +@privileged.default.entrypoint +def path_exists(_path): + return path.exists(_path) diff --git a/neutron/tests/common/net_helpers.py b/neutron/tests/common/net_helpers.py index 436e6045e5b..a070ec4eb41 100644 --- a/neutron/tests/common/net_helpers.py +++ b/neutron/tests/common/net_helpers.py @@ -620,8 +620,8 @@ class VethFixture(fixtures.Fixture): def destroy(self): for port in self.ports: ip_wrapper = ip_lib.IPWrapper(port.namespace) - if (ip_wrapper.netns.exists(port.namespace) or - port.namespace is None): + if (port.namespace is None or + ip_wrapper.netns.exists(port.namespace)): try: ip_wrapper.del_veth(port.name) break diff --git a/neutron/tests/functional/agent/linux/test_ip_lib.py b/neutron/tests/functional/agent/linux/test_ip_lib.py index f8dcc8b44b5..8fafefe7741 100644 --- a/neutron/tests/functional/agent/linux/test_ip_lib.py +++ b/neutron/tests/functional/agent/linux/test_ip_lib.py @@ -660,10 +660,16 @@ class NamespaceTestCase(functional_base.BaseSudoTestCase): ip_lib.delete_network_namespace(self.namespace) def test_network_namespace_exists_ns_exists(self): - self.assertTrue(ip_lib.network_namespace_exists(self.namespace)) + for use_helper_for_ns_read in (True, False): + cfg.CONF.set_override('use_helper_for_ns_read', + use_helper_for_ns_read, 'AGENT') + self.assertTrue(ip_lib.network_namespace_exists(self.namespace)) def test_network_namespace_exists_ns_doesnt_exists(self): - self.assertFalse(ip_lib.network_namespace_exists('another_ns')) + for use_helper_for_ns_read in (True, False): + cfg.CONF.set_override('use_helper_for_ns_read', + use_helper_for_ns_read, 'AGENT') + self.assertFalse(ip_lib.network_namespace_exists('another_ns')) def test_network_namespace_exists_ns_exists_try_is_ready(self): self.assertTrue(ip_lib.network_namespace_exists(self.namespace, diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 17e9216d593..07bdbc7aa50 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -61,6 +61,7 @@ from neutron.conf.agent import common as agent_config from neutron.conf.agent.l3 import config as l3_config from neutron.conf.agent.l3 import ha as ha_conf from neutron.conf import common as base_config +from neutron.privileged.agent.linux import utils as priv_utils from neutron.tests import base from neutron.tests.common import l3_test_common from neutron.tests.unit.agent.linux.test_utils import FakeUser @@ -101,6 +102,8 @@ class BasicRouterOperationsFramework(base.BaseTestCase): self.list_network_namespaces_p = mock.patch( 'neutron.agent.linux.ip_lib.list_network_namespaces') self.list_network_namespaces = self.list_network_namespaces_p.start() + self.path_exists_p = mock.patch.object(priv_utils, 'path_exists') + self.path_exists = self.path_exists_p.start() self.ensure_dir = mock.patch( 'oslo_utils.fileutils.ensure_tree').start() diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index a324658a25e..eb6b51baa76 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -863,31 +863,6 @@ class TestIpNetnsCommand(TestIPCmdBase): self.netns_cmd.delete('ns') remove.assert_called_once_with('ns') - @mock.patch.object(pyroute2.netns, 'listnetns') - @mock.patch.object(priv_lib, 'list_netns') - def test_namespace_exists_use_helper(self, priv_listnetns, listnetns): - self.config(group='AGENT', use_helper_for_ns_read=True) - priv_listnetns.return_value = NETNS_SAMPLE - # need another instance to avoid mocking - netns_cmd = ip_lib.IpNetnsCommand(ip_lib.SubProcessBase()) - self.assertTrue( - netns_cmd.exists('bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb')) - self.assertEqual(1, priv_listnetns.call_count) - self.assertFalse(listnetns.called) - - @mock.patch.object(pyroute2.netns, 'listnetns') - @mock.patch.object(priv_lib, 'list_netns') - def test_namespace_does_not_exist_no_helper(self, priv_listnetns, - listnetns): - self.config(group='AGENT', use_helper_for_ns_read=False) - listnetns.return_value = NETNS_SAMPLE - # need another instance to avoid mocking - netns_cmd = ip_lib.IpNetnsCommand(ip_lib.SubProcessBase()) - self.assertFalse( - netns_cmd.exists('bbbbbbbb-1111-2222-3333-bbbbbbbbbbbb')) - self.assertEqual(1, listnetns.call_count) - self.assertFalse(priv_listnetns.called) - def test_execute(self): self.parent.namespace = 'ns' with mock.patch('neutron.agent.common.utils.execute') as execute: