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

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

Fixes #12736

11 files changed, 140 insertions, 34 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..1e53d9191b 100644
--- a/lib/ansible/executor/process/result.py
+++ b/lib/ansible/executor/process/result.py
@@ -171,7 +171,10 @@ 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 = 'item'
+                            if result._task.loop_control:
+                                loop_var = result._task.loop_control.get('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..3563abbcf1 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -224,9 +224,17 @@ class TaskExecutor:
         #task_vars = self._job_vars.copy()
         task_vars = self._job_vars
 
-        items = self._squash_items(items, task_vars)
+        loop_var = 'item'
+        if self._task.loop_control:
+            # the value may be 'None', so we still need to default it back to 'item' 
+            loop_var = self._task.loop_control.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 in the `loop_control` option 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 +253,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 +292,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/attribute.py b/lib/ansible/playbook/attribute.py
index 0befb9d80d..9b4e6f26db 100644
--- a/lib/ansible/playbook/attribute.py
+++ b/lib/ansible/playbook/attribute.py
@@ -23,7 +23,7 @@ from copy import deepcopy
 
 class Attribute:
 
-    def __init__(self, isa=None, private=False, default=None, required=False, listof=None, priority=0, always_post_validate=False):
+    def __init__(self, isa=None, private=False, default=None, required=False, listof=None, priority=0, class_type=None, always_post_validate=False):
 
         self.isa = isa
         self.private = private
@@ -31,6 +31,7 @@ class Attribute:
         self.required = required
         self.listof = listof
         self.priority = priority
+        self.class_type = class_type
         self.always_post_validate = always_post_validate
 
         if default is not None and self.isa in ('list', 'dict', 'set'):
diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py
index c9fd2e84b4..7acd6e160e 100644
--- a/lib/ansible/playbook/base.py
+++ b/lib/ansible/playbook/base.py
@@ -304,6 +304,8 @@ class Base:
                 method = getattr(self, '_post_validate_%s' % name, None)
                 if method:
                     value = method(attribute, getattr(self, name), templar)
+                elif attribute.isa == 'class':
+                    value = getattr(self, name)
                 else:
                     # if the attribute contains a variable, template it now
                     value = templar.template(getattr(self, name))
@@ -363,6 +365,10 @@ class Base:
                             value = dict()
                         elif not isinstance(value, dict):
                             raise TypeError("%s is not a dictionary" % value)
+                    elif attribute.isa == 'class':
+                        if not isinstance(value, attribute.class_type):
+                            raise TypeError("%s is not a valid %s (got a %s instead)" % (name, attribute.class_type, type(value)))
+                        value.post_validate(templar=templar)
 
                 # and assign the massaged value back to the attribute field
                 setattr(self, name, value)
diff --git a/lib/ansible/playbook/included_file.py b/lib/ansible/playbook/included_file.py
index 1c0001f6b5..23a1f7860a 100644
--- a/lib/ansible/playbook/included_file.py
+++ b/lib/ansible/playbook/included_file.py
@@ -80,8 +80,11 @@ 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 = 'item'
+                    if res._task.loop_control:
+                        loop_var = res._task.loop_control.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/loop_control.py b/lib/ansible/playbook/loop_control.py
new file mode 100644
index 0000000000..9cdd18ffd9
--- /dev/null
+++ b/lib/ansible/playbook/loop_control.py
@@ -0,0 +1,40 @@
+# (c) 2012-2014, Michael DeHaan <michael.dehaan@gmail.com>
+#
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import itertools
+
+from ansible.compat.six import string_types
+from ansible.errors import AnsibleError
+from ansible.playbook.attribute import FieldAttribute
+from ansible.playbook.base import Base
+
+class LoopControl(Base):
+
+    _loop_var = FieldAttribute(isa='str')
+
+    def __init__(self):
+        super(LoopControl, self).__init__()
+
+    @staticmethod
+    def load(data, variable_manager=None, loader=None):
+        t = LoopControl()
+        return t.load_data(data, variable_manager=variable_manager, loader=loader)
+
diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py
index 8ee440386b..54bfdc960b 100644
--- a/lib/ansible/playbook/task.py
+++ b/lib/ansible/playbook/task.py
@@ -32,6 +32,7 @@ from ansible.playbook.base import Base
 from ansible.playbook.become import Become
 from ansible.playbook.block import Block
 from ansible.playbook.conditional import Conditional
+from ansible.playbook.loop_control import LoopControl
 from ansible.playbook.role import Role
 from ansible.playbook.taggable import Taggable
 
@@ -78,6 +79,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_control         = FieldAttribute(isa='class', class_type=LoopControl)
     _name                 = FieldAttribute(isa='string', default='')
     _notify               = FieldAttribute(isa='list')
     _poll                 = FieldAttribute(isa='int')
@@ -220,6 +222,16 @@ class Task(Base, Conditional, Taggable, Become):
 
         return super(Task, self).preprocess_data(new_ds)
 
+    def _load_loop_control(self, attr, ds):
+        if not isinstance(ds, dict):
+           raise AnsibleParserError(
+               "the `loop_control` value must be specified as a dictionary and cannot " \
+               "be a variable itself (though it can contain variables)",
+               obj=ds,
+           )
+
+        return LoopControl.load(data=ds, variable_manager=self._variable_manager, loader=self._loader)
+
     def post_validate(self, templar):
         '''
         Override of base class post_validate, to also do final validation on
diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index f06d4f6f75..80bda8078a 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -331,9 +331,12 @@ 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 = 'item'
+                        if task.loop_control:
+                            loop_var = task.loop_control.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..38bcd50dcc 100644
--- a/test/integration/roles/setup_postgresql_db/tasks/main.yml
+++ b/test/integration/roles/setup_postgresql_db/tasks/main.yml
@@ -9,13 +9,17 @@
 
 # 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_control:
+    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_control:
+    loop_var: postgresql_package_item
   when: ansible_pkg_mgr  ==  'apt'
 
 - name: remove old db (red hat)
@@ -35,13 +39,17 @@
   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_control:
+    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_control:
+    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):