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

[Barry Price]

[Barry Price (barryprice)]

Also upped the timeout for checks to an hour to cover sub-optimal internet connections and/or cloud environments.

[Stuart Bishop (stub)]

Mostly looks good.

I think the interface:nrpe-external-master needs to stay though, commented on inline. I suspect when you tested you had nrpe relation hooks hanging around from a previous build.

review: Approve

[Barry Price (barryprice)]

Local tests are now passing once again, but I want to reinstate the amulet test for nagios_context/servicegroups now those have been reinstated. Will likely pick this up again Monday.

review: Needs Fixing

[Barry Price (barryprice)]

All tests are now passing.

[Barry Price (barryprice)]

review: Abstain

diff --git a/README.md b/README.md
index 4cf16a5..8ac1e4a 100644
--- a/README.md
+++ b/README.md
@@ -24,10 +24,6 @@ Or via juju config (Juju 2.x)
 
     juju config canonical-livepatch livepatch_key=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
 
-## Known Limitations and Issues
-
-The Nagios nrpe check integration code is a work in progress (but is fully functional)
-
 # Configuration
 
 The livepatch_key setting must be configured in order for the software to function correctly - see above.
@@ -36,8 +32,6 @@ By default, the livepatch software will attempt to retrieve its patch data direc
 
 Likewise, if you need to use a proxy server to access the Snap Store, snap_proxy can also be set to the address of your proxy server.
 
-We set the Nagios servicegroups to 'livepatch' by default, this can also be reconfigured to suit your needs.
-
 # Contact Information
 
 This charm is maintained here, by the livepatch-charmers team:
diff --git a/config.yaml b/config.yaml
index 40694b6..298611b 100644
--- a/config.yaml
+++ b/config.yaml
@@ -13,7 +13,19 @@ 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: "livepatch"
+    default: ""
     type: string
-    description: "Nagios servicegroup(s) for alerts."
+    description: >
+        A comma-separated list of nagios servicegroups.
+        If left empty, the nagios_context will be used as the servicegroup.
diff --git a/reactive/canonical_livepatch.py b/reactive/canonical_livepatch.py
index 102df07..4c12fb0 100644
--- a/reactive/canonical_livepatch.py
+++ b/reactive/canonical_livepatch.py
@@ -1,11 +1,11 @@
 from charms.reactive import when, when_not, set_state
 from charmhelpers.core.host import write_file
 from charmhelpers.core import hookenv
+from charmhelpers.contrib.charmsupport import nrpe
 from subprocess import check_call, check_output, CalledProcessError
 from time import sleep
 from os import path
 from yaml import load, dump
-from glob import glob
 
 
 def file_to_units(local_path, unit_path):
@@ -102,30 +102,6 @@ def restart_livepatch():
         check_call(cmd, universal_newlines=True)
 
 
-def update_nagios_servicedef(service_name, service_value):
-    for service_file in glob("/var/lib/nagios/export/service__*_check_canonical-livepatch.cfg"):
-        with open(service_file, "r+") as fh:
-            output = ''
-            for line in fh:
-                if line.startswith('    {} '.format(service_name)):
-                    els = line.split(' ')
-                    new_els = []
-                    for e in els:
-                        if e == '':
-                            new_els.append(e)
-                        elif e == service_name:
-                            new_els.append(e)
-                        else:
-                            new_els.append(service_value)
-                    new_line = ' '.join(new_els) + "\n"
-                    output += new_line
-                else:
-                    output += line
-            fh.seek(0)
-            fh.write(output)
-            fh.truncate()
-
-
 @when('snap.installed.canonical-livepatch')
 @when_not('canonical-livepatch.connected')
 def canonical_livepatch_connect():
@@ -184,24 +160,14 @@ def update_livepatch_proxy():
     hookenv.status_set('active', 'Ready')
 
 
+@when('snap.installed.canonical-livepatch')
 @when('nrpe-external-master.available')
 def init_nagios_checks(nagios):
     hookenv.status_set('maintenance', 'Configuring Nagios checks')
 
