Merge remote-tracking branch 'origin/dmw'

- Travis fixes
- 3.x fixes
- #420

.travis/ansible_tests.py

@@ -63,5 +63,6 @@ with ci_lib.Fold('job_setup'):
 
 
 with ci_lib.Fold('ansible'):
-    run('/usr/bin/time ./run_ansible_playbook.py all.yml -i "%s" %s',
+    playbook = os.environ.get('PLAYBOOK', 'all.yml')
-        HOSTS_DIR, ' '.join(sys.argv[1:]))
+    run('/usr/bin/time ./run_ansible_playbook.py %s -i "%s" %s',
+        playbook, HOSTS_DIR, ' '.join(sys.argv[1:]))

.travis/ci_lib.py

@@ -37,11 +37,20 @@ if not hasattr(subprocess, 'check_output'):
 
 # Force stdout FD 1 to be a pipe, so tools like pip don't spam progress bars.
 
-sys.stdout = os.popen('stdbuf -oL cat', 'w', 1)
+proc = subprocess.Popen(
-os.dup2(sys.stdout.fileno(), 1)
+    args=['stdbuf', '-oL', 'cat'],
+    stdin=subprocess.PIPE
+)
+
+os.dup2(proc.stdin.fileno(), 1)
+os.dup2(proc.stdin.fileno(), 2)
+
+def cleanup_travis_junk(stdout=sys.stdout, stderr=sys.stderr, proc=proc):
+    stdout.close()
+    stderr.close()
+    proc.terminate()
 
-sys.stderr = sys.stdout
+atexit.register(cleanup_travis_junk)
-os.dup2(sys.stderr.fileno(), 2)
 
 # -----------------
 
@@ -54,7 +63,9 @@ def _argv(s, *args):
 def run(s, *args, **kwargs):
     argv = _argv(s, *args)
     print('Running: %s' % (argv,))
-    return subprocess.check_call(argv, **kwargs)
+    ret = subprocess.check_call(argv, **kwargs)
+    print('Finished running: %s' % (argv,))
+    return ret
 
 
 def get_output(s, *args, **kwargs):

ansible_mitogen/connection.py

@@ -42,6 +42,7 @@ import ansible.errors
 import ansible.plugins.connection
 import ansible.utils.shlex
 
+import mitogen.fork
 import mitogen.unix
 import mitogen.utils
 
@@ -802,19 +803,32 @@ class Connection(ansible.plugins.connection.ConnectionBase):
         self.init_child_result = None
         self.chain = None
 
-    def close(self):
+    def _shutdown_broker(self):
         """
-        Arrange for the mitogen.master.Router running in the worker to
+        Shutdown the broker thread during :meth:`close` or :meth:`reset`.
-        gracefully shut down, and wait for shutdown to complete. Safe to call
-        multiple times.
         """
-        self._mitogen_reset(mode='put')
         if self.broker:
             self.broker.shutdown()
             self.broker.join()
             self.broker = None
             self.router = None
 
+        # #420: Ansible executes "meta" actions in the top-level process,
+        # meaning "reset_connection" will cause :class:`mitogen.core.Latch` FDs
+        # to be cached and subsequently erroneously shared by children on
+        # subsequent task forks. To handle that, call on_fork() to ensure any
+        # shared state is discarded.
+        mitogen.fork.on_fork()
+
+    def close(self):
+        """
+        Arrange for the mitogen.master.Router running in the worker to
+        gracefully shut down, and wait for shutdown to complete. Safe to call
+        multiple times.
+        """
+        self._mitogen_reset(mode='put')
+        self._shutdown_broker()
+
     reset_compat_msg = (
         'Mitogen only supports "reset_connection" on Ansible 2.5.6 or later'
     )
@@ -835,6 +849,7 @@ class Connection(ansible.plugins.connection.ConnectionBase):
 
         self._connect()
         self._mitogen_reset(mode='reset')
