From 09c90f7f2f7b193ebdb9c51573b37678f35db192 Mon Sep 17 00:00:00 2001
From: James Cammarata <jimi@sngx.net>
Date: Fri, 29 Apr 2016 08:32:31 -0400
Subject: [PATCH] Fixing bugs in strategies

* Don't filter hosts remaining based on their failed state. Instead rely
  on the PlayIterator to return None/ITERATING_COMPLETE when the host is
  failed.
* In the free strategy, make sure we wait outside the host loop for all
  pending results to be processed.
* Use the internal _set_failed_state() instead of manually setting things
  when a failed child state is hit

Fixes #15623
---
 lib/ansible/executor/play_iterator.py  | 11 +++++------
 lib/ansible/plugins/strategy/free.py   |  5 ++++-
 lib/ansible/plugins/strategy/linear.py |  2 +-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/lib/ansible/executor/play_iterator.py b/lib/ansible/executor/play_iterator.py
index 3ab2e289ac1..9ecda9395cd 100644
--- a/lib/ansible/executor/play_iterator.py
+++ b/lib/ansible/executor/play_iterator.py
@@ -326,8 +326,7 @@ class PlayIterator:
                     if self._check_failed_state(state.tasks_child_state):
                         # failed child state, so clear it and move into the rescue portion
                         state.tasks_child_state = None
-                        state.fail_state |= self.FAILED_TASKS
-                        state.run_state = self.ITERATING_RESCUE
+                        self._set_failed_state(state)
                     else:
                         # get the next task recursively
                         if task is None or state.tasks_child_state.run_state == self.ITERATING_COMPLETE:
@@ -365,8 +364,7 @@ class PlayIterator:
                     (state.rescue_child_state, task) = self._get_next_task_from_state(state.rescue_child_state, host=host, peek=peek)
                     if self._check_failed_state(state.rescue_child_state):
                         state.rescue_child_state = None
-                        state.fail_state |= self.FAILED_RESCUE
-                        state.run_state = self.ITERATING_ALWAYS
+                        self._set_failed_state(state)
                     else:
                         if task is None or state.rescue_child_state.run_state == self.ITERATING_COMPLETE:
                             state.rescue_child_state = None
@@ -396,8 +394,7 @@ class PlayIterator:
                     (state.always_child_state, task) = self._get_next_task_from_state(state.always_child_state, host=host, peek=peek)
                     if self._check_failed_state(state.always_child_state):
                         state.always_child_state = None
-                        state.fail_state |= self.FAILED_ALWAYS
-                        state.run_state = self.ITERATING_COMPLETE
+                        self._set_failed_state(state)
                     else:
                         if task is None or state.always_child_state.run_state == self.ITERATING_COMPLETE:
                             state.always_child_state = None
@@ -466,7 +463,9 @@ class PlayIterator:
 
     def mark_host_failed(self, host):
         s = self.get_host_state(host)
+        display.debug("marking host %s failed, current state: %s" % (host, s))
         s = self._set_failed_state(s)
+        display.debug("^ failed state is now: %s" % s)
         self._host_states[host.name] = s
 
     def get_failed_hosts(self):
diff --git a/lib/ansible/plugins/strategy/free.py b/lib/ansible/plugins/strategy/free.py
index b64b5df53b3..fb0ae7dfcc5 100644
--- a/lib/ansible/plugins/strategy/free.py
+++ b/lib/ansible/plugins/strategy/free.py
@@ -58,7 +58,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 and not iterator.is_failed(host)]
+            hosts_left = [host for host in self._inventory.get_hosts(iterator._play.hosts) if host.name not in self._tqm._unreachable_hosts]
             if len(hosts_left) == 0:
                 self._tqm.send_callback('v2_playbook_on_no_hosts_remaining')
                 result = False
@@ -191,6 +191,9 @@ class StrategyModule(StrategyBase):
             # pause briefly so we don't spin lock
             time.sleep(0.001)
 
+        # collect all the final results
+        results = self._wait_on_pending_results(iterator)
+
         # run the base class run() method, which executes the cleanup function
         # and runs any outstanding handlers which have been triggered
         return super(StrategyModule, self).run(iterator, play_context, result)
diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py
index aa0f67e1dab..9ab302e0da1 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 and not iterator.is_failed(host)]
+                hosts_left = [host for host in self._inventory.get_hosts(iterator._play.hosts) if host.name not in self._tqm._unreachable_hosts]
                 display.debug("done getting the remaining hosts for this loop")
 
                 # queue up this task for each host in the inventory