From 8034651cd2626e0d634b2b52eeafc81852d8110d Mon Sep 17 00:00:00 2001 From: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> Date: Fri, 8 Sep 2023 12:11:48 -0400 Subject: [PATCH] Only mark a role as complete once a task in it executes for the target host (#81565) * If all tasks in the role are skipped or unreachable, the role is not marked as complete for the host. * Only mark the role as complete if a task in the role succeeds or fails for the host. --- .../role-deduplication-condition.yml | 2 + lib/ansible/plugins/strategy/__init__.py | 5 +- .../targets/roles/role_complete.yml | 47 +++++++++++++++++++ .../roles/roles/failed_when/tasks/main.yml | 4 ++ .../roles/roles/recover/tasks/main.yml | 1 + .../roles/roles/set_var/tasks/main.yml | 2 + .../roles/test_connectivity/tasks/main.yml | 2 + test/integration/targets/roles/runme.sh | 6 +++ 8 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 changelogs/fragments/role-deduplication-condition.yml create mode 100644 test/integration/targets/roles/role_complete.yml create mode 100644 test/integration/targets/roles/roles/failed_when/tasks/main.yml create mode 100644 test/integration/targets/roles/roles/recover/tasks/main.yml create mode 100644 test/integration/targets/roles/roles/set_var/tasks/main.yml create mode 100644 test/integration/targets/roles/roles/test_connectivity/tasks/main.yml diff --git a/changelogs/fragments/role-deduplication-condition.yml b/changelogs/fragments/role-deduplication-condition.yml new file mode 100644 index 00000000000000..365644e057866d --- /dev/null +++ b/changelogs/fragments/role-deduplication-condition.yml @@ -0,0 +1,2 @@ +bugfixes: + - role deduplication - don't deduplicate before a role has had a task run for that particular host (https://github.com/ansible/ansible/issues/81486). diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index fd16624bcf7796..eda0e0af62e381 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -1033,8 +1033,9 @@ def _evaluate_conditional(h): # How would this work with allow_duplicates?? if task.implicit: role_obj = self._get_cached_role(task, iterator._play) - role_obj._completed[target_host.name] = True - msg = 'role_complete for %s' % target_host.name + if target_host.name in role_obj._had_task_run: + role_obj._completed[target_host.name] = True + msg = 'role_complete for %s' % target_host.name elif meta_action == 'reset_connection': all_vars = self._variable_manager.get_vars(play=iterator._play, host=target_host, task=task, _hosts=self._hosts_cache, _hosts_all=self._hosts_cache_all) diff --git a/test/integration/targets/roles/role_complete.yml b/test/integration/targets/roles/role_complete.yml new file mode 100644 index 00000000000000..86cae772ad61cc --- /dev/null +++ b/test/integration/targets/roles/role_complete.yml @@ -0,0 +1,47 @@ +- name: test deduping allows for 1 successful execution of role after it is skipped + hosts: testhost + gather_facts: false + tags: [ 'conditional_skipped' ] + roles: + # Skipped the first time it executes + - role: a + when: role_set_var is defined + + - role: set_var + + # No longer skipped + - role: a + when: role_set_var is defined + # Deduplicated with the previous success + - role: a + when: role_set_var is defined + +- name: test deduping allows for successful execution of role after host is unreachable + hosts: fake,testhost + gather_facts: false + tags: [ 'unreachable' ] + ignore_unreachable: yes + roles: + # unreachable by the first host + - role: test_connectivity + + # unreachable host will try again, + # the successful host will not because it's deduplicated + - role: test_connectivity + +- name: test deduping role for failed host + hosts: testhost,localhost + gather_facts: false + tags: [ 'conditional_failed' ] + ignore_errors: yes + roles: + # Uses run_once to fail on the first host the first time it executes + - role: failed_when + + - role: set_var + - role: recover + + # Deduplicated after the failure, ONLY runs for localhost + - role: failed_when + # Deduplicated with the previous success + - role: failed_when diff --git a/test/integration/targets/roles/roles/failed_when/tasks/main.yml b/test/integration/targets/roles/roles/failed_when/tasks/main.yml new file mode 100644 index 00000000000000..6ca4d8cf4fd7cf --- /dev/null +++ b/test/integration/targets/roles/roles/failed_when/tasks/main.yml @@ -0,0 +1,4 @@ +- debug: + msg: "{{ role_set_var is undefined | ternary('failed_when task failed', 'failed_when task succeeded') }}" + failed_when: role_set_var is undefined + run_once: true diff --git a/test/integration/targets/roles/roles/recover/tasks/main.yml b/test/integration/targets/roles/roles/recover/tasks/main.yml new file mode 100644 index 00000000000000..72ea3ac1e55698 --- /dev/null +++ b/test/integration/targets/roles/roles/recover/tasks/main.yml @@ -0,0 +1 @@ +- meta: clear_host_errors diff --git a/test/integration/targets/roles/roles/set_var/tasks/main.yml b/test/integration/targets/roles/roles/set_var/tasks/main.yml new file mode 100644 index 00000000000000..45f83eb02bb062 --- /dev/null +++ b/test/integration/targets/roles/roles/set_var/tasks/main.yml @@ -0,0 +1,2 @@ +- set_fact: + role_set_var: true diff --git a/test/integration/targets/roles/roles/test_connectivity/tasks/main.yml b/test/integration/targets/roles/roles/test_connectivity/tasks/main.yml new file mode 100644 index 00000000000000..22fac6edf6e90c --- /dev/null +++ b/test/integration/targets/roles/roles/test_connectivity/tasks/main.yml @@ -0,0 +1,2 @@ +- ping: + data: 'reachable' diff --git a/test/integration/targets/roles/runme.sh b/test/integration/targets/roles/runme.sh index f6902d6344154c..fcaa91421d125e 100755 --- a/test/integration/targets/roles/runme.sh +++ b/test/integration/targets/roles/runme.sh @@ -10,6 +10,12 @@ set -eux # but still dupe across plays [ "$(ansible-playbook no_dupes.yml -i ../../inventory "$@" | grep -c '"msg": "A"')" = "3" ] +# and don't dedupe before the role successfully completes +[ "$(ansible-playbook role_complete.yml -i ../../inventory -i fake, --tags conditional_skipped "$@" | grep -c '"msg": "A"')" = "1" ] +[ "$(ansible-playbook role_complete.yml -i ../../inventory -i fake, --tags conditional_failed "$@" | grep -c '"msg": "failed_when task succeeded"')" = "1" ] +[ "$(ansible-playbook role_complete.yml -i ../../inventory -i fake, --tags unreachable "$@" | grep -c '"data": "reachable"')" = "1" ] +ansible-playbook role_complete.yml -i ../../inventory -i fake, --tags unreachable "$@" | grep -e 'ignored=2' + # include/import can execute another instance of role [ "$(ansible-playbook allowed_dupes.yml -i ../../inventory --tags importrole "$@" | grep -c '"msg": "A"')" = "2" ] [ "$(ansible-playbook allowed_dupes.yml -i ../../inventory --tags includerole "$@" | grep -c '"msg": "A"')" = "2" ]