Merge branch 'master' into patch-1

.ci/ansible_install.py

@@ -6,10 +6,12 @@ batches = [
     [
         # Must be installed separately, as PyNACL indirect requirement causes
         # newer version to be installed if done in a single pip run.
+        # Separately install ansible based on version passed in from azure-pipelines.yml or .travis.yml
         'pip install "pycparser<2.19" "idna<2.7"',
         'pip install '
             '-r tests/requirements.txt '
             '-r tests/ansible/requirements.txt',
+        'pip install -q ansible=={0}'.format(ci_lib.ANSIBLE_VERSION)
     ]
 ]
 

.ci/ansible_tests.py

@@ -66,10 +66,6 @@ with ci_lib.Fold('job_setup'):
     ci_lib.dump_file(inventory_path)
 
     if not ci_lib.exists_in_path('sshpass'):
-        # fix errors with apt-get update
-        run("sudo apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys 78BD65473CB3BD13")
-        run("sudo sed -i -e 's#deb https://downloads.apache.org/cassandra/debian 39x main#deb http://downloads.apache.org/cassandra/debian 39x main#g' /etc/apt/sources.list.d/cassandra.list")
-
         run("sudo apt-get update")
         run("sudo apt-get install -y sshpass")
 

.ci/azure-pipelines.yml

@@ -13,10 +13,10 @@ jobs:
   strategy:
     matrix:
       Mito27_27:
-        python.version: '2.7'
+        python.version: '2.7.18'
         MODE: mitogen
       Ans288_27:
-        python.version: '2.7'
+        python.version: '2.7.18'
         MODE: localhost_ansible
         VER: 2.8.8
 
@@ -46,6 +46,12 @@ jobs:
         MODE: mitogen
         DISTRO: centos6
 
+      Mito37Debian_27:
+        python.version: '3.7'
+        MODE: mitogen
+        DISTRO: debian
+        VER: 2.9.6
+
       #Py26CentOS7:
         #python.version: '2.7'
         #MODE: mitogen
@@ -97,3 +103,8 @@ jobs:
         python.version: '3.5'
         MODE: ansible
         VER: 2.8.0
+
+      Ansible_296_37:
+        python.version: '3.7'
+        MODE: ansible
+        VER: 2.9.6

.ci/localhost_ansible_install.py

@@ -6,10 +6,12 @@ batches = [
     [
         # Must be installed separately, as PyNACL indirect requirement causes
         # newer version to be installed if done in a single pip run.
+        # Separately install ansible based on version passed in from azure-pipelines.yml or .travis.yml
         'pip install "pycparser<2.19" "idna<2.7"',
         'pip install '
             '-r tests/requirements.txt '
             '-r tests/ansible/requirements.txt',
+        'pip install -q ansible=={}'.format(ci_lib.ANSIBLE_VERSION)
     ]
 ]
 

.ci/localhost_ansible_tests.py

@@ -1,9 +1,7 @@
 #!/usr/bin/env python
 # Run tests/ansible/all.yml under Ansible and Ansible-Mitogen
 
-import glob
 import os
-import shutil
 import sys
 
 import ci_lib
@@ -31,16 +29,17 @@ with ci_lib.Fold('job_setup'):
 
 
 with ci_lib.Fold('machine_prep'):
-    ssh_dir = os.path.expanduser('~/.ssh')
+    # generate a new ssh key for localhost ssh
-    if not os.path.exists(ssh_dir):
+    os.system("ssh-keygen -P '' -m pem -f ~/.ssh/id_rsa")
-        os.makedirs(ssh_dir, int('0700', 8))
+    os.system("cat ~/.ssh/id_rsa.pub >> ~/.ssh/authorized_keys")
-
+    # also generate it for the sudo user
-    key_path = os.path.expanduser('~/.ssh/id_rsa')
+    os.system("sudo ssh-keygen -P '' -m pem -f /var/root/.ssh/id_rsa")
-    shutil.copy(KEY_PATH, key_path)
+    os.system("sudo cat /var/root/.ssh/id_rsa.pub | sudo tee -a /var/root/.ssh/authorized_keys")
-
+    os.chmod(os.path.expanduser('~/.ssh'), int('0700', 8))
-    auth_path = os.path.expanduser('~/.ssh/authorized_keys')
+    os.chmod(os.path.expanduser('~/.ssh/authorized_keys'), int('0600', 8))
-    os.system('ssh-keygen -y -f %s >> %s' % (key_path, auth_path))
+    # run chmod through sudo since it's owned by root
-    os.chmod(auth_path, int('0600', 8))
+    os.system('sudo chmod 600 /var/root/.ssh')
+    os.system('sudo chmod 600 /var/root/.ssh/authorized_keys')
 
     if os.path.expanduser('~mitogen__user1') == '~mitogen__user1':
         os.chdir(IMAGE_PREP_DIR)

