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

set_extra_var module to set a global variable from play #20849

Closed
wants to merge 13 commits into from

Conversation

PieterVoet
Copy link

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

set_extra_var

ANSIBLE VERSION
ansible 2.3.0
  config file = /home/pieter/PycharmProjects/myansible/ansible.cfg
  configured module search path = ['/home/pieter/ansible/modules']

SUMMARY

Sets an extra variable from a playbook. If the 'extra variable' is already been set from
the command line, it will NOT be overruled by the module.

Example inventory :

group_all = { host_1, host_2, host_3 }
group_one = { host_1 }
group_two = { host_2, host3 }

Example playbook 'my_playbook.yml' :

hosts: group_one
tasks:

name: list /tmp
shell: ls /tmp
register: out

name: set extra variable
set_extra_var: name=myvar value="{{out}}"

hosts: group_two
tasks:
debug: msg="{{myvar}}"

Example command

ansible-playbook my_playbook.yml --limit group_all

To prevent breaking playbooks, I only allow setting an 'extra var' from the playbook if it doesn't already exists ! That should be safe enough to not break playbooks.

Next, this plugin specificly can be used when dealing with dynamic variables, i.e. variables that are discovered runtime, and hence cannot be set upfront using files or vars.
In addition to that, my sample playbook illustrates a playbook where there's no way to expose the variable to other hosts in the play, since a first play discovers the variable value for a different subset of hosts than the variable is used at. Therefore also 'set_fact' cannot be used to deal with this.
For me, I really required this in order to do proper playbook orchestration.


@jhawkesworth
Copy link
Contributor

I am not sure I see the need for this.
It adds another place in which vars can be defined and another level of variable precedence but I think the same results can be acheived without it.

You can refer to facts gathered about other hosts as long as you have previously gathered facts from those hosts using hostvars - see this part of the FAQ http://docs.ansible.com/ansible/faq.html#how-do-i-loop-over-a-list-of-hosts-in-a-group-inside-of-a-template

if necessary you can set_fact against localhost and then refer to that in later plays as well.

@PieterVoet
Copy link
Author

Hi jhawkesworth,

'set_fact' will not help in this case. The testcase contains two plays with different host groups. 'set_fact' sets a variable on a hostbase. You can probably use the playhosts array to look up the host that contains the variable, but you need to know the hostname for that. If using groups, that hostname is not known. So I wonder if you can get my example playbook ( with the example inventory ) to run using 'set_fact' module.

Thanks !

@ansibot
Copy link
Contributor

ansibot commented Jan 31, 2017

@bennojoy @gregswift @dagwieers @tbielawa

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

@ansibot ansibot added affects_2.3 This issue/PR affects Ansible v2.3 c:plugins/action c:plugins/strategy community_review In order to be merged, this PR must follow the community review workflow. feature_pull_request module This issue/PR relates to a module. needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. needs_triage Needs a first human triage before being processed. new_module This PR includes a new module. labels Jan 31, 2017
@dagwieers
Copy link
Contributor

If I read it correctly you need to set an "extra variable" that is system-wide (i.e. covers all systems at once, just like the --extra-vars arguments). Why don't you use play variables here ?

Personally, I would prefer to extend this kind of functionality to e.g. set variables to the host/group level, and in your case one would only need to set the variable for group "all". I think that offers everything people would desire. The only problem I can think of is that if you make the action conditional, it will be non-deterministic whether it runs (which is currently the case anyhow for run_once or specific action plugins anyhow, see #19966).

@PieterVoet
Copy link
Author

PieterVoet commented Jan 31, 2017

Hi Dag,

  • Why don't you use play variables here ?

in case the variable holds a value that is determined at runtime, i.e. during the run on the remote host
that is in 'group_one'. The testcase probably oversimplified this by running 'ls /tmp' for sample runtime data to assign as a value for the variable.
Same thing for variables at host/group level. It's value is not known upfront.

Thanks !

@dagwieers
Copy link
Contributor

@PieterVoet I would like to see a real-life example of what you are trying to do. Because in the case of the output for ls /tmp. The output is different for different targets, so how would your current module handle this ? Which output (for which target) is intended to end up in the variable you specified to set_extra_var ?

hosts: host1:host2:host3
tasks:
- shell: ls /tmp
  register: ls

- set_extra_var:
    name: foobar
    value: '{{ ls.stdout }}'

- debug:
    var: foobar

In the above case, would foobar hold the output from host1, host2 or host3 ?

@dagwieers
Copy link
Contributor

dagwieers commented Jan 31, 2017

Why don't you simply do this:

- hosts: host1
  tasks:
  - shell: ls /tmp
    register: ls

- hosts: group_all
  vars:
    foobar: '{{ hostvars.host1.ls.stdout }}'
  tasks:
  - debug:
      var: foobar

@PieterVoet
Copy link
Author

I gotta go now, but will get back to this tomorrow. In your example you use 'host1' for the first play. The difference with my scenario is that I use a group instead of a host. So I don't know that the remote host for 'group_one' is ' host1', and hence I cannot look it up in the hostvars array.

@bcoca
Copy link
Member

bcoca commented Jan 31, 2017

you can check any host in the group, this example uses the first, but you can select any rnadom one:

- hosts: hostgroup1
  tasks:
  - shell: ls /tmp
    register: ls

- hosts: group_all
  vars:
    foobar: '{{ hostvars[groups['hostgroup1'][0]].ls.stdout }}'
  tasks:
  - debug:
      var: foobar

@dagwieers
Copy link
Contributor

@PieterVoet The reason I used host1 in the example is because as I mentioned before, if you target multiple systems it becomes less clear which output needs to be used. You have only one target in group_one so there is no issue there. But in the general case, you want to use the output of a single specific target. Not one undefined possible target out of many (that would not make any sense). But @bcoca already answered you that it is possible to also reference it via a hostgroup. Again, makes even less sense to me ;-)

