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

Restoring commit 1ed7ac6a00fc66fd51285808a64f80a65575ec4e (do not override variables if host is skipped) #8062

Closed
wants to merge 1 commit into from

Conversation

adinapoli
Copy link

Hello guys,
this commit, of almost two years ago, fixed a problem when a skipped task was erroneously (in my opinion) overriding registered variables when skipping a task. This is the PR:

#1616

The PR got merged but there is no trace of its code neither inside playbook/init.py nor inside the commit list , tracking the history of the file. This PR reintroduces it.

Without this PR, this simple playbook was failing:


---
- hosts: localhost
  vars:
    my_val: 10
  remote_user: alfredo
  tasks:
  - name: Print my_val
    shell: echo {{ my_val }}
    register: my_val2

  - name: Print my_val
    shell: echo {{ my_val2.stdout }}

  - name: Skip
    shell: echo {{ my_val2.stdout }}
    register: my_val2
    when: False

  - name: Re-print
    shell: echo {{ my_val2.stdout }}

With:

TASK: [Re-print] **************************************************************
fatal: [localhost] => One or more undefined variables: 'dict object' has no attribute 'stdout'

FATAL: all hosts have already failed -- aborting

With this PR, now it works as expected:

TASK: [Skip] ******************************************************************
skipping: [localhost]

TASK: [Re-print] **************************************************************
changed: [localhost] => {"changed": true, "cmd": "echo 10 ", "delta": "0:00:00.013029", "end": "2014-07-08 11:53:58.444579", "rc": 0, "start": "2014-07-08 11:53:58.431550", "stderr": "", "stdout": "10"}

@jimi-c
Copy link
Member

jimi-c commented Jul 8, 2014

Registering upon a skip is important, since you may want to know if a previous task had been skipped, which is actually something we use in our integration tests now. Due to the fact that many people may be using register this way, we can't change direction on this. Our best recommendation is to use a different variable name there, to prevent stomping on it, or to use when: my_val2.stdout is defined on tasks that may use a registered variable from a skipped task. If you have any further questions regarding this, please let us know. Thanks!

@jimi-c jimi-c closed this Jul 8, 2014
@adinapoli
Copy link
Author

My problem with the current behaviour is that is basically goes in the opposite direction of what code reuse might be. If I define a task and such a task is skipped, I would expect that it doesn't mangle my already-registered variable, or that at least the is a configuration flag to turn this on and off. In my
provisioning scripts I have a three mutually exclusive tasks, each of which will create a new EC2 instance according to some external condition (therefore the "when" clause). Being mutually exclusive, I still expect one of those to run and to yield a valid, registered variable I can reuse in my script. I don't want to create three variables, because that won't make things dry, since the outcome of these three tasks is the same, and by information hiding the rest of the script shouldn't be concerned about how I do generate the data within my variable.

Is what I'm asking doable with a custom module or with handlers? Can you please point me out to any resource I can learn from? Otherwise I struggle to see how I can achieve DRY code without turning my provisioning script into a bit mud of tangled code.

@jimi-c
Copy link
Member

jimi-c commented Jul 9, 2014

I would recommend breaking those 3 tasks out into separate tasks files, which are included with your conditional when statements. When the includes are skipped, the tasks inside should not be executed and therefore should not register their variables.

@adinapoli
Copy link
Author

Thanks, I've ended up writing a dead simple module capable to unify my variables into a single one, so I was able to get rid of the redundancy :)

@hostmaster
Copy link
Contributor

It does't work that way. the tasks inside is executed and register it's variables.
It is documented http://docs.ansible.com/playbooks_conditionals.html#applying-when-to-roles-and-includes

@aioue
Copy link
Contributor

aioue commented Sep 12, 2015

There should be a way of least least selectively skipping register. It is backwards compatible:

- shell: echo foo
  register:  name=registered_variable set_when_task_skipped=false
  when: false

(allow register module to accept a skip argument)

@Dmitry1987
Copy link

@aioue is this a real workaround or feature request? (set_when_task_skipped=false)
like, does it actually work to set "set_when_task_skipped=false" ??

@aioue
Copy link
Contributor

aioue commented Aug 15, 2018

@Dmitry1987 it is a suggested "ansible style" enhancement. There is no such argument at present.

@ansible ansible locked and limited conversation to collaborators Aug 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants