From fa8a1f48bbba4b0d884931272bb93f5ee3fe5a5f 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 (cherry picked from commit b3d8cdde5d51958ee7b285bcbf5a4a13e0fc7654) 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 00000000000000..fb0c8171fc1f9b --- /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 b35519e4ca3d28..83ad7fc4df60ab 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -575,7 +575,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): ''' diff --git a/lib/ansible/plugins/strategy/free.py b/lib/ansible/plugins/strategy/free.py index f3bc3af80409ed..61f3ad4faca80f 100644 --- a/lib/ansible/plugins/strategy/free.py +++ b/lib/ansible/plugins/strategy/free.py @@ -177,7 +177,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 diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py index 9746fcd20b8eae..d3d06dddebfee5 100644 --- a/lib/ansible/plugins/strategy/linear.py +++ b/lib/ansible/plugins/strategy/linear.py @@ -172,7 +172,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 diff --git a/test/integration/targets/include_import/runme.sh b/test/integration/targets/include_import/runme.sh index 078f080b0dbe16..d85b22d900dd5a 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=4' 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 00000000000000..82e08b339a2d49 --- /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