Skip to content

Commit

Permalink
fix handling allow_duplicates with the role cache (#82691)
Browse files Browse the repository at this point in the history
allow_duplicates is not part of the role uniqueness, so the value on the cached role may not match the current role.

* remove the allow_duplicates check from Role.has_run() which operates on the deduplicated role
* check the current role's allow_duplicates value in the strategy

Co-authored-by: Martin Krizek <martin.krizek@gmail.com>
  • Loading branch information
s-hertel and mkrizek committed Mar 28, 2024
1 parent 8704b9f commit b3d8cdd
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 3 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/fix-allow-duplicates.yml
@@ -0,0 +1,2 @@
bugfixes:
- allow_duplicates - fix evaluating if the current role allows duplicates instead of using the initial value from the duplicate's cached role.
2 changes: 1 addition & 1 deletion lib/ansible/playbook/role/__init__.py
Expand Up @@ -584,7 +584,7 @@ def has_run(self, host):
at least one task was run
'''

return host.name in self._completed and not self._metadata.allow_duplicates
return host.name in self._completed

def compile(self, play, dep_chain=None):
'''
Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/plugins/strategy/free.py
Expand Up @@ -175,7 +175,7 @@ def run(self, iterator, play_context):
# role which has already run (and whether that role allows duplicate execution)
if not isinstance(task, Handler) and task._role:
role_obj = self._get_cached_role(task, iterator._play)
if role_obj.has_run(host) and role_obj._metadata.allow_duplicates is False:
if role_obj.has_run(host) and task._role._metadata.allow_duplicates is False:
display.debug("'%s' skipped because role has already run" % task, host=host_name)
del self._blocked_hosts[host_name]
continue
Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/plugins/strategy/linear.py
Expand Up @@ -170,7 +170,7 @@ def run(self, iterator, play_context):
# role which has already run (and whether that role allows duplicate execution)
if not isinstance(task, Handler) and task._role:
role_obj = self._get_cached_role(task, iterator._play)
if role_obj.has_run(host) and role_obj._metadata.allow_duplicates is False:
if role_obj.has_run(host) and task._role._metadata.allow_duplicates is False:
display.debug("'%s' skipped because role has already run" % task)
continue

Expand Down
4 changes: 4 additions & 0 deletions test/integration/targets/include_import/runme.sh
Expand Up @@ -121,6 +121,10 @@ ansible-playbook valid_include_keywords/playbook.yml "$@"
ansible-playbook tasks/test_allow_single_role_dup.yml 2>&1 | tee test_allow_single_role_dup.out
test "$(grep -c 'ok=3' test_allow_single_role_dup.out)" = 1

# Test allow_duplicate with include_role and import_role
test "$(ansible-playbook tasks/test_dynamic_allow_dup.yml --tags include | grep -c 'Tasks file inside role')" = 2
test "$(ansible-playbook tasks/test_dynamic_allow_dup.yml --tags import | grep -c 'Tasks file inside role')" = 2

# test templating public, allow_duplicates, and rolespec_validate
ansible-playbook tasks/test_templating_IncludeRole_FA.yml 2>&1 | tee IncludeRole_FA_template.out
test "$(grep -c 'ok=6' IncludeRole_FA_template.out)" = 1
Expand Down
@@ -0,0 +1,30 @@
---
- name: test for allow_duplicates with include_role
hosts: localhost
gather_facts: false
tags:
- include
tasks:
- include_role:
name: dup_allowed_role
allow_duplicates: false
- include_role:
name: dup_allowed_role
- include_role:
name: dup_allowed_role
allow_duplicates: false

- name: test for allow_duplicates with import_role
hosts: localhost
gather_facts: false
tags:
- import
tasks:
- import_role:
name: dup_allowed_role
allow_duplicates: false
- import_role:
name: dup_allowed_role
- import_role:
name: dup_allowed_role
allow_duplicates: false

0 comments on commit b3d8cdd

Please sign in to comment.