PlayIterator: skip tasks from already ran roles (#83793)

... so strategies do not have to filter them.

6 files changed, 23 insertions, 29 deletions

diff --git a/changelogs/fragments/skip-role-task-iterator.yml b/changelogs/fragments/skip-role-task-iterator.yml
new file mode 100644
index 0000000000..1cf6b4cbb8
--- /dev/null
+++ b/changelogs/fragments/skip-role-task-iterator.yml
@@ -0,0 +1,2 @@
+minor_changes:
+  - PlayIterator - do not return tasks from already executed roles so specific strategy plugins do not have to do the filtering of such tasks themselves
diff --git a/lib/ansible/executor/play_iterator.py b/lib/ansible/executor/play_iterator.py
index 83dd5d89c3..1ef7451095 100644
--- a/lib/ansible/executor/play_iterator.py
+++ b/lib/ansible/executor/play_iterator.py
@@ -440,7 +440,7 @@ class PlayIterator:
                     else:
                         state.cur_handlers_task += 1
                         if task.is_host_notified(host):
-                            break
+                            return state, task
 
             elif state.run_state == IteratingStates.COMPLETE:
                 return (state, None)
@@ -463,9 +463,15 @@ class PlayIterator:
                         and all(not h.notified_hosts for h in self.handlers)
                     )
                 ):
-                    continue
-
-                break
+                    display.debug("No handler notifications for %s, skipping." % host.name)
+                elif (
+                    (role := task._role)
+                    and role._metadata.allow_duplicates is False
+                    and host.name in self._play._get_cached_role(role)._completed
+                ):
+                    display.debug("'%s' skipped because role has already run" % task)
+                else:
+                    break
 
         return (state, task)
 
diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py
index 877af1e0d5..aea7ba5a80 100644
--- a/lib/ansible/playbook/play.py
+++ b/lib/ansible/playbook/play.py
@@ -19,7 +19,7 @@ from __future__ import annotations
 
 from ansible import constants as C
 from ansible import context
-from ansible.errors import AnsibleParserError, AnsibleAssertionError
+from ansible.errors import AnsibleParserError, AnsibleAssertionError, AnsibleError
 from ansible.module_utils.common.text.converters import to_native
 from ansible.module_utils.common.collections import is_sequence
 from ansible.module_utils.six import binary_type, string_types, text_type
@@ -100,6 +100,15 @@ class Play(Base, Taggable, CollectionSearch):
     def __repr__(self):
         return self.get_name()
 
+    def _get_cached_role(self, role):
+        role_path = role.get_role_path()
+        role_cache = self.role_cache[role_path]
+        try:
+            idx = role_cache.index(role)
+            return role_cache[idx]
+        except ValueError:
+            raise AnsibleError(f'Cannot locate {role.get_name()} in role cache')
+
     def _validate_hosts(self, attribute, name, value):
         # Only validate 'hosts' if a value was passed in to original data set.
         if 'hosts' in self._ds:
diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index ce4ae89744..c2a3e8a618 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -1102,13 +1102,7 @@ class StrategyBase:
         return [res]
 
     def _get_cached_role(self, task, play):
-        role_path = task._role.get_role_path()
-        role_cache = play.role_cache[role_path]
-        try:
-            idx = role_cache.index(task._role)
-            return role_cache[idx]
-        except ValueError:
-            raise AnsibleError(f'Cannot locate {task._role.get_name()} in role cache')
+        return play._get_cached_role(task._role)
 
     def get_hosts_left(self, iterator):
         ''' returns list of available hosts for this iterator by filtering out unreachables '''
diff --git a/lib/ansible/plugins/strategy/free.py b/lib/ansible/plugins/strategy/free.py
index 04c16024d9..90840dab63 100644
--- a/lib/ansible/plugins/strategy/free.py
+++ b/lib/ansible/plugins/strategy/free.py
@@ -172,15 +172,6 @@ class StrategyModule(StrategyBase):
                                 display.warning("Using run_once with the free strategy is not currently supported. This task will still be "
                                                 "executed for every host in the inventory list.")
 
-                        # check to see if this task should be skipped, due to it being a member of a
-                        # 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 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
-
                         if task.action in C._ACTION_META:
                             if self._host_pinned:
                                 meta_task_dummy_results_count += 1
diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py
index 49cfdd4bf4..c6815a38de 100644
--- a/lib/ansible/plugins/strategy/linear.py
+++ b/lib/ansible/plugins/strategy/linear.py
@@ -130,14 +130,6 @@ class StrategyModule(StrategyBase):
                     run_once = False
                     work_to_do = True
 
-                    # check to see if this task should be skipped, due to it being a member of a
-                    # 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 task._role._metadata.allow_duplicates is False:
-                            display.debug("'%s' skipped because role has already run" % task)
-                            continue
-
                     display.debug("getting variables")
                     task_vars = self._variable_manager.get_vars(play=iterator._play, host=host, task=task,
                                                                 _hosts=self._hosts_cache, _hosts_all=self._hosts_cache_all)