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

[Barry Price]

Write our actual live status for the nagios check, instead of an empty file (which the latest check script would see as an error)

[🤖 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)]

Yup. Comments inline, but its a matter of taste.

review: Approve

[Stuart Bishop (stub)]

Yup. Comments inline.

review: Approve

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

Merge proposal is approved, but source revision has changed, setting status to needs review.

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

Change successfully merged at revision d70cd8dadb3d78b4bc01559dd8cc1f0f788c2ec0

diff --git a/Makefile b/Makefile
index e8168bf..d608afb 100644
--- a/Makefile
+++ b/Makefile
@@ -12,7 +12,7 @@ clean:
 .PHONY: testdeps
 testdeps:
 	@sudo apt-get update
-	@sudo apt-get install -y make flake8 python3-flake8 python3-pip python-pip snapd
+	@sudo apt-get install -y make flake8 python3-flake8 python3-pip python-pip snapd libffi-dev
 	@which juju >/dev/null || (sudo snap install juju --classic)
 	@which charm >/dev/null || (sudo snap install charm)
 	@which bundletester >/dev/null || (pip2 install bundletester juju-deployer)
@@ -20,7 +20,7 @@ testdeps:
 
 .PHONY: lint
 lint:
-	@flake8 --max-complexity=16 --max-line-length=120 && echo OK
+	@flake8 --max-complexity=16 --max-line-length=120 --exclude=lib/* && echo OK
 
 .PHONY: charmbuild
 charmbuild:
diff --git a/files/canonical-livepatch-status.txt b/files/canonical-livepatch-status.txt
deleted file mode 100644
index e69de29..0000000
--- a/files/canonical-livepatch-status.txt
+++ /dev/null
diff --git a/files/check_canonical-livepatch.py b/files/check_canonical-livepatch.py
index f578f93..a40936a 100755
--- a/files/check_canonical-livepatch.py
+++ b/files/check_canonical-livepatch.py
@@ -26,7 +26,7 @@ def load_status():
     with open(livepatch_output_path, 'r') as canonical_livepatch_log:
         livepatch_status = canonical_livepatch_log.read()
 
-    return livepatch_status
+    return livepatch_status.strip()
 
 
 def check_serious_errors():
@@ -95,7 +95,7 @@ def check_check_state(check_state):
 
 def check_patch_state(patch_state):
     """Check for issues with patchState, including unexpected output"""
-    if patch_state in ['applied', 'nothing-to-apply']:
+    if patch_state in ['applied', 'applying', 'nothing-to-apply']:
         pass
     elif patch_state == 'unapplied':
         return 'Livepatch has not applied the downloaded patches.'
diff --git a/reactive/canonical_livepatch.py b/reactive/canonical_livepatch.py
index 03c1903..e8ba806 100644
--- a/reactive/canonical_livepatch.py
+++ b/reactive/canonical_livepatch.py
@@ -5,7 +5,7 @@ from charmhelpers.core.host import write_file, is_container
 from charmhelpers.core import hookenv
 from charmhelpers.contrib.charmsupport import nrpe
 from distutils.version import LooseVersion
-from os import environ, path, uname
+from os import environ, mkdir, path, uname
 from subprocess import check_call, check_output, CalledProcessError
 from time import sleep
 from yaml import safe_load
@@ -53,26 +53,46 @@ def unit_update(status=None, message=None):
         hookenv.status_set('active', 'Running kernel {}, patchState: {}'.format(*patch_details))
 
 
-def get_patch_details():
-    kernel = patch_state = 'unknown'
-
+def get_livepatch_status():
     cmd = ['/snap/bin/canonical-livepatch', 'status']
+    livepatch_status = ''
     try:
-        livepatch_state = check_output(cmd, universal_newlines=True)
-    except CalledProcessError as e:
-        hookenv.log('Unable to get status: {}'.format(str(e)), hookenv.ERROR)
-        return kernel, patch_state
+        livepatch_status = check_output(cmd, universal_newlines=True)
+    except CalledProcessError:
+        # If the service hasn't been enabled yet, we'll get a process error - this is fine
+        pass
+    return livepatch_status
+
+
+def write_status_to_disk():
+    # In a race with nrpe, create /var/lib/nagios if nrpe didn't do it yet
+    if not path.exists('/var/lib/nagios'):
+        mkdir('/var/lib/nagios')
+
+    current_status = get_livepatch_status()
+    perms = 0o644
+    write_file(
+        path='/var/lib/nagios/canonical-livepatch-status.txt',
+        content=current_status,
+        owner='root',
+        perms=perms,
+    )
+
+
+def get_patch_details():
+    kernel = 'unknown'
+    raw_status = get_livepatch_status()
 
     # status will usually pass YAML (but not always!)
     try:
-        status_yaml = safe_load(livepatch_state)
+        status = safe_load(raw_status)
     except Exception:
         hookenv.log('Unable to parse status yaml', hookenv.ERROR)
-        return kernel, patch_state
+        return kernel, raw_status
 
     # even if we got YAML, be paranoid
     try:
-        for k in status_yaml['status']:
+        for k in status['status']:
             if k['running'] is True:
                 kernel = k['kernel']
                 patch_state = k['livepatch']['patchState']
@@ -122,6 +142,11 @@ def activate_livepatch():
             unit_update()
     else:
         hookenv.log('Unable to activate canonical-livepatch as no key has been set', hookenv.ERROR)
+        cmd = ['/snap/bin/canonical-livepatch', 'disable']
+        try:
+            check_output(cmd, universal_newlines=True)
+        except CalledProcessError as e:
+            hookenv.log('Unable to deactivate: {}'.format(str(e)), hookenv.ERROR)
         clear_flag('canonical-livepatch.active')
         unit_update('blocked', 'Service disabled, please set livepatch_key to activate')
 
@@ -216,6 +241,7 @@ def init_key():
 def update_key():
     # handle regular config-changed hooks for the livepatch key
     activate_livepatch()
+    write_status_to_disk()
 
 
 @when('snap.installed.canonical-livepatch', 'config.changed.snap_channel')
@@ -247,12 +273,9 @@ def configure_nagios(nagios):
         'files/check_canonical-livepatch.py',
         '/usr/lib/nagios/plugins/check_canonical-livepatch.py',
     )
-    # don't overwrite an existing status file
-    if not path.exists('/var/lib/nagios/canonical-livepatch-status.txt'):
-        file_to_units(
-            'files/canonical-livepatch-status.txt',
-            '/var/lib/nagios/canonical-livepatch-status.txt',
-        )
+
+    # write current status to disk to satisfy the nagios check
+    write_status_to_disk()
 
     # remove check from previous release with poorly formed name
     nrpe_setup.remove_check(