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

import_role vars persist across plays #46824

Closed
torched99 opened this issue Oct 11, 2018 · 8 comments · Fixed by #46833
Closed

import_role vars persist across plays #46824

torched99 opened this issue Oct 11, 2018 · 8 comments · Fixed by #46833
Labels
affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@torched99
Copy link

torched99 commented Oct 11, 2018

SUMMARY

Please confirm the ansible 2.7 scope change for import_role vars. It now behaves differently to previous versions in that for example role/vars/main.yml (or role/defaults/main.yml) definition live across plays (unlike roles: keyword inclusions, and meta dependency based inclusions, and also include_role with public: no).

The presence of 'roles:' keyword in either play makes behaviour revert to 2.6 form.
Also being a static var definition (pre-processor) it also works in reverse, where the import can exist in a play later in sequence, such that at earlier sequenced plays will resolve the variable to the value defined by the later play's role.

As far as I can tell old 2.6 behaviour allowed for variables to have a scope across plays in a playbook only via:

static:
global config
inventory
host and local facts
extra vars

dynamic:
local_facts task
include_vars task

So this introduces a role level cross play static variable capability. Apologies if my terminology is confusing but I find the ansible use of the word 'static' confusing myself (as it more akin to a pre-processor scope, that generally behaves with a programmatic dynamic scope across role/block/task within a play).

ISSUE TYPE
  • Bug Report
COMPONENT NAME

import_role

ANSIBLE VERSION
2.7
CONFIGURATION
OS / ENVIRONMENT

Oracle Linux 6

STEPS TO REPRODUCE

Try the following playbook on 2.6 and 2.7, compare diff.

- name: testvars precedence
  hosts: localhost
  # role 'rolesetvar' has only a vars/main.yml setting testvar:  'roles/rolesetvar/vars/main.yml'
  # Uncomment just the roles: keyword alone and behaviour reverts to 2.6.x form
  # roles:
  #  - rolesetvar
  tasks:
  - include_role:
      name: rolesetvar
     #    public: no
  - debug:
      var: testvar
  - import_role:
      name: rolesetvar
  - debug:
      var: testvar
    #- set_fact:
    #    testvar: "play task set_fact"
  - debug:
      var: testvar
  - include_role:
      name: rolesetvar
      public: yes
  - name: conditional debug task
    debug:
      var: testvar
    when: testvar == 'roles/rolesetvar/vars/main.yml'
  vars:
    testvar: "play vars:"

- name: testvars play2
  hosts: localhost
  tasks:
  - debug:
      var: testvar

EXPECTED RESULTS

With version 2.6.x the second play would show variable undefined for testvar.
Expected? I have no idea what to expect any more with the super complex scope rules ansible has (and that keep subtly changing). So please clarify either way, and ideally describe any future plans to change.

ACTUAL RESULTS

Extract of last section of stdout:

PLAY [testvars play2] **********************************************************

TASK [Gathering Facts] *********************************************************
ok: [localhost]

TASK [debug] *******************************************************************
ok: [localhost] => {
    "testvar": "roles/rolesetvar/vars/main.yml"
}

@sivel
Copy link
Member

sivel commented Oct 11, 2018

This is indeed expected and is documented at https://docs.ansible.com/ansible/latest/porting_guides/porting_guide_2.7.html#include-role-and-import-role-variable-exposure

If you have further questions please stop by IRC or the mailing list:

@sivel sivel closed this as completed Oct 11, 2018
@torched99
Copy link
Author

Perhaps you have not read my issue correctly. If the behaviour is intended by this statement:

"import_role does not support the public argument, and will unconditionally expose the role’s defaults and vars to the rest of the playbook. This functionality brings import_role into closer alignment with roles listed within the roles header in a play."

then:-

  1. the statement is wrongly worded as import_role now behave VERY differently to 'roles listed within the roles header in a play', and
  2. the change is badly implemented as it does not behave that way if the roles: keyword exists (even with empty list of roles), so the result is really very unexpected behaviour.
  3. the docs page on variable precedence is now getting woefully out of whack with the implementation behaviour.

@sivel sivel changed the title Playbook's import_role role: {vars,defaults}/main.yml scope in 2.7 import_role vars persist across plays Oct 11, 2018
@sivel
Copy link
Member

sivel commented Oct 11, 2018

I will admit that I read your issue multiple times. However I struggled to see a problem outside of the documentation and implementation.

However, it seems I was wrong.

I've updated the title to better reflect your problem, in that vars from import_role seem to be persisting across plays, and more specifically, only when no roles are listed under the roles header within a play.

An example that misbehaves:

- hosts: localhost
  gather_facts: false
  tasks:
    - import_role:
        name: foo

    - debug:
        var: thing

- hosts: localhost
  gather_facts: false
  tasks:
    - debug:
        var: thing

This should produce the following in the 2nd play, but it does not.

ok: [localhost] => {
    "thing": "VARIABLE IS NOT DEFINED!: 'thing' is undefined"
}

When any other role is listed under roles:, then it behaves correctly.

@sivel sivel reopened this Oct 11, 2018
@torched99
Copy link
Author

torched99 commented Oct 11, 2018

Yes. I have no idea what is expected/correct though, only what it did before. The porting guide clearly says expose to the rest of playbook (not rest of play) - which is very new behaviour.

@sivel
Copy link
Member

sivel commented Oct 11, 2018

Just as an update here, the problem is that roles is defined in Play with a mutable default value.

As such, if roles is not listed in the play, then Play.roles isn't overridden per play, and instead the value is mutated across all plays.

This is a potentially big problem for the future, so I'm going to look into a solution to address that root cause, instead of a one off for just roles.

@torched99
Copy link
Author

torched99 commented Oct 11, 2018

I was thinking this scoping issue in general was likely to hit big problems if not well defined. I only came across this particular issue while trying to improve my own docs.

What would be most useful is to know what the ansible team want the scoping to work in the long term.

For your reference I am pasting the current incarnation of my attempt to document it - if only for you to see that it is seems more complex than the docs suggest, at least to me.

AnsibleVarsPrecedence.pdf

@torched99 torched99 reopened this Oct 11, 2018
@torched99
Copy link
Author

Sorry, hit the wrong button :-)

@ansibot
Copy link
Contributor

ansibot commented Oct 11, 2018

Hi @torched99, thank you for submitting this issue!

click here for bot help

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Oct 11, 2018
sivel added a commit to sivel/ansible that referenced this issue Oct 11, 2018
@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Oct 12, 2018
StephenSorriaux pushed a commit to StephenSorriaux/ansible that referenced this issue Oct 12, 2018
sivel added a commit to sivel/ansible that referenced this issue Oct 12, 2018
…llow supplying a callable for defaults of mutable types. Fixes ansible#46824 (ansible#46833).

(cherry picked from commit a06a5de)

Co-authored-by: Matt Martz <matt@sivel.net>
abadger pushed a commit that referenced this issue Oct 12, 2018
…llow supplying a callable for defaults of mutable types. Fixes #46824 (#46833).

(cherry picked from commit a06a5de)

Co-authored-by: Matt Martz <matt@sivel.net>
Tomorrow9 pushed a commit to Tomorrow9/ansible that referenced this issue Dec 4, 2018
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants