-
Notifications
You must be signed in to change notification settings - Fork 23.9k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Allow loading dirs from role defaults/vars #36357
Conversation
d3776ab
to
1515aa9
Compare
c3b541b
to
7efca52
Compare
e4eb6b1
to
d4aabda
Compare
94e363f
to
06dcc77
Compare
d090018
to
2fdaafc
Compare
d80f63c
to
a696cdd
Compare
7e44e00
to
c8c1b0e
Compare
Yes, that should probably be |
I'm having a hard time getting ansible to even hit that line of code. It should only get there on a role subdir such as |
I'm not familiar with ansible internals, but this seemed to do the trick: @patch('ansible.playbook.role.definition.unfrackpath', mock_unfrackpath_noop)
def test_load_role_with_tasks_dir_vs_file(self):
fake_loader = DictDataLoader({
"/etc/ansible/roles/foo_tasks/tasks/custom_main/foo.yml": """
- command: bar
""",
"/etc/ansible/roles/foo_tasks/tasks/custom_main.yml": """
- command: baz
""",
})
mock_play = MagicMock()
mock_play.ROLE_CACHE = {}
i = RoleInclude.load('foo_tasks', play=mock_play, loader=fake_loader)
r = Role.load(i, play=mock_play, from_files=dict(tasks='custom_main'))
self.assertEqual(r._task_blocks[0]._ds[0]['command'], 'baz') |
Yeah, that seems to trigger the bug (or something that could be attributed to it). I did something very similar, but I wasn't using |
Also add tests around that code path
Also add tests around that code path
Also add tests around that code path
Also add tests around that code path
(cherry picked from commit 2e62e36)
(cherry picked from commit 2e62e36)
This commit moves code to look for vars files/dirs to a common place and uses it for loading role defaults/vars. This allows things such as 'defaults/main' or 'vars/main' being a directory in a role, allowing splitting of defaults/vars into multiple files. This commit also fixes the role loading unit tests for py3 when bytestrings are used for paths instead of utf8 strings. This fixes ansible#14248 and ansible#11639.
It doesn't really work as i would expect it to work (why does 3rd scenario not work?). I tested different scenarios and this is what i found:
If this is intended, i am happy to update Playbooks Best Practices. |
SUMMARY
This commit moves code to look for vars files/dirs to a common place and uses it for loading role defaults/vars. This allows things such as
defaults/main
orvars/main
being a directory in a role, allowing splitting of defaults/vars into multiple files. This commit also fixes the role loading unit tests for py3 when bytestrings are used for paths instead of utf8 strings.This fixes #14248 and #11639.
ISSUE TYPE
COMPONENT NAME
vars loading
ANSIBLE VERSION
N/A
ADDITIONAL INFORMATION
N/A