allow incremental update for vars in loop for set_fact/include_vars (#38302)

* fix set_fact/include_vars looping

* tests for all behaviours affected

* add tests with injection off/on

lib/ansible/executor/task_executor.py

@@ -605,7 +605,10 @@ class TaskExecutor:
                     failed_when_result = False
                 return failed_when_result
 
-            if 'ansible_facts' in result and self._task.action not in ('set_fact', 'include_vars'):
+            if 'ansible_facts' in result:
+                if self._task.action in ('set_fact', 'include_vars'):
+                    vars_copy.update(result['ansible_facts'])
+                else:
                     vars_copy.update(namespace_facts(result['ansible_facts']))
                     if C.INJECT_FACTS_AS_VARS:
                         vars_copy.update(clean_facts(result['ansible_facts']))
@@ -663,7 +666,10 @@ class TaskExecutor:
         if self._task.register:
             variables[self._task.register] = wrap_var(result)
 
-        if 'ansible_facts' in result and self._task.action not in ('set_fact', 'include_vars'):
+        if 'ansible_facts' in result:
+            if self._task.action in ('set_fact', 'include_vars'):
+                variables.update(result['ansible_facts'])
+            else:
                 variables.update(namespace_facts(result['ansible_facts']))
                 if C.INJECT_FACTS_AS_VARS:
                     variables.update(clean_facts(result['ansible_facts']))

test/integration/targets/set_fact/incremental.yml

@@ -0,0 +1,19 @@
+- name: test set_fact incremental https://github.com/ansible/ansible/issues/38271
+  hosts: testhost
+  gather_facts: no
+  tasks:
+    - name: Generate inline loop for set_fact
+      set_fact:
+        dig_list: "{{ dig_list + [ item ] }}"
+      loop:
+        - two
+        - three
+        - four
+      vars:
+        dig_list:
+            - one
+
+    - name: verify cumulative set fact worked
+      assert:
+        that:
+           - dig_list == ['one', 'two', 'three', 'four']

test/integration/targets/set_fact/nowarn_clean_facts.yml

@@ -0,0 +1,7 @@
+- name: Test no warnings ref "http://github.com/ansible/ansible/issues/37535"
+  hosts: testhost
+  gather_facts: false
+  tasks:
+    - name: set ssh jump host args
+      set_fact:
+        ansible_ssh_common_args: "-o ProxyCommand='ssh -W %h:%p -q root@localhost'"

test/integration/targets/set_fact/runme.sh

@@ -5,9 +5,15 @@ set -eux
 MYTMPDIR=$(mktemp -d 2>/dev/null || mktemp -d -t 'mytmpdir')
 trap 'rm -rf "${MYTMPDIR}"' EXIT
 
-ANSIBLE_CACHE_PLUGIN=jsonfile ANSIBLE_CACHE_PLUGIN_CONNECTION="${MYTMPDIR}" ansible-playbook -i ../../inventory "$@" set_fact_cached_1.yml
+# ensure we can incrementally set fact via loopi, injection or not
+ANSIBLE_INJECT_FACT_VARS=0 ansible-playbook -i ../../inventory incremental.yml
+ANSIBLE_INJECT_FACT_VARS=1 ansible-playbook -i ../../inventory incremental.yml
 
-ANSIBLE_CACHE_PLUGIN=jsonfile ANSIBLE_CACHE_PLUGIN_CONNECTION="${MYTMPDIR}" ansible-playbook -i ../../inventory "$@" set_fact_cached_2.yml
-
-ANSIBLE_CACHE_PLUGIN=jsonfile ANSIBLE_CACHE_PLUGIN_CONNECTION="${MYTMPDIR}" ansible-playbook -i ../../inventory --flush-cache "$@" set_fact_no_cache.yml
+# ensure we dont have spurious warnings do to clean_facts
+ansible-playbook -i ../../inventory nowarn_clean_facts.yml | grep '[WARNING]: Removed restricted key from module data: ansible_ssh_common_args' && exit 1
 
+# test cached feature
+export ANSIBLE_CACHE_PLUGIN=jsonfile ANSIBLE_CACHE_PLUGIN_CONNECTION="${MYTMPDIR}"
+ansible-playbook -i ../../inventory "$@" set_fact_cached_1.yml
+ansible-playbook -i ../../inventory "$@" set_fact_cached_2.yml
+ansible-playbook -i ../../inventory --flush-cache "$@" set_fact_no_cache.yml