From 1401fa74cc39b44ea6ddd1d4b5c5eac9fb82bbf9 Mon Sep 17 00:00:00 2001
From: Matt Martz <matt@sivel.net>
Date: Tue, 26 Jun 2018 15:10:04 -0500
Subject: [PATCH] Cache items when task.loop/with_items is evaluated to set
 delegate_to vars (#41969)

* If we evaluate task.loop/with_items when calculating delegate_to vars, cache the items. Fixes #28231

* Add comments about caching loop items

* Add test for delegate_to+loop+random

* Be more careful about where we update task.loop
---
 lib/ansible/vars/manager.py                   |  9 +++
 test/integration/targets/delegate_to/runme.sh |  2 +
 .../test_delegate_to_loop_randomness.yml      | 58 +++++++++++++++++++
 3 files changed, 69 insertions(+)
 create mode 100644 test/integration/targets/delegate_to/test_delegate_to_loop_randomness.yml

diff --git a/lib/ansible/vars/manager.py b/lib/ansible/vars/manager.py
index 923c10dafd2..6f104b518d4 100644
--- a/lib/ansible/vars/manager.py
+++ b/lib/ansible/vars/manager.py
@@ -498,10 +498,19 @@ class VariableManager:
                     # This task will be skipped later due to this, so we just setup
                     # a dummy array for the later code so it doesn't fail
                     items = [None]
+                # Update task.loop with templated items, this ensures that delegate_to+loop
+                # doesn't produce different restuls than TaskExecutor which may reprocess the loop
+                # Set loop_with to None, so we don't do extra unexpected processing on the cached items later
+                # in TaskExecutor
+                task.loop_with = None
+                task.loop = items
             else:
                 raise AnsibleError("Failed to find the lookup named '%s' in the available lookup plugins" % task.loop_with)
         elif task.loop is not None:
             items = templar.template(task.loop)
+            # Update task.loop with templated items, this ensures that delegate_to+loop
+            # doesn't produce different restuls than TaskExecutor which may reprocess the loop
+            task.loop = items
         else:
             items = [None]
 
diff --git a/test/integration/targets/delegate_to/runme.sh b/test/integration/targets/delegate_to/runme.sh
index 3d2873ee447..38514c37b81 100755
--- a/test/integration/targets/delegate_to/runme.sh
+++ b/test/integration/targets/delegate_to/runme.sh
@@ -6,3 +6,5 @@ ANSIBLE_SSH_ARGS='-C -o ControlMaster=auto -o ControlPersist=60s -o UserKnownHos
     ANSIBLE_HOST_KEY_CHECKING=false ansible-playbook test_delegate_to.yml -i ../../inventory -v "$@"
 
 ansible-playbook test_loop_control.yml -v "$@"
+
+ansible-playbook test_delegate_to_loop_randomness.yml -v "$@"
diff --git a/test/integration/targets/delegate_to/test_delegate_to_loop_randomness.yml b/test/integration/targets/delegate_to/test_delegate_to_loop_randomness.yml
new file mode 100644
index 00000000000..b43e884a0c6
--- /dev/null
+++ b/test/integration/targets/delegate_to/test_delegate_to_loop_randomness.yml
@@ -0,0 +1,58 @@
+---
+- name: Integration tests for #28231
+  hosts: localhost
+  gather_facts: false
+  tasks:
+    - name: Add some test hosts
+      add_host:
+        name: "foo{{item}}"
+        groups: foo
+      loop: "{{ range(10)|list }}"
+
+    # We expect all of the next 3 runs to succeeed
+    # this is done multiple times to increase randomness
+    - assert:
+        that:
+          - item in ansible_delegated_vars
+      delegate_to: "{{ item }}"
+      loop:
+        - "{{ groups.foo|random }}"
+      ignore_errors: true
+      register: result1
+
+    - assert:
+        that:
+          - item in ansible_delegated_vars
+      delegate_to: "{{ item }}"
+      loop:
+        - "{{ groups.foo|random }}"
+      ignore_errors: true
+      register: result2
+
+    - assert:
+        that:
+          - item in ansible_delegated_vars
+      delegate_to: "{{ item }}"
+      loop:
+        - "{{ groups.foo|random }}"
+      ignore_errors: true
+      register: result3
+
+    - debug:
+        var: result1
+
+    - debug:
+        var: result2
+
+    - debug:
+        var: result3
+
+    - name: Ensure all of the 3 asserts were successful
+      assert:
+        that:
+          - results is all
+      vars:
+        results:
+          - "{{ (result1.results|first) is successful }}"
+          - "{{ (result2.results|first) is successful }}"
+          - "{{ (result3.results|first) is successful }}"