Merge pull request #322 from dw/dmw

Dmw

.travis/debops_common_tests.sh

@@ -81,10 +81,10 @@ echo travis_fold:end:job_setup
 
 
 echo travis_fold:start:first_run
-/usr/bin/time debops common
+/usr/bin/time debops common "$@"
 echo travis_fold:end:first_run
 
 
 echo travis_fold:start:second_run
-/usr/bin/time debops common
+/usr/bin/time debops common "$@"
 echo travis_fold:end:second_run

ansible_mitogen/connection.py

@@ -44,9 +44,10 @@
 import mitogen.unix
 import mitogen.utils
 
-import ansible_mitogen.target
+import ansible_mitogen.parsing
 import ansible_mitogen.process
 import ansible_mitogen.services
+import ansible_mitogen.target
 
 
 LOG = logging.getLogger(__name__)
@@ -248,6 +249,17 @@ def _connect_mitogen_doas(spec):
 }
 
 
+def parse_python_path(s):
+    """
+    Given the string set for ansible_python_interpeter, parse it using shell
+    syntax and return an appropriate argument vector.
+    """
+    if not s:
+        return None
+
+    return ansible.utils.shlex.shlex_split(s)
+
+
 def config_from_play_context(transport, inventory_name, connection):
     """
     Return a dict representing all important connection configuration, allowing
@@ -265,7 +277,7 @@ def config_from_play_context(transport, inventory_name, connection):
         'become_pass': connection._play_context.become_pass,
         'password': connection._play_context.password,
         'port': connection._play_context.port,
-        'python_path': connection.python_path,
+        'python_path': parse_python_path(connection.python_path),
         'private_key_file': connection._play_context.private_key_file,
         'ssh_executable': connection._play_context.ssh_executable,
         'timeout': connection._play_context.timeout,
@@ -314,7 +326,7 @@ def config_from_hostvars(transport, inventory_name, connection,
         'password': (hostvars.get('ansible_ssh_pass') or
                      hostvars.get('ansible_password')),
         'port': hostvars.get('ansible_port'),
-        'python_path': hostvars.get('ansible_python_interpreter'),
+        'python_path': parse_python_path(hostvars.get('ansible_python_interpreter')),
         'private_key_file': (hostvars.get('ansible_ssh_private_key_file') or
                              hostvars.get('ansible_private_key_file')),
         'mitogen_via': hostvars.get('mitogen_via'),
@@ -332,15 +344,19 @@ class Connection(ansible.plugins.connection.ConnectionBase):
     #: mitogen.master.Router for this worker.
     router = None
 
-    #: mitogen.master.Context representing the parent Context, which is
+    #: mitogen.parent.Context representing the parent Context, which is
     #: presently always the connection multiplexer process.
     parent = None
 
-    #: mitogen.master.Context connected to the target user account on the
-    #: target machine (i.e. via sudo).
+    #: mitogen.parent.Context for the target account on the target, possibly
+    #: reached via become.
     context = None
 
-    #: mitogen.master.Context connected to the fork parent process in the
+    #: mitogen.parent.Context for the login account on the target. This is
+    #: always the login account, even when become=True.
+    login_context = None
+
+    #: mitogen.parent.Context connected to the fork parent process in the
     #: target user account.
     fork_context = None
 
@@ -480,7 +496,7 @@ def _connect(self):
         """
         Establish a connection to the master process's UNIX listener socket,
         constructing a mitogen.master.Router to communicate with the master,
-        and a mitogen.master.Context to represent it.
+        and a mitogen.parent.Context to represent it.
 
         Depending on the original transport we should emulate, trigger one of
         the _connect_*() service calls defined above to cause the master
@@ -517,6 +533,11 @@ def _connect(self):
             raise ansible.errors.AnsibleConnectionFailure(dct['msg'])
 
         self.context = dct['context']
+        if self._play_context.become:
+            self.login_context = dct['via']
+        else:
+            self.login_context = self.context
+
         self.fork_context = dct['init_child_result']['fork_context']
         self.home_dir = dct['init_child_result']['home_dir']
 
@@ -534,6 +555,8 @@ def close(self, new_task=False):
             )
 
         self.context = None
+        self.fork_context = None
+        self.login_context = None
         if self.broker and not new_task:
             self.broker.shutdown()
             self.broker.join()
@@ -544,11 +567,18 @@ def call_async(self, func, *args, **kwargs):
         """
         Start a function call to the target.
 
+        :param bool use_login_context:
+            If present and :data:`True`, send the call to the login account
+            context rather than the optional become user context.
         :returns:
             mitogen.core.Receiver that receives the function call result.
         """
         self._connect()
-        return self.context.call_async(func, *args, **kwargs)
+        if kwargs.pop('use_login_context', None):
+            call_context = self.login_context
+        else:
+            call_context = self.context
+        return call_context.call_async(func, *args, **kwargs)
 
     def call(self, func, *args, **kwargs):
         """

ansible_mitogen/logging.py

@@ -34,15 +34,20 @@
 import mitogen.core
 import mitogen.utils
 
+try:
+    from __main__ import display
+except ImportError:
+    from ansible.utils.display import Display
+    display = Display()
+
 
 class Handler(logging.Handler):
     """
     Use Mitogen's log format, but send the result to a Display method.
     """
-    def __init__(self, display, normal_method):
+    def __init__(self, normal_method):
         logging.Handler.__init__(self)
         self.formatter = mitogen.utils.log_get_formatter()
-        self.display = display
         self.normal_method = normal_method
 
     #: Set of target loggers that produce warnings and errors that spam the
@@ -62,41 +67,34 @@ def emit(self, record):
 
         s = '[pid %d] %s' % (os.getpid(), self.format(record))
         if record.levelno >= logging.ERROR:
-            self.display.error(s, wrap_text=False)
+            display.error(s, wrap_text=False)
         elif record.levelno >= logging.WARNING:
-            self.display.warning(s, formatted=True)
+            display.warning(s, formatted=True)
         else:
             self.normal_method(s)
 
 
-def find_display():
-    """
-    Find the CLI tool's display variable somewhere up the stack. Why god why,
-    right? Because it's the the simplest way to get access to the verbosity
-    configured on the command line.
-    """
-    f = sys._getframe()
-    while f:
-        if 'display' in f.f_locals:
-            return f.f_locals['display']
-        f = f.f_back
-
-
 def setup():
     """
     Install a handler for Mitogen's logger to redirect it into the Ansible
     display framework, and prevent propagation to the root logger.
     """
-    display = find_display()
-
-    logging.getLogger('ansible_mitogen').handlers = [Handler(display, display.vvv)]
-    mitogen.core.LOG.handlers = [Handler(display, display.vvv)]
-    mitogen.core.IOLOG.handlers = [Handler(display, display.vvvv)]
+    logging.getLogger('ansible_mitogen').handlers = [Handler(display.vvv)]
+    mitogen.core.LOG.handlers = [Handler(display.vvv)]
+    mitogen.core.IOLOG.handlers = [Handler(display.vvvv)]
     mitogen.core.IOLOG.propagate = False
 
     if display.verbosity > 2:
-        logging.getLogger('ansible_mitogen').setLevel(logging.DEBUG)
         mitogen.core.LOG.setLevel(logging.DEBUG)
+        logging.getLogger('ansible_mitogen').setLevel(logging.DEBUG)
+    else:
+        # Mitogen copies the active log level into new children, allowing them
+        # to filter tiny messages before they hit the network, and therefore
+        # before they wake the IO loop. Explicitly setting INFO saves ~4%
+        # running against just the local machine.
+        mitogen.core.LOG.setLevel(logging.ERROR)
+        logging.getLogger('ansible_mitogen').setLevel(logging.ERROR)
 
     if display.verbosity > 3:
         mitogen.core.IOLOG.setLevel(logging.DEBUG)
+        logging.getLogger('ansible_mitogen').setLevel(logging.DEBUG)

ansible_mitogen/mixins.py

@@ -188,21 +188,23 @@ def _get_remote_tmp(self):
         except AttributeError:
             s = ansible.constants.DEFAULT_REMOTE_TMP  # <=2.4.x
 
-        return self._remote_expand_user(s)
+        return self._remote_expand_user(s, sudoable=False)
 
     def _make_tmp_path(self, remote_user=None):
         """
         Replace the base implementation's use of shell to implement mkdtemp()
-        with an actual call to mkdtemp().
+        with an actual call to mkdtemp(). Like vanilla, the directory is always
+        created in the login account context.
         """
         LOG.debug('_make_tmp_path(remote_user=%r)', remote_user)
 
         # _make_tmp_path() is basically a global stashed away as Shell.tmpdir.
         # The copy action plugin violates layering and grabs this attribute
         # directly.
-        self._connection._shell.tmpdir = self.call(
+        self._connection._shell.tmpdir = self._connection.call(
             ansible_mitogen.target.make_temp_directory,
             base_dir=self._get_remote_tmp(),
+            use_login_context=True,
         )
         LOG.debug('Temporary directory: %r', self._connection._shell.tmpdir)
         self._cleanup_remote_tmp = True
@@ -280,20 +282,26 @@ def _remote_expand_user(self, path, sudoable=True):
         """
         Replace the base implementation's attempt to emulate
         os.path.expanduser() with an actual call to os.path.expanduser().
+
+        :param bool sudoable:
+            If :data:`True`, indicate unqualified tilde ("~" with no username)
+            should be evaluated in the context of the login account, not any
+            become_user.
         """
         LOG.debug('_remote_expand_user(%r, sudoable=%r)', path, sudoable)
         if not path.startswith('~'):
             # /home/foo -> /home/foo
             return path
-        if path == '~':
-            # ~ -> /home/dmw
-            return self._connection.homedir
-        if path.startswith('~/'):
-            # ~/.ansible -> /home/dmw/.ansible
-            return os.path.join(self._connection.homedir, path[2:])
-        if path.startswith('~'):
-            # ~root/.ansible -> /root/.ansible
-            return self.call(os.path.expanduser, mitogen.utils.cast(path))
+        if sudoable or not self._play_context.become:
+            if path == '~':
+                # ~ -> /home/dmw
+                return self._connection.homedir
+            if path.startswith('~/'):
+                # ~/.ansible -> /home/dmw/.ansible
+                return os.path.join(self._connection.homedir, path[2:])
+        # ~root/.ansible -> /root/.ansible
+        return self.call(os.path.expanduser, mitogen.utils.cast(path),
+                         use_login_context=not sudoable)
 
     def get_task_timeout_secs(self):
         """

ansible_mitogen/parsing.py

@@ -0,0 +1,84 @@
+# Copyright 2017, David Wilson
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+# 1. Redistributions of source code must retain the above copyright notice,
+# this list of conditions and the following disclaimer.
+#
+# 2. Redistributions in binary form must reproduce the above copyright notice,
+# this list of conditions and the following disclaimer in the documentation
+# and/or other materials provided with the distribution.
+#
+# 3. Neither the name of the copyright holder nor the names of its contributors
+# may be used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+# POSSIBILITY OF SUCH DAMAGE.
+
+"""
+Classes to detect each case from [0] and prepare arguments necessary for the
+corresponding Runner class within the target, including preloading requisite
+files/modules known missing.
+
+[0] "Ansible Module Architecture", developing_program_flow_modules.html
+"""
+
+from __future__ import absolute_import
+from __future__ import unicode_literals
+
+import mitogen.core
+
+
+def parse_script_interpreter(source):
+    """
+    Parse the script interpreter portion of a UNIX hashbang using the rules
+    Linux uses.
+
+    :param str source: String like "/usr/bin/env python".
+
+    :returns:
+        Tuple of `(interpreter, arg)`, where `intepreter` is the script
+        interpreter and `arg` is its sole argument if present, otherwise
+        :py:data:`None`.
+    """
+    # Find terminating newline. Assume last byte of binprm_buf if absent.
+    nl = source.find(b'\n', 0, 128)
+    if nl == -1:
+        nl = min(128, len(source))
+
+    # Split once on the first run of whitespace. If no whitespace exists,
+    # bits just contains the interpreter filename.
+    bits = source[0:nl].strip().split(None, 1)
+    if len(bits) == 1:
+        return mitogen.core.to_text(bits[0]), None
+    return mitogen.core.to_text(bits[0]), mitogen.core.to_text(bits[1])
+
+
+def parse_hashbang(source):
+    """
+    Parse a UNIX "hashbang line" using the syntax supported by Linux.
+
+    :param str source: String like "#!/usr/bin/env python".
+
+    :returns:
+        Tuple of `(interpreter, arg)`, where `intepreter` is the script
+        interpreter and `arg` is its sole argument if present, otherwise
+        :py:data:`None`.
+    """
+    # Linux requires first 2 bytes with no whitespace, pretty sure it's the
+    # same everywhere. See binfmt_script.c.
+    if not source.startswith(b'#!'):
+        return None, None
+
+    return parse_script_interpreter(source[2:])