Introduce "this" for default task results

[dagwieers]

Proposal: Make task results available as "this" variable

Author: Dag Wieers <@dagwieers>

Date: 2017/09/20

• Status: New
• Proposal type: core design
• Targeted Release: 2.5
• Associated PR: https://github.com/ansible/ansible/pull/30957
• Estimated time to implement: 30 mins (already done)

Motivation

We would like to make it easier for users to influence when the task failed or changed.

Problems

• Registering return values that you don't need to expose to other tasks may overwrite an existing variable inadvertently
• It's an additional unnecessary statement
• Simplifies using changed_when:, failed_when: and until: for loop control

Solution proposal

I would like to introduce a reserved variable name this that would consist of the return values of the specific task.

  # Fail task when both files are identical
  - raw: diff foo/file1 bar/file2
    failed_when: this.rc == 0

  # Report a change when rc is not 2
  - shell: /usr/bin/billybass --mode="take me to the river"
    changed_when: this.rc != 2

  # Wait until file exists
  - stat:
      path: /tmp/some.file
    until: this.stat.exists

  # Wait until request works
  - uri:
      url: https://foo.bar/api/v1/cluster-manager/initial-install
      method: POST
      body:
        { "foo": "bar" }
      status_code: [ 202 ]
    until: this is successful
    retries: 10
    delay: 30
    delegate_to: localhost

This avoids having to needlessly register it to a specific (and unique) variable name, makes it very obvious where the values are coming from (without having to check what it was registered to).

It also makes the YAML file more readable with lines like: until: this is successful

Note: In case the user has registered a variable this already, the implementation will warn the user that it is overriding a reserved variable, but it will not touch it, so existing uses of this (if any) will keep on working. The warning allows the user to fix those cases.

Testing (optional)

We could extend the existing tests to also test this.

Documentation (optional)

A few examples could be added as part of the failed_when, changed_when and loop section.

[bcoca]

it is also interesting for controlling loop progression:

when: not (this.results[-1]|failed or this.results[-1]|skipped)

[dagwieers]

So first implementation is done: https://github.com/ansible/ansible/pull/30957. Please review.

@bcoca: make that:

    when: this.results|last is successful

[amenonsen]

I like both the proposal and the implementation (which I just applied locally and tried out for a while).

[amenonsen]

I wonder if 'result' wouldn't be a better name than 'this', but I don't feel strongly about it. I just think failed_when: result.rc != 42 scans more easily.

[jhawkesworth]

