From 94c20c9ac6aaf5707bfa508b37e0ebb9401ad1ed Mon Sep 17 00:00:00 2001
From: James Cammarata <jimi@sngx.net>
Date: Wed, 11 Nov 2015 14:23:08 -0500
Subject: [PATCH] Cleanup failed/changed logic in TaskExecutor

Fixes #12980
---
 lib/ansible/executor/task_executor.py | 78 +++++++++++++--------------
 1 file changed, 36 insertions(+), 42 deletions(-)

diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index 592f652a8b3..e9406609d39 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -373,7 +373,7 @@ class TaskExecutor:
         # make a copy of the job vars here, in case we need to update them
         # with the registered variable value later on when testing conditions
         #vars_copy = variables.copy()
-        vars_copy = variables
+        vars_copy = variables.copy()
 
         display.debug("starting attempt loop")
         result = None
@@ -400,6 +400,22 @@ class TaskExecutor:
                 if self._task.poll > 0:
                     result = self._poll_async_result(result=result, templar=templar)
 
+            # helper methods for use below in evaluating changed/failed_when
+            def _evaluate_changed_when_result(result):
+                if self._task.changed_when is not None:
+                    cond = Conditional(loader=self._loader)
+                    cond.when = [ self._task.changed_when ]
+                    result['changed'] = cond.evaluate_conditional(templar, vars_copy)
+
+            def _evaluate_failed_when_result(result):
+                if self._task.failed_when is not None:
+                    cond = Conditional(loader=self._loader)
+                    cond.when = [ self._task.failed_when ]
+                    failed_when_result = cond.evaluate_conditional(templar, vars_copy)
+                    result['failed_when_result'] = result['failed'] = failed_when_result
+                    return failed_when_result
+                return False
+
             # update the local copy of vars with the registered value, if specified,
             # or any facts which may have been generated by the module execution
             if self._task.register:
@@ -408,54 +424,32 @@ class TaskExecutor:
             if 'ansible_facts' in result:
                 vars_copy.update(result['ansible_facts'])
 
-            # create a conditional object to evaluate task conditions
-            cond = Conditional(loader=self._loader)
+            # set the failed property if the result has a non-zero rc. This will be
+            # overridden below if the failed_when property is set
+            if result.get('rc', 0) != 0:
+                result['failed'] = True
 
-            def _evaluate_changed_when_result(result):
-                if self._task.changed_when is not None:
-                    cond.when = [ self._task.changed_when ]
-                    result['changed'] = cond.evaluate_conditional(templar, vars_copy)
-
-            def _evaluate_failed_when_result(result):
-                if self._task.failed_when is not None:
-                    cond.when = [ self._task.failed_when ]
-                    failed_when_result = cond.evaluate_conditional(templar, vars_copy)
-                    result['failed_when_result'] = result['failed'] = failed_when_result
-                    return failed_when_result
-                return False
-
-            if self._task.until:
-                cond.when = self._task.until
-                if cond.evaluate_conditional(templar, vars_copy):
-                    _evaluate_changed_when_result(result)
-                    _evaluate_failed_when_result(result)
-                    break
-            elif (self._task.changed_when is not None or self._task.failed_when is not None) and 'skipped' not in result:
-                    _evaluate_changed_when_result(result)
-                    if _evaluate_failed_when_result(result):
-                        break
-            elif 'failed' not in result:
-                if result.get('rc', 0) != 0:
-                    result['failed'] = True
-                else:
-                    # if the result is not failed, stop trying
-                    break
-
-            if attempt < retries - 1:
-                time.sleep(delay)
-            else:
+            # if we didn't skip this task, use the helpers to evaluate the changed/
+            # failed_when properties
+            if 'skipped' not in result:
                 _evaluate_changed_when_result(result)
                 _evaluate_failed_when_result(result)
 
+            if attempt < retries - 1:
+                cond = Conditional(loader=self._loader)
+                cond.when = self._task.until
+                if cond.evaluate_conditional(templar, vars_copy):
+                    break
+
+                # no conditional check, or it failed, so sleep for the specified time
+                time.sleep(delay)
+
+            elif 'failed' not in result:
+                break
+
         # do the final update of the local variables here, for both registered
         # values and any facts which may have been created
         if self._task.register:
-            ### FIXME:
-            # If we remove invocation, we should also be removing _ansible*
-            # and maybe ansible_facts.
-            # Remove invocation from registered vars
-            #if 'invocation' in result:
-            #    del result['invocation']
             variables[self._task.register] = result
 
         if 'ansible_facts' in result: