openshift_checks: enable providing file outputs

Some refactoring of checks and the action plugin to enable writing files
locally about the check operation and results, if the user wants them.
This is aimed at enabling persistent and machine-readable results from
recurring runs of health checks.

Now, rather than trying to build a result hash to return from running
each check, checks can just register what they need to as they're going
along, and the action plugin processes state when the check is done.
Checks can register failures, notes about what they saw, and arbitrary
files to be saved into a directory structure where the user specifies.
If no directory is specified, no files are written.

At this time checks can still return a result hash, but that will likely
be refactored away in the next iteration.

Multiple failures can be registered without halting check execution.
Throwing an exception or returning a hash with "failed" is registered as
a failure.

execute_module now does a little more with the results. Results are
automatically included in notes and written individually as files.
"changed" results are propagated. Some json results are decoded.

A few of the checks were enhanced to use these features; all get some of
the features for free.

1 files changed, 108 insertions, 17 deletions

diff --git a/roles/openshift_health_checker/test/action_plugin_test.py b/roles/openshift_health_checker/test/action_plugin_test.py
index 58864da21..f14887303 100644
--- a/roles/openshift_health_checker/test/action_plugin_test.py
+++ b/roles/openshift_health_checker/test/action_plugin_test.py
@@ -3,10 +3,12 @@ import pytest
 from ansible.playbook.play_context import PlayContext
 
 from openshift_health_check import ActionModule, resolve_checks
-from openshift_checks import OpenShiftCheckException
+from openshift_health_check import copy_remote_file_to_dir, write_result_to_output_dir, write_to_output_file
+from openshift_checks import OpenShiftCheckException, FileToSave
 
 
-def fake_check(name='fake_check', tags=None, is_active=True, run_return=None, run_exception=None, changed=False):
+def fake_check(name='fake_check', tags=None, is_active=True, run_return=None, run_exception=None,
+               run_logs=None, run_files=None, changed=False, get_var_return=None):
     """Returns a new class that is compatible with OpenShiftCheck for testing."""
 
     _name, _tags = name, tags