I think 'this' is preferable to result as 'result' is sometimes used in examples for registered variables, and 'results' is used when registering loop results ( see http://docs.ansible.com/ansible/latest/playbooks_variables.html#registered-variables ) so it would be better to have a name this is distinct from 'result'.

I recall someone proposing something similar in the past and it was declined on the grounds of being too 'programmery' i.e. that it would reduce playbook readability for non programmers. If I recall though the suggestion then was to use something like '$_' which is definitely 'programmer magic' and bad for readability.

I would like a better word than 'this' that communicated the nicely-limited scope of such a variable, but cannot think of a better English word.

[rcarrillocruz]

+1 to this proposal, it striked when I started using Ansible why I had to manually register variables, it seems natural you want to do things per task outcome.

For reference, what @jhawkesworth refers about automatic 'results' on loops is described on http://docs.ansible.com/ansible/latest/playbooks_loops.html#using-register-with-a-loop .

I think precisely because 'result' is used in examples, it would be better to stick to that, for easier transition on already existing playbooks (I bet most variables used in the wild use 'result' with register) and it's more 'ansibly'. 'this' makes me remember Java :/ .

So, 'result' for a single task outcome as singular and 'results' as multiple tasks outcome as plural fits better IMHO.

[dagwieers]

I am not in favor of naming it result, for more than one reason:

• Chances of conflicts with existing playbooks is going to be very real and going to cause issues
• It does not clearly indicate the local scope
• Also result is often used for the previous task result, which is going to lead to confusion
• We are not going to add a second results variable for loops since it is already available from the task result and adds confusion. So I prefer this.results over result.results, keep it simple

I don't mind a better name, but result is not that better name.

[bcoca]

I like the direction, just one caveat. We made a rule that any new 'ansible variables' need to be ansible_ prefixed.

[amenonsen]

I think it would be better to not have the feature at all than to have it be named ansible_this.

[kustodian]

I agree with @amenonsen. this sounds so natural and easy, while ansible_this just feels wrong.

[maxamillion]

+1 to the change.

I think this is a good choice, if someone wanted to make it more "specific" to ansible somehow, or even a playbook/taskset I think something like this_task would be a good candidate.

[caphrim007]

+1 this would be useful in test situations (in my use case writing integration tests) so that I could eliminate all the register: result'ing I do to assert changes.

[sivel]

Copying my opinion here. I am -1 on the feature as a whole. We try to do less magic, and be more explicit. This is a magical feature in rough alignment with _ perl, and I prefer being more explicit.

To re-iterate an argument from bcoca, this actually makes reading a playbook less obvious, and potentially makes the lives of playbook linters harder.

[nitzmahone]

^-- if we end up following that logic, this arguably should be considered in loop as well (ie, making the user explicitly name the loop var instead of item)... Though I land on the side of this is a good thing, we should probably be consistent in the design of a new language feature, whichever way we go.

[thaumos]

@sivel, just to expand on what you mean for those who don't understand. You are suggesting that if a user wants to do anything with a returned result, they should register a variable and have a follow up task to use that. Correct?

[abadger]

I'm a strong -1 to anything that's not namespaced. it will break existing playbooks and be a trip hazard for new playbooks. So "this" earns a strong -1 from me.

bcoca proposed in the meeting using "_" as a namespace and I'm okay with that. So the variable could be "_this". Dunders were also proposed. I could also be okay with that. (__this__).

thaumos brought up that being too "programmery" is a bad thing. I somewhat agree with that but I think that if that's the case, then "this" is also programmery (and programmery with a programming language that isn't natural to ansible).... The only proposal that satisfies the namespace argument and the non-programmery aspect that I can think of is ansible_results. (or perhaps better: ansible_task_results). I suppose this could be a case where we ask whether it's more important to be readable ("ansible_task_results") or writable ("_this"). I think that the compromises in between these two extremes have all of the drawbacks of both sides without the benefits.

[tima]

strong +1 to this -- I think its a great idea.

-1 to _this -- that is a programming idiom bleeding thru where it shouldn't be. Ansible positions its playbook as human readable/meaningful that don't require programming skills. Programming idioms like _ should not be bleeding into there. I used to be a perl programmer and hated that crap. I also hated _meta in the Ansible inventory. I'm hoping that dies as inventory plugins become the norm.

We need to do more magic and do more convention over configuration. That is what customers want -- to make their lives easier getting stuff done and not prescribing to someone's sense of code purity.

[sivel]

@sivel, just to expand on what you mean for those who don't understand. You are suggesting that if a user wants to do anything with a returned result, they should register a variable and have a follow up task to use that. Correct?

Yes, I am in favor of the current behavior. If you want to do anything with the returned result, you should explicitly register a variable.

Do less magic, be more explicit. I will always land on the side of being explicit.

[samdoran]

+1 to the feature and using this as the var name.

I understand the desire for less magic, however this feature change removes some tedium and friction for the playbook author. That's a good thing.

I also see that _this would be far less problematic a name, but that forces a programming convention on playbook authors, and no other "magic" or built-in variable we have has an underscore prefix. I think that would be taking the style of Ansible in the wrong direction.

[thaumos]

+1 to this.

I echo everything that @samdoran has just eloquently said. Less code lines == less friction.

[abadger]

