Make the loop variable (item by default) settable per taskfeature_make_loop_variable_settable_per_task

Required for include+with* tasks which may include files that also
have tasks containing a with* loop.

Fixes #12736

8 files changed, 67 insertions, 33 deletions

diff --git a/docsite/rst/playbooks_loops.rst b/docsite/rst/playbooks_loops.rst
index e329d7650d..51d365a9c1 100644
--- a/docsite/rst/playbooks_loops.rst
+++ b/docsite/rst/playbooks_loops.rst
@@ -549,22 +549,43 @@ More information on the patterns can be found on :doc:`intro_patterns`
 Loops and Includes
 ``````````````````
 
-In 2.0 you are able to use `with_` loops and task includes (but not playbook includes), this adds the ability to loop over the set of tasks in one shot.
-There are a couple of things that you need to keep in mind, an included task that has its own `with_` loop will overwrite the value of the special `item` variable.
-So if you want access to both the include's `item` and the current task's `item` you should use `set_fact` to create an alias to the outer one.::
+In Ansible 2.0 you are able to use `with_` loops and task includes (but not playbook includes), this adds the ability to loop over the set of tasks in one shot.
+One thing to keep in mind, a nested include task which has a task with its own `with_` loop will overwrite the value of the special `item` variable.
+If you want access to both the outer includes `item` and the current task's `item`, the `loop_var` value should be set on each loop to ensure the variable is unique
+to each loop::
+
+    # outer.yml
+    - include: inner.yml
+      loop_var: outer_item
+      with_items:
+        - 1
+        - 2
+        - 3
+
+    # inner.yml
+    - debug:
+        msg: "outer item={{outer_item}} inner item={{inner_item}}"
+      loop_var: inner_item
+      with_items:
+        - a
+        - b
+        - c
 
+The `loop_var` option was added in 2.1.0. For those using Ansible 2.0, use `set_fact` to create an alias to the outer variable::
 
-    - include: test.yml
+    # outer.yml
+    - include: inner.yml
       with_items:
         - 1
         - 2
         - 3
 
-in test.yml::
-
-    - set_fact: outer_loop="{{item}}"
+    # inner.yml
+    - set_fact:
+        outer_item: "{{item}}"
 
-    - debug: msg="outer item={{outer_loop}} inner item={{item}}"
+    - debug:
+        msg: "outer item={{outer_item}} inner item={{item}}"
       with_items:
         - a
         - b
diff --git a/lib/ansible/executor/process/result.py b/lib/ansible/executor/process/result.py
index fce31db494..862c7fe5a2 100644
--- a/lib/ansible/executor/process/result.py
+++ b/lib/ansible/executor/process/result.py
@@ -171,7 +171,8 @@ class ResultProcess(multiprocessing.Process):
                             self._send_result(('add_group', result._host, result_item))
                         elif 'ansible_facts' in result_item:
                             # if this task is registering facts, do that now
-                            item = result_item.get('item', None)
+                            loop_var = result._task.loop_var or 'item'
+                            item = result_item.get(loop_var, None)
                             if result._task.action == 'include_vars':
                                 for (key, value) in iteritems(result_item['ansible_facts']):
                                     self._send_result(('set_host_var', result._host, result._task, item, key, value))
diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index e391ccc431..291e69e112 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -224,9 +224,13 @@ class TaskExecutor:
         #task_vars = self._job_vars.copy()
         task_vars = self._job_vars
 
-        items = self._squash_items(items, task_vars)
+        loop_var = self._task.loop_var or 'item'
+        if loop_var in task_vars:
+            raise AnsibleError("the loop variable '%s' is already in use. You should set the `loop_var` value for the task to something else to avoid variable collisions" % loop_var)
+
+        items = self._squash_items(items, loop_var, task_vars)
         for item in items:
-            task_vars['item'] = item
+            task_vars[loop_var] = item
 
             try:
                 tmp_task = self._task.copy()
@@ -245,15 +249,16 @@ class TaskExecutor:
 
             # now update the result with the item info, and append the result
             # to the list of results
-            res['item'] = item
+            res[loop_var] = item
             res['_ansible_item_result'] = True
 
             self._rslt_q.put(TaskResult(self._host, self._task, res), block=False)
             results.append(res)
+            del task_vars[loop_var]
 
         return results
 
-    def _squash_items(self, items, variables):
+    def _squash_items(self, items, loop_var, variables):
         '''
         Squash items down to a comma-separated list for certain modules which support it
         (typically package management modules).
@@ -283,18 +288,18 @@ class TaskExecutor:
                 template_no_item = template_with_item = None
                 if name:
                     if templar._contains_vars(name):
-                        variables['item'] = '\0$'
+                        variables[loop_var] = '\0$'
                         template_no_item = templar.template(name, variables, cache=False)
-                        variables['item'] = '\0@'
+                        variables[loop_var] = '\0@'
                         template_with_item = templar.template(name, variables, cache=False)
-                        del variables['item']
+                        del variables[loop_var]
 
                     # Check if the user is doing some operation that doesn't take
                     # name/pkg or the name/pkg field doesn't have any variables
                     # and thus the items can't be squashed
                     if template_no_item != template_with_item:
                         for item in items:
-                            variables['item'] = item
+                            variables[loop_var] = item
                             if self._task.evaluate_conditional(templar, variables):
                                 new_item = templar.template(name, cache=False)
                                 final_items.append(new_item)
diff --git a/lib/ansible/playbook/included_file.py b/lib/ansible/playbook/included_file.py
index 1c0001f6b5..eaca3b80c7 100644
--- a/lib/ansible/playbook/included_file.py
+++ b/lib/ansible/playbook/included_file.py
@@ -80,8 +80,9 @@ class IncludedFile:
                     templar = Templar(loader=loader, variables=task_vars)
 
                     include_variables = include_result.get('include_variables', dict())
-                    if 'item' in include_result:
-                        task_vars['item'] = include_variables['item'] = include_result['item']
+                    loop_var = res._task.loop_var or 'item'
+                    if loop_var in include_result:
+                        task_vars[loop_var] = include_variables[loop_var] = include_result[loop_var]
 
                     if original_task:
                         if original_task.static:
diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py
index 8ee440386b..f89635567c 100644
--- a/lib/ansible/playbook/task.py
+++ b/lib/ansible/playbook/task.py
@@ -78,6 +78,7 @@ class Task(Base, Conditional, Taggable, Become):
     _first_available_file = FieldAttribute(isa='list')
     _loop                 = FieldAttribute(isa='string', private=True)
     _loop_args            = FieldAttribute(isa='list', private=True)
+    _loop_var             = FieldAttribute(isa='string', default='item')
     _name                 = FieldAttribute(isa='string', default='')
     _notify               = FieldAttribute(isa='list')
     _poll                 = FieldAttribute(isa='int')
diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index f06d4f6f75..c511a3d0b3 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -331,9 +331,10 @@ class StrategyBase:
                     # be a host that is not really in inventory at all
                     if task.delegate_to is not None and task.delegate_facts:
                         task_vars = self._variable_manager.get_vars(loader=self._loader, play=iterator._play, host=host, task=task)
-                        self.add_tqm_variables(task_vars, play=iterator._play)
+                        task_vars = self.add_tqm_variables(task_vars, play=iterator._play)
+                        loop_var = task.loop_var or 'item'
                         if item is not None:
-                            task_vars['item'] = item
+                            task_vars[loop_var] = item
                         templar = Templar(loader=self._loader, variables=task_vars)
                         host_name = templar.template(task.delegate_to)
                         actual_host = self._inventory.get_host(host_name)
diff --git a/test/integration/roles/setup_postgresql_db/tasks/main.yml b/test/integration/roles/setup_postgresql_db/tasks/main.yml
index 48f9211e1b..68ecd595dc 100644
--- a/test/integration/roles/setup_postgresql_db/tasks/main.yml
+++ b/test/integration/roles/setup_postgresql_db/tasks/main.yml
@@ -9,13 +9,15 @@
 
 # Make sure we start fresh
 - name: remove rpm dependencies for postgresql test
-  package: name={{ item }} state=absent
+  package: name={{ postgresql_package_item }} state=absent
   with_items: "{{postgresql_packages}}"
+  loop_var: postgresql_package_item
   when: ansible_os_family == "RedHat"
 
 - name: remove dpkg dependencies for postgresql test
-  apt: name={{ item }} state=absent
+  apt: name={{ postgresql_package_item }} state=absent
   with_items: "{{postgresql_packages}}"
+  loop_var: postgresql_package_item
   when: ansible_pkg_mgr  ==  'apt'
 
 - name: remove old db (red hat)
@@ -35,13 +37,15 @@
   when: ansible_os_family == "Debian"
 
 - name: install rpm dependencies for postgresql test
-  package: name={{ item }} state=latest
+  package: name={{ postgresql_package_item }} state=latest
   with_items: "{{postgresql_packages}}"
+  loop_var: postgresql_package_item
   when: ansible_os_family == "RedHat"
 
 - name: install dpkg dependencies for postgresql test
-  apt: name={{ item }} state=latest
+  apt: name={{ postgresql_package_item }} state=latest
   with_items: "{{postgresql_packages}}"
+  loop_var: postgresql_package_item
   when: ansible_pkg_mgr  ==  'apt'
 
 - name: Initialize postgres (systemd)
diff --git a/test/units/executor/test_task_executor.py b/test/units/executor/test_task_executor.py
index b029f87114..3bca43b702 100644
--- a/test/units/executor/test_task_executor.py
+++ b/test/units/executor/test_task_executor.py
@@ -212,22 +212,22 @@ class TestTaskExecutor(unittest.TestCase):
         # No replacement
         #
         mock_task.action = 'yum'
-        new_items = te._squash_items(items=items, variables=job_vars)
+        new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
         self.assertEqual(new_items, ['a', 'b', 'c'])
 
         mock_task.action = 'foo'
         mock_task.args={'name': '{{item}}'}
-        new_items = te._squash_items(items=items, variables=job_vars)
+        new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
         self.assertEqual(new_items, ['a', 'b', 'c'])
 
         mock_task.action = 'yum'
         mock_task.args={'name': 'static'}
-        new_items = te._squash_items(items=items, variables=job_vars)
+        new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
         self.assertEqual(new_items, ['a', 'b', 'c'])
 
         mock_task.action = 'yum'
         mock_task.args={'name': '{{pkg_mgr}}'}
-        new_items = te._squash_items(items=items, variables=job_vars)
+        new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
         self.assertEqual(new_items, ['a', 'b', 'c'])
 
         #
@@ -235,12 +235,12 @@ class TestTaskExecutor(unittest.TestCase):
         #
         mock_task.action = 'yum'
         mock_task.args={'name': '{{item}}'}
-        new_items = te._squash_items(items=items, variables=job_vars)
+        new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
         self.assertEqual(new_items, [['a','c']])
 
         mock_task.action = '{{pkg_mgr}}'
         mock_task.args={'name': '{{item}}'}
-        new_items = te._squash_items(items=items, variables=job_vars)
+        new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
         self.assertEqual(new_items, [['a', 'c']])
 
         #
@@ -249,7 +249,7 @@ class TestTaskExecutor(unittest.TestCase):
         #
         mock_task.action = '{{unknown}}'
         mock_task.args={'name': '{{item}}'}
-        new_items = te._squash_items(items=items, variables=job_vars)
+        new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
         self.assertEqual(new_items, ['a', 'b', 'c'])
 
         items = [dict(name='a', state='present'),
@@ -257,7 +257,7 @@ class TestTaskExecutor(unittest.TestCase):
                 dict(name='c', state='present')]
         mock_task.action = 'yum'
         mock_task.args={'name': '{{item}}'}
-        new_items = te._squash_items(items=items, variables=job_vars)
+        new_items = te._squash_items(items=items, loop_var='item', variables=job_vars)
         self.assertEqual(new_items, items)
 
     def test_task_executor_execute(self):