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.

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).

lib/ansible/plugins/strategy/__init__.py

@@ -1033,8 +1033,9 @@ class StrategyBase:
             # 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)

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

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

test/integration/targets/roles/roles/recover/tasks/main.yml

@@ -0,0 +1 @@
+- meta: clear_host_errors

test/integration/targets/roles/roles/set_var/tasks/main.yml

@@ -0,0 +1,2 @@
+- set_fact:
+    role_set_var: true

test/integration/targets/roles/roles/test_connectivity/tasks/main.yml

@@ -0,0 +1,2 @@
+- ping:
+    data: 'reachable'

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" ]