+        self._shutdown_broker()
 
     # Compatibility with Ansible 2.4 wait_for_connection plug-in.
     _reset = reset
@@ -914,9 +929,9 @@ class Connection(ansible.plugins.connection.ConnectionBase):
             emulate_tty=emulate_tty,
         )
 
-        stderr += 'Shared connection to %s closed.%s' % (
+        stderr += b'Shared connection to %s closed.%s' % (
-            self._play_context.remote_addr,
+            self._play_context.remote_addr.encode(),
-            ('\r\n' if emulate_tty else '\n'),
+            (b'\r\n' if emulate_tty else b'\n'),
         )
         return rc, stdout, stderr
 

ansible_mitogen/target.py

@@ -541,7 +541,7 @@ def exec_args(args, in_data='', chdir=None, shell=None, emulate_tty=False):
 
     if emulate_tty:
         stdout = stdout.replace(b'\n', b'\r\n')
-    return proc.returncode, stdout, stderr or ''
+    return proc.returncode, stdout, stderr or b''
 
 
 def exec_command(cmd, in_data='', chdir=None, shell=None, emulate_tty=False):

dev_requirements.txt

@@ -1,4 +1,5 @@
 -r docs/docs-requirements.txt
+-r tests/ansible/requirements.txt
 psutil==5.4.8
 coverage==4.5.1
 Django==1.6.11  # Last version supporting 2.6.

docs/changelog.rst

@@ -186,6 +186,11 @@ Fixes
 * `#410 <https://github.com/dw/mitogen/issues/410>`_: the sudo method supports
   the SELinux ``--type`` and ``--role`` options.
 
+* `#420 <https://github.com/dw/mitogen/issues/420>`_: if a :class:`Connection`
+  was constructed in the Ansible top-level process, for example while executing
+  ``meta: reset_connection``, resources could become undesirably shared in
+  subsequent children.
+
 
 Core Library
 ~~~~~~~~~~~~

mitogen/core.py

@@ -1778,16 +1778,16 @@ class Latch(object):
             self._cls_all_sockets.extend((rsock, wsock))
             return rsock, wsock
 
-    COOKIE_SIZE = 33
-
     def _make_cookie(self):
         """
-        Return a 33-byte string encoding the ID of the instance and the current
+        Return a string encoding the ID of the instance and the current thread.
-        thread. This disambiguates legitimate wake-ups, accidental writes to
+        This disambiguates legitimate wake-ups, accidental writes to the FD,
-        the FD, and buggy internal FD sharing.
+        and buggy internal FD sharing.
         """
         ident = threading.currentThread().ident
-        return b(u'%016x-%016x' % (int(id(self)), ident))
+        return b(u'%010d-%016x-%016x' % (os.getpid(), int(id(self)), ident))
+
+    COOKIE_SIZE = len(_make_cookie(None))
 
     def get(self, timeout=None, block=True):
         """

mitogen/setns.py

@@ -69,7 +69,7 @@ def _run_command(args):
 
     output, _ = proc.communicate()
     if not proc.returncode:
-        return output
+        return output.decode('utf-8', 'replace')
 
     raise Error("%s exitted with status %d: %s",
                 mitogen.parent.Argv(args), proc.returncode, output)

tests/ansible/integration/action/synchronize.yml

@@ -5,6 +5,7 @@
   any_errors_fatal: true
   vars:
     ansible_user: mitogen__has_sudo_pubkey
+    ansible_become_pass: has_sudo_pubkey_password
     ansible_ssh_private_key_file: /tmp/synchronize-action-key
   tasks:
     # must copy git file to set proper file mode.

tests/ansible/integration/async/all.yml

@@ -1,8 +1,9 @@
+# - import_playbook: multiple_items_loop.yml
 - import_playbook: result_binary_producing_json.yml
 - import_playbook: result_binary_producing_junk.yml
 - import_playbook: result_shell_echo_hi.yml
 - import_playbook: runner_new_process.yml
 - import_playbook: runner_one_job.yml
 - import_playbook: runner_timeout_then_polling.yml
-- import_playbook: runner_with_polling_and_timeout.yml
 - import_playbook: runner_two_simultaneous_jobs.yml
+- import_playbook: runner_with_polling_and_timeout.yml

tests/ansible/integration/async/multiple_items_loop.yml

@@ -0,0 +1,36 @@
+# issue #414: verify behaviour of async tasks created in a loop.
+
+- name: integration/async/multiple_items_loop.yml
+  hosts: test-targets
+  any_errors_fatal: true
+  tasks:
+
+  - name: start long running ops
+    become: true
+    shell: "{{item}}"
+    async: 15
+    poll: 0
+    register: jobs
+    with_items:
+    - "sleep 3; echo hi-from-job-1"
+    - "sleep 5; echo hi-from-job-2"
+
+  - name: Ensure static files are collected and compressed
+    async_status:
+      jid: "{{ item.ansible_job_id }}"
+    become: yes
+    register: out
+    until: out.finished
+    retries: 30
+    with_items:
+      - "{{ jobs.results }}"
+
+  - assert:
+      that:
+      - out.results[0].stdout == 'hi-from-job-1'
+      - out.results[0].rc == 0
+      - out.results[0].delta > '0:00:03'
+
+      - out.results[1].stdout == 'hi-from-job-2'
+      - out.results[1].rc == 0
+      - out.results[1].delta > '0:00:05'

tests/ansible/integration/connection/_disconnect_during_module.yml

@@ -0,0 +1,13 @@
+# issue 352: test ability to notice disconnection during a module invocation.
+---
+
+- name: integration/connection/_disconnect_during_module.yml
+  hosts: test-targets
+  gather_facts: no
+  any_errors_fatal: false
+  tasks:
+  - run_once: true  # don't run against localhost
+    shell: |
+      kill -9 $PPID
+    register: out
+    ignore_errors: true

tests/ansible/integration/connection/disconnect_during_module.yml

@@ -2,18 +2,26 @@
 ---
 
 - name: integration/connection/disconnect_during_module.yml
-  hosts: test-targets localhost
+  hosts: test-targets
   gather_facts: no
   any_errors_fatal: false
   tasks:
-  - run_once: true  # don't run against localhost
+  - meta: end_play
-    shell: |
+    when: not is_mitogen
-      kill -9 $PPID
+
+  - connection: local
+    command: |
+      ansible-playbook
+      -i "{{inventory_file}}"
+      integration/connection/_disconnect_during_module.yml
+    args:
+      chdir: ../..
     register: out
     ignore_errors: true
 
+  - debug: var=out
+
   - assert:
       that:
-      - out.msg.startswith('Mitogen was disconnected from the remote environment while a call was in-progress.')
+      - out.rc == 4
-
+      - "'Mitogen was disconnected from the remote environment while a call was in-progress.' in out.stdout"
-  - meta: clear_host_errors

tests/ansible/integration/connection/disconnect_resets_connection.yml

@@ -14,6 +14,9 @@
   gather_facts: no
   any_errors_fatal: true
   tasks:
+  - meta: end_play
+    when: not is_mitogen
+
   - mitogen_action_script:
       script: |
         import sys

tests/ansible/integration/connection/exec_command.yml

@@ -15,5 +15,5 @@
   - assert:
       that:
       - out.result[0] == 0
-      - out.result[1] == "hello, world\r\n"
+      - out.result[1].decode() == "hello, world\r\n"
-      - out.result[2].startswith("Shared connection to ")
+      - out.result[2].decode().startswith("Shared connection to ")

tests/ansible/integration/context_service/disconnect_cleanup.yml

@@ -2,12 +2,15 @@
 # state of dependent contexts (e.g. sudo, connection delegation, ..).
 
 - name: integration/context_service/disconnect_cleanup.yml
-  hosts: test-targets
+  hosts: test-targets[0]
   any_errors_fatal: true
   tasks:
   - meta: end_play
     when: not is_mitogen
 
+  - meta: end_play
+    when: ansible_version.full < '2.5.6'
+
   # Start with a clean slate.
   - mitogen_shutdown_all:
 
@@ -28,7 +31,7 @@
     register: out
 
   - assert:
-      that: out.dump|length == 4  # ssh account + 3 sudo accounts
+      that: out.dump|length == (play_hosts|length) * 4 # ssh account + 3 sudo accounts
 
   - meta: reset_connection
 
@@ -43,4 +46,4 @@
     register: out
 
   - assert:
-      that: out.dump|length == 1  # just the ssh account
+      that: out.dump|length == play_hosts|length # just the ssh account

tests/ansible/integration/ssh/variables.yml

@@ -3,7 +3,7 @@
 # whatever reason.
 
 - name: integration/ssh/variables.yml
-  hosts: test-targets
+  hosts: test-targets[0]
   connection: local
   vars:
     # ControlMaster has the effect of caching the previous auth to the same

tests/ansible/integration/strategy/_mixed_mitogen_vanilla.yml

@@ -2,9 +2,10 @@
 # issue #294: ensure running mixed vanilla/Mitogen succeeds.
 
 - name: integration/strategy/_mixed_mitogen_vanilla.yml (mitogen_linear)
-  hosts: test-targets
+  hosts: test-targets[0]
   any_errors_fatal: true
   strategy: mitogen_linear
+  run_once: true
   tasks:
     - custom_python_detect_environment:
       register: out
@@ -15,8 +16,9 @@
     - assert:
         that: strategy == 'ansible.plugins.strategy.mitogen_linear.StrategyModule'
 
+
 - name: integration/strategy/_mixed_mitogen_vanilla.yml (linear)
-  hosts: test-targets
+  hosts: test-targets[0]
   any_errors_fatal: true
   strategy: linear
   tasks:
@@ -31,7 +33,7 @@
 
 
 - name: integration/strategy/_mixed_mitogen_vanilla.yml (mitogen_linear)
-  hosts: test-targets
+  hosts: test-targets[0]
   any_errors_fatal: true
   strategy: mitogen_linear
   tasks:

tests/ansible/integration/strategy/_mixed_vanilla_mitogen.yml

@@ -2,7 +2,7 @@
 # issue #294: ensure running mixed vanilla/Mitogen succeeds.
 
 - name: integration/strategy/_mixed_vanilla_mitogen.yml (linear)
-  hosts: test-targets
+  hosts: test-targets[0]
   any_errors_fatal: true
   strategy: linear
   tasks:
@@ -16,7 +16,7 @@
         that: strategy == 'ansible.plugins.strategy.linear.StrategyModule'
 
 - name: integration/strategy/_mixed_vanilla_mitogen.yml (mitogen_linear)
-  hosts: test-targets
+  hosts: test-targets[0]
   any_errors_fatal: true
   strategy: mitogen_linear
   tasks:
@@ -31,7 +31,7 @@
 
 
 - name: integration/strategy/_mixed_vanilla_mitogen.yml (linear)
-  hosts: test-targets
+  hosts: test-targets[0]
   any_errors_fatal: true
   strategy: linear
   tasks:

tests/ansible/integration/strategy/mixed_vanilla_mitogen.yml

@@ -1,12 +1,13 @@
 
 - name: integration/strategy/mixed_vanilla_mitogen.yml (linear->mitogen->linear)
-  hosts: test-targets
+  hosts: test-targets[0]
   any_errors_fatal: true
   tasks:
     - connection: local
       command: |
         ansible-playbook
         -i "{{inventory_file}}"
+        -vvv
         integration/strategy/_mixed_mitogen_vanilla.yml
       args:
         chdir: ../..
@@ -16,6 +17,7 @@
       command: |
         ansible-playbook
         -i "{{inventory_file}}"
+        -vvv
         integration/strategy/_mixed_vanilla_mitogen.yml
       args:
         chdir: ../..

tests/ansible/integration/stub_connections/mitogen_sudo.yml

@@ -10,6 +10,7 @@
   - custom_python_detect_environment:
     vars:
       ansible_connection: mitogen_sudo
+      ansible_user: root
       ansible_become_exe: stub-sudo.py
       ansible_become_flags: --type=sometype --role=somerole
     register: out

tests/ansible/integration/stub_connections/setns_lxc.yml

@@ -14,11 +14,11 @@
     - include_tasks: _end_play_if_not_sudo_linux.yml
 
     - command: |
-        sudo -nE ansible
+        sudo -nE "{{lookup('env', 'VIRTUAL_ENV')}}/bin/ansible"
         -i localhost,
         -c setns
         -e mitogen_kind=lxc
-        -e mitogen_lxc_info_path=stub-lxc-info.py
+        -e mitogen_lxc_info_path={{git_basedir}}/tests/data/stubs/stub-lxc-info.py
         -m shell
         -a "echo hi"
         localhost

tests/ansible/integration/stub_connections/setns_lxd.yml

@@ -14,11 +14,11 @@
     - include_tasks: _end_play_if_not_sudo_linux.yml
 
     - command: |
-        sudo -nE ansible
+        sudo -nE "{{lookup('env', 'VIRTUAL_ENV')}}/bin/ansible"
         -i localhost,
         -c setns
         -e mitogen_kind=lxd
-        -e mitogen_lxc_path=stub-lxc.py
+        -e mitogen_lxc_path={{git_basedir}}/tests/data/stubs/stub-lxc.py
         -m shell
         -a "echo hi"
         localhost

tests/ansible/mitogen_ansible_playbook.py

@@ -0,0 +1,6 @@
+#!/usr/bin/env python
+import os
+import subprocess
+import sys
+os.environ['ANSIBLE_STRATEGY'] = 'mitogen_linear'
+subprocess.check_call(['./run_ansible_playbook.py'] + sys.argv[1:])

tests/ansible/mitogen_ansible_playbook.sh

@@ -1,3 +0,0 @@
-#!/bin/bash
-export ANSIBLE_STRATEGY=mitogen_linear
-exec ./run_ansible_playbook.py "$@"

tests/ansible/requirements.txt

@@ -1,2 +1,4 @@
+ansible; python_version >= '2.7'
+ansible<2.7; python_version < '2.7'
 paramiko==2.3.2  # Last 2.6-compat version.
 google-api-python-client==1.6.5

tests/ansible/run_ansible_playbook.py

@@ -12,6 +12,11 @@ GIT_BASEDIR = os.path.dirname(
     )
 )
 
+# Ensure VIRTUAL_ENV is exported.
+os.environ.setdefault(
+    'VIRTUAL_ENV',
+    os.path.dirname(os.path.dirname(sys.executable))
+)
 
 # Used by delegate_to.yml to ensure "sudo -E" preserves environment.
 os.environ['I_WAS_PRESERVED'] = '1'

tests/image_prep/_container_setup.yml

@@ -2,7 +2,6 @@
 - hosts: all
   strategy: linear
   gather_facts: false
-  become: true
   tasks:
     - raw: >
         if ! python -c ''; then
@@ -96,6 +95,11 @@
         dest: /etc/sudoers.d/001-mitogen
         src: ../data/docker/001-mitogen.sudo
 
+    - lineinfile:
+        path: /etc/sudoers
+        line: "%wheel ALL=(ALL) ALL"
+      when: distro == "CentOS"
+
     - lineinfile:
         path: /etc/ssh/sshd_config
         line: Banner /etc/ssh/banner.txt

tests/image_prep/ansible.cfg

@@ -2,3 +2,5 @@
 [defaults]
 strategy_plugins = ../../ansible_mitogen/plugins/strategy
 retry_files_enabled = false
+display_args_to_stdout = True
+no_target_syslog = True

tests/image_prep/build_docker_images.py

@@ -6,9 +6,10 @@ Build the Docker images used for testing.
 
 import commands
 import os
-import tempfile
 import shlex
 import subprocess
+import sys
+import tempfile
 
 
 BASEDIR = os.path.dirname(os.path.abspath(__file__))
@@ -42,7 +43,7 @@ with tempfile.NamedTemporaryFile() as fp:
     try:
         subprocess.check_call(
             cwd=BASEDIR,
-            args=sh('ansible-playbook -i %s -c docker setup.yml', fp.name),
+            args=sh('ansible-playbook -i %s -c docker setup.yml', fp.name) + sys.argv[1:],
         )
 
         for container_id, label in label_by_id.items():

tests/parent_test.py

@@ -31,8 +31,25 @@ def wait_for_child(pid, timeout=1.0):
 
 
 @mitogen.core.takes_econtext
-def call_func_in_sibling(ctx, econtext):
+def call_func_in_sibling(ctx, econtext, sync_sender):
-    ctx.call(time.sleep, 99999)
+    recv = ctx.call_async(time.sleep, 99999)
+    sync_sender.send(None)
+    recv.get().unpickle()
+
+
+def wait_for_empty_output_queue(sync_recv, context):
+    # wait for sender to submit their RPC. Since the RPC is sent first, the
+    # message sent to this sender cannot arrive until we've routed the RPC.
+    sync_recv.get()
+
+    router = context.router
+    broker = router.broker
+    while True:
+        # Now wait for the RPC to exit the output queue.
+        stream = router.stream_by_id(context.context_id)
+        if broker.defer_sync(lambda: stream.pending_bytes()) == 0:
+            return
+        time.sleep(0.1)
 
 
 class GetDefaultRemoteNameTest(testlib.TestCase):
@@ -353,12 +370,17 @@ class DisconnectTest(testlib.RouterMixin, testlib.TestCase):
         self.router.stream_by_id(c1.context_id).auth_id = mitogen.context_id
         c1.call(mitogen.parent.upgrade_router)
 
-        recv = c1.call_async(call_func_in_sibling, c2)
+        sync_recv = mitogen.core.Receiver(self.router)
+        recv = c1.call_async(call_func_in_sibling, c2,
+            sync_sender=sync_recv.to_sender())
+
+        wait_for_empty_output_queue(sync_recv, c2)
         c2.shutdown(wait=True)
+
         e = self.assertRaises(mitogen.core.CallError,
             lambda: recv.get().unpickle())
         s = 'mitogen.core.ChannelError: ' + self.router.respondent_disconnect_msg
-        self.assertTrue(e.args[0].startswith(s))
+        self.assertTrue(e.args[0].startswith(s), str(e))
 
     def test_far_sibling_disconnected(self):
         # God mode: child of child notices child of child of parent has
@@ -373,8 +395,13 @@ class DisconnectTest(testlib.RouterMixin, testlib.TestCase):
         self.router.stream_by_id(c1.context_id).auth_id = mitogen.context_id
         c11.call(mitogen.parent.upgrade_router)
 
-        recv = c11.call_async(call_func_in_sibling, c22)
+        sync_recv = mitogen.core.Receiver(self.router)
+        recv = c11.call_async(call_func_in_sibling, c22,
+            sync_sender=sync_recv.to_sender())
+
+        wait_for_empty_output_queue(sync_recv, c22)
         c22.shutdown(wait=True)
+
         e = self.assertRaises(mitogen.core.CallError,
             lambda: recv.get().unpickle())
         s = 'mitogen.core.ChannelError: ' + self.router.respondent_disconnect_msg

tox.ini

@@ -8,6 +8,7 @@ envlist =
 [testenv]
 deps =
     -r{toxinidir}/dev_requirements.txt
+    -r{toxinidir}/tests/ansible/requirements.txt
 
 commands =
     {posargs:bash run_tests}