From 9281148b623d4e2e8302778d91af3e84ab9579a9 Mon Sep 17 00:00:00 2001
From: Brian Coca <bcoca@users.noreply.github.com>
Date: Wed, 20 May 2020 18:53:37 -0400
Subject: [PATCH] correctly merge multiple facts results (#68987)
* correctly merge multiple facts results
fixes #68532
---
changelogs/fragments/gf_fix.yml | 2 +
lib/ansible/plugins/action/gather_facts.py | 15 +++++--
.../targets/gathering_facts/aliases | 1 +
.../targets/gathering_facts/library/facts_one | 25 +++++++++++
.../targets/gathering_facts/library/facts_two | 24 +++++++++++
.../targets/gathering_facts/one_two.json | 27 ++++++++++++
.../targets/gathering_facts/runme.sh | 7 +++-
.../targets/gathering_facts/two_one.json | 27 ++++++++++++
.../gathering_facts/verify_merge_facts.yml | 41 +++++++++++++++++++
test/sanity/ignore.txt | 2 +
10 files changed, 165 insertions(+), 6 deletions(-)
create mode 100644 changelogs/fragments/gf_fix.yml
create mode 100644 test/integration/targets/gathering_facts/library/facts_one
create mode 100644 test/integration/targets/gathering_facts/library/facts_two
create mode 100644 test/integration/targets/gathering_facts/one_two.json
create mode 100644 test/integration/targets/gathering_facts/two_one.json
create mode 100644 test/integration/targets/gathering_facts/verify_merge_facts.yml
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