From de3f7c7739851852dec8ea99a76c353317270b70 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Fri, 22 May 2020 09:31:34 -0400 Subject: [PATCH] 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 --- .../fragments/discovery_delegation_fix.yml | 3 ++ lib/ansible/plugins/action/__init__.py | 29 ++++++++---- .../delegate_to/inventory_interpreters | 5 ++ .../delegate_to/library/detect_interpreter.py | 18 +++++++ test/integration/targets/delegate_to/runme.sh | 11 +++++ .../delegate_to/verify_interpreter.yml | 47 +++++++++++++++++++ test/units/plugins/action/test_action.py | 1 + 7 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 changelogs/fragments/discovery_delegation_fix.yml create mode 100644 test/integration/targets/delegate_to/inventory_interpreters create mode 100644 test/integration/targets/delegate_to/library/detect_interpreter.py create mode 100644 test/integration/targets/delegate_to/verify_interpreter.yml diff --git a/changelogs/fragments/discovery_delegation_fix.yml b/changelogs/fragments/discovery_delegation_fix.yml new file mode 100644 index 00000000000..949738042f7 --- /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 2f70946ee3c..aa001c405bf 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 00000000000..4c202ca5bd4 --- /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 00000000000..1f4016772ac --- /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 717809b3dca..730d5cc9c74 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 00000000000..63c60a41af7 --- /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 faa6c6e6391..1a8001df505 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()