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

Skip self._parent on dynamic, defer grandparent for attr lookup #38827

Merged
merged 3 commits into from
Apr 16, 2018

Conversation

sivel
Copy link
Member

@sivel sivel commented Apr 16, 2018

SUMMARY

Skip self._parent on dynamic, defer grandparent for attr lookup.

This solves the problem where we should look up the tree for attrs on static objects, when constructing attrs for tasks within dynamic.

Examples:

Block:

# delegate_to: localhost should be applied to tasks within the include as the attr is not applied to the include itself
- delegate_to: localhost
  block:
    - include_tasks: include_me.yml

import->include:

- import_tasks: import_me.yml
  delegate_to: localhost
# delegate_to: localhost should apply to tasks within include_tasks
- include_tasks: include_me.yml

This PR also backs out the _inheritable changes from 8e6ebae8bda

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/playbook/block.py
lib/ansible/playbook/task.py

ANSIBLE VERSION
devel
ADDITIONAL INFORMATION

@sivel
Copy link
Member Author

sivel commented Apr 16, 2018

@sivel sivel changed the title [WIP] Skip self._parent on dynamic, defer grandparent for attr lookup Skip self._parent on dynamic, defer grandparent for attr lookup Apr 16, 2018
@sivel
Copy link
Member Author

sivel commented Apr 16, 2018

I ran the new tests against devel, and received:

TASK [assert] ******************************************************************
fatal: [testhost]: FAILED! => {
    "assertion": "block_include_result is skipped",
    "changed": false,
    "evaluated_to": false
}
fatal: [testhost]: FAILED! => {
    "assertion": "import_include_include_result is skipped",
    "changed": false,
    "evaluated_to": false
}

@sivel
Copy link
Member Author

sivel commented Apr 16, 2018

From the reproducer in #36194 I get expected results with this change:

TASK [whoami : shell] ************************************************************************************************************************************************************************************************************************
changed: [localhost] => {..., "stdout": "matt", "stdout_lines": ["matt"]}

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

TASK [whoami : shell] ************************************************************************************************************************************************************************************************************************
changed: [localhost] => {..., "stdout": "matt", "stdout_lines": ["matt"]}

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

TASK [whoami : shell] ************************************************************************************************************************************************************************************************************************
changed: [localhost] => {..., "stdout": "root", "stdout_lines": ["root"]}

@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. 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. test This PR relates to tests. labels Apr 16, 2018
@sivel sivel merged commit 354aa8d into ansible:devel Apr 16, 2018
sivel added a commit to sivel/ansible that referenced this pull request Apr 16, 2018
…nsible#38827)

* Skip self._parent on dynamic, defer to grandparent for attr lookup

* Revert _inheritable

* Add tests for include inheritance from static blocks

Fixes ansible#38037 ansible#36194

(cherry picked from commit 354aa8d)
sivel added a commit to sivel/ansible that referenced this pull request Apr 16, 2018
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Apr 16, 2018
sivel added a commit that referenced this pull request Apr 20, 2018
…38847)

* Skip self._parent on dynamic, defer to grandparent for attr lookup (#38827)

* Skip self._parent on dynamic, defer to grandparent for attr lookup

* Revert _inheritable

* Add tests for include inheritance from static blocks

Fixes #38037 #36194

(cherry picked from commit 354aa8d)

* Add changelog for #38827
ilicmilan pushed a commit to ilicmilan/ansible that referenced this pull request Nov 7, 2018
…nsible#38827)

* Skip self._parent on dynamic, defer to grandparent for attr lookup

* Revert _inheritable

* Add tests for include inheritance from static blocks

Fixes ansible#38037 ansible#36194
@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: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. test This PR relates to tests.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants