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

Fix name parameter templating in include_role module #36372

Merged
merged 1 commit into from
Mar 20, 2018

Conversation

fullyint
Copy link
Contributor

SUMMARY

Followup to #33386 and #21285.

Fixes problem that include_role looping over with_items and name: "{{ item }}" will use the same name for all items in the loop.

In the current code, an IncludedFile() object built using the original_task will have its inc_file._task bound to the original_task. The iterative reassignment of original_task._role_name during with_items loops leaves all returned included_files with the same inc_file._task._role_name. All have the final name from the with_items list.

This PR builds IncludedFile() objects from an original_task.copy() to avoid the problematic binding.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

include_role module

ANSIBLE VERSION
ansible 2.6.0 (devel a5654bd63c) last updated 2018/02/18 20:37:44 (GMT -600)
  config file = None
  configured module search path = [u'/Users/philip/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/philip/projects/ansible/ansible/lib/ansible
  executable location = /Users/philip/projects/ansible/ansible/bin/ansible
  python version = 2.7.13 (default, Mar 13 2017, 12:42:50) [GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)]
ADDITIONAL INFORMATION

To reproduce, create a test playbook with a looping include_role:

# include-role-test.yml
- gather_facts: false
  hosts: localhost
  tasks:
    - include_role:
        name: "{{ role }}"
      with_items:
        - role1
        - role2
      loop_control:
        loop_var: role

Create role1 and role2:

# roles/role1/tasks/main.yml
- debug:
    msg: this is role 1
# roles/role2/tasks/main.yml
- debug:
    msg: this is role 2

Output from current devel branch

The include_role task runs role2 both times, despite with_items: ['role1', 'role2'].

$ ansible-playbook -i hosts include-role-test.yml

PLAY [localhost] ***************************************************************************

TASK [include_role] ************************************************************************

TASK [role2 : debug] ***********************************************************************
ok: [localhost] => {
    "failed": false,
    "msg": "this is role 2"
}

TASK [role2 : debug] ***********************************************************************
ok: [localhost] => {
    "failed": false,
    "msg": "this is role 2"
}

PLAY RECAP *******************************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0

Output after applying this PR

$ ansible-playbook -i hosts include-role-test.yml

PLAY [localhost] ***************************************************************************

TASK [include_role] ************************************************************************

TASK [role1 : debug] ***********************************************************************
ok: [localhost] => {
    "failed": false,
    "msg": "this is role 1"
}

TASK [role2 : debug] ***********************************************************************
ok: [localhost] => {
    "failed": false,
    "msg": "this is role 2"
}

PLAY RECAP *******************************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0

An IncludedFile() object built using the original_task will have
its _task bound to the original_task. The iterative reassignment of
original_task._role_name during with_item loops leaves all returned
included_files with the same ._task._role_name (the final name from
the with_items list). This commit builds IncludedFile() objects
from an original_task.copy() to avoid the problematic binding.
@ansibot ansibot added bugfix_pull_request 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 Feb 19, 2018
@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Feb 19, 2018
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Feb 27, 2018
@Im0
Copy link
Contributor

Im0 commented Mar 4, 2018

Thanks for logging this @fullyint , I'm hoping to use include_role: name: {{ loopedVar }} with with_items/with_dict but running into this exact problem (along with the other referenced issues). Hoping this can be merged!

You tested against 2.6, hopefully we can see this in 2.5 as well!

Only work around I can think of at the moment is to write out to a temporary playbook file and execute it.. seems messy. :(

Im0 pushed a commit to Im0/ansible that referenced this pull request Mar 5, 2018
Im0 pushed a commit to Im0/ansible that referenced this pull request Mar 5, 2018
Im0 pushed a commit to Im0/ansible that referenced this pull request Mar 5, 2018
@sivel sivel self-assigned this Mar 9, 2018
@sivel
Copy link
Member

sivel commented Mar 20, 2018

rebuild_merge

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Mar 20, 2018
@ansibot ansibot merged commit 54e70fc into ansible:devel Mar 20, 2018
@fullyint fullyint deleted the include_role_with_items_in_name branch March 20, 2018 18:18
sivel pushed a commit to sivel/ansible that referenced this pull request Mar 20, 2018
An IncludedFile() object built using the original_task will have
its _task bound to the original_task. The iterative reassignment of
original_task._role_name during with_item loops leaves all returned
included_files with the same ._task._role_name (the final name from
the with_items list). This commit builds IncludedFile() objects
from an original_task.copy() to avoid the problematic binding.

(cherry picked from commit 54e70fc)
sivel pushed a commit to sivel/ansible that referenced this pull request Mar 20, 2018
sivel added a commit to sivel/ansible that referenced this pull request Mar 20, 2018
@sivel sivel mentioned this pull request Mar 20, 2018
sivel pushed a commit that referenced this pull request Mar 20, 2018
sivel added a commit that referenced this pull request Mar 28, 2018
* Fix name parameter templating in include_role module (#36372)

An IncludedFile() object built using the original_task will have
its _task bound to the original_task. The iterative reassignment of
original_task._role_name during with_item loops leaves all returned
included_files with the same ._task._role_name (the final name from
the with_items list). This commit builds IncludedFile() objects
from an original_task.copy() to avoid the problematic binding.

(cherry picked from commit 54e70fc)

* Test include role with items in name #36372 (#37001)

* Tests for #36372

* Tests for #36372

* Tests for #36372

(cherry picked from commit 8c4f349)

* Add changelog for #36372
@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug. 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

4 participants