diff --git a/changelogs/fragments/gf_fix.yml b/changelogs/fragments/gf_fix.yml new file mode 100644 index 00000000000..7be81b99b41 --- /dev/null +++ b/changelogs/fragments/gf_fix.yml @@ -0,0 +1,2 @@ +bugfixes: + - now correclty merge and not just overwrite facts when gathering using multiple modules. diff --git a/lib/ansible/plugins/action/gather_facts.py b/lib/ansible/plugins/action/gather_facts.py index 8cc769a7a83..d58e9dd6bca 100644 --- a/lib/ansible/plugins/action/gather_facts.py +++ b/lib/ansible/plugins/action/gather_facts.py @@ -9,8 +9,9 @@ import time from ansible import constants as C from ansible.executor.module_common import get_action_args_with_defaults +from ansible.module_utils.parsing.convert_bool import boolean from ansible.plugins.action import ActionBase -from ansible.utils.vars import combine_vars +from ansible.utils.vars import merge_hash class ActionModule(ActionBase): @@ -52,7 +53,8 @@ class ActionModule(ActionBase): 'deprecations': task_result.get('deprecations', []), } - return combine_vars(result, filtered_res) + # on conflict the last plugin processed wins, but try to do deep merge and append to lists. + return merge_hash(result, filtered_res, list_merge='append_rp') def run(self, tmp=None, task_vars=None): @@ -71,7 +73,13 @@ class ActionModule(ActionBase): failed = {} skipped = {} - if parallel is False or (len(modules) == 1 and parallel is None): + + if parallel is None and len(modules) >= 1: + parallel = True + else: + parallel = boolean(parallel) + + if parallel: # serially execute each module for fact_module in modules: # just one module, no need for fancy async @@ -89,7 +97,6 @@ class ActionModule(ActionBase): # do it async jobs = {} for fact_module in modules: - mod_args = self._get_module_args(fact_module, task_vars) self._display.vvvv("Running %s" % fact_module) jobs[fact_module] = (self._execute_module(module_name=fact_module, module_args=mod_args, task_vars=task_vars, wrap_async=True)) diff --git a/test/integration/targets/gathering_facts/aliases b/test/integration/targets/gathering_facts/aliases index b59832142f2..0ee704e1163 100644 --- a/test/integration/targets/gathering_facts/aliases +++ b/test/integration/targets/gathering_facts/aliases @@ -1 +1,2 @@ shippable/posix/group3 +needs/root diff --git a/test/integration/targets/gathering_facts/library/facts_one b/test/integration/targets/gathering_facts/library/facts_one new file mode 100644 index 00000000000..c74ab9a7df9 --- /dev/null +++ b/test/integration/targets/gathering_facts/library/facts_one @@ -0,0 +1,25 @@ +#!/bin/sh + +echo '{ + "changed": false, + "ansible_facts": { + "factsone": "from facts_one module", + "common_fact": "also from facts_one module", + "common_dict_fact": { + "key_one": "from facts_one", + "key_two": "from facts_one" + }, + "common_list_fact": [ + "one", + "three", + "five" + ], + "common_list_fact2": [ + "one", + "two", + "three", + "five", + "five" + ] + } +}' diff --git a/test/integration/targets/gathering_facts/library/facts_two b/test/integration/targets/gathering_facts/library/facts_two new file mode 100644 index 00000000000..4e7c6684504 --- /dev/null +++ b/test/integration/targets/gathering_facts/library/facts_two @@ -0,0 +1,24 @@ +#!/bin/sh + +echo '{ + "changed": false, + "ansible_facts": { + "factstwo": "from facts_two module", + "common_fact": "also from facts_two module", + "common_dict_fact": { + "key_two": "from facts_two", + "key_four": "from facts_two" + }, + "common_list_fact": [ + "one", + "two", + "four" + ], + "common_list_fact2": [ + "one", + "two", + "four", + "four" + ] + } +}' diff --git a/test/integration/targets/gathering_facts/one_two.json b/test/integration/targets/gathering_facts/one_two.json new file mode 100644 index 00000000000..ecc698c3b5c --- /dev/null +++ b/test/integration/targets/gathering_facts/one_two.json @@ -0,0 +1,27 @@ +{ + "_ansible_facts_gathered": true, + "common_dict_fact": { + "key_four": "from facts_two", + "key_one": "from facts_one", + "key_two": "from facts_two" + }, + "common_fact": "also from facts_two module", + "common_list_fact": [ + "three", + "five", + "one", + "two", + "four" + ], + "common_list_fact2": [ + "three", + "five", + "five", + "one", + "two", + "four", + "four" + ], + "factsone": "from facts_one module", + "factstwo": "from facts_two module" +} \ No newline at end of file diff --git a/test/integration/targets/gathering_facts/runme.sh b/test/integration/targets/gathering_facts/runme.sh index ccea7662811..770cd674c39 100755 --- a/test/integration/targets/gathering_facts/runme.sh +++ b/test/integration/targets/gathering_facts/runme.sh @@ -2,11 +2,14 @@ set -eux -# ANSIBLE_CACHE_PLUGINS=cache_plugins/ ANSIBLE_CACHE_PLUGIN=none ansible-playbook test_gathering_facts.yml -i inventory -v "$@" +#ANSIBLE_CACHE_PLUGINS=cache_plugins/ ANSIBLE_CACHE_PLUGIN=none ansible-playbook test_gathering_facts.yml -i inventory -v "$@" ansible-playbook test_gathering_facts.yml -i inventory -v "$@" -# ANSIBLE_CACHE_PLUGIN=base ansible-playbook test_gathering_facts.yml -i inventory -v "$@" +#ANSIBLE_CACHE_PLUGIN=base ansible-playbook test_gathering_facts.yml -i inventory -v "$@" ANSIBLE_GATHERING=smart ansible-playbook test_run_once.yml -i inventory -v "$@" # ensure clean_facts is working properly ansible-playbook test_prevent_injection.yml -i inventory -v "$@" + +# ensure fact merging is working properly +ansible-playbook verify_merge_facts.yml -v "$@" -e 'ansible_facts_parallel: False' diff --git a/test/integration/targets/gathering_facts/two_one.json b/test/integration/targets/gathering_facts/two_one.json new file mode 100644 index 00000000000..4b34a2d5752 --- /dev/null +++ b/test/integration/targets/gathering_facts/two_one.json @@ -0,0 +1,27 @@ +{ + "_ansible_facts_gathered": true, + "common_dict_fact": { + "key_four": "from facts_two", + "key_one": "from facts_one", + "key_two": "from facts_one" + }, + "common_fact": "also from facts_one module", + "common_list_fact": [ + "two", + "four", + "one", + "three", + "five" + ], + "common_list_fact2": [ + "four", + "four", + "one", + "two", + "three", + "five", + "five" + ], + "factsone": "from facts_one module", + "factstwo": "from facts_two module" +} \ No newline at end of file diff --git a/test/integration/targets/gathering_facts/verify_merge_facts.yml b/test/integration/targets/gathering_facts/verify_merge_facts.yml new file mode 100644 index 00000000000..d2144024382 --- /dev/null +++ b/test/integration/targets/gathering_facts/verify_merge_facts.yml @@ -0,0 +1,41 @@ +- name: rune one and two, verify merge is as expected + hosts: localhost + vars: + ansible_facts_modules: + - facts_one + - facts_two + tasks: + + - name: populate original + include_vars: + name: original + file: one_two.json + + - name: fail if ref file is updated + assert: + msg: '{{ansible_facts}} vs {{original}}' + that: + - ansible_facts|to_json(indent=4, sort_keys=True) == original|to_json(indent=4, sort_keys=True) + + - name: clear existing facts for next play + meta: clear_facts + + +- name: rune two and one, verify merge is as expected + hosts: localhost + vars: + ansible_facts_modules: + - facts_two + - facts_one + tasks: + + - name: populate original + include_vars: + name: original + file: two_one.json + + - name: fail if ref file is updated + assert: + msg: '{{ansible_facts}} vs {{original}}' + that: + - ansible_facts|to_json(indent=4, sort_keys=True) == original|to_json(indent=4, sort_keys=True) diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index dd3f289e4d2..8e7cbb9b418 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -282,6 +282,8 @@ test/integration/targets/collections_relative_imports/collection_root/ansible_co test/integration/targets/expect/files/test_command.py future-import-boilerplate test/integration/targets/expect/files/test_command.py metaclass-boilerplate test/integration/targets/gathering_facts/library/bogus_facts shebang +test/integration/targets/gathering_facts/library/facts_one shebang +test/integration/targets/gathering_facts/library/facts_two shebang test/integration/targets/get_url/files/testserver.py future-import-boilerplate test/integration/targets/get_url/files/testserver.py metaclass-boilerplate test/integration/targets/group/files/gidget.py future-import-boilerplate