Reworking the way end of role detection is donereworking_role_end_detection

Rather than trying to enumerate tasks or track an ever changing cur_role
flag in PlayIterator, this change simply sets a flag on the last block in
the list of blocks returned by Role.compile(). The PlayIterator then checks
for that flag when the cur_block number is incremented, and marks the role
as complete if the given host had any tasks run in that role.

Fixes #20224

3 files changed, 16 insertions, 98 deletions

diff --git a/lib/ansible/executor/play_iterator.py b/lib/ansible/executor/play_iterator.py
index 702dd7a5c7..02052f56b3 100644
--- a/lib/ansible/executor/play_iterator.py
+++ b/lib/ansible/executor/play_iterator.py
@@ -48,8 +48,6 @@ class HostState:
         self.cur_regular_task   = 0
         self.cur_rescue_task    = 0
         self.cur_always_task    = 0
-        self.cur_role           = None
-        self.cur_role_task      = None
         self.cur_dep_chain      = None
         self.run_state          = PlayIterator.ITERATING_SETUP
         self.fail_state         = PlayIterator.FAILED_NONE
@@ -82,12 +80,11 @@ class HostState:
                         ret.append(states[i])
                 return "|".join(ret)
 
-        return "HOST STATE: block=%d, task=%d, rescue=%d, always=%d, role=%s, run_state=%s, fail_state=%s, pending_setup=%s, tasks child state? (%s), rescue child state? (%s), always child state? (%s), did rescue? %s, did start at task? %s" % (
+        return "HOST STATE: block=%d, task=%d, rescue=%d, always=%d, run_state=%s, fail_state=%s, pending_setup=%s, tasks child state? (%s), rescue child state? (%s), always child state? (%s), did rescue? %s, did start at task? %s" % (
             self.cur_block,
             self.cur_regular_task,
             self.cur_rescue_task,
             self.cur_always_task,
-            self.cur_role,
             _run_state_to_string(self.run_state),
             _failed_state_to_string(self.fail_state),
             self.pending_setup,
@@ -104,7 +101,7 @@ class HostState:
 
         for attr in (
             '_blocks', 'cur_block', 'cur_regular_task', 'cur_rescue_task', 'cur_always_task',
-            'cur_role', 'run_state', 'fail_state', 'pending_setup', 'cur_dep_chain',
+            'run_state', 'fail_state', 'pending_setup', 'cur_dep_chain',
             'tasks_child_state', 'rescue_child_state', 'always_child_state'
             ):
             if getattr(self, attr) != getattr(other, attr):
@@ -121,9 +118,6 @@ class HostState:
         new_state.cur_regular_task = self.cur_regular_task
         new_state.cur_rescue_task = self.cur_rescue_task
         new_state.cur_always_task = self.cur_always_task
-        new_state.cur_role = self.cur_role
-        if self.cur_role_task:
-            new_state.cur_role_task = self.cur_role_task[:]
         new_state.run_state = self.run_state
         new_state.fail_state = self.fail_state
         new_state.pending_setup = self.pending_setup
@@ -272,92 +266,6 @@ class PlayIterator:
         old_s = s
         (s, task) = self._get_next_task_from_state(s, host=host, peek=peek)
 
-        def _roles_are_different(ra, rb):
-            if ra != rb:
-                return True
-            else:
-                return old_s.cur_dep_chain != task.get_dep_chain()
-
-        def _role_is_child(r):
-            parent = task._parent
-            while parent:
-                if hasattr(parent, '_role') and parent._role == r and isinstance(parent, IncludeRole):
-                    return True
-                parent = parent._parent
-            return False
-
-        def _get_cur_task(s, depth=0):
-            res = [s.run_state, depth, s.cur_block, -1]
-            if s.run_state == self.ITERATING_TASKS:
-                if s.tasks_child_state:
-                    res[-1] = [s.cur_regular_task, _get_cur_task(s.tasks_child_state, depth=depth+1)]
-                else:
-                    res[-1] = s.cur_regular_task
-            elif s.run_state == self.ITERATING_RESCUE:
-                if s.rescue_child_state:
-                    res[-1] = [s.cur_rescue_task, _get_cur_task(s.rescue_child_state, depth=depth+1)]
-                else:
-                    res[-1] = s.cur_rescue_task
-            elif s.run_state == self.ITERATING_ALWAYS:
-                if s.always_child_state:
-                    res[-1] = [s.cur_always_task, _get_cur_task(s.always_child_state, depth=depth+1)]
-                else:
-                    res[-1] = s.cur_always_task
-            return res
-
-        def _do_task_cmp(a, b):
-            '''
-            Does the heavy lifting for _role_task_cmp() of comparing task state objects
-            returned by _get_cur_task() above.
-            '''
-            res = cmp(a[0], b[0])
-            if res == 0:
-                res = cmp(a[1], b[1])
-                if res == 0:
-                    res = cmp(a[2], b[2])
-                    if res == 0:
-                        # if there were child states, the last value in the list may be
-                        # a list itself representing the current task position plus a new
-                        # list representing the child state. So here we normalize that so
-                        # we can call this method recursively when all else is equal.
-                        if isinstance(a[3], list):
-                            a_i, a_il = a[3]
-                        else:
-                            a_i = a[3]
-                            a_il = [-1, -1, -1, -1]
-                        if isinstance(b[3], list):
-                            b_i, b_il = b[3]
-                        else:
-                            b_i = b[3]
-                            b_il = [-1, -1, -1, -1]
-
-                        res = cmp(a_i, b_i)
-                        if res == 0:
-                            res = _do_task_cmp(a_il, b_il)
-            return res
-
-        def _role_task_cmp(s):
-            '''
-            Compares the given state against the stored state from the previous role task.
-            '''
-            if not s.cur_role_task:
-                return 1
-            cur_task = _get_cur_task(s)
-            return _do_task_cmp(cur_task, s.cur_role_task)
-
-        if task and task._role:
-            # if we had a current role, mark that role as completed
-            if s.cur_role:
-                role_diff  = _roles_are_different(task._role, s.cur_role)
-                role_child = _role_is_child(s.cur_role)
-                tasks_cmp  = _role_task_cmp(s)
-                host_done  = host.name in s.cur_role._had_task_run
-                if (role_diff or (not role_diff and tasks_cmp <= 0)) and host_done and not role_child and not peek:
-                    s.cur_role._completed[host.name] = True
-            s.cur_role = task._role
-            s.cur_role_task = _get_cur_task(s)
-            s.cur_dep_chain = task.get_dep_chain()
-
         if not peek:
             self._host_states[host.name] = s
 
@@ -462,7 +370,6 @@ class PlayIterator:
                         if isinstance(task, Block) or state.tasks_child_state is not None:
                             state.tasks_child_state = HostState(blocks=[task])
                             state.tasks_child_state.run_state = self.ITERATING_TASKS
-                            state.tasks_child_state.cur_role = state.cur_role
                             # since we've created the child state, clear the task
                             # so we can pick up the child state on the next pass
                             task = None
@@ -493,7 +400,6 @@ class PlayIterator:
                         if isinstance(task, Block) or state.rescue_child_state is not None:
                             state.rescue_child_state = HostState(blocks=[task])
                             state.rescue_child_state.run_state = self.ITERATING_TASKS
-                            state.rescue_child_state.cur_role = state.cur_role
                             task = None
                         state.cur_rescue_task += 1
 
@@ -525,12 +431,16 @@ class PlayIterator:
                             state.rescue_child_state = None
                             state.always_child_state = None
                             state.did_rescue = False
+
+                            # we're advancing blocks, so if this was an end-of-role block we
+                            # mark the current role complete
+                            if block._eor and host.name in block._role._had_task_run:
+                                block._role._completed[host.name] = True
                     else:
                         task = block.always[state.cur_always_task]
                         if isinstance(task, Block) or state.always_child_state is not None:
                             state.always_child_state = HostState(blocks=[task])
                             state.always_child_state.run_state = self.ITERATING_TASKS
-                            state.always_child_state.cur_role = state.cur_role
                             task = None
                         state.cur_always_task += 1
 
diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py
index bbb287420b..5b9585238c 100644
--- a/lib/ansible/playbook/block.py
+++ b/lib/ansible/playbook/block.py
@@ -53,6 +53,9 @@ class Block(Base, Become, Conditional, Taggable):
         self._use_handlers = use_handlers
         self._implicit     = implicit
 
+        # end of role flag
+        self._eor = False
+
         if task_include:
             self._parent = task_include
         elif parent_block:
@@ -182,6 +185,7 @@ class Block(Base, Become, Conditional, Taggable):
         new_me = super(Block, self).copy()
         new_me._play         = self._play
         new_me._use_handlers = self._use_handlers
+        new_me._eor          = self._eor
 
         if self._dep_chain is not None:
             new_me._dep_chain = self._dep_chain[:]
@@ -214,6 +218,7 @@ class Block(Base, Become, Conditional, Taggable):
                 data[attr] = getattr(self, attr)
 
         data['dep_chain'] = self.get_dep_chain()
+        data['eor'] = self._eor
 
         if self._role is not None:
             data['role'] = self._role.serialize()
@@ -241,6 +246,7 @@ class Block(Base, Become, Conditional, Taggable):
                 setattr(self, attr, data.get(attr))
 
         self._dep_chain = data.get('dep_chain', None)
+        self._eor = data.get('eor', False)
 
         # if there was a serialized role, unpack it too
         role_data = data.get('role')
diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py
index 2fea450428..ab2a657d62 100644
--- a/lib/ansible/playbook/role/__init__.py
+++ b/lib/ansible/playbook/role/__init__.py
@@ -408,12 +408,14 @@ class Role(Base, Become, Conditional, Taggable):
             dep_blocks = dep.compile(play=play, dep_chain=new_dep_chain)
             block_list.extend(dep_blocks)
 
-        for task_block in self._task_blocks:
+        for idx, task_block in enumerate(self._task_blocks):
             new_task_block = task_block.copy(exclude_parent=True)
             if task_block._parent:
                 new_task_block._parent = task_block._parent.copy()
             new_task_block._dep_chain = new_dep_chain
             new_task_block._play = play
+            if idx == len(self._task_blocks) - 1:
+                new_task_block._eor = True
             block_list.append(new_task_block)
 
         return block_list