From 00caeff928d7e78397fd16a0af5d61e796f46ecf Mon Sep 17 00:00:00 2001
From: Brian Coca <bcoca@users.noreply.github.com>
Date: Thu, 27 Aug 2020 14:39:19 -0400
Subject: [PATCH] dont clobber facts in loop (#71032) (#71095)

(cherry picked from commit f9af27c6317d46f262aa146aa491aa42cc3204bb)
---
 changelogs/fragments/no_fact_loop_loss.yml                | 2 ++
 lib/ansible/executor/task_executor.py                     | 4 ++--
 .../targets/gathering_facts/prevent_clobbering.yml        | 8 ++++++++
 test/integration/targets/gathering_facts/runme.sh         | 3 +++
 4 files changed, 15 insertions(+), 2 deletions(-)
 create mode 100644 changelogs/fragments/no_fact_loop_loss.yml
 create mode 100644 test/integration/targets/gathering_facts/prevent_clobbering.yml

diff --git a/changelogs/fragments/no_fact_loop_loss.yml b/changelogs/fragments/no_fact_loop_loss.yml
new file mode 100644
index 00000000000..d988809713a
--- /dev/null
+++ b/changelogs/fragments/no_fact_loop_loss.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - avoid clobbering existing facts inside loop when task also returns ansible_facts.
diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index 61cf6e6e74a..a0f8d212495 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -719,7 +719,7 @@ class TaskExecutor:
                 else:
                     # TODO: cleaning of facts should eventually become part of taskresults instead of vars
                     af = wrap_var(result['ansible_facts'])
-                    vars_copy.update(namespace_facts(af))
+                    vars_copy['ansible_facts'] = combine_vars(vars_copy.get('ansible_facts', {}), namespace_facts(af))
                     if C.INJECT_FACTS_AS_VARS:
                         vars_copy.update(clean_facts(af))
 
@@ -783,7 +783,7 @@ class TaskExecutor:
             else:
                 # TODO: cleaning of facts should eventually become part of taskresults instead of vars
                 af = wrap_var(result['ansible_facts'])
-                variables.update(namespace_facts(af))
+                variables['ansible_facts'] = combine_vars(variables.get('ansible_facts', {}), namespace_facts(af))
                 if C.INJECT_FACTS_AS_VARS:
                     variables.update(clean_facts(af))
 
diff --git a/test/integration/targets/gathering_facts/prevent_clobbering.yml b/test/integration/targets/gathering_facts/prevent_clobbering.yml
new file mode 100644
index 00000000000..94bb451200d
--- /dev/null
+++ b/test/integration/targets/gathering_facts/prevent_clobbering.yml
@@ -0,0 +1,8 @@
+- name: Verify existing facts don't go undefined on unrelated new facts in loop
+  hosts: localhost
+  gather_facts: True
+  tasks:
+  - name: Ensure that 'virtualization_type' is not undefined after first loop iteration
+    bogus_facts:
+    loop: [1, 2, 3]
+    when: ansible_facts['virtualization_type'] != 'NotDocker'
diff --git a/test/integration/targets/gathering_facts/runme.sh b/test/integration/targets/gathering_facts/runme.sh
index 5244b9ed051..4635562710e 100755
--- a/test/integration/targets/gathering_facts/runme.sh
+++ b/test/integration/targets/gathering_facts/runme.sh
@@ -13,3 +13,6 @@ 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'
+
+# ensure we dont clobber facts in loop
+ansible-playbook prevent_clobbering.yml -v "$@"