fix delegated interpreter discovery (#69604)

* fix delegated interpeter
* allow returning fact if it is 'the right host'
* added note for future fix/efficiency
 as it stands we rerun discovery for the delegated host
unless its saving facts to itself
 * fixed test lacking delegate_to mock

7 files changed, 105 insertions, 9 deletions

diff --git a/changelogs/fragments/discovery_delegation_fix.yml b/changelogs/fragments/discovery_delegation_fix.yml
new file mode 100644
index 0000000000..949738042f
--- /dev/null
+++ b/changelogs/fragments/discovery_delegation_fix.yml
@@ -0,0 +1,3 @@
+bugfixes:
+    - interpreter discovery will now use correct vars (from delegated host) when in delegate_to task.
+    - discovery will NOT update incorrect host anymore when in delegate_to task.
diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py
index 2f70946ee3..aa001c405b 100644
--- a/lib/ansible/plugins/action/__init__.py
+++ b/lib/ansible/plugins/action/__init__.py
@@ -158,8 +158,14 @@ class ActionBase(with_metaclass(ABCMeta, object)):
         Handles the loading and templating of the module code through the
         modify_module() function.
         '''
+
         if task_vars is None:
-            task_vars = dict()
+            use_vars = dict()
+
+        if self._task.delegate_to:
+            use_vars = task_vars.get('ansible_delegated_vars')[self._task.delegate_to]
+        else:
+            use_vars = task_vars
 
         # Search module path(s) for named module.
         for mod_type in self._connection.module_implementation_preferences:
@@ -210,7 +216,7 @@ class ActionBase(with_metaclass(ABCMeta, object)):
         for dummy in (1, 2):
             try:
                 (module_data, module_style, module_shebang) = modify_module(module_name, module_path, module_args, self._templar,
-                                                                            task_vars=task_vars,
+                                                                            task_vars=use_vars,
                                                                             module_compression=self._play_context.module_compression,
                                                                             async_timeout=self._task.async_val,
                                                                             environment=final_environment,
@@ -221,17 +227,22 @@ class ActionBase(with_metaclass(ABCMeta, object)):
                     action=self,
                     interpreter_name=idre.interpreter_name,
                     discovery_mode=idre.discovery_mode,
-                    task_vars=task_vars))
+                    task_vars=use_vars))
 
                 # update the local task_vars with the discovered interpreter (which might be None);
                 # we'll propagate back to the controller in the task result
                 discovered_key = 'discovered_interpreter_%s' % idre.interpreter_name
-                # store in local task_vars facts collection for the retry and any other usages in this worker
-                if task_vars.get('ansible_facts') is None:
-                    task_vars['ansible_facts'] = {}
-                task_vars['ansible_facts'][discovered_key] = self._discovered_interpreter
-                # preserve this so _execute_module can propagate back to controller as a fact
-                self._discovered_interpreter_key = discovered_key
+
+                # TODO: this condition prevents 'wrong host' from being updated
+                # but in future we would want to be able to update 'delegated host facts'
+                # irrespective of task settings
+                if not self._task.delegate_to or self._task.delegate_facts:
+                    # store in local task_vars facts collection for the retry and any other usages in this worker
+                    if use_vars.get('ansible_facts') is None:
+                        task_vars['ansible_facts'] = {}
+                    task_vars['ansible_facts'][discovered_key] = self._discovered_interpreter
+                    # preserve this so _execute_module can propagate back to controller as a fact
+                    self._discovered_interpreter_key = discovered_key
 
         return (module_style, module_shebang, module_data, module_path)
 
diff --git a/test/integration/targets/delegate_to/inventory_interpreters b/test/integration/targets/delegate_to/inventory_interpreters
new file mode 100644
index 0000000000..4c202ca5bd
--- /dev/null
+++ b/test/integration/targets/delegate_to/inventory_interpreters
@@ -0,0 +1,5 @@
+testhost ansible_python_interpreter=firstpython
+testhost2 ansible_python_interpreter=secondpython
+
+[all:vars]
+ansible_connection=local
diff --git a/test/integration/targets/delegate_to/library/detect_interpreter.py b/test/integration/targets/delegate_to/library/detect_interpreter.py
new file mode 100644
index 0000000000..1f4016772a
--- /dev/null
+++ b/test/integration/targets/delegate_to/library/detect_interpreter.py
@@ -0,0 +1,18 @@
+#!/usr/bin/python
+
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
+import sys
+
+from ansible.module_utils.basic import AnsibleModule
+
+
+def main():
+    module = AnsibleModule(argument_spec={})
+    module.exit_json(**dict(found=sys.executable))
+
+
+if __name__ == '__main__':
+    main()
diff --git a/test/integration/targets/delegate_to/runme.sh b/test/integration/targets/delegate_to/runme.sh
index 717809b3dc..730d5cc9c7 100755
--- a/test/integration/targets/delegate_to/runme.sh
+++ b/test/integration/targets/delegate_to/runme.sh
@@ -58,3 +58,14 @@ ansible-playbook test_delegate_to_loop_caching.yml -i inventory -v "$@"
 
 # ensure we are using correct settings when delegating
 ANSIBLE_TIMEOUT=3 ansible-playbook delegate_vars_hanldling.yml -i inventory -v "$@"
+
+
+# test ansible_x_interpreter
+# python
+source virtualenv.sh
+(
+cd "${OUTPUT_DIR}"/venv/bin
+ln -s python firstpython
+ln -s python secondpython
+)
+ansible-playbook verify_interpreter.yml -i inventory_interpreters -v "$@"
diff --git a/test/integration/targets/delegate_to/verify_interpreter.yml b/test/integration/targets/delegate_to/verify_interpreter.yml
new file mode 100644
index 0000000000..63c60a41af
--- /dev/null
+++ b/test/integration/targets/delegate_to/verify_interpreter.yml
@@ -0,0 +1,47 @@
+- name: ensure they are different
+  hosts: localhost
+  tasks:
+    - name: dont game me
+      assert:
+        msg: 'expected different values but got ((hostvars["testhost"]["ansible_python_interpreter"]}} and {{hostvars["testhost2"]["ansible_python_interpreter"]}}'
+        that:
+           - hostvars["testhost"]["ansible_python_interpreter"] != hostvars["testhost2"]["ansible_python_interpreter"]
+
+- name: no delegation
+  hosts: all
+  gather_facts: false
+  tasks:
+    - name: detect interpreter used by each host
+      detect_interpreter:
+      register: baseline
+
+    - name: verify it
+      assert:
+        msg: 'expected {{ansible_python_interpreter}} but got {{baseline.found|basename}}'
+        that:
+           - baseline.found|basename == ansible_python_interpreter
+
+- name: actual test
+  hosts: testhost
+  gather_facts: false
+  tasks:
+    - name: original host
+      detect_interpreter:
+      register: found
+
+    - name: verify it orig host
+      assert:
+        msg: 'expected {{ansible_python_interpreter}} but got {{found.found|basename}}'
+        that:
+           - found.found|basename == ansible_python_interpreter
+
+    - name: delegated host
+      detect_interpreter:
+      register: found2
+      delegate_to: testhost2
+
+    - name: verify it delegated
+      assert:
+        msg: 'expected {{hostvars["testhost2"]["ansible_python_interpreter"]}} but got {{found2.found|basename}}'
+        that:
+           - found2.found|basename == hostvars["testhost2"]["ansible_python_interpreter"]
diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py
index faa6c6e639..1a8001df50 100644
--- a/test/units/plugins/action/test_action.py
+++ b/test/units/plugins/action/test_action.py
@@ -116,6 +116,7 @@ class TestActionBase(unittest.TestCase):
         mock_task = MagicMock()
         mock_task.action = "copy"
         mock_task.async_val = 0
+        mock_task.delegate_to = None
 
         # create a mock connection, so we don't actually try and connect to things
         mock_connection = MagicMock()