Still I would like to understand what you are trying to accomplish, because I think there may be a more elegant solution than this if we know what you are doing specifically.

@ryansb ryansb removed the needs_triage Needs a first human triage before being processed. label Jan 31, 2017
@mattclay
Copy link
Member

Closing and re-opening to trigger CI.

@mattclay mattclay closed this Jan 31, 2017
@mattclay mattclay reopened this Jan 31, 2017
@ansibot ansibot removed the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. label Jan 31, 2017
@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Feb 6, 2017
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Feb 7, 2017
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 7, 2017
@PieterVoet
Copy link
Author

I was wondering what will happen with this pull request ? Is there something I should do ? Thanks !

@gundalow
Copy link
Contributor

@PieterVoet Looks like there is still some discussion to be had, which can be continued here, or in the Core IRC meeting today ansible/community#156

@gundalow
Copy link
Contributor

If you are available on IRC at 1500 UTC today (Thursday) please add it to the agenda ansible/community#156 and we can discuss there.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 11, 2017
@ansibot ansibot added the support:community This issue/PR relates to code supported by the Ansible community. label Jun 29, 2017
@PieterVoet
Copy link
Author

I was wondering what will happen with this pull request ? Is there something I should do ? Thanks !

@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. new_module This PR includes a new module. labels Jul 10, 2017
@mattclay
Copy link
Member

@PieterVoet Given the amount of discussion on this PR already, I recommend bringing it up in the Public Core Team Meeting on IRC. The schedule is here:

https://github.com/ansible/community/tree/master/meetings

If you're able to attend, please add it to the agenda:

ansible/community#162

If you can't attend, discussion can continue here. However, it will likely be much slower.

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. new_module This PR includes a new module. labels Jul 25, 2017
@abadger abadger changed the title set_extra_var module to set extra variable from play set_extra_var module to set a global variable from play Aug 11, 2017
@abadger
Copy link
Contributor

abadger commented Aug 11, 2017

Changed the title because we wouldn't want to allow overriding extra_vars from playbooks but a global var is a potential feature.

@jctanner
Copy link
Contributor

Hi!

As of April of 2016, we have started using the Ansible Proposal process for large feature ideas or changes in current functionality, such as this. If you are still interested in seeing this new feature get into Ansible, please submit a proposal for it using this process.

https://github.com/ansible/proposals/blob/master/proposals_process_proposal.md

If you have any further questions, please let us know by stopping by our devel mailing list, or our devel IRC channel:

Thank you!

@jctanner jctanner closed this Aug 11, 2017
@abadger
Copy link
Contributor

abadger commented Aug 11, 2017

Reasoning for why this should be a proposal is that although a global variable store would be nice to have, there's many open questions both on how it should be deigned and on what is required to implement it. Some of the things that a proposal should touch on are:

  • precedence, where in the variable precedence order should global vars fall?
  • What are some of the pitfalls that can be encountered?
    • This needs to bypass the host loop so we don't have a race with multiple hosts trying to set it
    • Perhaps a short list of alternatives (the one that people seem to use presently is setting the variable on localhost).

@PieterVoet
Copy link
Author

PieterVoet commented Aug 12, 2017 via email

@abadger
Copy link
Contributor

abadger commented Aug 12, 2017

Yep, for that reason I changed the title.

@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 4, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.3 This issue/PR affects Ansible v2.3 c:plugins/action c:plugins/strategy community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. new_module This PR includes a new module. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet