Repository: ansible/ansible
Pull request: Don't use the task for a cache, return a special cache var
##### SUMMARY

This PR addresses an issue where looping over a hostvar, with a templated `delegate_to` would create a loop cache on the task for the last host in the loop.  Fixes #47207 

The original problem that was being solved was the following:

```
- command: whoami
  delegate_to: "{{ item }}"
  with_random_choice: "{{ ansible_play_hosts }}"
```

Because we process the loop in `VariableManager._get_delegated_vars` and in `TaskExecutor._get_loop_items` that loop could produce different results, causing `ansible_delegated_vars` to not exist for the host later selected in `TaskExecutor._get_loop_items`

We solved that by caching the resultant loop as `task.loop`.  The problem with that is when the variable being looped is different per host.

This fix allows the original case to work, as well as the case of:

```
- command: whoami
  delegate_to: "{{ item }}"
  loop: "{{ some_per_host_list }}"
```

##### ISSUE TYPE
- Bugfix Pull Request

##### COMPONENT NAME
lib/ansible/executor/task_executor.py
lib/ansible/vars/manager.py

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes -->
```paste below
2.7
2.8
```

##### ADDITIONAL INFORMATION
Also, not mentioned above is that an inventory was supplied to some other tests that were skipping as a result of the inventory not matching.

Problem statement:
'with_dict' not working correctly when used in combination with a variable as input for 'delegate_to'
##### SUMMARY
While using a variable as input to 'delegate_to' in combination with
'with_dict' a dictionary configured in host_vars of host A is also
used for host B.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
delegate_to
with_dict

##### ANSIBLE VERSION
```
ansible 2.7.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/usr/lib/python2.7/site-packages/ansible/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, Jul 13 2018, 13:06:57) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]

```
```
ansible 2.7.0
 config file = /etc/ansible/ansible.cfg
 configured module search path = ['/usr/share/ansible/plugins/modules']
 ansible python module location = /usr/lib/python3.7/site-packages/ansible executable location = /usr/bin/ansible
 python version = 3.7.0 (default, Sep 15 2018, 19:13:07) [GCC 8.2.1 20180831]
```

##### CONFIGURATION
```
ANSIBLE_SSH_ARGS(/etc/ansible/ansible.cfg) = -o ControlMaster=auto -o ControlPersist=60s
ANSIBLE_SSH_RETRIES(/etc/ansible/ansible.cfg) = 23
CACHE_PLUGIN(/etc/ansible/ansible.cfg) = memory
DEFAULT_ACTION_PLUGIN_PATH(/etc/ansible/ansible.cfg) = [u'/usr/share/ansible_plugins/action_plugins']
DEFAULT_CALLBACK_PLUGIN_PATH(/etc/ansible/ansible.cfg) = [u'/usr/share/ansible_plugins/callback_plugins']
DEFAULT_CONNECTION_PLUGIN_PATH(/etc/ansible/ansible.cfg) = [u'/usr/share/ansible_plugins/connection_plugins']
DEFAULT_FILTER_PLUGIN_PATH(/etc/ansible/ansible.cfg) = [u'/usr/share/ansible_plugins/filter_plugins']
DEFAULT_FORKS(/etc/ansible/ansible.cfg) = 20
DEFAULT_GATHERING(/etc/ansible/ansible.cfg) = implicit
DEFAULT_HOST_LIST(/etc/ansible/ansible.cfg) = [u'/usr/bin/get_ansible_inventory.py']
DEFAULT_LOOKUP_PLUGIN_PATH(/etc/ansible/ansible.cfg) = [u'/usr/share/ansible_plugins/lookup_plugins']
DEFAULT_MANAGED_STR(/etc/ansible/ansible.cfg) = Ansible managed: {file} modified on %Y-%m-%d %H:%M:%S by {uid
DEFAULT_MODULE_PATH(/etc/ansible/ansible.cfg) = [u'/usr/lib/python2.7/site-packages/ansible/modules']
DEFAULT_POLL_INTERVAL(/etc/ansible/ansible.cfg) = 15
DEFAULT_REMOTE_PORT(/etc/ansible/ansible.cfg) = 22
DEFAULT_TIMEOUT(/etc/ansible/ansible.cfg) = 10
DEFAULT_TRANSPORT(/etc/ansible/ansible.cfg) = paramiko
DEFAULT_VARS_PLUGIN_PATH(/etc/ansible/ansible.cfg) = [u'/usr/share/ansible_plugins/vars_plugins']
DISPLAY_SKIPPED_HOSTS(/etc/ansible/ansible.cfg) = False
HOST_KEY_CHECKING(/etc/ansible/ansible.cfg) = False
```

##### OS / ENVIRONMENT
Tested on 
 - Centos 7.5.1804 (Core)
 - Arch Linux

##### STEPS TO REPRODUCE
Create inventory
```
balrog    gandalf="{'shout':'You shall not pass!'}"
fellow    gandalf="{'speak:':'Run you fools!'}"
```

Playbook
```
---
- name: with_dict in combination with delegate_to
  hosts: "all"
  gather_facts: no

  vars:
    delegate_to_host: "localhost"

  tasks:
    - name: works correctly
      debug: var=item
      delegate_to: localhost
      with_dict: "{{ gandalf }}"

    - name: shows same item for all hosts
      debug: var=item
      delegate_to: "{{ delegate_to_host }}"
      with_dict: "{{ gandalf }}"

```

##### EXPECTED RESULTS
Both tasks produce the same output as shown here with older version:
```
# ansible-playbook issue.yml -i inventory -vvvv
ansible-playbook 2.6.1
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/usr/share/ansible']
  ansible python module location = /usr/local/lib/python2.7/dist-packages/ansible
  executable location = /usr/local/bin/ansible-playbook
  python version = 2.7.12 (default, Dec  4 2017, 14:50:18) [GCC 5.4.0 20160609]
Using /etc/ansible/ansible.cfg as config file
setting up inventory plugins
Parsed /data/ansible/inventory inventory source with ini plugin
Loading callback plugin default of type stdout, v2.0 from /usr/local/lib/python2.7/dist-packages/ansible/plugins/callback/default.pyc

PLAYBOOK: issue.yml *****************************************************************************************
1 plays in issue.yml

PLAY [with_dict in combination with delegate_to] ************************************************************
META: ran handlers

TASK [works correctly] **************************************************************************************
task path: /data/ansible/issue.yml:20
ok: [balrog -> localhost] => (item={'value': u'You shall not pass!', 'key': u'shout'}) => {
    "item": {
        "key": "shout", 
        "value": "You shall not pass!"
    }
}
ok: [fellow -> localhost] => (item={'value': u'Run you fools!', 'key': u'speak:'}) => {
    "item": {
        "key": "speak:", 
        "value": "Run you fools!"
    }
}

TASK [shows same item for all hosts] ************************************************************************
task path: /data/ansible/issue.yml:25
ok: [balrog -> localhost] => (item={'value': u'You shall not pass!', 'key': u'shout'}) => {
    "item": {
        "key": "shout", 
        "value": "You shall not pass!"
    }
}
ok: [fellow -> localhost] => (item={'value': u'Run you fools!', 'key': u'speak:'}) => {
    "item": {
        "key": "speak:", 
        "value": "Run you fools!"
    }
}
META: ran handlers
META: ran handlers

PLAY RECAP **************************************************************************************************
balrog                     : ok=2    changed=0    unreachable=0    failed=0   
fellow                     : ok=2    changed=0    unreachable=0    failed=0   

```



##### ACTUAL RESULTS
If a variable is used to choose target of delegate_to all hosts are using the same dictionary,
even if they should not be able to access the host_vars of another host.

```
# ansible-playbook issue.yml -i inventory -vvvv
ansible-playbook 2.7.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/usr/lib/python2.7/site-packages/ansible/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible-playbook
  python version = 2.7.5 (default, Jul 13 2018, 13:06:57) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]
Using /etc/ansible/ansible.cfg as config file
setting up inventory plugins
Parsed /data/ansible/inventory inventory source with ini plugin
Loading callback plugin default of type stdout, v2.0 from /usr/lib/python2.7/site-packages/ansible/plugins/callback/default.pyc

PLAYBOOK: issue.yml *****************************************************************************************
1 plays in issue.yml

PLAY [with_dict in combination with delegate_to] ************************************************************
META: ran handlers

TASK [works correctly] **************************************************************************************
task path: /data/ansible/issue.yml:10
ok: [balrog -> localhost] => (item={'value': u'You shall not pass!', 'key': u'shout'}) => {
    "item": {
        "key": "shout", 
        "value": "You shall not pass!"
    }
}
ok: [fellow -> localhost] => (item={'value': u'Run you fools!', 'key': u'speak:'}) => {
    "item": {
        "key": "speak:", 
        "value": "Run you fools!"
    }
}

TASK [shows same item for all hosts] ************************************************************************
task path: /data/ansible/issue.yml:15
ok: [balrog -> localhost] => (item={'key': u'shout', 'value': u'You shall not pass!'}) => {
    "item": {
        "key": "shout", 
        "value": "You shall not pass!"
    }
}
ok: [fellow -> localhost] => (item={'value': u'You shall not pass!', 'key': u'shout'}) => {
    "item": {
        "key": "shout", 
        "value": "You shall not pass!"
    }
}
META: ran handlers
META: ran handlers

PLAY RECAP **************************************************************************************************
balrog                     : ok=2    changed=0    unreachable=0    failed=0   
fellow                     : ok=2    changed=0    unreachable=0    failed=0   

```

