Merge pull request #658 from s1113950/complexAnsiblePythonInterpreterArg

Adds support for special ansible_python_interpreter values, ansible_python_interpreter discovery, and fixes tests

Author: s1113950

.ci/ansible_tests.py

@@ -66,6 +66,10 @@ def pause_if_interactive():
     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-steps.yml

@@ -14,9 +14,17 @@ steps:
 # stuff into. The virtualenv can probably be removed again, but this was a
 # hard-fought battle and for now I am tired of this crap.
 - script: |
-    sudo ln -fs /usr/bin/python$(python.version) /usr/bin/python
-    /usr/bin/python -m pip install -U virtualenv setuptools wheel
-    /usr/bin/python -m virtualenv /tmp/venv -p /usr/bin/python$(python.version)
+    # need wheel before building virtualenv because of bdist_wheel and setuptools deps
+    # Mac's System Integrity Protection prevents symlinking /usr/bin
+    # and Azure isn't allowing disabling it apparently: https://developercommunityapi.westus.cloudapp.azure.com/idea/558702/allow-disabling-sip-on-microsoft-hosted-macos-agen.html
+    # the || will activate when running python3 tests
+    # TODO: get python3 tests passing
+    (sudo ln -fs /usr/bin/python$(python.version) /usr/bin/python &&
+      /usr/bin/python -m pip install -U pip wheel setuptools &&
+      /usr/bin/python -m pip install -U virtualenv &&
+      /usr/bin/python -m virtualenv /tmp/venv -p /usr/bin/python$(python.version)) ||
+    (sudo /usr/bin/python$(python.version) -m pip install -U pip wheel setuptools &&
+      /usr/bin/python$(python.version) -m venv /tmp/venv)
     echo "##vso[task.prependpath]/tmp/venv/bin"
 
   displayName: activate venv

.ci/azure-pipelines.yml

@@ -9,20 +9,21 @@ jobs:
   steps:
   - template: azure-pipelines-steps.yml
   pool:
-    vmImage: macOS-10.13
+    vmImage: macOS-10.14
   strategy:
     matrix:
       Mito27_27:
         python.version: '2.7'
         MODE: mitogen
-      Ans280_27:
+      Ans288_27:
         python.version: '2.7'
         MODE: localhost_ansible
+        VER: 2.8.8
 
 
 - job: Linux
   pool:
-    vmImage: "Ubuntu 16.04"
+    vmImage: "Ubuntu 18.04"
   steps:
   - template: azure-pipelines-steps.yml
   strategy:
@@ -45,10 +46,6 @@ jobs:
         MODE: mitogen
         DISTRO: centos6
 
-      #
-      #
-      #
-
       #Py26CentOS7:
         #python.version: '2.7'
         #MODE: mitogen

.ci/localhost_ansible_tests.py

@@ -44,11 +44,11 @@
 
     if os.path.expanduser('~mitogen__user1') == '~mitogen__user1':
         os.chdir(IMAGE_PREP_DIR)
-        run("ansible-playbook -c local -i localhost, _user_accounts.yml")
+        run("ansible-playbook -c local -i localhost, _user_accounts.yml -vvv")
 
 
 with ci_lib.Fold('ansible'):
     os.chdir(TESTS_DIR)
     playbook = os.environ.get('PLAYBOOK', 'all.yml')
