Merge ~barryprice/charm-canonical-livepatch/+git/canonical-livepatch-charm:master into ~livepatch-charmers/charm-canonical-livepatch:master

[Barry Price]

Clearer check output, and warn if the client output format changes (i.e. if checkState or patchState go away).

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Stuart Bishop (stub)]

Seems ok. Some comments inline.

review: Approve

[Stuart Bishop (stub)]

Yup, but see comments.

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 6308443772e8c506cde9b2b61caec86c443cae6f

diff --git a/files/check_canonical-livepatch.py b/files/check_canonical-livepatch.py
index 7878169..87c7204 100755
--- a/files/check_canonical-livepatch.py
+++ b/files/check_canonical-livepatch.py
@@ -5,70 +5,108 @@
 import os
 import nagios_plugin
 from subprocess import check_output, call
+from yaml import safe_load
 
 supported_archs = ['x86_64']
 
 
-##############################################################################
-
-def check_package_installed():
+def check_snap_installed():
+    """Confirm the snap is installed, raise an error if not"""
     cmd = ['snap', 'list', 'canonical-livepatch']
     try:
         check_output(cmd, universal_newlines=True)
     except Exception:
-        raise nagios_plugin.CriticalError("canonical-livepatch snap is not installed")
+        raise nagios_plugin.CriticalError('canonical-livepatch snap is not installed')
 
 
-##############################################################################
+def load_status():
+    """Load the cached status from disk, return it as a string"""
+    livepatch_output_path = '/var/lib/nagios/canonical-livepatch-status.txt'
 
-def check_vmlinuz():
-    vmlinuz = '/vmlinuz'
-    if os.path.exists(vmlinuz):
-        full_kernel_path = os.path.realpath(vmlinuz)
-    elif os.path.exists('/boot' + vmlinuz):
-        vmlinuz = '/boot' + vmlinuz
-        full_kernel_path = os.path.realpath(vmlinuz)
-    else:
-        return 'no /vmlinuz or /boot/vmlinuz'
-    kernel_filename = os.path.basename(full_kernel_path)
-    # remove 'vmlinuz'-' from start:
-    kernel_version = '-'.join(kernel_filename.split('-', 1)[1:])
-    # check for '-generic-pae' kernels that need two removes:
-    if '-generic-pae' in kernel_version:
-        kernel_version = '-'.join(kernel_version.split('-')[:-1])
-    # remove e.g. '-generic' from end:
-    kernel_version = '-'.join(kernel_version.split('-')[:-1])
-    return kernel_version.strip()
+    with open(livepatch_output_path, 'r') as canonical_livepatch_log:
+        livepatch_status = canonical_livepatch_log.read()
+
+    return livepatch_status
+
+
+def check_serious_errors():
+    """In serious error cases, the output will not be YAML"""
+    livepatch_status = load_status()
+
+    # if it's empty, something's definitely wrong
+    if livepatch_status == '':
+        raise nagios_plugin.CriticalError('No output from canonical-livepatch status')
+
+    # in some cases, it's obviously not valid YAML
+    try:
+        livepatch_yaml = safe_load(livepatch_status)
+    except Exception:
+        raise nagios_plugin.CriticalError(livepatch_status.replace('\n', ' '))
+
+    # in other cases, it's less obvious until we try to parse
+    try:
+        livepatch_yaml['status']
+    except KeyError:
+        raise nagios_plugin.CriticalError(livepatch_status.replace('\n', ' '))
 
 
-##############################################################################
+def active_kernel_version():
+    """Return the active kernel version, from livepatch's perspective"""
+    livepatch_status = load_status()
+    status_yaml = safe_load(livepatch_status)
+    for kernel in status_yaml.get('status'):
+        if kernel.get('running') is True:
+            return kernel.get('kernel')
+
 
 def check_status():
-    livepatch_output_path = '/var/lib/nagios/canonical-livepatch-status.txt'
-    err_lines = []
-    wrn_lines = []
+    """Check the cached status, raise an error if we find any issues"""
+    livepatch_status = load_status()
+    errors = []
 
-    with open(livepatch_output_path, 'r') as canonical_livepatch_log:
-        for line in canonical_livepatch_log:
-            line = line.strip()
-            if 'State:' in line:
-                if 'apply-failed' in line:
-                    err_lines.append('Livepatch failed to apply patches.')
-                elif 'check-failed' in line:
-                    err_lines.append('Livepatch failed to check the remote service for patches.')
-                elif 'unknown' in line:
-                    err_lines.append('Livepatch reports an unknown error.')
-                elif 'kernel-upgrade-required' in line:
-                    err_lines.append('A kernel upgrade (and reboot) is required.')
-            elif 'Machine is not enabled' in line:
-                err_lines.append('Machine is not enabled.')
-
-    if err_lines:
-        err = " ".join(err_lines)
-        raise nagios_plugin.CriticalError(err)
-    elif wrn_lines:
-        wrn = " ".join(wrn_lines)
-        raise nagios_plugin.WarnError(wrn)
+    status_yaml = safe_load(livepatch_status)
+
+    for kernel in status_yaml.get('status'):
+        if kernel.get('running') is True:
+            check_state = kernel.get('livepatch').get('checkState')
+            patch_state = kernel.get('livepatch').get('patchState')
+
+    check_state_error = check_check_state(check_state)
+    patch_state_error = check_patch_state(patch_state)
+
+    if check_state_error:
+        errors.append(check_state_error)
+    if patch_state_error:
+        errors.append(patch_state_error)
+
+    if errors:
+        raise nagios_plugin.CriticalError(' '.join(errors))
+
+
+def check_check_state(check_state):
+    """Check for issues with checkState, including unexpected output"""
+    if check_state in ['checked', 'needs-check']:
+        pass
+    elif check_state == 'check-failed':
+        return 'Livepatch failed to check the remote service for patches.'
+    else:
+        return 'Unknown check state: {}'.format(check_state)
+
+
+def check_patch_state(patch_state):
+    """Check for issues with patchState, including unexpected output"""
+    if patch_state in ['applied', 'nothing-to-apply']:
+        pass
+    elif patch_state == 'unapplied':
+        return 'Livepatch has not applied the downloaded patches.'
+    elif patch_state == 'applied-with-bug':
+        return 'Livepatch has detected a kernel bug while applying patches.'
+    elif patch_state == 'apply-failed':
+        return 'Livepatch failed to apply patches.'
+    elif patch_state == 'kernel-upgrade-required':
+        return 'A kernel upgrade (and reboot) is required.'
+    else:
+        return 'Unknown patch state: {}'.format(patch_state)
 
 
 def lsb_release():
@@ -101,8 +139,6 @@ def is_container():
         return os.path.exists('/run/container_type')
 
 
-##############################################################################
-
 def main():
     arch = os.uname()[4]
     if arch not in supported_archs:
@@ -112,12 +148,12 @@ def main():
     elif is_container():
         print("canonical-livepatch not needed in OS containers.")
     else:
-        nagios_plugin.try_check(check_package_installed)
+        nagios_plugin.try_check(check_snap_installed)
+        nagios_plugin.try_check(check_serious_errors)
         nagios_plugin.try_check(check_status)
-        print("OK - canonical-livepatch seems to be installed and working")
-
+        kernel_version = active_kernel_version()
+        print("OK - canonical-livepatch is active on kernel {}".format(kernel_version))
 
-##############################################################################
 
 if __name__ == "__main__":
     main()