.travis.yml

@@ -36,6 +36,9 @@ matrix:
 
   include:
     # Debops tests.
+    # 2.9.6; 3.6 -> 2.7
+    - python: "3.6"
+      env: MODE=debops_common VER=2.9.6
     # 2.8.3; 3.6 -> 2.7
     - python: "3.6"
       env: MODE=debops_common VER=2.8.3
@@ -49,6 +52,9 @@ matrix:
 
     # ansible_mitogen tests.
 
+    # 2.9.6 -> {debian, centos6, centos7}
+    - python: "3.6"
+      env: MODE=ansible VER=2.9.6
     # 2.8.3 -> {debian, centos6, centos7}
     - python: "3.6"
       env: MODE=ansible VER=2.8.3
@@ -75,6 +81,8 @@ matrix:
     # 2.7 -> 2.6
     #- python: "2.7"
       #env: MODE=mitogen DISTRO=centos6
+    - python: "3.6"
+      env: MODE=mitogen DISTRO=centos7
     # 2.6 -> 2.7
     - python: "2.6"
       env: MODE=mitogen DISTRO=centos7

README.md

@@ -1,4 +1,3 @@
-
 # Mitogen
 
 <!-- [![Build Status](https://travis-ci.org/dw/mitogen.png?branch=master)](https://travis-ci.org/dw/mitogen}) -->

ansible_mitogen/mixins.py

@@ -371,6 +371,13 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase):
         self._compute_environment_string(env)
         self._set_temp_file_args(module_args, wrap_async)
 
+        # there's a case where if a task shuts down the node and then immediately calls
+        # wait_for_connection, the `ping` test from Ansible won't pass because we lost connection
+        # clearing out context forces a reconnect
+        # see https://github.com/dw/mitogen/issues/655 and Ansible's `wait_for_connection` module for more info
+        if module_name == 'ping' and type(self).__name__ == 'wait_for_connection':
+            self._connection.context = None
+
         self._connection._connect()
         result = ansible_mitogen.planner.invoke(
             ansible_mitogen.planner.Invocation(

ansible_mitogen/planner.py

@@ -96,6 +96,9 @@ class Invocation(object):
         #: Initially ``None``, but set by :func:`invoke`. The raw source or
         #: binary contents of the module.
         self._module_source = None
+        #: Initially ``{}``, but set by :func:`invoke`. Optional source to send
+        #: to :func:`propagate_paths_and_modules` to fix Python3.5 relative import errors
+        self._overridden_sources = {}
 
     def get_module_source(self):
         if self._module_source is None:
@@ -476,6 +479,7 @@ def _propagate_deps(invocation, planner, context):
         context=context,
         paths=planner.get_push_files(),
         modules=planner.get_module_deps(),
+        overridden_sources=invocation._overridden_sources
     )
 
 
@@ -533,6 +537,26 @@ def _get_planner(name, path, source):
     raise ansible.errors.AnsibleError(NO_METHOD_MSG + repr(invocation))
 
 
+def _fix_py35(invocation, module_source):
+    """
+    super edge case with a relative import error in Python 3.5.1-3.5.3
+    in Ansible's setup module when using Mitogen
+    https://github.com/dw/mitogen/issues/672#issuecomment-636408833
+    We replace a relative import in the setup module with the actual full file path
+    This works in vanilla Ansible but not in Mitogen otherwise
+    """
+    if invocation.module_name == 'setup' and \
+            invocation.module_path not in invocation._overridden_sources:
+        # in-memory replacement of setup module's relative import
+        # would check for just python3.5 and run this then but we don't know the
+        # target python at this time yet
+        module_source = module_source.replace(
+            b"from ...module_utils.basic import AnsibleModule",
+            b"from ansible.module_utils.basic import AnsibleModule"
+        )
+        invocation._overridden_sources[invocation.module_path] = module_source
+
+
 def invoke(invocation):
     """
     Find a Planner subclass corresponding to `invocation` and use it to invoke
@@ -555,10 +579,12 @@ def invoke(invocation):
 
     invocation.module_path = mitogen.core.to_text(path)
     if invocation.module_path not in _planner_by_path:
+        module_source = invocation.get_module_source()
+        _fix_py35(invocation, module_source)
         _planner_by_path[invocation.module_path] = _get_planner(
             invocation.module_name,
             invocation.module_path,
-            invocation.get_module_source()
+            module_source
         )
 
     planner = _planner_by_path[invocation.module_path](invocation)

ansible_mitogen/plugins/action/mitogen_fetch.py

@@ -157,6 +157,10 @@ class ActionModule(ActionBase):
                 result.update(dict(changed=False, md5sum=local_md5, file=source, dest=dest, checksum=local_checksum))
 
         finally:
+            try:
                 self._remove_tmp_path(self._connection._shell.tmpdir)
+            except AttributeError:
+                # .tmpdir was added to ShellModule in v2.6.0, so old versions don't have it
+                pass
 
         return result

ansible_mitogen/services.py

@@ -170,6 +170,12 @@ class ContextService(mitogen.service.Service):
         """
         LOG.debug('%r.reset(%r)', self, stack)
 
+        # this could happen if we have a `shutdown -r` shell command
+        # and then a `wait_for_connection` right afterwards
+        # in this case, we have no stack to disconnect from
+        if not stack:
+            return False
+
         l = mitogen.core.Latch()
         context = None
         with self._lock:

docs/ansible_detailed.rst

@@ -9,7 +9,7 @@ Mitogen for Ansible
 
 **Mitogen for Ansible** is a completely redesigned UNIX connection layer and
 module runtime for `Ansible`_. Requiring minimal configuration changes, it
-updates Ansible's slow and wasteful shell-centic implementation with
+updates Ansible's slow and wasteful shell-centric implementation with
 pure-Python equivalents, invoked via highly efficient remote procedure calls to
 persistent interpreters tunnelled over SSH. No changes are required to target
 hosts.
@@ -1009,7 +1009,7 @@ Like the :ans:conn:`ssh` except connection delegation is supported.
 * ``mitogen_ssh_keepalive_count``: integer count of server keepalive messages to
   which no reply is received before considering the SSH server dead. Defaults
   to 10.
-* ``mitogen_ssh_keepalive_count``: integer seconds delay between keepalive
+* ``mitogen_ssh_keepalive_interval``: integer seconds delay between keepalive
   messages. Defaults to 30.
 
 

mitogen/service.py

@@ -74,7 +74,7 @@ else:
 
 
 @mitogen.core.takes_router
-def get_or_create_pool(size=None, router=None):
+def get_or_create_pool(size=None, router=None, context=None):
     global _pool
     global _pool_pid
 
@@ -84,6 +84,12 @@ def get_or_create_pool(size=None, router=None):
         _pool_lock.acquire()
         try:
             if _pool_pid != my_pid:
+                if router is None:
+                    # fallback to trying to get router from context if that exists
+                    if context is not None:
+                        router = context.router
+                    else:
+                        raise ValueError("Unable to create Pool! Missing router.")
                 _pool = Pool(
                     router,
                     services=[],
@@ -119,7 +125,7 @@ def call(service_name, method_name, call_context=None, **kwargs):
     if call_context:
         return call_context.call_service(service_name, method_name, **kwargs)
     else:
-        pool = get_or_create_pool()
+        pool = get_or_create_pool(context=kwargs.get('context'))
         invoker = pool.get_invoker(service_name, msg=None)
         return getattr(invoker.service, method_name)(**kwargs)
 
@@ -740,13 +746,18 @@ class PushFileService(Service):
         'paths': list,
         'modules': list,
     })
-    def propagate_paths_and_modules(self, context, paths, modules):
+    def propagate_paths_and_modules(self, context, paths, modules, overridden_sources=None):
         """
         One size fits all method to ensure a target context has been preloaded
         with a set of small files and Python modules.
+
+        overridden_sources: optional dict containing source code to override path's source code
         """
         for path in paths:
-            self.propagate_to(context, mitogen.core.to_text(path))
+            overridden_source = None
+            if overridden_sources is not None and path in overridden_sources:
+                overridden_source = overridden_sources[path]
+            self.propagate_to(context, mitogen.core.to_text(path), overridden_source)
         #self.router.responder.forward_modules(context, modules) TODO
 
     @expose(policy=AllowParents())
@@ -754,14 +765,22 @@ class PushFileService(Service):
         'context': mitogen.core.Context,
         'path': mitogen.core.FsPathTypes,
     })
-    def propagate_to(self, context, path):
+    def propagate_to(self, context, path, overridden_source=None):
+        """
+        If the optional parameter 'overridden_source' is passed, use
+        that instead of the path's code as source code. This works around some bugs
+        of source modules such as relative imports on unsupported Python versions
+        """
         if path not in self._cache:
             LOG.debug('caching small file %s', path)
+            if overridden_source is None:
                 fp = open(path, 'rb')
                 try:
                     self._cache[path] = mitogen.core.Blob(fp.read())
                 finally:
                     fp.close()
+            else:
+                self._cache[path] = mitogen.core.Blob(overridden_source)
         self._forward(context, path)
 
     @expose(policy=AllowParents())

tests/ansible/regression/all.yml

@@ -12,3 +12,4 @@
 - include: issue_590__sys_modules_crap.yml
 - include: issue_591__setuptools_cwd_crash.yml
 - include: issue_615__streaming_transfer.yml
+- include: issue_655__wait_for_connection_error.yml

tests/ansible/regression/issue_655__wait_for_connection_error.yml

@@ -0,0 +1,85 @@
+# https://github.com/dw/mitogen/issues/655
+# Spins up a Centos8 container and runs the wait_for_connection test inside of it
+# Doing it this way because the shutdown command causes issues in our tests
+#   since things are ran on localhost; Azure DevOps loses connection and fails
+# TODO: do we want to install docker a different way to be able to do this for other tests too
+---
+# this should only run on our Mac hosts
+- hosts: target
+  any_errors_fatal: True
+  gather_facts: yes
+  become: no
+  tasks:
+    - name: set up test container and run tests inside it
+      block:
+        - name: install deps
+          block:
+            - name: install docker
+              shell: |
+                # NOTE: for tracking purposes: https://github.com/docker/for-mac/issues/2359
+                # using docker for mac CI workaround: https://github.com/drud/ddev/pull/1748/files#diff-19288f650af2dabdf1dcc5b354d1f245
+                DOCKER_URL=https://download.docker.com/mac/stable/31259/Docker.dmg && 
+                curl -O -sSL $DOCKER_URL && 
+                open -W Docker.dmg && cp -r /Volumes/Docker/Docker.app /Applications
+                sudo /Applications/Docker.app/Contents/MacOS/Docker --quit-after-install --unattended && 
+                ln -s /Applications/Docker.app/Contents/Resources/bin/docker /usr/local/bin/docker && 
+                nohup /Applications/Docker.app/Contents/MacOS/Docker --unattended & 
+                # wait 2 min for docker to come up
+                counter=0 && 
+                  while ! /usr/local/bin/docker ps 2>/dev/null ; do
+                    if [ $counter -lt 24 ]; then
+                      let counter=counter+1
+                    else
+                      exit 1
+                    fi
+                    sleep 5
+                  done
+
+        # python bindings (docker_container) aren't working on this host, so gonna shell out
+        - name: create docker container
+          shell: /usr/local/bin/docker run --name testMitogen -d --rm centos:8 bash -c "sleep infinity & wait"
+
+        - name: add container to inventory
+          add_host:
+            name: testMitogen
+            ansible_connection: docker
+            ansible_user: root
+          changed_when: false
+          environment:
+            PATH: /usr/local/bin/:{{ ansible_env.PATH }}
+
+        - name: run tests
+          block:
+            # to repro the issue, will create /var/run/reboot-required
+            - name: create test file
+              file:
+                path: /var/run/reboot-required
+                state: touch
+
+            - name: Check if reboot is required
+              stat:
+                path: /var/run/reboot-required
+              register: reboot_required
+
+            - name: Reboot server
+              shell: sleep 2 && shutdown -r now "Ansible updates triggered"
+              async: 1
+              poll: 0
+              when: reboot_required.stat.exists == True
+
+            - name: Wait 300 seconds for server to become available
+              wait_for_connection:
+                delay: 30
+                timeout: 300
+              when: reboot_required.stat.exists == True
+
+            - name: cleanup test file
+              file:
+                path: /var/run/reboot-required
+                state: absent
+          delegate_to: testMitogen
+          environment:
+            PATH: /usr/local/bin/:{{ ansible_env.PATH }}
+
+        - name: remove test container
+          shell: /usr/local/bin/docker stop testMitogen

tests/ansible/requirements.txt

@@ -1,5 +1,3 @@
-ansible==2.8.8; python_version >= '2.7'
-ansible<2.7; python_version < '2.7'
 paramiko==2.3.2  # Last 2.6-compat version.
 hdrhistogram==0.6.1
 PyYAML==3.11; python_version < '2.7'

tests/log_handler_test.py

@@ -87,8 +87,8 @@ class StartupTest(testlib.RouterMixin, testlib.TestCase):
         self.assertTrue(expect in logs)
 
 StartupTest = unittest2.skipIf(
-    condition=sys.version_info < (2, 7),
+    condition=sys.version_info < (2, 7) or sys.version_info >= (3, 6),
-    reason="Message log flaky on Python < 2.7"
+    reason="Message log flaky on Python < 2.7 or >= 3.6"
 )(StartupTest)