From 7dfda4026ec6f82247c08df7c0fb8e8287dce2be Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Fri, 17 Jul 2020 21:40:00 +0200 Subject: [PATCH] Fix delegate_facts with interpreter not being set (#70293) (#70384) Fixes #70168 ci_complete Co-authored-by: Brian Coca Co-authored-by: Matt Clay (cherry picked from commit b05e00e99a91b08cc2de95c6e151c9eb268a01d5) --- ...-fix-delegate_facts-without-interpreter-set.yml | 2 ++ lib/ansible/plugins/action/__init__.py | 14 ++++---------- .../aliases | 2 ++ .../delegate_facts.yml | 10 ++++++++++ .../inventory | 2 ++ .../runme.sh | 5 +++++ test/units/plugins/action/test_action.py | 6 +++--- 7 files changed, 28 insertions(+), 13 deletions(-) create mode 100644 changelogs/fragments/70168-fix-delegate_facts-without-interpreter-set.yml create mode 100644 test/integration/targets/interpreter_discovery_python_delegate_facts/aliases create mode 100644 test/integration/targets/interpreter_discovery_python_delegate_facts/delegate_facts.yml create mode 100644 test/integration/targets/interpreter_discovery_python_delegate_facts/inventory create mode 100755 test/integration/targets/interpreter_discovery_python_delegate_facts/runme.sh diff --git a/changelogs/fragments/70168-fix-delegate_facts-without-interpreter-set.yml b/changelogs/fragments/70168-fix-delegate_facts-without-interpreter-set.yml new file mode 100644 index 00000000000..371778eabb2 --- /dev/null +++ b/changelogs/fragments/70168-fix-delegate_facts-without-interpreter-set.yml @@ -0,0 +1,2 @@ +bugfixes: + - "Fix ``delegate_facts: true`` when ``ansible_python_interpreter`` is not set. (https://github.com/ansible/ansible/issues/70168)" diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 83c7715d3ee..f08a084608f 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -153,15 +153,11 @@ class ActionBase(with_metaclass(ABCMeta, object)): return True return False - def _configure_module(self, module_name, module_args, task_vars=None): + def _configure_module(self, module_name, module_args, task_vars): ''' Handles the loading and templating of the module code through the modify_module() function. ''' - - if task_vars is None: - use_vars = dict() - if self._task.delegate_to: use_vars = task_vars.get('ansible_delegated_vars')[self._task.delegate_to] else: @@ -242,20 +238,18 @@ class ActionBase(with_metaclass(ABCMeta, object)): # we'll propagate back to the controller in the task result discovered_key = 'discovered_interpreter_%s' % idre.interpreter_name + # update the local vars copy for the retry + use_vars['ansible_facts'][discovered_key] = self._discovered_interpreter + # 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 else: - task_vars['ansible_delegated_vars'][self._task.delegate_to] - if task_vars['ansible_delegated_vars'][self._task.delegate_to].get('ansible_facts') is None: - task_vars['ansible_delegated_vars'][self._task.delegate_to]['ansible_facts'] = {} task_vars['ansible_delegated_vars'][self._task.delegate_to]['ansible_facts'][discovered_key] = self._discovered_interpreter return (module_style, module_shebang, module_data, module_path) diff --git a/test/integration/targets/interpreter_discovery_python_delegate_facts/aliases b/test/integration/targets/interpreter_discovery_python_delegate_facts/aliases new file mode 100644 index 00000000000..dc9ac4682ba --- /dev/null +++ b/test/integration/targets/interpreter_discovery_python_delegate_facts/aliases @@ -0,0 +1,2 @@ +shippable/posix/group1 +non_local # this test requires interpreter discovery, which means code coverage must be disabled diff --git a/test/integration/targets/interpreter_discovery_python_delegate_facts/delegate_facts.yml b/test/integration/targets/interpreter_discovery_python_delegate_facts/delegate_facts.yml new file mode 100644 index 00000000000..535269d1602 --- /dev/null +++ b/test/integration/targets/interpreter_discovery_python_delegate_facts/delegate_facts.yml @@ -0,0 +1,10 @@ +- hosts: localhost + gather_facts: no + tasks: + - name: Test python interpreter discovery with delegate_to without delegate_facts + ping: + delegate_to: testhost + - name: Test python interpreter discovery with delegate_to with delegate_facts + ping: + delegate_to: testhost + delegate_facts: yes diff --git a/test/integration/targets/interpreter_discovery_python_delegate_facts/inventory b/test/integration/targets/interpreter_discovery_python_delegate_facts/inventory new file mode 100644 index 00000000000..350f3e89996 --- /dev/null +++ b/test/integration/targets/interpreter_discovery_python_delegate_facts/inventory @@ -0,0 +1,2 @@ +[local] +testhost ansible_connection=local ansible_python_interpreter=auto # interpreter discovery required diff --git a/test/integration/targets/interpreter_discovery_python_delegate_facts/runme.sh b/test/integration/targets/interpreter_discovery_python_delegate_facts/runme.sh new file mode 100755 index 00000000000..ca2caa1c86d --- /dev/null +++ b/test/integration/targets/interpreter_discovery_python_delegate_facts/runme.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +set -eux + +ansible-playbook delegate_facts.yml -i inventory "$@" diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 3f80913c1f9..124880196ad 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -164,19 +164,19 @@ class TestActionBase(unittest.TestCase): self.assertEqual(shebang, u"#!/usr/bin/python") # test module not found - self.assertRaises(AnsibleError, action_base._configure_module, 'badmodule', mock_task.args) + self.assertRaises(AnsibleError, action_base._configure_module, 'badmodule', mock_task.args, {}) # test powershell module formatting with patch.object(builtins, 'open', mock_open(read_data=to_bytes(powershell_module_replacers.strip(), encoding='utf-8'))): mock_task.action = 'win_copy' mock_task.args = dict(b=2) mock_connection.module_implementation_preferences = ('.ps1',) - (style, shebang, data, path) = action_base._configure_module('stat', mock_task.args) + (style, shebang, data, path) = action_base._configure_module('stat', mock_task.args, {}) self.assertEqual(style, "new") self.assertEqual(shebang, u'#!powershell') # test module not found - self.assertRaises(AnsibleError, action_base._configure_module, 'badmodule', mock_task.args) + self.assertRaises(AnsibleError, action_base._configure_module, 'badmodule', mock_task.args, {}) def test_action_base__compute_environment_string(self): fake_loader = DictDataLoader({