Add rootwrap daemon mode support

This patch introduces support for rootwrap daemon mode. It adds
a new config option, AGENT.root_helper_daemon with no default. To
enable, set to something like:

root_helper_daemon = sudo neutron-rootwrap-daemon /etc/neutron/rootwrap.conf

The patch currently assumes that the root_helper_daemon value, and specifically
the rootwrap config, will not change once calls to execute() happen. While it
would not be hard to generate a rootwrap daemon client for each new config, I
couldn't think of a legitimate reason to support it and left it out as YAGNI.

This patch does change the behavior of the addl_env argument to create_process
and execute. Previously, an environment dict would be passed to Popen. If
a root helper was used, this environemnt would actually be passed to 'sudo'
which would filter it before passing it to the underlying command. In the case
of daemon mode, this would cause a problem as the enviornment is filtered by
sudo only once, at daemon startup. Any environment variables added at execute
time would then just be passed directly to the underyling command unfiltered.

oslo.rootwrap 1.6.0 fixes this issue by denying the passing of environment
variables to the daemon altogether. Instead, anything using rootwrap and needing
to pass additional environment variables should define an EnvFilter and run the
command with env var=val cmd. utils.execute/create_process have been modified to
run code in this way (which netns.execute already did).

No code in neutron currently uses both run_as_root=True and addl_env, so this
change does not require any change in code or filters.

DocImpact
Implements: blueprint rootwrap-daemon-mode
Change-Id: I567334bb611253c7b9d830d50c5be308a5153baf

neutron/agent/common/config.py

@@ -31,6 +31,12 @@ ROOT_HELPER_OPTS = [
                 default=True,
                 help=_('Use the root helper to read the namespaces from '
                        'the operating system.')),
+    # We can't just use root_helper=sudo neutron-rootwrap-daemon $cfg because
+    # it isn't appropriate for long-lived processes spawned with create_process
+    # Having a bool use_rootwrap_daemon option precludes specifying the
+    # rootwrap daemon command, which may be necessary for Xen?
+    cfg.StrOpt('root_helper_daemon',
+               help=_('Root helper daemon application to use when possible.')),
 ]
 
 AGENT_STATE_OPTS = [

neutron/agent/linux/utils.py

@@ -20,12 +20,14 @@ import shlex
 import socket
 import struct
 import tempfile
+import threading
 
 import eventlet
 from eventlet.green import subprocess
 from eventlet import greenthread
 from oslo_config import cfg
 from oslo_log import log as logging
+from oslo_rootwrap import client
 from oslo_utils import excutils
 
 from neutron.agent.common import config
@@ -38,56 +40,95 @@ LOG = logging.getLogger(__name__)
 config.register_root_helper(cfg.CONF)
 
 
+class RootwrapDaemonHelper(object):
+    __client = None
+    __lock = threading.Lock()
+
+    def __new__(cls):
+        """There is no reason to instantiate this class"""
+        raise NotImplementedError()
+
+    @classmethod
+    def get_client(cls):
+        with cls.__lock:
+            if cls.__client is None:
+                cls.__client = client.Client(
+                    shlex.split(cfg.CONF.AGENT.root_helper_daemon))
+            return cls.__client
+
+
+def addl_env_args(addl_env):
+    """Build arugments for adding additional environment vars with env"""
+
+    # NOTE (twilson) If using rootwrap, an EnvFilter should be set up for the
+    # command instead of a CommandFilter.
+    if addl_env is None:
+        return []
+    return ['env'] + ['%s=%s' % pair for pair in addl_env.items()]
+
+
 def create_process(cmd, run_as_root=False, addl_env=None):
     """Create a process object for the given command.
 
     The return value will be a tuple of the process object and the
     list of command arguments used to create it.
     """
+    cmd = map(str, addl_env_args(addl_env) + cmd)
     if run_as_root:
         cmd = shlex.split(config.get_root_helper(cfg.CONF)) + cmd
-    cmd = map(str, cmd)
-
     LOG.debug("Running command: %s", cmd)
-    env = os.environ.copy()
-    if addl_env:
-        env.update(addl_env)
-
     obj = utils.subprocess_popen(cmd, shell=False,
                                  stdin=subprocess.PIPE,
                                  stdout=subprocess.PIPE,
-                                 stderr=subprocess.PIPE,
-                                 env=env)
+                                 stderr=subprocess.PIPE)
 
     return obj, cmd
 
 
+def execute_rootwrap_daemon(cmd, process_input, addl_env):
+    cmd = map(str, addl_env_args(addl_env) + cmd)
+    # NOTE(twilson) oslo_rootwrap.daemon will raise on filter match
+    # errors, whereas oslo_rootwrap.cmd converts them to return codes.
+    # In practice, no neutron code should be trying to execute something that
+    # would throw those errors, and if it does it should be fixed as opposed to
+    # just logging the execution error.
+    LOG.debug("Running command (rootwrap daemon): %s", cmd)
+    client = RootwrapDaemonHelper.get_client()
+    return client.execute(cmd, process_input)
+
+
 def execute(cmd, process_input=None, addl_env=None,
             check_exit_code=True, return_stderr=False, log_fail_as_error=True,
             extra_ok_codes=None, run_as_root=False):
     try:
-        obj, cmd = create_process(cmd, run_as_root=run_as_root,
-                                  addl_env=addl_env)
-        _stdout, _stderr = obj.communicate(process_input)
-        obj.stdin.close()
-        m = _("\nCommand: %(cmd)s\nExit code: %(code)s\nStdin: %(stdin)s\n"
-              "Stdout: %(stdout)s\nStderr: %(stderr)s") % \
-            {'cmd': cmd,
-             'code': obj.returncode,
-             'stdin': process_input or '',
-             'stdout': _stdout,
-             'stderr': _stderr}
+        if run_as_root and cfg.CONF.AGENT.root_helper_daemon:
+            returncode, _stdout, _stderr = (
+                execute_rootwrap_daemon(cmd, process_input, addl_env))
+        else:
+            obj, cmd = create_process(cmd, run_as_root=run_as_root,
+                                      addl_env=addl_env)
+            _stdout, _stderr = obj.communicate(process_input)
+            returncode = obj.returncode
+            obj.stdin.close()
+
+        m = _("\nCommand: {cmd}\nExit code: {code}\nStdin: {stdin}\n"
+              "Stdout: {stdout}\nStderr: {stderr}").format(
+                  cmd=cmd,
+                  code=returncode,
+                  stdin=process_input or '',
+                  stdout=_stdout,
+                  stderr=_stderr)
 
         extra_ok_codes = extra_ok_codes or []
-        if obj.returncode and obj.returncode in extra_ok_codes:
-            obj.returncode = None
+        if returncode and returncode in extra_ok_codes:
+            returncode = None
 
-        if obj.returncode and log_fail_as_error:
+        if returncode and log_fail_as_error:
             LOG.error(m)
         else:
             LOG.debug(m)
 
-        if obj.returncode and check_exit_code:
+        if returncode and check_exit_code:
             raise RuntimeError(m)
     finally:
         # NOTE(termie): this appears to be necessary to let the subprocess

neutron/tests/functional/agent/test_l3_agent.py

@@ -92,7 +92,7 @@ class L3AgentTestFramework(base.BaseOVSLinuxTestCase):
                           get_temp_file_path('external/pids'))
         conf.set_override('host', host)
         agent = neutron_l3_agent.L3NATAgentWithStateReport(host, conf)
-        mock.patch.object(ip_lib, 'send_gratuitous_arp').start()
+        mock.patch.object(ip_lib, '_arping').start()
 
         return agent
 

neutron/tests/functional/base.py

@@ -56,3 +56,6 @@ class BaseSudoTestCase(base.BaseTestCase):
         config.register_root_helper(cfg.CONF)
         self.config(group='AGENT',
                     root_helper=os.environ.get('OS_ROOTWRAP_CMD', SUDO_CMD))
+        self.config(group='AGENT',
+                    root_helper_daemon=os.environ.get(
+                        'OS_ROOTWRAP_DAEMON_CMD'))

neutron/tests/unit/test_agent_config.py

@@ -34,3 +34,10 @@ class TestRootHelper(base.BaseTestCase):
         conf = config.setup_conf()
         config.register_root_helper(conf)
         self.assertEqual(config.get_root_helper(conf), 'sudo')
+
+    def test_agent_root_helper_daemon(self):
+        conf = config.setup_conf()
+        config.register_root_helper(conf)
+        rhd = 'my_root_helper_daemon'
+        conf.set_override('root_helper_daemon', rhd, 'AGENT')
+        self.assertEqual(rhd, conf.AGENT.root_helper_daemon)

setup.cfg

@@ -107,6 +107,7 @@ console_scripts =
     neutron-restproxy-agent = neutron.plugins.bigswitch.agent.restproxy_agent:main
     neutron-server = neutron.cmd.eventlet.server:main
     neutron-rootwrap = oslo_rootwrap.cmd:main
+    neutron-rootwrap-daemon = oslo_rootwrap.cmd:daemon
     neutron-usage-audit = neutron.cmd.usage_audit:main
     neutron-metering-agent = neutron.cmd.eventlet.services.metering_agent:main
     neutron-sriov-nic-agent = neutron.plugins.sriovnicagent.sriov_nic_agent:main

tools/configure_for_func_testing.sh

@@ -178,6 +178,7 @@ function _install_rootwrap_sudoers {
     VENV_NAME=${venv:-dsvm-functional}
     VENV_PATH=$NEUTRON_PATH/.tox/$VENV_NAME
     ROOTWRAP_SUDOER_CMD="$VENV_PATH/bin/neutron-rootwrap $VENV_PATH/etc/neutron/rootwrap.conf *"
+    ROOTWRAP_DAEMON_SUDOER_CMD="$VENV_PATH/bin/neutron-rootwrap-daemon $VENV_PATH/etc/neutron/rootwrap.conf"
     TEMPFILE=$(mktemp)
     cat << EOF > $TEMPFILE
 # A bug in oslo.rootwrap [1] prevents commands executed with 'ip netns
@@ -194,6 +195,7 @@ function _install_rootwrap_sudoers {
 #
 Defaults:$STACK_USER  secure_path="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:$VENV_PATH/bin"
 $STACK_USER ALL=(root) NOPASSWD: $ROOTWRAP_SUDOER_CMD
+$STACK_USER ALL=(root) NOPASSWD: $ROOTWRAP_DAEMON_SUDOER_CMD
 EOF
     chmod 0440 $TEMPFILE
     sudo chown root:root $TEMPFILE

tox.ini

@@ -39,6 +39,7 @@ deps =
 setenv = OS_TEST_PATH=./neutron/tests/functional
          OS_SUDO_TESTING=1
          OS_ROOTWRAP_CMD=sudo {envbindir}/neutron-rootwrap {envdir}/etc/neutron/rootwrap.conf
+         OS_ROOTWRAP_DAEMON_CMD=sudo {envbindir}/neutron-rootwrap-daemon {envdir}/etc/neutron/rootwrap.conf
          OS_FAIL_ON_MISSING_DEPS=1
          OS_TEST_TIMEOUT=90
 sitepackages=True