From 4753c70c55ef27297d4795ffa33a333a762cfddc Mon Sep 17 00:00:00 2001
From: Elvira Garcia Ruiz <elviragr@riseup.net>
Date: Sun, 26 Apr 2020 02:19:25 +0200
Subject: [PATCH] library: Separate debug and info logs from warnings

Logs are now separed by severity level. Warnings and failures are the only logs
that appear now on the output. All logs are saved into a new json parameter
called "stderr" that is later shown on a different task. In case of
failure, all logs are shown as output. Tests have been created and modified in
order to assure that this feature works.

Signed-off-by: Elvira Garcia Ruiz <elviragr@riseup.net>
---
 library/network_connections.py                | 34 ++++++++++++-------
 tasks/main.yml                                |  4 +++
 tests/playbooks/tests_ethernet.yml            |  2 ++
 tests/playbooks/tests_reapply.yml             |  2 +-
 ...sert_output_in_stderr_without_warnings.yml | 12 +++++++
 tests/tests_default.yml                       |  6 ++++
 6 files changed, 46 insertions(+), 14 deletions(-)
 create mode 100644 tests/tasks/assert_output_in_stderr_without_warnings.yml

diff --git a/library/network_connections.py b/library/network_connections.py
index a72f346..cd64abb 100644
--- a/library/network_connections.py
+++ b/library/network_connections.py
@@ -1559,24 +1559,32 @@ class RunEnvironmentAnsible(RunEnvironment):
                 c["persistent_state"],
             )
             prefix = prefix + (", '%s'" % (c["name"]))
-        for r in rr["log"]:
-            yield (r[2], "[%03d] %s %s: %s" % (r[2], LogLevel.fmt(r[0]), prefix, r[1]))
-
-    def _complete_kwargs(self, connections, kwargs, traceback_msg=None):
-        if "warnings" in kwargs:
-            logs = list(kwargs["warnings"])
-        else:
-            logs = []
+        for severity, msg, idx in rr["log"]:
+            yield (
+                idx,
+                "[%03d] %s %s: %s" % (idx, LogLevel.fmt(severity), prefix, msg),
+                severity,
+            )
 
+    def _complete_kwargs(self, connections, kwargs, traceback_msg=None, fail=False):
+        warning_logs = kwargs.get("warnings", [])
+        debug_logs = []
         loglines = []
         for res in self._run_results:
             for idx, rr in enumerate(res):
                 loglines.extend(self._complete_kwargs_loglines(rr, connections, idx))
-        loglines.sort(key=lambda x: x[0])
-        logs.extend([x[1] for x in loglines])
+        loglines.sort(key=lambda log_line: log_line[0])
+        for idx, log_line, severity in loglines:
+            debug_logs.append(log_line)
+            if fail:
+                warning_logs.append(log_line)
+            elif severity >= LogLevel.WARN:
+                warning_logs.append(log_line)
         if traceback_msg is not None:
-            logs.append(traceback_msg)
-        kwargs["warnings"] = logs
+            warning_logs.append(traceback_msg)
+        kwargs["warnings"] = warning_logs
+        stderr = "\n".join(debug_logs) + "\n"
+        kwargs["stderr"] = stderr
         return kwargs
 
     def exit_json(self, connections, changed=False, **kwargs):
@@ -1595,7 +1603,7 @@ class RunEnvironmentAnsible(RunEnvironment):
         kwargs["msg"] = msg
         kwargs["changed"] = changed
         self.module.fail_json(
-            **self._complete_kwargs(connections, kwargs, traceback_msg)
+            **self._complete_kwargs(connections, kwargs, traceback_msg, fail=True)
         )
 
 
diff --git a/tasks/main.yml b/tasks/main.yml
index 94c7680..4b36c5c 100644
--- a/tasks/main.yml
+++ b/tasks/main.yml
@@ -64,6 +64,10 @@
     ignore_errors: "{{ network_ignore_errors | default(omit) }}"
     force_state_change: "{{ network_force_state_change | default(omit) }}"
     connections: "{{ network_connections | default([]) }}"
+  register: __network_connections_result
+
+- name: Show debug messages
+  debug: var=__network_connections_result
 
 - name: Re-test connectivity
   ping:
diff --git a/tests/playbooks/tests_ethernet.yml b/tests/playbooks/tests_ethernet.yml
index 0479947..cd02579 100644
--- a/tests/playbooks/tests_ethernet.yml
+++ b/tests/playbooks/tests_ethernet.yml
@@ -37,6 +37,8 @@
           address: 192.0.2.1/24
   roles:
     - linux-system-roles.network
+  tasks:
+    - include_tasks: tasks/assert_output_in_stderr_without_warnings.yml
 
 - hosts: all
   tasks:
diff --git a/tests/playbooks/tests_reapply.yml b/tests/playbooks/tests_reapply.yml
index adea4f5..4b1cb09 100644
--- a/tests/playbooks/tests_reapply.yml
+++ b/tests/playbooks/tests_reapply.yml
@@ -50,7 +50,7 @@
         - name: Assert that reapply is found in log output
           assert:
             fail_msg: Reapply not found in log output
-            that: "{{ 'connection reapplied' in test_module_run.warnings[2] }}"
+            that: "{{ 'connection reapplied' in test_module_run.stderr }}"
       always:
         - block:
             # Use internal module directly for speedup
diff --git a/tests/tasks/assert_output_in_stderr_without_warnings.yml b/tests/tasks/assert_output_in_stderr_without_warnings.yml
new file mode 100644
index 0000000..d760d3d
--- /dev/null
+++ b/tests/tasks/assert_output_in_stderr_without_warnings.yml
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: BSD-3-Clause
+---
+- name: "Assert that warnings is empty"
+  assert:
+    that:
+      - "'warnings' not in __network_connections_result"
+    msg: "There are unexpected warnings"
+- name: "Assert that there is output in stderr"
+  assert:
+    that:
+      - "'stderr' in __network_connections_result"
+    msg: "There are no messages in stderr"
diff --git a/tests/tests_default.yml b/tests/tests_default.yml
index fda6ed5..f6f7550 100644
--- a/tests/tests_default.yml
+++ b/tests/tests_default.yml
@@ -4,3 +4,9 @@
   hosts: all
   roles:
     - linux-system-roles.network
+  tasks:
+    - name: Test warning and info logs
+      assert:
+        that:
+          - "'warnings' not in __network_connections_result"
+        msg: "There are warnings"