Hints from issue discussion:
Hi @christian-simmen, thank you for submitting this issue!

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: thank_submitter --->
Files identified in the description:
* [lib/ansible/playbook/task.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/playbook/task.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->

Commit message: Add changelog fragment

Patch to review:
diff --git a/changelogs/fragments/delegate_to_loop_hostvars.yaml b/changelogs/fragments/delegate_to_loop_hostvars.yaml
new file mode 100644
index 00000000000000..26680f3ae5e77e
--- /dev/null
+++ b/changelogs/fragments/delegate_to_loop_hostvars.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+- delegate_to - When templating ``delegate_to`` in a loop, don't use the task for a cache, return a special cache through ``get_vars`` allowing looping over a hostvar (https://github.com/ansible/ansible/issues/47207)
diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index d1f67af8a2abf9..6d779dcd46bac5 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -4,6 +4,7 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
+import copy
 import os
 import pty
 import time
@@ -210,7 +211,12 @@ def _get_loop_items(self):
 
         templar = Templar(loader=self._loader, shared_loader_obj=self._shared_loader_obj, variables=self._job_vars)
         items = None
-        if self._task.loop_with:
+        loop_cache = self._job_vars.get('_ansible_loop_cache')
+        if loop_cache is not None:
+            # _ansible_loop_cache may be set in `get_vars` when calculating `delegate_to`
+            # to avoid reprocessing the loop
+            items = copy.copy(loop_cache)
+        elif self._task.loop_with:
             if self._task.loop_with in self._shared_loader_obj.lookup_loader:
                 fail = True
                 if self._task.loop_with == 'first_found':
diff --git a/lib/ansible/vars/manager.py b/lib/ansible/vars/manager.py
index ba0e56d376ef99..6ac8c658ae8dd3 100644
--- a/lib/ansible/vars/manager.py
+++ b/lib/ansible/vars/manager.py
@@ -430,7 +430,7 @@ def plugins_by_groups():
         # if we have a task and we're delegating to another host, figure out the
         # variables for that host now so we don't have to rely on hostvars later
         if task and task.delegate_to is not None and include_delegate_to:
-            all_vars['ansible_delegated_vars'] = self._get_delegated_vars(play, task, all_vars)
+            all_vars['ansible_delegated_vars'], all_vars['_ansible_loop_cache'] = self._get_delegated_vars(play, task, host, all_vars)
 
         # 'vars' magic var
         if task or play:
@@ -486,7 +486,7 @@ def _get_magic_variables(self, play, host, task, include_hostvars, include_deleg
 
         return variables
 
-    def _get_delegated_vars(self, play, task, existing_variables):
+    def _get_delegated_vars(self, play, task, host, existing_variables):
         if not hasattr(task, 'loop'):
             # This "task" is not a Task, so we need to skip it
             return {}
@@ -594,16 +594,15 @@ def _get_delegated_vars(self, play, task, existing_variables):
                 include_hostvars=False,
             )
 
+        _ansible_loop_cache = None
         if has_loop and cache_items:
-            # delegate_to templating produced a change, update task.loop with templated items,
+            # delegate_to templating produced a change, so we will cache the templated items
+            # in a special private hostvar
             # this ensures that delegate_to+loop doesn't produce different results 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
+            _ansible_loop_cache = items
 
-        return delegated_host_vars
+        return delegated_host_vars, _ansible_loop_cache
 
     def clear_facts(self, hostname):
         '''
diff --git a/test/integration/targets/delegate_to/runme.sh b/test/integration/targets/delegate_to/runme.sh
index d9978837b20a97..733274cdaf8a7b 100755
--- a/test/integration/targets/delegate_to/runme.sh
+++ b/test/integration/targets/delegate_to/runme.sh
@@ -9,6 +9,8 @@ ansible-playbook test_loop_control.yml -v "$@"
 
 ansible-playbook test_delegate_to_loop_randomness.yml -v "$@"
 
-ansible-playbook delegate_and_nolog.yml -v "$@"
+ansible-playbook delegate_and_nolog.yml -i ../../inventory -v "$@"
 
-ansible-playbook delegate_facts_block.yml -v "$@"
+ansible-playbook delegate_facts_block.yml -i ../../inventory -v "$@"
+
+ansible-playbook test_delegate_to_loop_caching.yml -i ../../inventory -v "$@"
diff --git a/test/integration/targets/delegate_to/test_delegate_to_loop_caching.yml b/test/integration/targets/delegate_to/test_delegate_to_loop_caching.yml
new file mode 100644
index 00000000000000..6ea08f72d899c7
--- /dev/null
+++ b/test/integration/targets/delegate_to/test_delegate_to_loop_caching.yml
@@ -0,0 +1,45 @@
+- hosts: testhost,testhost2
+  gather_facts: false
+  vars:
+    delegate_to_host: "localhost"
+  tasks:
+    - set_fact:
+        gandalf:
+          shout: 'You shall not pass!'
+      when: inventory_hostname == 'testhost'
+
+    - set_fact:
+        gandalf:
+          speak: 'Run you fools!'
+      when: inventory_hostname == 'testhost2'
+
+    - name: works correctly
+      debug: var=item
+      delegate_to: localhost
+      with_dict: "{{ gandalf }}"
+      register: result1
+
+    - name: shows same item for all hosts
+      debug: var=item
+      delegate_to: "{{ delegate_to_host }}"
+      with_dict: "{{ gandalf }}"
+      register: result2
+
+    - debug:
+        var: result2.results[0].item.value
+
+    - assert:
+        that:
+          - result1.results[0].item.value == 'You shall not pass!'
+          - result2.results[0].item.value == 'You shall not pass!'
+      when: inventory_hostname == 'testhost'
+
+    - assert:
+        that:
+          - result1.results[0].item.value == 'Run you fools!'
+          - result2.results[0].item.value == 'Run you fools!'
+      when: inventory_hostname == 'testhost2'
+
+    - assert:
+        that:
+          - _ansible_loop_cache is undefined


Code review comments:
diff_hunk: @@ -210,7 +211,12 @@ def _get_loop_items(self):
 
         templar = Templar(loader=self._loader, shared_loader_obj=self._shared_loader_obj, variables=self._job_vars)
         items = None
-        if self._task.loop_with:
+        loop_cache = self._job_vars.get('_ansible_loop_cache')
+        if loop_cache is not None:
+            # _ansible_loop_cache may be set in `get_vars` when calculating `delegate_to`
+            # to avoid reprocessing the loop
+            items = copy.copy(loop_cache)
path: lib/ansible/executor/task_executor.py
text: @user1:
wouldn't we need a deepcopy here?

@author:
Oh, actually, I can remove that all together.  I had added it when I was removing it from `_job_vars`.

@author:
Removed