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.

Closes-Bug: #1947974
Change-Id: I558d50d28378beb3710d98a2113ff9549c82ae17
(cherry picked from commit 8127221479)
This commit is contained in:
Rodolfo Alonso Hernandez 2021-10-21 08:49:41 +00:00
parent 64069352e1
commit c05d07bd8f
6 changed files with 32 additions and 33 deletions

View File

@ -14,6 +14,7 @@
# under the License. # under the License.
import errno import errno
from os import path
import re import re
import threading import threading
import time import time
@ -37,6 +38,7 @@ from neutron._i18n import _
from neutron.agent.common import utils from neutron.agent.common import utils
from neutron.common import utils as common_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 ip_lib as privileged
from neutron.privileged.agent.linux import utils as priv_utils
LOG = logging.getLogger(__name__) LOG = logging.getLogger(__name__)
@ -921,9 +923,16 @@ def network_namespace_exists(namespace, try_is_ready=False, **kwargs):
is ready to be operated. is ready to be operated.
:param kwargs: Callers add any filters they use as kwargs :param kwargs: Callers add any filters they use as kwargs
""" """
if not namespace:
return False
if not try_is_ready: if not try_is_ready:
output = list_network_namespaces(**kwargs) nspath = kwargs.get('nspath') or netns.NETNS_RUN_DIR
return namespace in output nspath += '/' + namespace
if cfg.CONF.AGENT.use_helper_for_ns_read:
return priv_utils.path_exists(nspath)
else:
return path.exists(nspath)
try: try:
privileged.open_namespace(namespace) privileged.open_namespace(namespace)

View File

@ -13,6 +13,7 @@
# under the License. # under the License.
import os import os
from os import path
import re import re
from eventlet.green import subprocess from eventlet.green import subprocess
@ -47,8 +48,8 @@ def _find_listen_pids_namespace(namespace):
@privileged.default.entrypoint @privileged.default.entrypoint
def delete_if_exists(path, remove=os.unlink): def delete_if_exists(_path, remove=os.unlink):
fileutils.delete_if_exists(path, remove=remove) fileutils.delete_if_exists(_path, remove=remove)
@privileged.default.entrypoint @privileged.default.entrypoint
@ -82,3 +83,8 @@ def _create_process(cmd, addl_env=None):
obj = subprocess.Popen(cmd, shell=False, stdin=subprocess.PIPE, obj = subprocess.Popen(cmd, shell=False, stdin=subprocess.PIPE,
stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdout=subprocess.PIPE, stderr=subprocess.PIPE)
return obj, cmd return obj, cmd
@privileged.default.entrypoint
def path_exists(_path):
return path.exists(_path)

View File

@ -651,8 +651,8 @@ class VethFixture(fixtures.Fixture):
def destroy(self): def destroy(self):
for port in self.ports: for port in self.ports:
ip_wrapper = ip_lib.IPWrapper(port.namespace) ip_wrapper = ip_lib.IPWrapper(port.namespace)
if (ip_wrapper.netns.exists(port.namespace) or if (port.namespace is None or
port.namespace is None): ip_wrapper.netns.exists(port.namespace)):
try: try:
ip_wrapper.del_veth(port.name) ip_wrapper.del_veth(port.name)
break break

View File

@ -659,10 +659,16 @@ class NamespaceTestCase(functional_base.BaseSudoTestCase):
ip_lib.delete_network_namespace(self.namespace) ip_lib.delete_network_namespace(self.namespace)
def test_network_namespace_exists_ns_exists(self): 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): 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): def test_network_namespace_exists_ns_exists_try_is_ready(self):
self.assertTrue(ip_lib.network_namespace_exists(self.namespace, self.assertTrue(ip_lib.network_namespace_exists(self.namespace,

View File

@ -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 config as l3_config
from neutron.conf.agent.l3 import ha as ha_conf from neutron.conf.agent.l3 import ha as ha_conf
from neutron.conf import common as base_config 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 import base
from neutron.tests.common import l3_test_common from neutron.tests.common import l3_test_common
from neutron.tests.unit.agent.linux.test_utils import FakeUser 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( self.list_network_namespaces_p = mock.patch(
'neutron.agent.linux.ip_lib.list_network_namespaces') 'neutron.agent.linux.ip_lib.list_network_namespaces')
self.list_network_namespaces = self.list_network_namespaces_p.start() 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( self.ensure_dir = mock.patch(
'oslo_utils.fileutils.ensure_tree').start() 'oslo_utils.fileutils.ensure_tree').start()

View File

@ -824,31 +824,6 @@ class TestIpNetnsCommand(TestIPCmdBase):
self.netns_cmd.delete('ns') self.netns_cmd.delete('ns')
remove.assert_called_once_with('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): def test_execute(self):
self.parent.namespace = 'ns' self.parent.namespace = 'ns'
with mock.patch('neutron.agent.common.utils.execute') as execute: with mock.patch('neutron.agent.common.utils.execute') as execute: