Merge remote-tracking branch 'origin/dmw'

* origin/dmw:
  docs: update Changelog; closes #511, closes #536.
  docs: update Changelog release date.
  issue #536: disable transport_config tests on vanilla
  issue #536: restore correct Python interpreter selection behaviour.
  issue #536: connection_delegation/ tests were erroneously broken
  tests: define MITOGEN_INVENTORY_FILE even if -i unspecified.
  issue #536: add tests for each ansible_python_interpreter case.
  issue #536: stop defining explicit localhost in inventory.
  tests: allow running Ansible tests locally without -udmw again.
  stable: fix preamble_size on stable docs.
  issue #481: add test.

ansible_mitogen/transport_config.py

@@ -329,9 +329,11 @@ class PlayContextSpec(Spec):
         return self._play_context.port
 
     def python_path(self):
-        return parse_python_path(
-            self._connection.get_task_var('ansible_python_interpreter')
-        )
+        s = self._connection.get_task_var('ansible_python_interpreter')
+        # #511, #536: executor/module_common.py::_get_shebang() hard-wires
+        # "/usr/bin/python" as the default interpreter path if no other
+        # interpreter is specified.
+        return parse_python_path(s or '/usr/bin/python')
 
     def private_key_file(self):
         return self._play_context.private_key_file
@@ -487,11 +489,12 @@ class MitogenViaSpec(Spec):
         )
 
     def python_path(self):
-        return parse_python_path(
-            self._host_vars.get('ansible_python_interpreter')
-            # This variable has no default for remote hosts. For local hosts it
-            # is sys.executable.
-        )
+        s = self._host_vars.get('ansible_python_interpreter')
+        # #511, #536: executor/module_common.py::_get_shebang() hard-wires
+        # "/usr/bin/python" as the default interpreter path if no other
+        # interpreter is specified.
+        return parse_python_path(s or '/usr/bin/python')
+
 
     def private_key_file(self):
         # TODO: must come from PlayContext too.

docs/changelog.rst

@@ -125,7 +125,24 @@ Core Library
   series.
 
 
-v0.2.4 (2018-??-??)
+v0.2.5 (2019-02-1?)
+-------------------
+
+* `#511 <https://github.com/dw/mitogen/issues/511>`_,
+  `#536 <https://github.com/dw/mitogen/issues/536>`_: changes in 0.2.4 to
+  repair ``delegate_to`` handling broke default ``ansible_python_interpreter``
+  handling. Test coverage was added.
+
+Thanks!
+~~~~~~~
+
+Mitogen would not be possible without the support of users. A huge thanks for
+bug reports, testing, features and fixes in this release contributed by
+`Guy Knights <https://github.com/knightsg>`_, and
+`Josh Smift <https://github.com/jbscare>`_,
+
+
+v0.2.4 (2019-02-10)
 -------------------
 
 Mitogen for Ansible

preamble_size.py

@@ -8,6 +8,7 @@ import sys
 import zlib
 
 import mitogen.fakessh
+import mitogen.fork
 import mitogen.master
 import mitogen.minify
 import mitogen.parent
@@ -16,9 +17,6 @@ import mitogen.service
 import mitogen.ssh
 import mitogen.sudo
 
-import ansible_mitogen.runner
-import ansible_mitogen.target
-
 router = mitogen.master.Router()
 context = mitogen.parent.Context(router, 0)
 stream = mitogen.ssh.Stream(router, 0, max_message_size=0, hostname='foo')
@@ -46,8 +44,6 @@ print(
 for mod in (
         mitogen.parent,
         mitogen.fork,
-        ansible_mitogen.target,
-        ansible_mitogen.runner,
         mitogen.ssh,
         mitogen.sudo,
         mitogen.select,

tests/ansible/hosts/default.hosts

@@ -1,8 +1,9 @@
 # vim: syntax=dosini
 
 # When running the tests outside CI, make a single 'target' host which is the
-# local machine.
-target ansible_host=localhost
+# local machine. The ansible_user override is necessary since some tests want a
+# fixed ansible.cfg remote_user setting to test against.
+target ansible_host=localhost ansible_user="{{lookup('env', 'USER')}}"
 
 [test-targets]
 target

tests/ansible/hosts/localhost.hosts

@@ -1,9 +1,8 @@
 # vim: syntax=dosini
 
-# This must be defined explicitly, otherwise _create_implicit_localhost()
-# generates its own copy, which includes an ansible_python_interpreter that
-# varies according to host machine.
-localhost
+# issue #511, #536: we must not define an explicit localhost, as some
+# transport_config/python_path.yml needs to test the implicit localhost
+# behaviour.
 
 # This is only used for manual testing.
 [localhost-x10]

tests/ansible/hosts/transport_config.hosts

@@ -0,0 +1,10 @@
+# integration/transport_config
+# Hosts with twiddled configs that need to be checked somehow.
+
+tc-python-path-unset
+tc-python-path-hostvar  ansible_python_interpreter=/hostvar/path/to/python
+
+# local connections get virtualenv python path
+tc-python-path-local-unset ansible_connection=local
+tc-python-path-local-explicit ansible_connection=local ansible_python_interpreter=/a/b/c
+

tests/ansible/integration/all.yml

@@ -19,3 +19,4 @@
 - include: ssh/all.yml
 - include: strategy/all.yml
 - include: stub_connections/all.yml
+- include: transport_config/all.yml

tests/ansible/integration/connection_delegation/delegate_to_template.yml

@@ -39,7 +39,7 @@
               'identity_file': null,
               'password': null,
               'port': null,
-              'python_path': null,
+              'python_path': ["/usr/bin/python"],
               'ssh_args': [
                 '-o',
                 'UserKnownHostsFile=/dev/null',
@@ -66,7 +66,7 @@
               'identity_file': null,
               'password': null,
               'port': null,
-              'python_path': null,
+              'python_path': ["/usr/bin/python"],
               'ssh_args': [
                 '-o',
                 'UserKnownHostsFile=/dev/null',

tests/ansible/integration/connection_delegation/local_action.yml

@@ -15,7 +15,7 @@
         right: [
           {
             'kwargs': {
-              'python_path': null
+              'python_path': ["{{ansible_playbook_python}}"],
             },
             'method': 'local',
           },
@@ -23,7 +23,7 @@
             'enable_lru': true,
             'kwargs': {
               'connect_timeout': 10,
-              'python_path': null,
+              'python_path': ["{{ansible_playbook_python}}"],
               'password': null,
               'username': 'root',
               'sudo_path': null,

tests/ansible/integration/connection_delegation/osa_delegate_to_self.yml

@@ -24,7 +24,7 @@
             'lxc_info_path': null,
             'lxc_path': null,
             'machinectl_path': null,
-            'python_path': null,
+            'python_path': ["/usr/bin/python"],
             'username': 'ansible-cfg-remote-user',
           },
           'method': 'setns',

tests/ansible/integration/connection_delegation/stack_construction.yml

@@ -43,7 +43,7 @@
               "connect_timeout": 10,
               "doas_path": null,
               "password": null,
-              "python_path": null,
+              "python_path": ["/usr/bin/python"],
               "username": "normal-user",
             },
             "method": "doas",
@@ -72,7 +72,7 @@
                       'identity_file': null,
                       'password': null,
                       'port': null,
-                      'python_path': null,
+                      "python_path": ["/usr/bin/python"],
                       'ssh_args': [
                           '-o',
                           'UserKnownHostsFile=/dev/null',
@@ -112,7 +112,7 @@
                       'identity_file': null,
                       'password': null,
                       'port': null,
-                      'python_path': null,
+                      "python_path": ["/usr/bin/python"],
                       'ssh_args': [
                           '-o',
                           'UserKnownHostsFile=/dev/null',
@@ -147,7 +147,7 @@
                       'connect_timeout': 10,
                       'doas_path': null,
                       'password': null,
-                      'python_path': null,
+                      "python_path": ["/usr/bin/python"],
                       'username': 'normal-user',
                   },
                   'method': 'doas',
@@ -162,7 +162,7 @@
                       'identity_file': null,
                       'password': null,
                       'port': null,
-                      'python_path': null,
+                      "python_path": ["/usr/bin/python"],
                       'ssh_args': [
                           '-o',
                           'UserKnownHostsFile=/dev/null',
@@ -202,7 +202,7 @@
                       'identity_file': null,
                       'password': null,
                       'port': null,
-                      'python_path': null,
+                      "python_path": ["/usr/bin/python"],
                       'ssh_args': [
                           '-o',
                           'UserKnownHostsFile=/dev/null',
@@ -229,7 +229,7 @@
                       'identity_file': null,
                       'password': null,
                       'port': null,
-                      'python_path': null,
+                      "python_path": ["/usr/bin/python"],
                       'ssh_args': [
                           '-o',
                           'UserKnownHostsFile=/dev/null',
@@ -264,7 +264,7 @@
                       'connect_timeout': 10,
                       'doas_path': null,
                       'password': null,
-                      'python_path': null,
+                      "python_path": ["/usr/bin/python"],
                       'username': 'normal-user',
                   },
                   'method': 'doas',
@@ -279,7 +279,7 @@
                       'identity_file': null,
                       'password': null,
                       'port': null,
-                      'python_path': null,
+                      "python_path": ["/usr/bin/python"],
                       'ssh_args': [
                           '-o',
                           'UserKnownHostsFile=/dev/null',
@@ -320,7 +320,7 @@
                       'identity_file': null,
                       'password': null,
                       'port': null,
-                      'python_path': null,
+                      "python_path": ["/usr/bin/python"],
                       'ssh_args': [
                           '-o',
                           'UserKnownHostsFile=/dev/null',
@@ -352,7 +352,7 @@
         right: [
           {
             'kwargs': {
-              'python_path': null
+              "python_path": ["{{ansible_playbook_python}}"],
             },
             'method': 'local',
           },
@@ -374,7 +374,7 @@
               'connect_timeout': 10,
               'doas_path': null,
               'password': null,
-              'python_path': null,
+              'python_path': ["/usr/bin/python"],
               'username': 'normal-user',
             },
             'method': 'doas',
@@ -384,7 +384,7 @@
               'connect_timeout': 10,
               'doas_path': null,
               'password': null,
-              'python_path': null,
+              'python_path': ["/usr/bin/python"],
               'username': 'newuser-doas-normal-user',
             },
             'method': 'doas',

tests/ansible/integration/transport_config/README.md

@@ -0,0 +1,7 @@
+
+# Tests for correct selection of connection variables.
+
+This directory is a placeholder for a work-in-progress test set that tries
+every combination of the variables extracted via `transport_config.py`.
+
+In the meantime, it has ad-hoc scripts for bugs already encountered.

tests/ansible/integration/transport_config/all.yml

@@ -0,0 +1,2 @@
+
+- include: python_path.yml

tests/ansible/integration/transport_config/python_path.yml

@@ -0,0 +1,62 @@
+# related: issue #511, #536
+
+
+# When no ansible_python_interpreter is set, executor/module_common.py chooses
+# "/usr/bin/python".
+- name: integration/transport_config/python_path.yml
+  hosts: tc-python-path-unset
+  tasks:
+  - meta: end_play
+    when: not is_mitogen
+  - mitogen_get_stack:
+    register: out
+  - assert_equal:
+      left: out.result[0].kwargs.python_path
+      right: ["/usr/bin/python"]
+
+
+# Non-localhost with explicit ansible_python_interpreter
+- hosts: tc-python-path-hostvar
+  tasks:
+  - meta: end_play
+    when: not is_mitogen
+  - mitogen_get_stack:
+    register: out
+  - assert_equal:
+      left: out.result[0].kwargs.python_path
+      right: [/hostvar/path/to/python]
+
+
+# Implicit localhost gets ansible_python_interpreter=virtualenv interpreter
+- hosts: localhost
+  tasks:
+  - meta: end_play
+    when: not is_mitogen
+  - mitogen_get_stack:
+    register: out
+  - assert_equal:
+      left: out.result[0].kwargs.python_path
+      right: ["{{ansible_playbook_python}}"]
+
+
+# explicit local connections get the same treatment as everything else.
+- hosts: tc-python-path-local-unset
+  tasks:
+  - meta: end_play
+    when: not is_mitogen
+  - mitogen_get_stack:
+    register: out
+  - assert_equal:
+      left: out.result[0].kwargs.python_path
+      right: ["/usr/bin/python"]
+
+
+- hosts: tc-python-path-local-explicit
+  tasks:
+  - meta: end_play
+    when: not is_mitogen
+  - mitogen_get_stack:
+    register: out
+  - assert_equal:
+      left: out.result[0].kwargs.python_path
+      right: ["/a/b/c"]

tests/ansible/run_ansible_playbook.py

@@ -46,6 +46,10 @@ if '-i' in sys.argv:
     extra['MITOGEN_INVENTORY_FILE'] = (
         os.path.abspath(sys.argv[1 + sys.argv.index('-i')])
     )
+else:
+    extra['MITOGEN_INVENTORY_FILE'] = (
+        os.path.join(GIT_BASEDIR, 'tests/ansible/hosts')
+    )
 
 args = ['ansible-playbook']
 args += ['-e', json.dumps(extra)]

tests/data/stubs/stub-sudo.py

@@ -8,7 +8,12 @@ import sys
 os.environ['ORIGINAL_ARGV'] = json.dumps(sys.argv)
 os.environ['THIS_IS_STUB_SUDO'] = '1'
 
-# This must be a child process and not exec() since Mitogen replaces its stderr
-# descriptor, causing the last user of the slave PTY to close it, resulting in
-# the master side indicating EIO.
+if os.environ.get('PREHISTORIC_SUDO'):
+    # issue #481: old versions of sudo did in fact use execve, thus we must
+    # have TTY handle preservation in core.py.
+    os.execv(sys.executable, sys.argv[sys.argv.index('--') + 1:])
+else:
+    # This must be a child process and not exec() since Mitogen replaces its
+    # stderr descriptor, causing the last user of the slave PTY to close it,
+    # resulting in the master side indicating EIO.
     subprocess.check_call(sys.argv[sys.argv.index('--') + 1:])

tests/sudo_test.py

@@ -55,6 +55,15 @@ class ConstructorTest(testlib.RouterMixin, testlib.TestCase):
             '--'
         ])
 
+    def test_tty_preserved(self):
+        # issue #481
+        os.environ['PREHISTORIC_SUDO'] = '1'
+        try:
+            context, argv = self.run_sudo()
+            self.assertEquals('1', context.call(os.getenv, 'PREHISTORIC_SUDO'))
+        finally:
+            del os.environ['PREHISTORIC_SUDO']
+
 
 class NonEnglishPromptTest(testlib.DockerMixin, testlib.TestCase):
     # Only mitogen/debian-test has a properly configured sudo.