@@ -14,12 +16,16 @@ def fake_check(name='fake_check', tags=None, is_active=True, run_return=None, ru
     class FakeCheck(object):
         name = _name
         tags = _tags or []
-        changed = False
 
-        def __init__(self, execute_module=None, task_vars=None, tmp=None):
-            pass
+        def __init__(self, **_):
+            self.changed = False
+            self.failures = []
+            self.logs = run_logs or []
+            self.files_to_save = run_files or []
 
         def is_active(self):
+            if isinstance(is_active, Exception):
+                raise is_active
             return is_active
 
         def run(self):
@@ -28,6 +34,13 @@ def fake_check(name='fake_check', tags=None, is_active=True, run_return=None, ru
                 raise run_exception
             return run_return
 
+        def get_var(*args, **_):
+            return get_var_return
+
+        def register_failure(self, exc):
+            self.failures.append(OpenShiftCheckException(str(exc)))
+            return
+
     return FakeCheck
 
 
@@ -98,13 +111,18 @@ def test_action_plugin_cannot_load_checks_with_the_same_name(plugin, task_vars,
     assert failed(result, msg_has=['duplicate', 'duplicate_name', 'FakeCheck'])
 
 
-def test_action_plugin_skip_non_active_checks(plugin, task_vars, monkeypatch):
-    checks = [fake_check(is_active=False)]
+@pytest.mark.parametrize('is_active, skipped_reason', [
+    (False, "Not active for this host"),
+    (Exception("borked"), "exception"),
+])
+def test_action_plugin_skip_non_active_checks(is_active, skipped_reason, plugin, task_vars, monkeypatch):
+    checks = [fake_check(is_active=is_active)]
     monkeypatch.setattr('openshift_checks.OpenShiftCheck.subclasses', classmethod(lambda cls: checks))
 
     result = plugin.run(tmp=None, task_vars=task_vars)
 
-    assert result['checks']['fake_check'] == dict(skipped=True, skipped_reason="Not active for this host")
+    assert result['checks']['fake_check'].get('skipped')
+    assert skipped_reason in result['checks']['fake_check'].get('skipped_reason')
     assert not failed(result)
     assert not changed(result)
     assert not skipped(result)
@@ -128,10 +146,21 @@ def test_action_plugin_skip_disabled_checks(to_disable, plugin, task_vars, monke
     assert not skipped(result)
 
 
+def test_action_plugin_run_list_checks(monkeypatch):
+    task = FakeTask('openshift_health_check', {'checks': []})
+    plugin = ActionModule(task, None, PlayContext(), None, None, None)
+    monkeypatch.setattr(plugin, 'load_known_checks', lambda *_: {})
+    result = plugin.run()
+
+    assert failed(result, msg_has="Available checks")
+    assert not changed(result)
+    assert not skipped(result)
+
+
 def test_action_plugin_run_check_ok(plugin, task_vars, monkeypatch):
     check_return_value = {'ok': 'test'}
-    check_class = fake_check(run_return=check_return_value)
-    monkeypatch.setattr(plugin, 'load_known_checks', lambda tmp, task_vars: {'fake_check': check_class()})
+    check_class = fake_check(run_return=check_return_value, run_files=[None])
+    monkeypatch.setattr(plugin, 'load_known_checks', lambda *_: {'fake_check': check_class()})
     monkeypatch.setattr('openshift_health_check.resolve_checks', lambda *args: ['fake_check'])
 
     result = plugin.run(tmp=None, task_vars=task_vars)
@@ -145,7 +174,7 @@ def test_action_plugin_run_check_ok(plugin, task_vars, monkeypatch):
 def test_action_plugin_run_check_changed(plugin, task_vars, monkeypatch):
     check_return_value = {'ok': 'test'}
     check_class = fake_check(run_return=check_return_value, changed=True)
-    monkeypatch.setattr(plugin, 'load_known_checks', lambda tmp, task_vars: {'fake_check': check_class()})
+    monkeypatch.setattr(plugin, 'load_known_checks', lambda *_: {'fake_check': check_class()})
     monkeypatch.setattr('openshift_health_check.resolve_checks', lambda *args: ['fake_check'])
 
     result = plugin.run(tmp=None, task_vars=task_vars)
@@ -158,9 +187,9 @@ def test_action_plugin_run_check_changed(plugin, task_vars, monkeypatch):
 
 
 def test_action_plugin_run_check_fail(plugin, task_vars, monkeypatch):
-    check_return_value = {'failed': True}
+    check_return_value = {'failed': True, 'msg': 'this is a failure'}
     check_class = fake_check(run_return=check_return_value)
-    monkeypatch.setattr(plugin, 'load_known_checks', lambda tmp, task_vars: {'fake_check': check_class()})
+    monkeypatch.setattr(plugin, 'load_known_checks', lambda *_: {'fake_check': check_class()})
     monkeypatch.setattr('openshift_health_check.resolve_checks', lambda *args: ['fake_check'])
 
     result = plugin.run(tmp=None, task_vars=task_vars)
@@ -171,24 +200,51 @@ def test_action_plugin_run_check_fail(plugin, task_vars, monkeypatch):
     assert not skipped(result)
 
 
-def test_action_plugin_run_check_exception(plugin, task_vars, monkeypatch):
+@pytest.mark.parametrize('exc_class, expect_traceback', [
+    (OpenShiftCheckException, False),
+    (Exception, True),
+])
+def test_action_plugin_run_check_exception(plugin, task_vars, exc_class, expect_traceback, monkeypatch):
     exception_msg = 'fake check has an exception'
-    run_exception = OpenShiftCheckException(exception_msg)
+    run_exception = exc_class(exception_msg)
     check_class = fake_check(run_exception=run_exception, changed=True)
-    monkeypatch.setattr(plugin, 'load_known_checks', lambda tmp, task_vars: {'fake_check': check_class()})
+    monkeypatch.setattr(plugin, 'load_known_checks', lambda *_: {'fake_check': check_class()})
     monkeypatch.setattr('openshift_health_check.resolve_checks', lambda *args: ['fake_check'])
 
     result = plugin.run(tmp=None, task_vars=task_vars)
 
     assert failed(result['checks']['fake_check'], msg_has=exception_msg)
+    assert expect_traceback == ("Traceback" in result['checks']['fake_check']['msg'])
     assert failed(result, msg_has=['failed'])
     assert changed(result['checks']['fake_check'])
     assert changed(result)
     assert not skipped(result)
 
 
+def test_action_plugin_run_check_output_dir(plugin, task_vars, tmpdir, monkeypatch):
+    check_class = fake_check(
+        run_return={},
+        run_logs=[('thing', 'note')],
+        run_files=[
+            FileToSave('save.file', 'contents', None),
+            FileToSave('save.file', 'duplicate', None),
+            FileToSave('copy.file', None, 'foo'),  # note: copy runs execute_module => exception
+        ],
+    )
+    task_vars['openshift_checks_output_dir'] = str(tmpdir)
+    check_class.get_var = lambda self, name, **_: task_vars.get(name)
+    monkeypatch.setattr(plugin, 'load_known_checks', lambda *_: {'fake_check': check_class()})
+    monkeypatch.setattr('openshift_health_check.resolve_checks', lambda *args: ['fake_check'])
+
+    plugin.run(tmp=None, task_vars=task_vars)
+    assert any(path.basename == task_vars['ansible_host'] for path in tmpdir.listdir())
+    assert any(path.basename == 'fake_check.log.json' for path in tmpdir.visit())
+    assert any(path.basename == 'save.file' for path in tmpdir.visit())
+    assert any(path.basename == 'save.file.2' for path in tmpdir.visit())
+
+
 def test_action_plugin_resolve_checks_exception(plugin, task_vars, monkeypatch):
-    monkeypatch.setattr(plugin, 'load_known_checks', lambda tmp, task_vars: {})
+    monkeypatch.setattr(plugin, 'load_known_checks', lambda *_: {})
 
     result = plugin.run(tmp=None, task_vars=task_vars)
 
@@ -254,3 +310,38 @@ def test_resolve_checks_failure(names, all_checks, words_in_exception):
         resolve_checks(names, all_checks)
     for word in words_in_exception:
         assert word in str(excinfo.value)
+
+
+@pytest.mark.parametrize('give_output_dir, result, expect_file', [
+    (False, None, False),
+    (True, dict(content="c3BhbQo=", encoding="base64"), True),
+    (True, dict(content="encoding error", encoding="base64"), False),
+    (True, dict(content="spam", no_encoding=None), True),
+    (True, dict(failed=True, msg="could not slurp"), False),
+])
+def test_copy_remote_file_to_dir(give_output_dir, result, expect_file, tmpdir):
+    check = fake_check()()
+    check.execute_module = lambda *args, **_: result
+    copy_remote_file_to_dir(check, "remote_file", str(tmpdir) if give_output_dir else "", "local_file")
+    assert expect_file == any(path.basename == "local_file" for path in tmpdir.listdir())
+
+
+def test_write_to_output_exceptions(tmpdir, monkeypatch, capsys):
+
+    class Spam(object):
+        def __str__(self):
+            raise Exception("break str")
+
+    test = {1: object(), 2: Spam()}
+    test[3] = test
+    write_result_to_output_dir(str(tmpdir), test)
+    assert "Error writing" in test["output_files"]
+
+    output_dir = tmpdir.join("eggs")
+    output_dir.write("spam")  # so now it's not a dir
+    write_to_output_file(str(output_dir), "somefile", "somedata")
+    assert "Could not write" in capsys.readouterr()[1]
+
+    monkeypatch.setattr("openshift_health_check.prepare_output_dir", lambda *_: False)
+    write_result_to_output_dir(str(tmpdir), test)
+    assert "Error creating" in test["output_files"]