-    # We are a subordinate, try to find the primary hostname
-    nagios_hostname = None
-    for hostfile in glob("/var/lib/nagios/export/host__*"):
-        with open(hostfile) as fh:
-            for line in fh:
-                if 'host_name' in line:
-                    nagios_hostname = line.split()[1]
-
-    # Otherwise, fall back to our generated hostname, probably wrong!
-    if nagios_hostname is None:
-        nagios_hostname = hookenv.local_unit()
-
-    config = hookenv.config()
-    nagios_servicegroups = config.get('nagios_servicegroups')
+    # Ask charmhelpers.contrib.charmsupport's nrpe to work out our hostname
+    hostname = nrpe.get_nagios_hostname()
+    nrpe_setup = nrpe.NRPE(hostname=hostname)
 
     # install nagios support files
     file_to_units(
@@ -219,23 +185,11 @@ def init_nagios_checks(nagios):
             '/var/lib/nagios/canonical-livepatch-status.txt',
         )
 
-    # use the interface to create the check
-    nagios.add_check(["/usr/lib/nagios/plugins/check_canonical-livepatch.py", ],
-                     name="check_canonical-livepatch",
-                     description="Verify canonical-livepatch is working",
-                     servicegroups=nagios_servicegroups,
-                     unit=nagios_hostname)
-
-    # clean up after https://github.com/cmars/nrpe-external-master-interface/issues/5
-    update_nagios_servicedef('host_name', nagios_hostname)
+    # use charmhelpers to create the check
+    nrpe_setup.add_check('check_canonical-livepatch',
+                         'Verify canonical-livepatch is working',
+                         '/usr/lib/nagios/plugins/check_canonical-livepatch.py'
+                         )
 
+    nrpe_setup.write()
     hookenv.status_set('active', 'Ready')
-
-
-@when('config.changed.nagios_servicegroups')
-def update_nagios_servicegroups():
-    hookenv.status_set('maintenance', 'Configuring Nagios checks')
-
-    config = hookenv.config()
-    nagios_servicegroups = config.get('nagios_servicegroups')
-    update_nagios_servicedef('servicegroups', nagios_servicegroups)
diff --git a/tests/99-autogen b/tests/99-autogen
index 1527d4f..c57bbf4 100755
--- a/tests/99-autogen
+++ b/tests/99-autogen
@@ -2,6 +2,7 @@
 
 import amulet
 import unittest
+from time import sleep
 
 
 class TestDeployment(unittest.TestCase):
@@ -26,7 +27,7 @@ class TestDeployment(unittest.TestCase):
         cls.deployment.relate('canonical-livepatch:nrpe-external-master', 'nrpe:nrpe-external-master')
 
         try:
-            cls.deployment.setup(timeout=900)
+            cls.deployment.setup(timeout=3600)
             cls.deployment.sentry.wait()
         except amulet.helpers.TimeoutError:
             amulet.raise_status(amulet.SKIP, msg="Environment wasn't stood up in time")
@@ -59,21 +60,44 @@ class TestDeployment(unittest.TestCase):
             output, exit_code = livepatch.run('stat {}'.format(path))
             self.assertEqual(exit_code, 0)
 
+    def test_nagios_context_change(self):
+        livepatch = self.deployment.sentry['canonical-livepatch'][0]
+
+        test_context_name = 'amulet1'
+
+        # set context name
+        self.deployment.configure('canonical-livepatch', {
+            'nagios_context': test_context_name,
+        })
+
+        # nrpe updates can take a while
+        sleep(30)
+
+        # confirm it showed up in the right place
+        output, exit_code = livepatch.run(
+            'grep {} /var/lib/nagios/export/service__*_check_canonical-livepatch.cfg'.format(test_context_name)
+        )
+        self.assertEqual(exit_code, 0)
+
     def test_nagios_servicegroup_change(self):
         livepatch = self.deployment.sentry['canonical-livepatch'][0]
 
-        test_servicegroup_name = 'amulet'
+        test_servicegroup_name = 'amulet2'
 
         # set servicegroup name
         self.deployment.configure('canonical-livepatch', {
             'nagios_servicegroups': test_servicegroup_name,
         })
 
+        # nrpe updates can take a while
+        sleep(30)
+
         # confirm it showed up in the right place
         output, exit_code = livepatch.run(
             'grep {} /var/lib/nagios/export/service__*_check_canonical-livepatch.cfg'.format(test_servicegroup_name)
         )
         self.assertEqual(exit_code, 0)
 
+
 if __name__ == '__main__':
     unittest.main()