From 9833364fbd4705fc4a563192cf2707ffe8cf763d Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Fri, 25 Jul 2014 14:27:00 -0700 Subject: [PATCH] Option for root_helper when checking namespace Adds a configuration option to use the root helper in the ip netns list command executed by the IP library when checking for the existence of a namespace. This prevents an unprivileged l3 agent from erroneously trying to create another namespace when one already exists. This is necessary in environments with constrained permissions on /var/run/netns via umask or other access controls. However, due to the overhead incurred by calling sudo every time on systems where this restriction isn't in place, this configuration won't be desired all of the time. So this patch also adds a sanity check that reports back whether or not the root_helper is required for a deployment. DocImpact Closes-Bug: #1348812 Closes-Bug: #1311804 Change-Id: If7560161de3be6066af0d9866e6b5cd7c7247c33 --- etc/neutron.conf | 5 +++ neutron/agent/common/config.py | 4 ++ neutron/agent/linux/ip_lib.py | 7 +++- neutron/cmd/sanity/checks.py | 15 +++++++ neutron/cmd/sanity_check.py | 40 +++++++++++++++---- neutron/tests/functional/agent/linux/base.py | 6 +++ .../tests/functional/sanity/test_sanity.py | 3 ++ 7 files changed, 71 insertions(+), 9 deletions(-) diff --git a/etc/neutron.conf b/etc/neutron.conf index e1c2cc258a2..95b3af1afbd 100644 --- a/etc/neutron.conf +++ b/etc/neutron.conf @@ -554,6 +554,11 @@ lock_path = $state_path/lock # each rule's purpose. (System must support the iptables comments module.) # comment_iptables_rules = True +# Use the root helper when listing the namespaces on a system. This may not +# be required depending on the security configuration. If the root helper is +# not required, set this to False for a performance improvement. +# use_helper_for_ns_read = True + # =========== items for agent management extension ============= # seconds between nodes reporting state to server; should be less than # agent_down_time, best if it is half or less than agent_down_time diff --git a/neutron/agent/common/config.py b/neutron/agent/common/config.py index 3ee41dde851..43335fe4578 100644 --- a/neutron/agent/common/config.py +++ b/neutron/agent/common/config.py @@ -27,6 +27,10 @@ LOG = logging.getLogger(__name__) ROOT_HELPER_OPTS = [ cfg.StrOpt('root_helper', default='sudo', help=_('Root helper application.')), + cfg.BoolOpt('use_helper_for_ns_read', + default=True, + help=_('Use the root helper to read the namespaces from ' + 'the operating system.')), ] AGENT_STATE_OPTS = [ diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index e7a4e56613b..4ca7591df61 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -554,8 +554,11 @@ class IpNetnsCommand(IpCommandBase): check_exit_code=check_exit_code, extra_ok_codes=extra_ok_codes) def exists(self, name): - output = self._parent._execute('o', 'netns', ['list']) - + root_helper = self._parent.root_helper + if not cfg.CONF.AGENT.use_helper_for_ns_read: + root_helper = None + output = self._parent._execute('o', 'netns', ['list'], + root_helper=root_helper) for line in output.split('\n'): if name == line.strip(): return True diff --git a/neutron/cmd/sanity/checks.py b/neutron/cmd/sanity/checks.py index c672546a1bf..de7de5482f0 100644 --- a/neutron/cmd/sanity/checks.py +++ b/neutron/cmd/sanity/checks.py @@ -15,12 +15,14 @@ import netaddr +from neutron.agent.linux import ip_lib from neutron.agent.linux import ip_link_support from neutron.agent.linux import ovs_lib from neutron.agent.linux import utils as agent_utils from neutron.common import utils from neutron.i18n import _LE from neutron.openstack.common import log as logging +from neutron.openstack.common import uuidutils from neutron.plugins.common import constants as const from neutron.plugins.openvswitch.common import constants as ovs_const @@ -107,3 +109,16 @@ def vf_management_supported(root_helper): "ip link command")) return False return True + + +def netns_read_requires_helper(root_helper): + ipw = ip_lib.IPWrapper(root_helper) + nsname = "netnsreadtest-" + uuidutils.generate_uuid() + ipw.netns.add(nsname) + try: + # read without root_helper. if exists, not required. + ipw_nohelp = ip_lib.IPWrapper() + exists = ipw_nohelp.netns.exists(nsname) + finally: + ipw.netns.delete(nsname) + return not exists diff --git a/neutron/cmd/sanity_check.py b/neutron/cmd/sanity_check.py index 1ab1ff57ed2..76bb46b801b 100644 --- a/neutron/cmd/sanity_check.py +++ b/neutron/cmd/sanity_check.py @@ -17,7 +17,7 @@ import sys from neutron.cmd.sanity import checks from neutron.common import config -from neutron.i18n import _LE +from neutron.i18n import _LE, _LW from neutron.openstack.common import log as logging from oslo.config import cfg @@ -31,6 +31,8 @@ cfg.CONF.import_group('ml2_sriov', class BoolOptCallback(cfg.BoolOpt): def __init__(self, name, callback, **kwargs): + if 'default' not in kwargs: + kwargs['default'] = False self.callback = callback super(BoolOptCallback, self).__init__(name, **kwargs) @@ -54,6 +56,27 @@ def check_ovs_patch(): return result +def check_read_netns(): + required = checks.netns_read_requires_helper( + root_helper=cfg.CONF.AGENT.root_helper) + if not required and cfg.CONF.AGENT.use_helper_for_ns_read: + LOG.warning(_LW("The user that is executing neutron can read the " + "namespaces without using the root_helper. Disable " + "the use_helper_for_ns_read option to avoid a " + "performance impact.")) + # Don't fail because nothing is actually broken. Just not optimal. + result = True + elif required and not cfg.CONF.AGENT.use_helper_for_ns_read: + LOG.error(_LE("The user that is executing neutron does not have " + "permissions to read the namespaces. Enable the " + "use_helper_for_ns_read configuration option.")) + result = False + else: + # everything is configured appropriately + result = True + return result + + def check_nova_notify(): result = checks.nova_notify_supported() if not result: @@ -85,17 +108,18 @@ def check_vf_management(): # Define CLI opts to test specific features, with a calback for the test OPTS = [ - BoolOptCallback('ovs_vxlan', check_ovs_vxlan, default=False, + BoolOptCallback('ovs_vxlan', check_ovs_vxlan, help=_('Check for vxlan support')), - BoolOptCallback('ovs_patch', check_ovs_patch, default=False, + BoolOptCallback('ovs_patch', check_ovs_patch, help=_('Check for patch port support')), - BoolOptCallback('nova_notify', check_nova_notify, default=False, + BoolOptCallback('nova_notify', check_nova_notify, help=_('Check for nova notification support')), - BoolOptCallback('arp_responder', check_arp_responder, default=False, + BoolOptCallback('arp_responder', check_arp_responder, help=_('Check for ARP responder support')), - BoolOptCallback('vf_management', check_vf_management, default=False, + BoolOptCallback('vf_management', check_vf_management, help=_('Check for VF management support')), - + BoolOptCallback('read_netns', check_read_netns, + help=_('Check netns permission settings')), ] @@ -118,6 +142,8 @@ def enable_tests_from_config(): cfg.CONF.set_override('arp_responder', True) if cfg.CONF.ml2_sriov.agent_required: cfg.CONF.set_override('vf_management', True) + if not cfg.CONF.AGENT.use_helper_for_ns_read: + cfg.CONF.set_override('read_netns', True) def all_tests_passed(): diff --git a/neutron/tests/functional/agent/linux/base.py b/neutron/tests/functional/agent/linux/base.py index b900d56fd65..034632deaad 100644 --- a/neutron/tests/functional/agent/linux/base.py +++ b/neutron/tests/functional/agent/linux/base.py @@ -15,7 +15,9 @@ import random import netaddr +from oslo.config import cfg +from neutron.agent.common import config from neutron.agent.linux import ip_lib from neutron.agent.linux import ovs_lib from neutron.agent.linux import utils @@ -44,6 +46,10 @@ def get_rand_veth_name(): class BaseLinuxTestCase(functional_base.BaseSudoTestCase): + def setUp(self): + super(BaseLinuxTestCase, self).setUp() + config.register_root_helper(cfg.CONF) + def check_command(self, cmd, error_text, skip_msg, root_helper=None): try: utils.execute(cmd, root_helper=root_helper) diff --git a/neutron/tests/functional/sanity/test_sanity.py b/neutron/tests/functional/sanity/test_sanity.py index 4ee357d4267..3006ffa5e33 100644 --- a/neutron/tests/functional/sanity/test_sanity.py +++ b/neutron/tests/functional/sanity/test_sanity.py @@ -55,3 +55,6 @@ class SanityTestCaseRoot(functional_base.BaseSudoTestCase): def test_vf_management_runs(self): checks.vf_management_supported(self.root_helper) + + def test_namespace_root_read_detection_runs(self): + checks.netns_read_requires_helper(self.root_helper)