I'm strongly enough against "this" that I'll do what I can to veto it (do what I can because we don't actually have a veto power...)

[bcoca]

-1 to this as it breaks actual playbooks i know of.

If we implement this feature (i'm 1/2 of a mind with @sivel that explicit > magic for auditability) it has to be namespaced, i proposed _this as a compromise, but in the end the 'correct' one would be ansible_task_result as @abadger notes as this follows existing convention, not only in naming but as how we expose many 'magic vars' to the users.

@bcoca grants @abadger veto power on this issue (not that i have that power either)

[samdoran]

To do anything beyond the task, you have to register. This this is very short-lived — just a tiny bit of magic.

But if this is problematic, then ansible_task_result is the most logical name. Less elegant, but also less problematic.

[dagwieers]

-1 to this as it breaks actual playbooks i know of.

Obviously I prefer what I wrote, but the concern that this is already registered is moot. The implementation takes care of this situation, and we can make it more explicit. If we consider it a reserved keyword, its use will become deprecated over time. Currently, if it is defined, we won't touch it so it doesn't affect the end-user.

[bcoca]

it does not solve it, it annoys the user that is already using it and prevents them from using the new feature

[thaumos]

I am fine with ansible_task_result as well.

[dagwieers]

Well, if you don't want to annoy users, you shouldn't be rewriting loops either :-) It's a fraction of people who use this (if any), we don't have to overstate the problem.

[bcoca]

@dagwieers that is actually trying to remove annoyance and confusion, if you think we accomplish the opposite, we can revisit it

[dagwieers]

@bcoca: Same for this proposal, for 99.99% of the users there's no impact. And you don't have to use it if you don't want to.

[maxamillion]

+1 to @abadger in respect to namespaces. I think ansible_task_result is a good option, it's clear and concise. A Playbook reader will have a good hint at what that variable means by looking at it even if they have no prior knowledge of the feature. I do like this because it's short and a relatively well known idiom for programmers but I do agree with the arguments that it's venturing too much into the programmer world and could be off-putting or confusing to some.

[tima]

Right @dagwieers. I mean how far are we supposed to impeded progress in making automation with ansible easier and more frictionless for the overall user community because of what some seemingly small number of users may or may not have done in their playbooks? There are probably some users out there that used ansible_results and ansible_task_results out there already.

[bcoca]

@tima the ansible_ namespace is reserved and we made the rule to make all new 'magic vars' use it, we are even moving from old vars play_hosts to ansible_play_hosts to keep this pattern, anyone using ansible_ in their vars should already be aware of their special nature. this and other names are not in the same category.

[dagwieers]

@bcoca I think the ansible_ namespace is an unfortunate decision, because it makes everything a lot more verbose for no good reason. ansible_facts is an unfortunate result, I think a reserved dictionary facts (or another idiom/term) is preferable. Since that will become the namespace for all facts in Ansible there's no need to add ansible_ in front.

Granted it is unfortunate that we want to redesign things in Ansible this late in the game, but making things less convenient and intuitive because there may be a conflict with what people are using, is a problem in itself standing in the way of nice design. Harming all future users for a limited set of existing users.

If we were designing a new language nobody would be considering ansible_task_results, but we're blinded by concerns from the past to make a good judgment for the future IMO.

(Consider the python language that namespaces all reserved keywords with python_ shiver)

[abadger]

While I mention "ansible_task_results" as being the logical conclusion of "non-programmery" it has one fatal flaw... it takes more typing to type that then "register: this". So it's not going to be a usability improvement. ("ansible_result" breaks even. "_result" could also be a compromise between namespaced and less programmery than "this")

[maxamillion]

Ultimately I like the ansible_ namespacing of things, I know it's more verbose and we all hate typing but it's removes the guess work of "where did that variable come from?" when reading a playbook you didn't write for at least the specific classification of variable that is "ansible built-in" but I'm open to other opinions on the topic.

[dagwieers]

@maxamillion I am not comparing with or without ansible_, but adding ansible_ to everything compared to more sensible reserved dictionaries as namespaces. Names that don't need ansible_.

When we decided to namespace facts with ansible_ it was for different reasons (and the convenience you're referring to). Today we have better ways to avoid conflicting names, there's no need anymore to prepend things with ansible_ to make stuff 'unique".

[bcoca]

@dagwieers I disagree on the account we still get users confused and frustrated with the few 'non namespaced' variables we have out there (inventory_file, playhosts, etc). the reason behind `ansible` prefixing was to avoid user confusion and frustration, which it has done pretty good job with as the count of queries of that nature have almost disappeared.

[maxamillion]

@dagwieers In the current line of discussion I'm not concerned about uniqueness from the perspective of ansible, I'm concerned about it from the perspective of an user. In the case of the user, I like the explicit "things that start with ansible_ come from ansible" which as an universal rule (in my opinion anyways) keeps the learning curve low and maintains a level of consistency that aids in over all user experience.

[dagwieers]

@maxamillion Would be the same for a reserved facts dictionary, it comes from the system.

And what you say isn't really true, as some ansible_ namespaced variables are actually set by users. And there's no guarantee at all. Which there would be for reserved keywords.

@bcoca: I am not contesting it was a means to an end, but it's not the most elegant or convenient one IMO. If it was properly designed from the start it wouldn't need to be introduced like that. But building on top of a workaround (which I don't contest worked) is not something I like very much. A redesign shouldn't set workarounds into stone, but ought to find new convenient ways. Like you have been doing with loops.

[maxamillion]

@dagwieers users deciding to set ansible_ variables does provide confusion, but if we're going to change a bunch of stuff then why not make that truly reserved and not allow variables set by users to be in that namespace?

The point I was trying to make is that if there is a definition of "all variables that start with ansible_ come from ansible itself" and the held true in practice, it would provide a pleasant consistency. Is it perfect? It is not, but I think it would be a net win.

An alternative, if it is the consensus that having a set of reserved words similar to how a programming language would, is to augment ansible-doc to provide that list upon request for quick reference as well as a listing in the documentation on the website.

I'm not strongly for or against either option, but I generally prefer namespacing and that holds true here as well.

[dagwieers]

Well, I don't like string-based namespaces, that's what we used to do. That's why I am pointing to using a dictionary instead for the future. We already have hostvars and groups, we could have facts for all facts, etc...

I hope nobody is suggesting to start using ansible_hostvars, and ansible_groups now...

Using facts.hostname is so much prettier than ansible_facts.ansible_hostname or even ansible_facts.hostname. If it wasn't for that convention to have everything prefixed with ansible_ things could look prettier. (And so we're getting back on topic with this).

If facts is too generic, we can think of a new name for the facts-backend and introduce new terminology, like facter and ohai actually did. In their respective worlds, everybody would understand facter.hostname or ohai.hostname, even if newcomers have no clue and need to learn.

[maxamillion]

@dagwieers that's fair.

[mgedmin]

Even if the accepted name is ansible_task_result, I could still define an alias this: "{{ ansible_task_result }}" in my role/vars/main.yml and use failed_when: this.rc == 2 etc., because of Ansible's lazy evaluation, right?

[agaffney]

Probably, but why the indirection?

[bcoca]

CLT disease, chronic lazy typer

[agaffney]

Good point. You can define the this variable for indirection once and use it all over the place.

[kustodian]

Or you can make this variable name (or its alias name) configurable in ansible.cfg to make more people happy.

[kustodian]

I'm all for this idea, any default variable will be better than nothing.

Regarding the name, if you are looking from the users' understand-ability perspective ansible_task_result is probably the best choice, but it's long and boring to write. On the other hand I'm not sure this is the best short choice, because if you're note a coder, you probably won't know about it. Maybe a better variable name would be just plain and simple task, e.g.:

  - raw: diff foo/file1 bar/file2
    failed_when: task.rc == 0

  - shell: cat /some/file
    changed_when: task.stdout != ""

  - stat:
      path: /tmp/some.file
    until: task.stat.exists

[agaffney]

Making the variable name configurable in ansible.cfg will probably lead to playbooks/roles that only work on a single machine. If someone wants to change the var, using another var for indirection is a better approach.

[amenonsen]

Making the variable name configurable in ansible.cfg will probably lead to playbooks/roles that only work on a single machine. If someone wants to change the var, using another var for indirection is a better approach.

I agree, I see no need for configurability beyond @mgedmin's suggestion.