Fixing bugs related to nested blocks inside roles

* Make sure dep chains are checked recursively for nested blocks
* Fixing iterator is_failed() check to make sure we're not in a
  rescue block before returning True
* Use is_failed() to test whether a host should be added to the TQM
  failed_hosts list
* Use is_failed() when compiling the list of hosts left to iterate
  over in both the linear and free strategies

Fixes #14222

6 files changed, 46 insertions, 28 deletions

diff --git a/lib/ansible/executor/play_iterator.py b/lib/ansible/executor/play_iterator.py
index 8c6fc29493..33bf94e5c2 100644
--- a/lib/ansible/executor/play_iterator.py
+++ b/lib/ansible/executor/play_iterator.py
@@ -198,7 +198,7 @@ class PlayIterator:
         task = None
         if s.run_state == self.ITERATING_COMPLETE:
             display.debug("host %s is done iterating, returning" % host.name)
-            return (None, None)
+            return (s, None)
 
         old_s = s
         (s, task) = self._get_next_task_from_state(s, host=host, peek=peek)
@@ -207,14 +207,14 @@ class PlayIterator:
             if ra != rb:
                 return True
             else:
-                return old_s.cur_dep_chain != task._block._dep_chain
+                return old_s.cur_dep_chain != task._block.get_dep_chain()
 
         if task and task._role:
             # if we had a current role, mark that role as completed
             if s.cur_role and _roles_are_different(task._role, s.cur_role) and host.name in s.cur_role._had_task_run and not peek:
                 s.cur_role._completed[host.name] = True
             s.cur_role = task._role
-            s.cur_dep_chain = task._block._dep_chain
+            s.cur_dep_chain = task._block.get_dep_chain()
 
         if not peek:
             self._host_states[host.name] = s
@@ -409,7 +409,11 @@ class PlayIterator:
         if state is None:
             return False
         elif state.run_state == self.ITERATING_TASKS and self._check_failed_state(state.tasks_child_state):
-            return True
+            cur_block = self._blocks[state.cur_block]
+            if len(cur_block.rescue) > 0 and state.fail_state & self.FAILED_RESCUE == 0:
+                return False
+            else:
+                return True
         elif state.run_state == self.ITERATING_RESCUE and self._check_failed_state(state.rescue_child_state):
             return True
         elif state.run_state == self.ITERATING_ALWAYS and self._check_failed_state(state.always_child_state):
diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py
index 095e6b338d..b16316ab2e 100644
--- a/lib/ansible/playbook/block.py
+++ b/lib/ansible/playbook/block.py
@@ -46,6 +46,7 @@ class Block(Base, Become, Conditional, Taggable):
         self._role         = role
         self._task_include = None
         self._parent_block = None
+        self._dep_chain    = None
         self._use_handlers = use_handlers
         self._implicit     = implicit
 
@@ -54,11 +55,6 @@ class Block(Base, Become, Conditional, Taggable):
         elif parent_block:
             self._parent_block = parent_block
 
-        if parent_block:
-            self._dep_chain = parent_block._dep_chain[:]
-        else:
-            self._dep_chain = []
-
         super(Block, self).__init__()
 
     def get_vars(self):
@@ -153,6 +149,15 @@ class Block(Base, Become, Conditional, Taggable):
         except AssertionError:
             raise AnsibleParserError("A malformed block was encountered.", obj=self._ds)
 
+    def get_dep_chain(self):
+        if self._dep_chain is None:
+            if self._parent_block:
+                return self._parent_block.get_dep_chain()
+            else:
+                return None
+        else:
+            return self._dep_chain[:]
+
     def copy(self, exclude_parent=False, exclude_tasks=False):
         def _dupe_task_list(task_list, new_block):
             new_task_list = []
@@ -169,7 +174,9 @@ 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._dep_chain    = self._dep_chain[:]
+
+        if self._dep_chain:
+            new_me._dep_chain = self._dep_chain[:]
 
         if not exclude_tasks:
             new_me.block  = _dupe_task_list(self.block or [], new_me)
@@ -201,7 +208,7 @@ class Block(Base, Become, Conditional, Taggable):
             if attr not in ('block', 'rescue', 'always'):
                 data[attr] = getattr(self, attr)
 
-        data['dep_chain'] = self._dep_chain
+        data['dep_chain'] = self.get_dep_chain()
 
         if self._role is not None:
             data['role'] = self._role.serialize()
@@ -226,7 +233,7 @@ class Block(Base, Become, Conditional, Taggable):
             if attr in data and attr not in ('block', 'rescue', 'always'):
                 setattr(self, attr, data.get(attr))
 
-        self._dep_chain = data.get('dep_chain', [])
+        self._dep_chain = data.get('dep_chain', None)
 
         # if there was a serialized role, unpack it too
         role_data = data.get('role')
@@ -247,10 +254,12 @@ class Block(Base, Become, Conditional, Taggable):
             pb = Block()
             pb.deserialize(pb_data)
             self._parent_block = pb
+            self._dep_chain = self._parent_block.get_dep_chain()
 
     def evaluate_conditional(self, templar, all_vars):
-        if len(self._dep_chain):
-            for dep in self._dep_chain:
+        dep_chain = self.get_dep_chain()
+        if dep_chain:
+            for dep in dep_chain:
                 if not dep.evaluate_conditional(templar, all_vars):
                     return False
         if self._task_include is not None:
@@ -274,8 +283,10 @@ class Block(Base, Become, Conditional, Taggable):
         if self._task_include:
             self._task_include.set_loader(loader)
 
-        for dep in self._dep_chain:
-            dep.set_loader(loader)
+        dep_chain = self.get_dep_chain()
+        if dep_chain:
+            for dep in dep_chain:
+                dep.set_loader(loader)
 
     def _get_parent_attribute(self, attr, extend=False):
         '''
@@ -305,10 +316,10 @@ class Block(Base, Become, Conditional, Taggable):
                 else:
                     value = parent_value
 
-                if len(self._dep_chain) and (value is None or extend):
-                    reverse_dep_chain = self._dep_chain[:]
-                    reverse_dep_chain.reverse()
-                    for dep in reverse_dep_chain:
+                dep_chain = self.get_dep_chain()
+                if dep_chain and (value is None or extend):
+                    dep_chain.reverse()
+                    for dep in dep_chain:
                         dep_value = getattr(dep, attr, None)
                         if extend:
                             value = self._extend_value(value, dep_value)
diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py
index f192ea6c94..9b406ae7ba 100644
--- a/lib/ansible/playbook/role/__init__.py
+++ b/lib/ansible/playbook/role/__init__.py
@@ -262,10 +262,11 @@ class Role(Base, Become, Conditional, Taggable):
     def get_inherited_vars(self, dep_chain=[], include_params=True):
         inherited_vars = dict()
 
-        for parent in dep_chain:
-            inherited_vars = combine_vars(inherited_vars, parent._role_vars)
-            if include_params:
-                inherited_vars = combine_vars(inherited_vars, parent._role_params)
+        if dep_chain:
+            for parent in dep_chain:
+                inherited_vars = combine_vars(inherited_vars, parent._role_vars)
+                if include_params:
+                    inherited_vars = combine_vars(inherited_vars, parent._role_params)
         return inherited_vars
 
     def get_role_params(self):
diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index 4b098ddce6..52f1e588cc 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -210,8 +210,10 @@ class StrategyBase:
                                 [iterator.mark_host_failed(h) for h in self._inventory.get_hosts(iterator._play.hosts) if h.name not in self._tqm._unreachable_hosts]
                             else:
                                 iterator.mark_host_failed(host)
-                            (state, tmp_task) = iterator.get_next_task_for_host(host, peek=True)
-                            if not state or state.run_state != PlayIterator.ITERATING_RESCUE:
+
+                            # only add the host to the failed list officially if it has
+                            # been failed by the iterator
+                            if iterator.is_failed(host):
                                 self._tqm._failed_hosts[host.name] = True
                                 self._tqm._stats.increment('failures', host.name)
                         else:
diff --git a/lib/ansible/plugins/strategy/free.py b/lib/ansible/plugins/strategy/free.py
index da123ce3b7..a63ebdf6a6 100644
--- a/lib/ansible/plugins/strategy/free.py
+++ b/lib/ansible/plugins/strategy/free.py
@@ -56,7 +56,7 @@ class StrategyModule(StrategyBase):
         work_to_do = True
         while work_to_do and not self._tqm._terminated:
 
-            hosts_left = [host for host in self._inventory.get_hosts(iterator._play.hosts) if host.name not in self._tqm._unreachable_hosts]
+            hosts_left = [host for host in self._inventory.get_hosts(iterator._play.hosts) if host.name not in self._tqm._unreachable_hosts and not iterator.is_failed(host)]
             if len(hosts_left) == 0:
                 self._tqm.send_callback('v2_playbook_on_no_hosts_remaining')
                 result = False
diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py
index 804cfadc77..b47c6ffaf0 100644
--- a/lib/ansible/plugins/strategy/linear.py
+++ b/lib/ansible/plugins/strategy/linear.py
@@ -163,7 +163,7 @@ class StrategyModule(StrategyBase):
 
             try:
                 display.debug("getting the remaining hosts for this loop")
-                hosts_left = [host for host in self._inventory.get_hosts(iterator._play.hosts) if host.name not in self._tqm._unreachable_hosts]
+                hosts_left = [host for host in self._inventory.get_hosts(iterator._play.hosts) if host.name not in self._tqm._unreachable_hosts and not iterator.is_failed(host)]
                 display.debug("done getting the remaining hosts for this loop")
 
                 # queue up this task for each host in the inventory