corrected no_log for items and skipped tasks

corrected output from default callback
added new tests for no_log loops
updated makefile test to check for both positive and negative occurrences of no_log

5 files changed, 16 insertions, 13 deletions

diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index 6bf5f0c203..f13029d82c 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -277,7 +277,7 @@ class TaskExecutor:
         # variable not being present which would otherwise cause validation to fail
         if not self._task.evaluate_conditional(templar, variables):
             self._display.debug("when evaulation failed, skipping this task")
-            return dict(changed=False, skipped=True, skip_reason='Conditional check failed')
+            return dict(changed=False, skipped=True, skip_reason='Conditional check failed', _ansible_no_log=self._play_context.no_log)
 
         # Now we do final validation on the task, which sets all fields to their final values.
         # In the case of debug tasks, we save any 'var' params and restore them after validating
diff --git a/lib/ansible/plugins/callback/__init__.py b/lib/ansible/plugins/callback/__init__.py
index 4c736999fc..c517a1337a 100644
--- a/lib/ansible/plugins/callback/__init__.py
+++ b/lib/ansible/plugins/callback/__init__.py
@@ -108,15 +108,14 @@ class CallbackBase:
                 ret.append(">> the files are different, but the diff library cannot compare unicode strings\n\n")
 
     def _get_item(self, result):
-        if '_ansible_no_log' in result:
+        if '_ansible_no_log' in result and result['_ansible_no_log']:
             item = "(censored due to no_log)"
         else:
-            item = getattr(result, 'item', "(censored due to no_log)")
+            item = result['item']
 
         return item
 
     def _process_items(self, result):
-
         for res in result._result['results']:
             newres = deepcopy(result)
             res['item'] = self._get_item(res)
diff --git a/lib/ansible/plugins/callback/default.py b/lib/ansible/plugins/callback/default.py
index 8a66f9ba62..e5f8ed12cc 100644
--- a/lib/ansible/plugins/callback/default.py
+++ b/lib/ansible/plugins/callback/default.py
@@ -88,13 +88,13 @@ class CallbackModule(CallbackBase):
 
     def v2_runner_on_skipped(self, result):
         if C.DISPLAY_SKIPPED_HOSTS:
-            msg = "skipping: [%s]" % result._host.get_name()
             if result._task.loop and 'results' in result._result:
                 self._process_items(result)
             else:
+                msg = "skipping: [%s]" % result._host.get_name()
                 if (self._display.verbosity > 0 or '_ansible_verbose_always' in result._result) and not '_ansible_verbose_override' in result._result:
                     msg += " => %s" % self._dump_results(result._result)
-            self._display.display(msg, color='cyan')
+                self._display.display(msg, color='cyan')
 
     def v2_runner_on_unreachable(self, result):
         if result._task.delegate_to:
diff --git a/test/integration/Makefile b/test/integration/Makefile
index c2ebec6d09..ab3262d01a 100644
--- a/test/integration/Makefile
+++ b/test/integration/Makefile
@@ -192,5 +192,6 @@ test_lookup_paths:
 	ansible-playbook lookup_paths/play.yml -i $(INVENTORY) -v $(TEST_FLAGS)
 
 no_log:
-	[ "$$(ansible-playbook no_log_local.yml -i $(INVENTORY) -vvvvv | grep DO_NOT_LOG)" = "" ]
+	# This test expects 7 loggable vars and 0 non loggable ones, if either mismatches it fails, run the ansible-playbook command to debug
+	[ "$$(ansible-playbook no_log_local.yml -i $(INVENTORY) -vvvvv | awk -e 'BEGIN { logme = 0; nolog = 0; } /LOG_ME/ { logme += 1;} /DO_NOT_LOG/ { nolog += 1;} END { printf "%d/%d", logme, nolog; }')" = "7/0" ]
 
diff --git a/test/integration/no_log_local.yml b/test/integration/no_log_local.yml
index 7cfec32713..d68dc03a4c 100644
--- a/test/integration/no_log_local.yml
+++ b/test/integration/no_log_local.yml
@@ -16,10 +16,11 @@
       failed_when: true
       ignore_errors: true
 
-    - name: failed item args should be logged in the absence of no_log
+    - name: item args should be logged in the absence of no_log
       shell: echo {{ item }}
-      with_items: [ "LOG_ME_ITEM_FAILED" ]
-      failed_when: true
+      with_items: [ "LOG_ME_ITEM", "LOG_ME_SKIPPED", "LOG_ME_ITEM_FAILED" ]
+      when: item != "LOG_ME_SKIPPED"
+      failed_when: item == "LOG_ME_ITEM_FAILED"
       ignore_errors: true
 
     - name: args should not be logged when task-level no_log set
@@ -49,11 +50,13 @@
       no_log: true
       when: false
 
-    - name: skipped item args should be suppressed with no_log
+    - name: items args should be suppressed with no_log in every state
       shell: echo {{ item }}
       no_log: true
-      with_items: [ "DO_NOT_LOG_ITEM_SKIPPED", "DO_NOT_LOG_ITEM_SKIPPED_2" ]
-      when: item == False
+      with_items: [ "DO_NOT_LOG_ITEM", "DO_NOT_LOG_ITEM_SKIPPED", "DO_NOT_LOG_ITEM_FAILED" ]
+      when: item != "DO_NOT_LOG_ITEM_SKIPPED"
+      failed_when: item == "DO_NOT_LOG_ITEM_FAILED"
+      ignore_errors: yes
 
     - name: async task args should suppressed with no_log
       async: 10