From b3d8cdde5d51958ee7b285bcbf5a4a13e0fc7654 Mon Sep 17 00:00:00 2001 From: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> Date: Thu, 28 Mar 2024 15:33:09 -0400 Subject: [PATCH] fix handling allow_duplicates with the role cache (#82691) 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 --- changelogs/fragments/fix-allow-duplicates.yml | 2 ++ lib/ansible/playbook/role/__init__.py | 2 +- lib/ansible/plugins/strategy/free.py | 2 +- lib/ansible/plugins/strategy/linear.py | 2 +- .../targets/include_import/runme.sh | 4 +++ .../tasks/test_dynamic_allow_dup.yml | 30 +++++++++++++++++++ 6 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/fix-allow-duplicates.yml create mode 100644 test/integration/targets/include_import/tasks/test_dynamic_allow_dup.yml diff --git a/changelogs/fragments/fix-allow-duplicates.yml b/changelogs/fragments/fix-allow-duplicates.yml new file mode 100644 index 00000000000..fb0c8171fc1 --- /dev/null +++ b/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. diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index 9bf28cd26b8..1c82e5335c4 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -584,7 +584,7 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable): 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): ''' diff --git a/lib/ansible/plugins/strategy/free.py b/lib/ansible/plugins/strategy/free.py index fda3a0dd633..6f33a68920b 100644 --- a/lib/ansible/plugins/strategy/free.py +++ b/lib/ansible/plugins/strategy/free.py @@ -175,7 +175,7 @@ class StrategyModule(StrategyBase): # 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 diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py index 71d6380211f..29f94c4699b 100644 --- a/lib/ansible/plugins/strategy/linear.py +++ b/lib/ansible/plugins/strategy/linear.py @@ -170,7 +170,7 @@ class StrategyModule(StrategyBase): # 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 diff --git a/test/integration/targets/include_import/runme.sh b/test/integration/targets/include_import/runme.sh index 98bc8c31612..12ee15b1476 100755 --- a/test/integration/targets/include_import/runme.sh +++ b/test/integration/targets/include_import/runme.sh @@ -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 diff --git a/test/integration/targets/include_import/tasks/test_dynamic_allow_dup.yml b/test/integration/targets/include_import/tasks/test_dynamic_allow_dup.yml new file mode 100644 index 00000000000..82e08b339a2 --- /dev/null +++ b/test/integration/targets/include_import/tasks/test_dynamic_allow_dup.yml @@ -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