Merge pull request #1123 from moreati/release-0.3.10

Release 0.3.10

.ci/ansible_tests.py

@@ -6,11 +6,13 @@ import glob
 import os
 import signal
 import sys
-import textwrap
+
+import jinja2
 
 import ci_lib
 
 
+TEMPLATES_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/ansible/templates')
 TESTS_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/ansible')
 HOSTS_DIR = os.path.join(ci_lib.TMP, 'hosts')
 
@@ -52,37 +54,19 @@ with ci_lib.Fold('job_setup'):
         distros[container['distro']].append(container['name'])
         families[container['family']].append(container['name'])
 
+    jinja_env = jinja2.Environment(
+        loader=jinja2.FileSystemLoader(searchpath=TEMPLATES_DIR),
+        lstrip_blocks=True,  # Remove spaces and tabs from before a block
+        trim_blocks=True,  # Remove first newline after a block
+    )
+    inventory_template = jinja_env.get_template('test-targets.j2')
     inventory_path = os.path.join(HOSTS_DIR, 'target')
+
     with open(inventory_path, 'w') as fp:
-        fp.write('[test-targets]\n')
+        fp.write(inventory_template.render(
-        fp.writelines(
+            containers=containers,
-            "%(name)s "
+            distros=distros,
-            "ansible_host=%(hostname)s "
+            families=families,
-            "ansible_port=%(port)s "
-            "ansible_python_interpreter=%(python_path)s "
-            "ansible_user=mitogen__has_sudo_nopw "
-            "ansible_password=has_sudo_nopw_password"
-            "\n"
-            % container
-            for container in containers
-        )
-
-        for distro, hostnames in sorted(distros.items(), key=lambda t: t[0]):
-            fp.write('\n[%s]\n' % distro)
-            fp.writelines('%s\n' % name for name in hostnames)
-
-        for family, hostnames in sorted(families.items(), key=lambda t: t[0]):
-            fp.write('\n[%s]\n' % family)
-            fp.writelines('%s\n' % name for name in hostnames)
-
-        fp.write(textwrap.dedent(
-            '''
-            [linux:children]
-            test-targets
-
-            [linux_containers:children]
-            test-targets
-            '''
         ))
 
     ci_lib.dump_file(inventory_path)

.ci/azure-pipelines.yml

@@ -28,10 +28,6 @@ jobs:
     matrix:
       Mito_312:
         tox.env: py312-mode_mitogen
-      Loc_312_9:
-        tox.env: py312-mode_localhost-ansible9
-      Van_312_9:
-        tox.env: py312-mode_localhost-ansible9-strategy_linear
       Loc_312_10:
         tox.env: py312-mode_localhost-ansible10
       Van_312_10:

ansible_mitogen/connection.py

@@ -1129,6 +1129,6 @@ class Connection(ansible.plugins.connection.ConnectionBase):
         self.get_chain().call(
             ansible_mitogen.target.transfer_file,
             context=self.binding.get_child_service_context(),
-            in_path=in_path,
+            in_path=ansible_mitogen.utils.unsafe.cast(in_path),
-            out_path=out_path
+            out_path=ansible_mitogen.utils.unsafe.cast(out_path)
         )

ansible_mitogen/mixins.py

@@ -280,7 +280,9 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase):
                   paths, mode, sudoable)
         return self.fake_shell(lambda: mitogen.select.Select.all(
             self._connection.get_chain().call_async(
-                ansible_mitogen.target.set_file_mode, path, mode
+                ansible_mitogen.target.set_file_mode,
+                ansible_mitogen.utils.unsafe.cast(path),
+                mode,
             )
             for path in paths
         ))

ansible_mitogen/transport_config.py

@@ -498,12 +498,13 @@ class PlayContextSpec(Spec):
         )
 
     def ssh_args(self):
+        local_vars = self._task_vars.get("hostvars", {}).get(self._inventory_name, {})
         return [
             mitogen.core.to_text(term)
             for s in (
-                C.config.get_config_value("ssh_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
+                C.config.get_config_value("ssh_args", plugin_type="connection", plugin_name="ssh", variables=local_vars),
-                C.config.get_config_value("ssh_common_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
+                C.config.get_config_value("ssh_common_args", plugin_type="connection", plugin_name="ssh", variables=local_vars),
-                C.config.get_config_value("ssh_extra_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {}))
+                C.config.get_config_value("ssh_extra_args", plugin_type="connection", plugin_name="ssh", variables=local_vars)
             )
             for term in ansible.utils.shlex.shlex_split(s or '')
         ]
@@ -738,12 +739,13 @@ class MitogenViaSpec(Spec):
         )
 
     def ssh_args(self):
+        local_vars = self._task_vars.get("hostvars", {}).get(self._inventory_name, {})
         return [
             mitogen.core.to_text(term)
             for s in (
-                C.config.get_config_value("ssh_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
+                C.config.get_config_value("ssh_args", plugin_type="connection", plugin_name="ssh", variables=local_vars),
-                C.config.get_config_value("ssh_common_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
+                C.config.get_config_value("ssh_common_args", plugin_type="connection", plugin_name="ssh", variables=local_vars),
-                C.config.get_config_value("ssh_extra_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {}))
+                C.config.get_config_value("ssh_extra_args", plugin_type="connection", plugin_name="ssh", variables=local_vars)
             )
             for term in ansible.utils.shlex.shlex_split(s)
             if s

docs/changelog.rst

@@ -18,6 +18,22 @@ To avail of fixes in an unreleased version, please download a ZIP file
 `directly from GitHub <https://github.com/mitogen-hq/mitogen/>`_.
 
 
+v0.3.10 (2024-09-20)
+--------------------
+
+* :gh:issue:`950` Fix Solaris/Illumos/SmartOS compatibility with become
+* :gh:issue:`1087` Fix :exc:`mitogen.core.StreamError` when Ansible template
+  module is called with a ``dest:`` filename that has an extension
+* :gh:issue:`1110` Fix :exc:`mitogen.core.StreamError` when Ansible copy
+  module is called with a file larger than 124 kibibytes
+  (:data:`ansible_mitogen.connection.Connection.SMALL_FILE_LIMIT`)
+* :gh:issue:`905` Initial support for templated ``ansible_ssh_args``,
+  ``ansible_ssh_common_args``, and ``ansible_ssh_extra_args`` variables.
+  NB: play or task scoped variables will probably still fail.
+* :gh:issue:`694` CI: Fixed a race condition and some resource leaks causing
+  some of intermittent failures when running the test suite.
+
+
 v0.3.9 (2024-08-13)
 -------------------
 

docs/contributors.rst

@@ -116,6 +116,7 @@ sponsorship and outstanding future-thinking of its early adopters.
 
     <ul>
     <li>Alex Willmer</li>
+    <li><a href="https://github.com/momiji">Christian Bourgeois </a></li>
     <li><a href="https://underwhelm.net/">Dan Dorman</a> &mdash; - <em>When I truly understand my enemy … then in that very moment I also love him.</em></li>
     <li>Daniel Foerster</li>
     <li><a href="https://www.deps.co/">Deps</a> &mdash; <em>Private Maven Repository Hosting for Java, Scala, Groovy, Clojure</em></li>
@@ -125,6 +126,7 @@ sponsorship and outstanding future-thinking of its early adopters.
     <li><a href="https://www.channable.com">rkrzr</a></li>
     <li>jgadling</li>
     <li>John F Wall &mdash; <em>Making Ansible Great with Massive Parallelism</em></li>
+    <li><a href="https://github.com/jrosser">Jonathan Rosser</a></li>
     <li>KennethC</li>
     <li><a href="https://github.com/lberruti">Luca Berruti</li>
     <li>Lewis Bellwood &mdash; <em>Happy to be apart of a great project.</em></li>

mitogen/__init__.py

@@ -35,7 +35,7 @@ be expected. On the slave, it is built dynamically during startup.
 
 
 #: Library version as a tuple.
-__version__ = (0, 3, 9)
+__version__ = (0, 3, 10)
 
 
 #: This is :data:`False` in slave contexts. Previously it was used to prevent

mitogen/parent.py

@@ -147,6 +147,8 @@ LINUX_TIOCGPTN = _ioctl_cast(2147767344)
 LINUX_TIOCSPTLCK = _ioctl_cast(1074025521)
 
 IS_LINUX = os.uname()[0] == 'Linux'
+IS_SOLARIS = os.uname()[0] == 'SunOS'
+
 
 SIGNAL_BY_NUM = dict(
     (getattr(signal, name), name)
@@ -411,7 +413,7 @@ def _acquire_controlling_tty():
         # On Linux, the controlling tty becomes the first tty opened by a
         # process lacking any prior tty.
         os.close(os.open(os.ttyname(2), os.O_RDWR))
-    if hasattr(termios, 'TIOCSCTTY') and not mitogen.core.IS_WSL:
+    if hasattr(termios, 'TIOCSCTTY') and not mitogen.core.IS_WSL and not IS_SOLARIS:
         # #550: prehistoric WSL does not like TIOCSCTTY.
         # On BSD an explicit ioctl is required. For some inexplicable reason,
         # Python 2.6 on Travis also requires it.
@@ -479,7 +481,8 @@ def openpty():
 
     master_fp = os.fdopen(master_fd, 'r+b', 0)
     slave_fp = os.fdopen(slave_fd, 'r+b', 0)
-    disable_echo(master_fd)
+    if not IS_SOLARIS:
+        disable_echo(master_fd)
     disable_echo(slave_fd)
     mitogen.core.set_block(slave_fd)
     return master_fp, slave_fp
@@ -2542,7 +2545,7 @@ class Reaper(object):
         # because it is setuid, so this is best-effort only.
         LOG.debug('%r: sending %s', self.proc, SIGNAL_BY_NUM[signum])
         try:
-            os.kill(self.proc.pid, signum)
+            self.proc.send_signal(signum)
         except OSError:
             e = sys.exc_info()[1]
             if e.args[0] != errno.EPERM:
@@ -2662,6 +2665,17 @@ class Process(object):
         """
         raise NotImplementedError()
 
+    def send_signal(self, sig):
+        os.kill(self.pid, sig)
+
+    def terminate(self):
+        "Ask the process to gracefully shutdown."
+        self.send_signal(signal.SIGTERM)
+
+    def kill(self):
+        "Ask the operating system to forcefully destroy the process."
+        self.send_signal(signal.SIGKILL)
+
 
 class PopenProcess(Process):
     """
@@ -2678,6 +2692,9 @@ class PopenProcess(Process):
     def poll(self):
         return self.proc.poll()
 
+    def send_signal(self, sig):
+        self.proc.send_signal(sig)
+
 
 class ModuleForwarder(object):
     """

mitogen/unix.py

@@ -143,19 +143,23 @@ class Listener(mitogen.core.Protocol):
     def on_accept_client(self, sock):
         sock.setblocking(True)
         try:
-            pid, = struct.unpack('>L', sock.recv(4))
+            data = sock.recv(4)
+            pid, = struct.unpack('>L', data)
         except (struct.error, socket.error):
-            LOG.error('listener: failed to read remote identity: %s',
+            LOG.error('listener: failed to read remote identity, got %d bytes: %s',
-                      sys.exc_info()[1])
+                      len(data), sys.exc_info()[1])
+            sock.close()
             return
 
         context_id = self._router.id_allocator.allocate()
         try:
+            # FIXME #1109 send() returns number of bytes sent, check it
             sock.send(struct.pack('>LLL', context_id, mitogen.context_id,
                                   os.getpid()))
         except socket.error:
             LOG.error('listener: failed to assign identity to PID %d: %s',
                       pid, sys.exc_info()[1])
+            sock.close()
             return
 
         context = mitogen.parent.Context(self._router, context_id)

tests/ansible/hosts/default.hosts

@@ -12,3 +12,10 @@ target ansible_host=localhost ansible_user="{{ lookup('pipe', 'whoami') }}"
 target
 
 [linux_containers]
+
+[issue905]
+ssh-common-args ansible_host=localhost ansible_user="{{ lookup('pipe', 'whoami') }}"
+
+[issue905:vars]
+ansible_ssh_common_args=-o PermitLocalCommand=yes -o LocalCommand="touch {{ ssh_args_canary_file }}"
+ssh_args_canary_file=/tmp/ssh_args_{{ inventory_hostname }}

tests/ansible/integration/action/copy.yml

@@ -1,92 +1,94 @@
 # Verify copy module for small and large files, and inline content.
+# To exercise https://github.com/mitogen-hq/mitogen/pull/1110 destination
+# files must have extensions and loops must use `with_items:`.
 
 - name: integration/action/copy.yml
   hosts: test-targets
-  tasks:
+  vars:
-    - name: Create tiny file
+    sourced_files:
-      copy:
+      - src: /tmp/copy-tiny-file
-        dest: /tmp/copy-tiny-file
+        dest: /tmp/copy-tiny-file.out
-        content:
+        content: this is a tiny file.
-          this is a tiny file.
+        expected_checksum: f29faa9a6f19a700a941bf2aa5b281643c4ec8a0
-      delegate_to: localhost
+      - src: /tmp/copy-large-file
-      run_once: true
+        dest: /tmp/copy-large-file.out
+        content: "{{ 'x' * 200000 }}"
+        expected_checksum: 62951f943c41cdd326e5ce2b53a779e7916a820d
+
+    inline_files:
+      - dest: /tmp/copy-tiny-inline-file.out
+        content: tiny inline content
+        expected_checksum: b26dd6444595e2bdb342aa0a91721b57478b5029
+      - dest: /tmp/copy-large-inline-file.out
+        content: |
+          {{ 'y' * 200000 }}
+        expected_checksum: d675f47e467eae19e49032a2cc39118e12a6ee72
 
-    - name: Create large file
+    files: "{{ sourced_files + inline_files }}"
+  tasks:
+    - name: Create sourced files
       copy:
-        dest: /tmp/copy-large-file
+        dest: "{{ item.src }}"
-        # Must be larger than Connection.SMALL_SIZE_LIMIT.
+        content: "{{ item.content }}"
-        content: "{% for x in range(200000) %}x{% endfor %}"
+        mode: u=rw,go=r
+      with_items: "{{ sourced_files }}"
+      loop_control:
+        label: "{{ item.src }}"
       delegate_to: localhost
       run_once: true
 
-    - name: Cleanup copied files
+    - name: Cleanup lingering destination files
       file:
+        path: "{{ item.dest }}"
         state: absent
-        path: "{{item}}"
+      with_items: "{{ files }}"
-      with_items:
+      loop_control:
-      - /tmp/copy-tiny-file.out
+        label: "{{ item.dest }}"
-      - /tmp/copy-large-file.out
-      - /tmp/copy-tiny-inline-file.out
-      - /tmp/copy-large-inline-file.out
 
-    - name: Copy large file
+    - name: Copy sourced files
       copy:
-        dest: /tmp/copy-large-file.out
+        src: "{{ item.src }}"
-        src: /tmp/copy-large-file
+        dest: "{{ item.dest }}"
-
+        mode: u=rw,go=r
-    - name: Copy tiny file
+      with_items: "{{ sourced_files }}"
-      copy:
+      loop_control:
-        dest: /tmp/copy-tiny-file.out
+        label: "{{ item.dest }}"
-        src: /tmp/copy-tiny-file
 
-    - name: Copy tiny inline file
+    - name: Copy inline files
       copy:
-        dest: /tmp/copy-tiny-inline-file.out
+        dest: "{{ item.dest }}"
-        content: "tiny inline content"
+        content: "{{ item.content }}"
-
+        mode: u=rw,go=r
-    - name: Copy large inline file
+      with_items: "{{ inline_files }}"
-      copy:
+      loop_control:
-        dest: /tmp/copy-large-inline-file.out
+        label: "{{ item.dest }}"
-        content: |
-          {% for x in range(200000) %}y{% endfor %}
 
     # stat results
 
     - name: Stat copied files
       stat:
-        path: "{{item}}"
+        path: "{{ item.dest }}"
-      with_items:
+      with_items: "{{ files }}"
-      - /tmp/copy-tiny-file.out
+      loop_control:
-      - /tmp/copy-large-file.out
+        label: "{{ item.dest }}"
-      - /tmp/copy-tiny-inline-file.out
-      - /tmp/copy-large-inline-file.out
       register: stat
 
     - assert:
         that:
-        - stat.results[0].stat.checksum == "f29faa9a6f19a700a941bf2aa5b281643c4ec8a0"
+          - item.stat.checksum == item.item.expected_checksum
-        - stat.results[1].stat.checksum == "62951f943c41cdd326e5ce2b53a779e7916a820d"
+        quiet: true  # Avoid spamming stdout with 400 kB of item.item.content
-        - stat.results[2].stat.checksum == "b26dd6444595e2bdb342aa0a91721b57478b5029"
+        fail_msg: item={{ item }}
-        - stat.results[3].stat.checksum == "d675f47e467eae19e49032a2cc39118e12a6ee72"
+      with_items: "{{ stat.results }}"
-        fail_msg: stat={{stat}}
+      loop_control:
+        label: "{{ item.stat.path }}"
 
-    - name: Cleanup files
+    - name: Cleanup destination files
       file:
+        path: "{{ item.dest }}"
         state: absent
-        path: "{{item}}"
+      with_items: "{{ files }}"
-      with_items:
+      loop_control:
-      - /tmp/copy-tiny-file
+        label: "{{ item.dest }}"
-      - /tmp/copy-tiny-file.out
-      - /tmp/copy-no-mode
-      - /tmp/copy-no-mode.out
-      - /tmp/copy-with-mode
-      - /tmp/copy-with-mode.out
-      - /tmp/copy-large-file
-      - /tmp/copy-large-file.out
-      - /tmp/copy-tiny-inline-file.out
-      - /tmp/copy-large-inline-file
-      - /tmp/copy-large-inline-file.out
-
-    # end of cleaning out files (again)
   tags:
     - copy
+    - issue_1110

tests/ansible/integration/ssh/all.yml

@@ -1,3 +1,5 @@
+- import_playbook: args.yml
 - import_playbook: config.yml
+- import_playbook: password.yml
 - import_playbook: timeouts.yml
 - import_playbook: variables.yml

tests/ansible/integration/ssh/args.yml

@@ -0,0 +1,48 @@
+- name: integration/ssh/args.yml
+  hosts: issue905
+  gather_facts: false
+  tasks:
+    # Test that ansible_ssh_common_args are templated; ansible_ssh_args &
+    # ansible_ssh_extra_args aren't directly tested, we assume they're similar.
+    # FIXME This test currently relies on variables set in the host group.
+    #       Ideally they'd be set here, and the host group eliminated, but
+    #       Mitogen currently fails to template when defined in the play.
+    # TODO Replace LocalCommand canary with SetEnv canary, to simplify test.
+    #      Requires modification of sshd_config files to add AcceptEnv ...
+    - name: Test templating of ansible_ssh_common_args et al
+      block:
+        - name: Ensure no lingering canary files
+          file:
+            path: "{{ ssh_args_canary_file }}"
+            state: absent
+          delegate_to: localhost
+
+        - name: Reset connections to force new ssh execution
+          meta: reset_connection
+
+        - name: Perform SSH connection, to trigger side effect
+          ping:
+
+        # LocalCommand="touch {{ ssh_args_canary_file }}" in ssh_*_args
+        - name: Stat for canary file created by side effect
+          stat:
+            path: "{{ ssh_args_canary_file }}"
+          delegate_to: localhost
+          register: ssh_args_canary_stat
+
+        - assert:
+            that:
+              - ssh_args_canary_stat.stat.exists == true
+            quiet: true
+            success_msg: "Canary found: {{ ssh_args_canary_file }}"
+            fail_msg: |
+              ssh_args_canary_file={{ ssh_args_canary_file }}
+              ssh_args_canary_stat={{ ssh_args_canary_stat }}
+      always:
+        - name: Cleanup canary files
+          file:
+            path: "{{ ssh_args_canary_file }}"
+            state: absent
+          delegate_to: localhost
+  tags:
+    - issue_905

tests/ansible/integration/ssh/password.yml

@@ -0,0 +1,51 @@
+- name: integration/ssh/password.yml
+  hosts: test-targets[0]
+  gather_facts: false
+  vars:
+    ansible_user: mitogen__user1
+  tasks:
+    - meta: reset_connection
+    - name: ansible_password
+      vars:
+        ansible_password: user1_password
+      ping:
+
+    - meta: reset_connection
+    - name: ansible_ssh_pass
+      vars:
+        ansible_ssh_pass: user1_password
+      ping:
+
+    - meta: reset_connection
+    - name: absent password should fail
+      ping:
+      ignore_errors: true
+      ignore_unreachable: true
+      register: ssh_no_password_result
+    - assert:
+        that:
+          - ssh_no_password_result.unreachable == True
+        fail_msg: ssh_no_password_result={{ ssh_no_password_result }}
+
+    - meta: reset_connection
+    - name: ansible_ssh_pass should override ansible_password
+      ping:
+      vars:
+        ansible_password: wrong
+        ansible_ssh_pass: user1_password
+
+    # Tests that ansible_ssh_pass has priority over ansible_password
+    # and that a wrong password causes a target to be marked unreachable.
+    - meta: reset_connection
+    - name: ansible_password should not override
+      vars:
+        ansible_password: user1_password
+        ansible_ssh_pass: wrong
+      ping:
+      ignore_errors: true
+      ignore_unreachable: true
+      register: ssh_wrong_password_result
+    - assert:
+        that:
+          - ssh_wrong_password_result.unreachable == True
+        fail_msg: ssh_wrong_password_result={{ ssh_wrong_password_result }}

tests/ansible/integration/ssh/variables.yml

@@ -13,134 +13,6 @@
       -o "ControlPath /tmp/mitogen-ansible-test-{{18446744073709551615|random}}"
 
   tasks:
-    - include_tasks: ../_mitogen_only.yml
-
-    - name: ansible_ssh_user, ansible_ssh_pass
-      shell: >
-        ANSIBLE_ANY_ERRORS_FATAL=false
-        ANSIBLE_STRATEGY=mitogen_linear
-        ANSIBLE_SSH_ARGS="-o HostKeyAlgorithms=+ssh-rsa -o PubkeyAcceptedKeyTypes=+ssh-rsa"
-        ANSIBLE_VERBOSITY="{{ ansible_verbosity }}"
-        ansible -m shell -a whoami
-        {% for inv in ansible_inventory_sources %}
-        -i "{{ inv }}"
-        {% endfor %}
-        test-targets
-        -e ansible_ssh_user=mitogen__has_sudo
-        -e ansible_ssh_pass=has_sudo_password
-      args:
-        chdir: ../..
-      register: out
-
-    - name: ansible_ssh_user, wrong ansible_ssh_pass
-      shell: >
-        ANSIBLE_ANY_ERRORS_FATAL=false
-        ANSIBLE_STRATEGY=mitogen_linear
-        ANSIBLE_SSH_ARGS="-o HostKeyAlgorithms=+ssh-rsa -o PubkeyAcceptedKeyTypes=+ssh-rsa"
-        ANSIBLE_VERBOSITY="{{ ansible_verbosity }}"
-        ansible -m shell -a whoami
-        {% for inv in ansible_inventory_sources %}
-        -i "{{ inv }}"
-        {% endfor %}
-        test-targets
-        -e ansible_ssh_user=mitogen__has_sudo
-        -e ansible_ssh_pass=wrong_password
-        -e ansible_python_interpreter=python3000
-      args:
-        chdir: ../..
-      register: out
-      ignore_errors: true
-
-    - assert:
-        that:
-          - out.rc == 4  # ansible.executor.task_queue_manager.TaskQueueManager.RUN_UNREACHABLE_HOSTS
-        fail_msg: out={{out}}
-
-
-    - name: ansible_user, ansible_ssh_pass
-      shell: >
-        ANSIBLE_ANY_ERRORS_FATAL=false
-        ANSIBLE_STRATEGY=mitogen_linear
-        ANSIBLE_SSH_ARGS="-o HostKeyAlgorithms=+ssh-rsa -o PubkeyAcceptedKeyTypes=+ssh-rsa"
-        ANSIBLE_VERBOSITY="{{ ansible_verbosity }}"
-        ansible -m shell -a whoami
-        {% for inv in ansible_inventory_sources %}
-        -i "{{ inv }}"
-        {% endfor %}
-        test-targets
-        -e ansible_user=mitogen__has_sudo
-        -e ansible_ssh_pass=has_sudo_password
-      args:
-        chdir: ../..
-      register: out
-
-    - name: ansible_user, wrong ansible_ssh_pass
-      shell: >
-        ANSIBLE_ANY_ERRORS_FATAL=false
-        ANSIBLE_STRATEGY=mitogen_linear
-        ANSIBLE_SSH_ARGS="-o HostKeyAlgorithms=+ssh-rsa -o PubkeyAcceptedKeyTypes=+ssh-rsa"
-        ANSIBLE_VERBOSITY="{{ ansible_verbosity }}"
-        ansible -m shell -a whoami
-        {% for inv in ansible_inventory_sources %}
-        -i "{{ inv }}"
-        {% endfor %}
-        test-targets
-        -e ansible_user=mitogen__has_sudo
-        -e ansible_ssh_pass=wrong_password
-        -e ansible_python_interpreter=python3000
-      args:
-        chdir: ../..
-      register: out
-      ignore_errors: true
-
-    - assert:
-        that:
-          - out.rc == 4  # ansible.executor.task_queue_manager.TaskQueueManager.RUN_UNREACHABLE_HOSTS
-        fail_msg: out={{out}}
-
-
-    - name: ansible_user, ansible_password
-      shell: >
-        ANSIBLE_ANY_ERRORS_FATAL=false
-        ANSIBLE_STRATEGY=mitogen_linear
-        ANSIBLE_SSH_ARGS="-o HostKeyAlgorithms=+ssh-rsa -o PubkeyAcceptedKeyTypes=+ssh-rsa"
-        ANSIBLE_VERBOSITY="{{ ansible_verbosity }}"
-        ansible -m shell -a whoami
-        {% for inv in ansible_inventory_sources %}
-        -i "{{ inv }}"
-        {% endfor %}
-        test-targets
-        -e ansible_user=mitogen__has_sudo
-        -e ansible_password=has_sudo_password
-      args:
-        chdir: ../..
-      register: out
-
-    - name: ansible_user, wrong ansible_password
-      shell: >
-        ANSIBLE_ANY_ERRORS_FATAL=false
-        ANSIBLE_STRATEGY=mitogen_linear
-        ANSIBLE_SSH_ARGS="-o HostKeyAlgorithms=+ssh-rsa -o PubkeyAcceptedKeyTypes=+ssh-rsa"
-        ANSIBLE_VERBOSITY="{{ ansible_verbosity }}"
-        ansible -m shell -a whoami
-        {% for inv in ansible_inventory_sources %}
-        -i "{{ inv }}"
-        {% endfor %}
-        test-targets
-        -e ansible_user=mitogen__has_sudo
-        -e ansible_password=wrong_password
-        -e ansible_python_interpreter=python3000
-      args:
-        chdir: ../..
-      register: out
-      ignore_errors: true
-
-    - assert:
-        that:
-          - out.rc == 4  # ansible.executor.task_queue_manager.TaskQueueManager.RUN_UNREACHABLE_HOSTS
-        fail_msg: out={{out}}
-
-
     - name: setup ansible_ssh_private_key_file
       shell: chmod 0600 ../data/docker/mitogen__has_sudo_pubkey.key
       args:

tests/ansible/regression/all.yml

@@ -16,3 +16,4 @@
 - import_playbook: issue_776__load_plugins_called_twice.yml
 - import_playbook: issue_952__ask_become_pass.yml
 - import_playbook: issue_1066__add_host__host_key_checking.yml
+- import_playbook: issue_1087__template_streamerror.yml

tests/ansible/regression/issue_1087__template_streamerror.yml

@@ -0,0 +1,43 @@
+- name: regression/issue_1087__template_streamerror.yml
+  # Ansible's template module has been seen to raise mitogen.core.StreamError
+  # iif  there is a with_items loop and the destination path has an extension.
+  # This printed an error message and left file permissions incorrect,
+  # but did not cause the task/playbook to fail.
+  hosts: test-targets
+  gather_facts: false
+  become: false
+  vars:
+    foos:
+      - dest: /tmp/foo
+      - dest: /tmp/foo.txt
+    foo: Foo
+    bar: Bar
+  tasks:
+    - block:
+        - name: Test template does not cause StreamError
+          delegate_to: localhost
+          run_once: true
+          environment:
+            ANSIBLE_VERBOSITY: "{{ ansible_verbosity }}"
+          command:
+            cmd: >
+              ansible-playbook
+              {% for inv in ansible_inventory_sources %}
+              -i "{{ inv }}"
+              {% endfor %}
+              regression/template_test.yml
+            chdir: ../
+          register: issue_1087_cmd
+          failed_when:
+            - issue_1087_cmd is failed
+              or issue_1087_cmd.stdout is search('ERROR|mitogen\.core\.CallError')
+              or issue_1087_cmd.stderr is search('ERROR|mitogen\.core\.CallError')
+
+      always:
+        - name: Cleanup
+          file:
+            path: "{{ item.dest }}"
+            state: absent
+          with_items: "{{ foos }}"
+  tags:
+    - issue_1087

tests/ansible/regression/template_test.yml

@@ -0,0 +1,28 @@
+- name: regression/template_test.yml
+  # Ansible's template module has been seen to raise mitogen.core.StreamError
+  # iif  there is a with_items loop and the destination path has an extension
+  hosts: test-targets
+  gather_facts: false
+  become: false
+  vars:
+    foos:
+      - dest: /tmp/foo
+      - dest: /tmp/foo.txt
+    foo: Foo
+    bar: Bar
+  tasks:
+    - block:
+        - name: Template files
+          template:
+            src: foo.bar.j2
+            dest: "{{ item.dest }}"
+            mode: u=rw,go=r
+          # This has to be with_items, loop: doesn't trigger the bug
+          with_items: "{{ foos }}"
+
+      always:
+        - name: Cleanup
+          file:
+            path: "{{ item.dest }}"
+            state: absent
+          with_items: "{{ foos }}"

tests/ansible/regression/templates/foo.bar.j2

@@ -0,0 +1 @@
+A {{ foo }} walks into a {{ bar }}. Ow!

tests/ansible/templates/test-targets.j2

@@ -0,0 +1,39 @@
+[test-targets]
+{% for c in containers %}
+{{ c.name }} ansible_host={{ c.hostname }} ansible_port={{ c.port }} ansible_python_interpreter={{ c.python_path }}
+{% endfor %}
+
+[test-targets:vars]
+ansible_user=mitogen__has_sudo_nopw
+ansible_password=has_sudo_nopw_password
+
+{% for distro, hostnames in distros | dictsort %}
+[{{ distro }}]
+{% for hostname in hostnames %}
+{{ hostname }}
+{% endfor %}
+{% endfor %}
+
+{% for family, hostnames in families | dictsort %}
+[{{ family }}]
+{% for hostname in hostnames %}
+{{ hostname }}
+{% endfor %}
+{% endfor %}
+
+[linux:children]
+test-targets
+
+[linux_containers:children]
+test-targets
+
+[issue905]
+{% for c in containers[:1] %}
+ssh-common-args ansible_host={{ c.hostname }} ansible_port={{ c.port }} ansible_python_interpreter={{ c.python_path }}
+{% endfor %}
+
+[issue905:vars]
+ansible_user=mitogen__has_sudo_nopw
+ansible_password=has_sudo_nopw_password
+ansible_ssh_common_args=-o PermitLocalCommand=yes -o LocalCommand="touch {{ '{{' }} ssh_args_canary_file {{ '}}' }}"
+ssh_args_canary_file=/tmp/ssh_args_{{ '{{' }} inventory_hostname {{ '}}' }}

tests/connection_test.py

@@ -1,3 +1,4 @@
+import logging
 import os
 import signal
 import sys
@@ -54,7 +55,9 @@ def do_detach(econtext):
 class DetachReapTest(testlib.RouterMixin, testlib.TestCase):
     def test_subprocess_preserved_on_shutdown(self):
         c1 = self.router.local()
+        c1_stream = self.router.stream_by_id(c1.context_id)
         pid = c1.call(os.getpid)
+        self.assertEqual(pid, c1_stream.conn.proc.pid)
 
         l = mitogen.core.Latch()
         mitogen.core.listen(c1, 'disconnect', l.put)
@@ -64,8 +67,8 @@ class DetachReapTest(testlib.RouterMixin, testlib.TestCase):
         self.broker.shutdown()
         self.broker.join()
 
-        os.kill(pid, 0)  # succeeds if process still alive
+        self.assertIsNone(os.kill(pid, 0))  # succeeds if process still alive
 
         # now clean up
-        os.kill(pid, signal.SIGTERM)
+        c1_stream.conn.proc.terminate()
-        os.waitpid(pid, 0)
+        c1_stream.conn.proc.proc.wait()

tests/create_child_test.py

@@ -76,6 +76,7 @@ def close_proc(proc):
     proc.stdout.close()
     if proc.stderr:
         proc.stderr.close()
+    proc.proc.wait()
 
 
 def wait_read(fp, n):

tests/data/importer/six_brokenpkg/__init__.py

@@ -53,4 +53,4 @@ if _system_six:
 else:
     from . import _six as six
 six_py_file = '{0}.py'.format(os.path.splitext(six.__file__)[0])
-exec(open(six_py_file, 'rb').read())
+with open(six_py_file, 'rb') as f: exec(f.read())

tests/id_allocation_test.py

@@ -27,3 +27,6 @@ class SlaveTest(testlib.RouterMixin, testlib.TestCase):
         # Subsequent master allocation does not collide
         c2 = self.router.local()
         self.assertEqual(1002, c2.context_id)
+
+        context.shutdown()
+        c2.shutdown()

tests/reaper_test.py

@@ -10,8 +10,7 @@ import mitogen.parent
 
 
 class ReaperTest(testlib.TestCase):
-    @mock.patch('os.kill')
+    def test_calc_delay(self):
-    def test_calc_delay(self, kill):
         broker = mock.Mock()
         proc = mock.Mock()
         proc.poll.return_value = None
@@ -24,8 +23,7 @@ class ReaperTest(testlib.TestCase):
         self.assertEqual(752, int(1000 * reaper._calc_delay(5)))
         self.assertEqual(1294, int(1000 * reaper._calc_delay(6)))
 
-    @mock.patch('os.kill')
+    def test_reap_calls(self):
-    def test_reap_calls(self, kill):
         broker = mock.Mock()
         proc = mock.Mock()
         proc.poll.return_value = None
@@ -33,20 +31,20 @@ class ReaperTest(testlib.TestCase):
         reaper = mitogen.parent.Reaper(broker, proc, True, True)
 
         reaper.reap()
-        self.assertEqual(0, kill.call_count)
+        self.assertEqual(0, proc.send_signal.call_count)
 
         reaper.reap()
-        self.assertEqual(1, kill.call_count)
+        self.assertEqual(1, proc.send_signal.call_count)
 
         reaper.reap()
         reaper.reap()
         reaper.reap()
-        self.assertEqual(1, kill.call_count)
+        self.assertEqual(1, proc.send_signal.call_count)
 
         reaper.reap()
-        self.assertEqual(2, kill.call_count)
+        self.assertEqual(2, proc.send_signal.call_count)
 
-        self.assertEqual(kill.mock_calls, [
+        self.assertEqual(proc.send_signal.mock_calls, [
-            mock.call(proc.pid, signal.SIGTERM),
+            mock.call(signal.SIGTERM),
-            mock.call(proc.pid, signal.SIGKILL),
+            mock.call(signal.SIGKILL),
         ])

tests/ssh_test.py

@@ -190,6 +190,7 @@ class BannerTest(testlib.DockerMixin, testlib.TestCase):
             self.dockerized_ssh.port,
         )
         self.assertEqual(name, context.name)
+        context.shutdown(wait=True)
 
 
 class StubPermissionDeniedTest(StubSshMixin, testlib.TestCase):

tests/testlib.py

@@ -146,6 +146,17 @@ def data_path(suffix):
     return path
 
 
+def retry(fn, on, max_attempts, delay):
+    for i in range(max_attempts):
+        try:
+            return fn()
+        except on:
+            if i >= max_attempts - 1:
+                raise
+            else:
+                time.sleep(delay)
+
+
 def threading__thread_is_alive(thread):
     """Return whether the thread is alive (Python version compatibility shim).
 
@@ -562,18 +573,24 @@ class DockerizedSshDaemon(object):
         wait_for_port(self.get_host(), self.port, pattern='OpenSSH')
 
     def check_processes(self):
-        args = ['docker', 'exec', self.container_name, 'ps', '-o', 'comm=']
+        # Get Accounting name (ucomm) & command line (args) of each process
+        # in the container. No truncation (-ww). No column headers (foo=).
+        ps_output = subprocess.check_output([
+            'docker', 'exec', self.container_name,
+            'ps', '-w', '-w', '-o', 'ucomm=', '-o', 'args=',
+        ])
+        ps_lines = ps_output.decode().splitlines()
+        processes = [tuple(line.split(None, 1)) for line in ps_lines]
         counts = {}
-        for comm in subprocess.check_output(args).decode().splitlines():
+        for ucomm, _ in processes:
-            comm = comm.strip()
+            counts[ucomm] = counts.get(ucomm, 0) + 1
-            counts[comm] = counts.get(comm, 0) + 1
 
         if counts != {'ps': 1, 'sshd': 1}:
             assert 0, (
                 'Docker container %r contained extra running processes '
                 'after test completed: %r' % (
                     self.container_name,
-                    counts
+                    processes,
                 )
             )
 
@@ -630,7 +647,12 @@ class DockerMixin(RouterMixin):
 
     @classmethod
     def tearDownClass(cls):
-        cls.dockerized_ssh.check_processes()
+        retry(
+            cls.dockerized_ssh.check_processes,
+            on=AssertionError,
+            max_attempts=5,
+            delay=0.1,
+        )
         cls.dockerized_ssh.close()
         super(DockerMixin, cls).tearDownClass()
 

tests/unix_test.py

@@ -65,17 +65,13 @@ class ListenerTest(testlib.RouterMixin, testlib.TestCase):
 
     def test_constructor_basic(self):
         listener = self.klass.build_stream(router=self.router)
-        capture = testlib.LogCapturer()
+        self.assertFalse(mitogen.unix.is_path_dead(listener.protocol.path))
-        capture.start()
+        os.unlink(listener.protocol.path)
-        try:
+
-            self.assertFalse(mitogen.unix.is_path_dead(listener.protocol.path))
+        # ensure we catch 0 byte read error log message
-            os.unlink(listener.protocol.path)
+        self.broker.shutdown()
-            # ensure we catch 0 byte read error log message
+        self.broker.join()
-            self.broker.shutdown()
+        self.broker_shutdown = True
-            self.broker.join()
-            self.broker_shutdown = True
-        finally:
-            capture.stop()
 
 
 class ClientTest(testlib.TestCase):

tox.ini

@@ -74,9 +74,9 @@ basepython =
 deps =
     -r{toxinidir}/tests/requirements.txt
     mode_ansible: -r{toxinidir}/tests/ansible/requirements.txt
-    ansible2.10: ansible==2.10.7
+    ansible2.10: ansible~=2.10.0
-    ansible3: ansible==3.4.0
+    ansible3: ansible~=3.0
-    ansible4: ansible==4.10.0
+    ansible4: ansible~=4.0
     ansible5: ansible~=5.0
     ansible6: ansible~=6.0
     ansible7: ansible~=7.0