correctly merge multiple facts results (#68987)

* correctly merge multiple facts results

  fixes #68532

changelogs/fragments/gf_fix.yml

@@ -0,0 +1,2 @@
+bugfixes:
+    - now correclty merge and not just overwrite facts when gathering using multiple modules.

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))

test/integration/targets/gathering_facts/aliases

@@ -1 +1,2 @@
 shippable/posix/group3
+needs/root

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"
+		]
+    }
+}'

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"
+		]
+    }
+}'

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"
+}

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'

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"
+}

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)

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