-    run('./run_ansible_playbook.py %s -l target %s',
+    run('./run_ansible_playbook.py %s -l target %s -vvv',
         playbook, ' '.join(sys.argv[1:]))

ansible_mitogen/connection.py

@@ -183,7 +183,7 @@ def _connect_docker(spec):
         'kwargs': {
             'username': spec.remote_user(),
             'container': spec.remote_addr(),
-            'python_path': spec.python_path(),
+            'python_path': spec.python_path(rediscover_python=True),
             'connect_timeout': spec.ansible_ssh_timeout() or spec.timeout(),
             'remote_name': get_remote_name(spec),
         }
@@ -503,6 +503,9 @@ class Connection(ansible.plugins.connection.ConnectionBase):
     #: matching vanilla Ansible behaviour.
     loader_basedir = None
 
+    # set by `_get_task_vars()` for interpreter discovery
+    _action = None
+
     def __del__(self):
         """
         Ansible cannot be trusted to always call close() e.g. the synchronize
@@ -551,6 +554,23 @@ def _get_task_vars(self):
         connection passed into any running action.
         """
         if self._task_vars is not None:
+            # check for if self._action has already been set or not
+            # there are some cases where the ansible executor passes in task_vars
+            # so we don't walk the stack to find them
+            # TODO: is there a better way to get the ActionModuleMixin object?
+            # ansible python discovery needs it to run discover_interpreter()
+            if not isinstance(self._action, ansible_mitogen.mixins.ActionModuleMixin):
+                f = sys._getframe()
+                while f:
+                    if f.f_code.co_name == 'run':
+                        f_self = f.f_locals.get('self')
+                        if isinstance(f_self, ansible_mitogen.mixins.ActionModuleMixin):
+                            self._action = f_self
+                            break
+                    elif f.f_code.co_name == '_execute_meta':
+                        break
+                    f = f.f_back
+
             return self._task_vars
 
         f = sys._getframe()
@@ -559,6 +579,9 @@ def _get_task_vars(self):
                 f_locals = f.f_locals
                 f_self = f_locals.get('self')
                 if isinstance(f_self, ansible_mitogen.mixins.ActionModuleMixin):
+                    # backref for python interpreter discovery, should be safe because _get_task_vars
+                    # is always called before running interpreter discovery
+                    self._action = f_self
                     task_vars = f_locals.get('task_vars')
                     if task_vars:
                         LOG.debug('recovered task_vars from Action')
@@ -600,16 +623,33 @@ def get_task_var(self, key, default=None):
         does not make sense to extract connection-related configuration for the
         delegated-to machine from them.
         """
+        def _fetch_task_var(task_vars, key):
+            """
+            Special helper func in case vars can be templated
+            """
+            SPECIAL_TASK_VARS = [
+                'ansible_python_interpreter'
+            ]
+            if key in task_vars:
+                val = task_vars[key]
+                if '{' in str(val) and key in SPECIAL_TASK_VARS:
+                    # template every time rather than storing in a cache
+                    # in case a different template value is used in a different task
+                    val = self.templar.template(
+                        val,
+                        preserve_trailing_newlines=True,
+                        escape_backslashes=False
+                    )
+                return val
+
         task_vars = self._get_task_vars()
         if self.delegate_to_hostname is None:
-            if key in task_vars:
-                return task_vars[key]
+            return _fetch_task_var(task_vars, key)
         else:
             delegated_vars = task_vars['ansible_delegated_vars']
             if self.delegate_to_hostname in delegated_vars:
                 task_vars = delegated_vars[self.delegate_to_hostname]
-                if key in task_vars:
-                    return task_vars[key]
+                return _fetch_task_var(task_vars, key)
 
         return default
 
@@ -654,6 +694,8 @@ def _spec_from_via(self, proxied_inventory_name, via_spec):
             inventory_name=inventory_name,
             play_context=self._play_context,
             host_vars=dict(via_vars),  # TODO: make it lazy
+            task_vars=self._get_task_vars(),    # needed for interpreter discovery in parse_python_path
+            action=self._action,
             become_method=become_method or None,
             become_user=become_user or None,
         )
@@ -847,6 +889,18 @@ def reset(self):
                 self.reset_compat_msg
             )
 
+        # Strategy's _execute_meta doesn't have an action obj but we'll need one for
+        # running interpreter_discovery
+        # will create a new temporary action obj for this purpose
+        self._action = ansible_mitogen.mixins.ActionModuleMixin(
+            task=0,
+            connection=self,
+            play_context=self._play_context,
+            loader=0,
+            templar=0,
+            shared_loader_obj=0
+        )
+
         # Clear out state in case we were ever connected.
         self.close()
 

ansible_mitogen/mixins.py

@@ -60,6 +60,17 @@
 except ImportError:
     from ansible.vars.unsafe_proxy import wrap_var
 
+try:
+    # ansible 2.8 moved remove_internal_keys to the clean module
+    from ansible.vars.clean import remove_internal_keys
+except ImportError:
+    try:
+        from ansible.vars.manager import remove_internal_keys
+    except ImportError:
+        # ansible 2.3.3 has remove_internal_keys as a protected func on the action class
+        # we'll fallback to calling self._remove_internal_keys in this case
+        remove_internal_keys = lambda a: "Not found"
+
 
 LOG = logging.getLogger(__name__)
 
@@ -108,6 +119,16 @@ def __init__(self, task, connection, *args, **kwargs):
         if not isinstance(connection, ansible_mitogen.connection.Connection):
             _, self.__class__ = type(self).__bases__
 
+        # required for python interpreter discovery
+        connection.templar = self._templar
+        self._finding_python_interpreter = False
+        self._rediscovered_python = False
+        # redeclaring interpreter discovery vars here in case running ansible < 2.8.0
+        self._discovered_interpreter_key = None
+        self._discovered_interpreter = False
+        self._discovery_deprecation_warnings = []
+        self._discovery_warnings = []
+
     def run(self, tmp=None, task_vars=None):
         """
         Override run() to notify Connection of task-specific data, so it has a
@@ -370,6 +391,34 @@ def _execute_module(self, module_name=None, module_args=None, tmp=None,
             # on _execute_module().
             self._remove_tmp_path(tmp)
 
+        # prevents things like discovered_interpreter_* or ansible_discovered_interpreter_* from being set
+        # handle ansible 2.3.3 that has remove_internal_keys in a different place
+        check = remove_internal_keys(result)
+        if check == 'Not found':
+            self._remove_internal_keys(result)
+
+        # taken from _execute_module of ansible 2.8.6
+        # propagate interpreter discovery results back to the controller
+        if self._discovered_interpreter_key:
+            if result.get('ansible_facts') is None:
+                result['ansible_facts'] = {}
+
+            # only cache discovered_interpreter if we're not running a rediscovery
+            # rediscovery happens in places like docker connections that could have different
+            # python interpreters than the main host
+            if not self._rediscovered_python:
+                result['ansible_facts'][self._discovered_interpreter_key] = self._discovered_interpreter
+
+        if self._discovery_warnings:
+            if result.get('warnings') is None:
+                result['warnings'] = []
+            result['warnings'].extend(self._discovery_warnings)
+
+        if self._discovery_deprecation_warnings:
+            if result.get('deprecations') is None:
+                result['deprecations'] = []
+            result['deprecations'].extend(self._discovery_deprecation_warnings)
+
         return wrap_var(result)
 
     def _postprocess_response(self, result):
@@ -407,17 +456,54 @@ def _low_level_execute_command(self, cmd, sudoable=True, in_data=None,
         """
         LOG.debug('_low_level_execute_command(%r, in_data=%r, exe=%r, dir=%r)',
                   cmd, type(in_data), executable, chdir)
+
         if executable is None:  # executable defaults to False
             executable = self._play_context.executable
         if executable:
             cmd = executable + ' -c ' + shlex_quote(cmd)
 
-        rc, stdout, stderr = self._connection.exec_command(
-            cmd=cmd,
-            in_data=in_data,
-            sudoable=sudoable,
-            mitogen_chdir=chdir,
-        )
+        # TODO: HACK: if finding python interpreter then we need to keep
+        # calling exec_command until we run into the right python we'll use
+        # chicken-and-egg issue, mitogen needs a python to run low_level_execute_command
+        # which is required by Ansible's discover_interpreter function
+        if self._finding_python_interpreter:
+            possible_pythons = [
+                '/usr/bin/python',
+                'python3',
+                'python3.7',
+                'python3.6',
+                'python3.5',
+                'python2.7',
+                'python2.6',
+                '/usr/libexec/platform-python',
+                '/usr/bin/python3',
+                'python'
+            ]
+        else:
+            # not used, just adding a filler value
+            possible_pythons = ['python']
+
+        def _run_cmd():
+            return self._connection.exec_command(
+                cmd=cmd,
+                in_data=in_data,
+                sudoable=sudoable,
+                mitogen_chdir=chdir,
+            )
+
+        for possible_python in possible_pythons:
+            try:
+                self._possible_python_interpreter = possible_python
+                rc, stdout, stderr = _run_cmd()
+            # TODO: what exception is thrown?
+            except:
+                # we've reached the last python attempted and failed
+                # TODO: could use enumerate(), need to check which version of python first had it though
+                if possible_python == 'python':
+                    raise
+                else:
+                    continue
+
         stdout_text = to_text(stdout, errors=encoding_errors)
 
         return {

ansible_mitogen/planner.py

@@ -535,7 +535,7 @@ def _get_planner(name, path, source):
 
 def invoke(invocation):
     """
-    Find a Planner subclass corresnding to `invocation` and use it to invoke
+    Find a Planner subclass corresponding to `invocation` and use it to invoke
     the module.
 
     :param Invocation invocation:

ansible_mitogen/plugins/action/mitogen_get_stack.py

@@ -52,4 +52,6 @@ def run(self, tmp=None, task_vars=None):
             'changed': True,
             'result': stack,
             '_ansible_verbose_always': True,
+            # for ansible < 2.8, we'll default to /usr/bin/python like before
+            'discovered_interpreter': self._connection._action._discovered_interpreter
         }