Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ansible loop interface redesign #140

Open
bcoca opened this issue Aug 9, 2018 · 7 comments
Open

Ansible loop interface redesign #140

bcoca opened this issue Aug 9, 2018 · 7 comments

Comments

@bcoca
Copy link
Member

bcoca commented Aug 9, 2018

Proposal: future loops

Author: Brian Coca <@bcoca> IRC: bcoca

Date: 2016-02-32

  • Status: New
  • Proposal type: core design
  • Targeted release: future release
  • Estimated time to implement: weeks

Motivation

Current with_ loops are misleading and have much magic that confuses users and obfuscates what really goes on.
The until/retry keywords are 'also loops' and don't interact well with with_ loops.

Problems

  • squashing, but only on certain hardcoded list of modules (makes list of arguments into single argument for module, optimizing
    execution)
  • auto association with lookups of same name is not obvious
  • the default lookup 'items' does auto flattening (1 level), so do many others to keep parity
  • requires {{ }} Jinja2 to disambiguate between strings and variables .. due to 'auto flatten' above
    was worse before when we allowed 'naked' Jinja2 and you could not give errors on missing var as it could be meant as string.
  • awkward to pass other fields to lookups, mixed list/dict is not possible in YAML syntax
  • lack of retry/until per loop item

Solution proposal

Redesign loop syntax (names not final, feel free to bikeshed)

    # loops, default values!
- action: ...
  loop:
    over: []
    until:
      when: result is failed
      retry: 0
      scope: item
    pause: 0
    display: item
    variable: item
    index: _index
    last: _last
    squash_option: name
    forks: 1

The terms

  • over: list of items to loop over, or a string representing a Jinaj2 expression that evaluates into a list, if empty we skip the task, if undefined we assume 1 True item (really needs better name)
  • until:
    • when: Jinja2 conditional expression that we'll retry until it is met defaults to result is failed
    • retry: Number of times to retry until giving up if 'until.when' condition is not met
      possibly make a boolean and then let 'when condition' get a retries value to decide end. default to 0 (currently we have 3 but only if until: is defined)
    • scope: if we should retry whole task from start or just the current item on failure. defaults to 'item' but can also be 'all'
  • pause: seconds paused between item/execution attempts, defaults to 0
  • display: Jinja2 expression which to display when reporting about the loop item, defaults to 'item'
  • variable: variable in which we stuff the loop item, defaults to 'item'
  • index: var that holds the index of the 'current item' in the provided list, defaults to '_index'
  • last: var that holds the index number of the last item in the loop, no first as that is always 0, defaults to '_last'

OPTIONAL

  • squash_option: 'optimization' that will pass all the loop items in 1 shot to the task (action must support this) to the option given. defaults to None, which means it won't squash
  • forks: number of parallel forks of the task to run, any value other than 1 removes ability to make items conditional on previous items or 'quantified' until conditions, defaults to 1
  • NOTE: I'm leaving squash_option here because of 'historical reasons' but I'm not in favor of adding this magic on the loop side anymore, I think modules that accept lists should be fed such if the user wants to use them that way and the loop should not attempt to deal with this.

This would give a cleaner and clearer interface for all looping needs as well as unify the until/retry and 'list looping' into one
structure and hopefully do the same in execution so they can work together intelligently.

A lot of this is base on feedback for ansible/ansible#12086, some we are implementing already in other wa
ys.

@caphrim007
Copy link

for the squash_option field, was wondering how might modules decide to support this? And/or Ansible report if a module does not support this (like interrogating the module argspec before exec'ing it)

for example, if a module foo_member today recommends that a user use the loop construct to create X members;

foo_member:
   first: "{{ item.first }}"
   middle: "{{ item.middle }"
   last: "{{ item.last }}"
loop:
  - first: Alice
    middle: M
    last: User
  - first: Bob
    middle: C
    last: User

Is this module a candidate for squash_option? and how would the definitions of the arguments in this module's argument spec need to change? For instance, if today they were type='str'?

What would be the recommendation from Ansible?

@bcoca
Copy link
Member Author

bcoca commented Aug 9, 2018

actually, that is one i'm now partial to dropping (i really just pasted pretty old proposal) .. squashing should not be needed, people should just pass the list directly to fields for modules that support it.

@bcoca
Copy link
Member Author

bcoca commented Aug 16, 2018

one thing the current loop has, it avoids 'flattening' magic so now it would be possible to use 'naked' jinja2 expresions again. Since now it is simple to disambiguate between a string as part of a list vs a string that represents a variable or expression.

Even the patch is simple:

diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index 84c2dad295..6de788ca87 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -236,7 +236,12 @@ class TaskExecutor:
                 raise AnsibleError("Unexpected failure in finding the lookup named '%s' in the available lookup plugins" % self._task.loop_with)

         elif self._task.loop:
-            items = templar.template(self._task.loop)
+
+            if isinstance(self._task.loop, string_types) and not templar.is_template(self._task.loop):
+                items = templar.template('{{%s}}' % self._task.loop)
+            else:
+                items = templar.template(self._task.loop)
+
             if not isinstance(items, list):
                 raise AnsibleError(
                     "Invalid data passed to 'loop', it requires a list, got this instead: %s."

so now you could do:

- debug: var=item
  loop: test
  vars:
    test: [1,2,3]

@webknjaz
Copy link
Member

Ref: ansible/ansible#44741

@webknjaz
Copy link
Member

webknjaz commented Sep 2, 2018

Integration tests for currently supported interaction between loop and until: ansible/ansible#44927

@dagwieers
Copy link
Contributor

dagwieers commented Sep 4, 2018

@caphrim007 For squashing and pure functionality, we still have this one open: #71

@vbotka
Copy link

vbotka commented Jun 25, 2020

(ansible 2.9.6) FWIW, the task below

    - command: '[ "{{ item }}" -gt "3" ]'
      loop: "{{ range(1, 5 + 1)|list }}"
      register: result
      ignore_errors: true
      when: not condition
      vars:
        condition: '{{ (result|default({"rc": 1})).rc == 0 }}'

gives (abridged)

    changed: [localhost] => (item=4)
    skipping: [localhost] => (item=5) 
    ...ignoring

samdoran added a commit to ansible/ansible that referenced this issue Nov 30, 2021
Ref #44741
Ref ansible/proposals#140

* Replace select filter with a more portable thing
* Add context
    This is needed for split controller/remote

Co-authored-by: Sam Doran <sdoran@redhat.com>
@bcoca bcoca changed the title future loops Ansible loop interface redesign Nov 30, 2021
clementmartin pushed a commit to clementmartin/ansible that referenced this issue Nov 30, 2021
Ref ansible#44741
Ref ansible/proposals#140

* Replace select filter with a more portable thing
* Add context
    This is needed for split controller/remote

Co-authored-by: Sam Doran <sdoran@redhat.com>
konstruktoid pushed a commit to konstruktoid/ansible-upstream that referenced this issue Feb 2, 2022
Ref ansible#44741
Ref ansible/proposals#140

* Replace select filter with a more portable thing
* Add context
    This is needed for split controller/remote

Co-authored-by: Sam Doran <sdoran@redhat.com>
bcoca pushed a commit to bcoca/ansible that referenced this issue Feb 7, 2022
Ref ansible#44741
Ref ansible/proposals#140

* Replace select filter with a more portable thing
* Add context
    This is needed for split controller/remote

Co-authored-by: Sam Doran <sdoran@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants