livepatch-charmers team mailing list archive

[Barry Price <barry.price@xxxxxxxxxxxxx>]

Barry Price has proposed merging ~barryprice/canonical-livepatch-charm/+git/canonical-livepatch-charm:master into canonical-livepatch-charm:master.

Commit message:
Use layer:nagios instead of interface:nrpe-external-master, and address LP#1795902

Requested reviews:
  Livepatch charm developers (livepatch-charmers)
Related bugs:
  Bug #1795902 in Canonical Livepatch Charm: "The configure_nagios() function runs on every hook"
  https://bugs.launchpad.net/canonical-livepatch-charm/+bug/1795902

For more details, see:
https://code.launchpad.net/~barryprice/canonical-livepatch-charm/+git/canonical-livepatch-charm/+merge/356576
-- 
Your team Livepatch charm developers is requested to review the proposed merge of ~barryprice/canonical-livepatch-charm/+git/canonical-livepatch-charm:master into canonical-livepatch-charm:master.

diff --git a/Makefile b/Makefile
index d608afb..e7f78ad 100644
--- a/Makefile
+++ b/Makefile
@@ -14,7 +14,7 @@ testdeps:
 	@sudo apt-get update
 	@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 charm >/dev/null || (sudo snap install charm --classic)
 	@which bundletester >/dev/null || (pip2 install bundletester juju-deployer)
 	@pip3 install amulet
 
diff --git a/config.yaml b/config.yaml
index 556aaee..ee164fe 100644
--- a/config.yaml
+++ b/config.yaml
@@ -13,22 +13,6 @@ options:
     description: |
         The address of a proxy server to use for livepatch traffic
         e.g. http://proxy.example.com:3128
-  nagios_context:
-    default: "juju"
-    type: string
-    description: |
-      Used by the nrpe-external-master subordinate charm.
-      A string that will be prepended to instance name to set the host name
-      in nagios. So for instance the hostname would be something like:
-          juju-myservice-0
-      If you're running multiple environments with the same services in them
-      this allows you to differentiate between them.
-  nagios_servicegroups:
-    default: ""
-    type: string
-    description: >
-        A comma-separated list of nagios servicegroups.
-        If left empty, the nagios_context will be used as the servicegroup.
   snap_channel:
     default: "stable"
     type: string
diff --git a/layer.yaml b/layer.yaml
index 0fd758a..fabbd11 100644
--- a/layer.yaml
+++ b/layer.yaml
@@ -1,7 +1,7 @@
 includes:
     - layer:basic
+    - layer:nagios
     - layer:snap
-    - interface:nrpe-external-master
 options:
     basic:
         use_venv: true
diff --git a/reactive/canonical_livepatch.py b/reactive/canonical_livepatch.py
index a1bac0d..3f30019 100644
--- a/reactive/canonical_livepatch.py
+++ b/reactive/canonical_livepatch.py
@@ -1,10 +1,9 @@
 from charms.layer import snap
-from charms.reactive import when, when_not, set_flag, clear_flag
+from charms.reactive import when, when_not, set_flag, clear_flag, hook
 from charms.reactive.flags import register_trigger
 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, mkdir, path, uname
 from subprocess import check_call, check_output, CalledProcessError
 from time import sleep
@@ -210,17 +209,10 @@ def proxy_settings():
 @when('livepatch-proxy.configured')
 @when_not('canonical-livepatch.connected')
 def canonical_livepatch_connect():
-    # So if we've just installed snapd on a trusty system, we will not be on
-    # the HWE kernel yet and unfortunately need to reboot first!
-    current = LooseVersion(uname()[2])
-    required = LooseVersion('4.4')
     uptrack_path = '/usr/sbin/uptrack-upgrade'
     if path.exists(uptrack_path):
         hookenv.log('Ksplice/Uptrack detected, please remove it and reboot')
         unit_update('blocked', 'Remove ksplice and then reboot')
-    elif current < required:
-        hookenv.log('Reboot required, kernel {} is too old'.format(current))
-        unit_update('blocked', 'A reboot is required')
     else:
         unit_update('maintenance', 'Connecting to the livepatch service')
         # Make sure the service is ready for us
@@ -232,9 +224,6 @@ def canonical_livepatch_connect():
 @when('canonical-livepatch.connected')
 @when_not('config.changed.livepatch_key', 'canonical-livepatch.active')
 def init_key():
-    # If deployed under Trusty before rebooting into the HWE kernel
-    # the config-changed hook won't fire post-reboot as the state
-    # isn't tracked, but we didn't initialise yet! So, handle it here
     activate_livepatch()
 
 
@@ -256,7 +245,9 @@ def change_channel():
     )
 
 
+# Set up Nagios checks when the nrpe-external-master subordinate is related
 @when('nrpe-external-master.available')
+@when_not('canonical-livepatch.nagios-setup.complete')
 def configure_nagios(nagios):
     if hookenv.hook_name() == 'update-status':
         return
@@ -292,10 +283,19 @@ def configure_nagios(nagios):
 
     nrpe_setup.write()
 
-    # Remove obsolete state from older charm.
+    # Remove obsolete state from older charm, if present
     clear_flag('canonical-livepatch.nagios-configured')
+    # Set our preferred state
+    set_flag('canonical-livepatch.nagios-setup.complete')
 
 
 @when('snap.installed.canonical-livepatch', 'canonical-livepatch.active')
 def update_kernel_version():
     unit_update()
+
+
+# This is triggered on any config-changed, and after an upgrade-charm - you
+# don't get the latter with @when('config.changed')
+@hook('config-changed')
+def set_nrpe_flag():
+    clear_flag('canonical-livepatch.nagios-setup.complete')