Merge lp:~gnuoy/charms/trusty/nrpe/sibling-subordinate into lp:charms/trusty/nrpe

[Liam Young]

Add support for multiple charms to be related to nrpe via the nrpe, juju-info or local-monitors relation. This is useful when joining an additional local subordinate to this charm. See README changes.

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/3192/

review: Needs Fixing (automated testing)

[David Ames (thedac)]

Liam running a manual amulet test fails:

2016-03-18 15:08:25 Deploying services...
2016-03-18 15:09:37 Config specifies num units for subordinate: nrpe
2016-03-18 15:09:39 Adding relations...
2016-03-18 15:09:40 Adding relation nrpe:local-monitors <-> mysql:local-monitors
Traceback (most recent call last):
  File "/usr/bin/juju-deployer", line 9, in <module>
    load_entry_point('juju-deployer==0.6.4', 'console_scripts', 'juju-deployer')()
  File "/usr/lib/python2.7/dist-packages/deployer/cli.py", line 135, in main
    run()
  File "/usr/lib/python2.7/dist-packages/deployer/cli.py", line 234, in run
    importer.Importer(env, deployment, options).run()
  File "/usr/lib/python2.7/dist-packages/deployer/action/importer.py", line 325, in run
    rels_created = self.add_relations()
  File "/usr/lib/python2.7/dist-packages/deployer/action/importer.py", line 235, in add_relations
    self.env.add_relation(end_a, end_b)
  File "/usr/lib/python2.7/dist-packages/deployer/env/go.py", line 51, in add_relation
    return self.client.add_relation(endpoint_a, endpoint_b)
  File "/usr/lib/python2.7/dist-packages/jujuclient.py", line 1122, in add_relation
    'Endpoints': [endpoint_a, endpoint_b]
  File "/usr/lib/python2.7/dist-packages/jujuclient.py", line 317, in _rpc
    raise EnvError(result)
jujuclient.EnvError: <Env Error - Details:
 { u'Error': u'cannot add relation "nrpe:local-monitors mysql:local-monitors": principal and subordinate services\' series
must match',
    u'RequestId': 1,
    u'Response': { }}
 >
E
======================================================================
ERROR: setUpClass (__main__.TestDeployment)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/10-tests", line 28, in setUpClass
    cls.deployment.setup(timeout=900)
  File "/usr/lib/python2.7/dist-packages/amulet/deployer.py", line 430, in setup
    subprocess.check_call(shlex.split(cmd))
  File "/usr/lib/python2.7/subprocess.py", line 541, in check_call
    raise CalledProcessError(retcode, cmd)
CalledProcessError: Command '['juju-deployer', '-W', '-c', '/tmp/amulet-juju-deployer-IiT6ij/deployer-schema.json', '-e', 'th
edac-laptop', '-t', '1000', 'thedac-laptop']' returned non-zero exit status 1

----------------------------------------------------------------------
Ran 0 tests in 77.503s

FAILED (errors=1)

review: Needs Fixing

[Review Queue (review-queue)]

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-aws/3230/

review: Needs Fixing (automated testing)

[lp:~gnuoy/charms/trusty/nrpe/sibling-subordinate]

44. By Liam Young on 2016-03-20

Fix bug which was stopping local checks being rendered

45. By Liam Young on 2016-03-20

Merge tests/11-monitors-configurations tests/13-monitors-config into tests/10-tests. Promote 14-basic-nrpe-external-master to me run from Makefile along with 10-tests. Fix bug causing checks with a _default prefix being defined on the monitors relation

46. By Liam Young on 2016-03-20

Remove debug print

[David Ames (thedac)]

I have manually run the amulet tests successfully.

+1 from me.

We may still need to figure out why the automated testing is having problems.

review: Approve

[Review Queue (review-queue)]

The results (PASS) are in and available here: http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/3230/

review: Approve (automated testing)

[David Ames (thedac)]

Approved as a ~charmer(s)

Merging.

review: Approve

=== modified file 'Makefile'
--- Makefile	2015-04-19 07:52:43 +0000
+++ Makefile	2016-03-20 13:11:29 +0000
@@ -25,4 +25,4 @@
 	# raise_status() messages to stderr:
 	#   https://bugs.launchpad.net/amulet/+bug/1320357
 	@juju test -v -p AMULET_HTTP_PROXY --timeout 900 \
-        00-setup 10-tests
+        00-setup 10-tests 14-basic-nrpe-external-master

=== modified file 'README.md'
--- README.md	2015-03-23 09:45:10 +0000
+++ README.md	2016-03-20 13:11:29 +0000
@@ -27,6 +27,26 @@
 section and finally it can request external monitors are setup by using one of
 the other remote types. See "Monitors yaml" below.
 
+Other Subordinate Charms
+========================
+
+If another subordinate charm deployed to the same principle has a
+local-monitors or nrpe-external-master relation then it can also be related to
+the local nrpe charm. For example,
+
+echo -e "glance:\n  vip: 10.5.106.1" > glance.yaml
+juju deploy -n3 --config glance.yaml glance
+juju deploy hacluster glance-hacluster
+juju deploy nrpe glance-nrpe
+juju deploy nagios
+juju add-relation glance glance-hacluster
+juju add-relation glance-nrpe:monitors nagios:monitors
+juju add-relation glance glance-nrpe
+juju add-relation glance-hacluster glance-nrpe
+
+The glance-hacluster charm will pass montioring information to glance-nrpe
+which will amalgamate all montior definitions before passing them to nagios
+
 Check sources
 =============
 

=== modified file 'hooks/nrpe_helpers.py'
--- hooks/nrpe_helpers.py	2016-02-10 13:01:51 +0000
+++ hooks/nrpe_helpers.py	2016-03-20 13:11:29 +0000
@@ -6,16 +6,42 @@
 from charmhelpers.core import hookenv
 
 
-class MonitorsRelation(helpers.RelationContext):
-    name = 'monitors'
-    interface = 'monitors'
-
-    def __init__(self, *args, **kwargs):
-        self.principle_relation = PrincipleRelation()
-        super(MonitorsRelation, self).__init__(*args, **kwargs)
-
-    def is_ready(self):
-        return self.principle_relation.is_ready()
+class Monitors(dict):
+    """ Represent the list of checks that a remote Nagios can query converting
+        local check to ones that can be queried remotely
+    """
+
+    def __init__(self, version='0.3'):
+        self['monitors'] = {
+            'local': {},
+            'remote': {
+                'nrpe': {}
+            }
+        }
+        self['version'] = version
+
+    def add_monitors(self, mdict, monitor_label='default'):
+        if not mdict or not mdict.get('monitors'):
+            return
+
+        for checktype in mdict['monitors'].get('remote', []):
+            check_details = mdict['monitors']['remote'][checktype]
+            if self['monitors']['remote'].get(checktype):
+                self['monitors']['remote'][checktype].update(check_details)
+            else:
+                self['monitors']['remote'][checktype] = check_details
+
+        if mdict['monitors'].get('local'):
+            self['monitors']['local'].update(mdict['monitors'].get('local'))
+            for checktype in mdict['monitors']['local']:
+                check_details = self.convert_local_checks(
+                    mdict['monitors']['local'],
+                    monitor_label,
+                )
+                self['monitors']['remote']['nrpe'].update(check_details)
+
+    def add_nrpe_check(self, check_name, command):
+        self['monitors']['remote']['nrpe'][check_name] = {'command': command}
 
     def convert_local_checks(self, monitors, monitor_src):
         """ Convert check from local checks to remote nrpe checks
@@ -35,23 +61,31 @@
                     {'command': check_def['cmd_name']}
         return mons
 
+
+class MonitorsRelation(helpers.RelationContext):
+    name = 'monitors'
+    interface = 'monitors'
+
+    def __init__(self, *args, **kwargs):
+        self.principle_relation = PrincipleRelation()
+        super(MonitorsRelation, self).__init__(*args, **kwargs)
+
+    def is_ready(self):
+        return self.principle_relation.is_ready()
+
     def get_subordinate_monitors(self):
         """ Return default monitors defined by this charm """
-        monitors = {
-            'monitors': {
-                'remote': {
-                    'nrpe': {}
-                }
-            }
-        }
+        monitors = Monitors()
         for check in SubordinateCheckDefinitions()['checks']:
-            monitors['monitors']['remote']['nrpe'][check['cmd_name']] = \
-                {'command': check['cmd_name']}
+            monitors.add_nrpe_check(check['cmd_name'], check['cmd_name'])
         return monitors
 
     def get_user_defined_monitors(self):
         """ Return monitors defined by monitors config option """
-        return yaml.safe_load(hookenv.config('monitors'))
+        monitors = Monitors()
+        monitors.add_monitors(yaml.safe_load(hookenv.config('monitors')),
+                              'user')
+        return monitors
 
     def get_principle_monitors(self):
         """ Return monitors passed by relation with principle """
@@ -70,37 +104,14 @@
         """ Return monitor dict of all monitors merged together and local
             monitors converted to remote nrpe checks
         """
-        monitors = {
-            'monitors': {
-                'remote': {
-                    'nrpe': {}
-                }
-            }
-        }
-        monitor_dicts = self.get_monitor_dicts()
-        for monitor_src in monitor_dicts.keys():
-            monitor_dict = monitor_dicts[monitor_src]
-            if not monitor_dict or 'monitors' not in monitor_dict:
-                continue
-            if 'remote' in monitor_dict['monitors']:
-                remote_mons = monitor_dict['monitors']['remote']
-                for checktype in remote_mons.keys():
-                    if checktype in monitors['monitors']['remote']:
-                        monitors['monitors']['remote'][checktype].update(
-                            remote_mons[checktype]
-                        )
-                    else:
-                        monitors['monitors']['remote'][checktype] = \
-                            remote_mons[checktype]
-            if 'local' in monitor_dict['monitors']:
-                monitors['monitors']['remote']['nrpe'].update(
-                    self.convert_local_checks(
-                        monitor_dict['monitors']['local'],
-                        monitor_src,
-                    )
-                )
-        monitors['version'] = '0.3'
-        return monitors
+        all_monitors = Monitors()
+        all_monitors.add_monitors(self.get_principle_monitors(), 'principle')
+        all_monitors.add_monitors(
+            self.get_subordinate_monitors(),
+            str(hookenv.config("sub_postfix"))
+        )
+        all_monitors.add_monitors(self.get_user_defined_monitors(), 'user')
+        return all_monitors
 
     def get_data(self):
         super(MonitorsRelation, self).get_data()
@@ -149,11 +160,15 @@
             return nagios_hostname
 
     def get_monitors(self):
-        """ Return monitors passed by principle charm """
+        """ Return monitors passed by services on the self.interface relation
+        """
         if not self.is_ready():
             return
-        if 'monitors' in self[self.name][0]:
-            return yaml.load(self[self.name][0]['monitors'])
+        monitors = Monitors()
+        for rel in self[self.name]:
+            if rel.get('monitors'):
+                monitors.add_monitors(yaml.load(rel['monitors']), 'principle')
+        return monitors
 
     def provide_data(self):
         # Provide this data to principals because get_nagios_hostname expects

=== modified file 'tests/10-tests'
--- tests/10-tests	2016-02-11 10:24:41 +0000
+++ tests/10-tests	2016-03-20 13:11:29 +0000
@@ -1,9 +1,10 @@
 #!/usr/bin/python
 
 import amulet
+import yaml
 import unittest
 import time
-import yaml
+import requests
 from charmhelpers.contrib.amulet.utils import (
     AmuletUtils,
 )
@@ -245,11 +246,12 @@
         self.check_nrpe_setting(**test_config)
 
     def test_sub_postfix(self):
-        sentry = self.deployment.sentry['nrpe'][0]
+        check_cmd = ('/usr/lib/nagios/plugins/check_disk -u GB -w 25% -c 20% '
+                     '-K 5% -p /')
         test_config = {
             'filename': '/etc/nagios/nrpe.d/check_disk_root_testing.cfg',
-            'expected_settings': {'command[check_disk_root]': '/usr/lib/nagios/plugins/check_disk -u GB -w 25% -c 20% -K 5% -p /'},
-            'juju_kv': { 'sub_postfix': '_testing'},
+            'expected_settings': {'command[check_disk_root]': check_cmd},
+            'juju_kv': {'sub_postfix': '_testing'},
             'filedelim': '=',
         }
         self.check_nrpe_setting(**test_config)
@@ -259,7 +261,10 @@
         test_config = {
             'filename': '/etc/nagios/nrpe.d/check_disk_root_sub.cfg',
             'expected_settings': {chk_key: '-u GB -w 5% -c 1% -K 10% -p /'},
-            'juju_kv': {'disk_root': '-u GB -w 5% -c 1% -K 10%'}
+            'juju_kv': {
+                'disk_root': '-u GB -w 5% -c 1% -K 10%',
+                'sub_postfix': '_sub',
+            }
         }
         self.check_nrpe_setting(**test_config)
 
@@ -269,7 +274,10 @@
         test_config = {
             'filename': '/etc/nagios/nrpe.d/check_zombie_procs_sub.cfg',
             'expected_settings': {chk_key: '-w 6 -c 12 -s Z'},
-            'juju_kv': {'zombies': '-w 6 -c 12 -s Z'}
+            'juju_kv': {
+                'zombies': '-w 6 -c 12 -s Z',
+                'sub_postfix': '_sub',
+            }
         }
         self.check_nrpe_setting(**test_config)
 
@@ -279,7 +287,10 @@
         test_config = {
             'filename': '/etc/nagios/nrpe.d/check_total_procs_sub.cfg',
             'expected_settings': {chk_key: '-w 40 -c 60'},
-            'juju_kv': {'procs': '-w 40 -c 60'}
+            'juju_kv': {
+                'procs': '-w 40 -c 60',
+                'sub_postfix': '_sub',
+            }
         }
         self.check_nrpe_setting(**test_config)
 
@@ -288,7 +299,10 @@
         test_config = {
             'filename': '/etc/nagios/nrpe.d/check_load_sub.cfg',
             'expected_settings': {chk_key: '-w 9,9,9 -c 16,16,16'},
-            'juju_kv': {'load': '-w 9,9,9 -c 16,16,16'}
+            'juju_kv': {
+                'load': '-w 9,9,9 -c 16,16,16',
+                'sub_postfix': '_sub',
+            }
         }
         self.check_nrpe_setting(**test_config)
 
@@ -297,7 +311,10 @@
         test_config = {
             'filename': '/etc/nagios/nrpe.d/check_users_sub.cfg',
             'expected_settings': {chk_key: '-w 40 -c 50'},
-            'juju_kv': {'users': '-w 40 -c 50'}
+            'juju_kv': {
+                'users': '-w 40 -c 50',
+                'sub_postfix': '_sub',
+            }
         }
         self.check_nrpe_setting(**test_config)
 
@@ -306,9 +323,34 @@
         test_config = {
             'filename': '/etc/nagios/nrpe.d/check_swap_sub.cfg',
             'expected_settings': {chk_key: '-w 5% -c 1%'},
-            'juju_kv': {'swap': '-w 5% -c 1%'}
-        }
-        self.check_nrpe_setting(**test_config)
+            'juju_kv': {
+                'swap': '-w 5% -c 1%',
+                'sub_postfix': '_sub',
+            }
+        }
+        self.check_nrpe_setting(**test_config)
+
+    def test_nrpe_monitors_config(self):
+        chk_key = 'command[check_swap]=/usr/lib/nagios/plugins/check_procs'
+        test_config = {
+            'filename': '/etc/nagios/nrpe.d/check_proc_mysqld_principle.cfg',
+            'expected_settings': {chk_key: '-w 1 -c 1 -C mysqld'},
+            'juju_kv': {},
+        }
+        self.check_nrpe_setting(**test_config)
+
+    def test_nagios_monitors_response(self):
+        # look for mysql_database requests
+        sentry = self.deployment.sentry['nagios'][0]
+        nagpwd = sentry.file_contents('/var/lib/juju/nagios.passwd').strip()
+        host_url = ("http://%s/cgi-bin/nagios3/status.cgi?"
+                    "host=all")
+        r = requests.get(host_url % sentry.info['public-address'],
+                         auth=('nagiosadmin', nagpwd))
+        if not r.text.find('check_proc_mysqld_principle'):
+            amulet.raise_status(amulet.ERROR,
+                                msg='Nagios is not monitoring the' +
+                                    ' hosts it supposed to.')
 
 
 if __name__ == '__main__':

=== removed file 'tests/11-monitors-configurations'
--- tests/11-monitors-configurations	2016-02-03 22:17:17 +0000
+++ tests/11-monitors-configurations	1970-01-01 00:00:00 +0000
@@ -1,60 +0,0 @@
-#!/usr/bin/python3
-
-import amulet
-import requests
-
-seconds = 20000
-
-d = amulet.Deployment(series='trusty')
-
-d.add('nagios')
-d.add('mysql')
-d.add('nrpe')
-
-d.configure('mysql', {'dataset-size': '10%'})
-d.relate('nagios:monitors', 'mysql:monitors')
-d.relate('nrpe:monitors', 'nagios:monitors')
-d.relate('nrpe:local-monitors', 'mysql:local-monitors')
-
-d.expose('nagios')
-
-try:
-    d.setup(timeout=seconds)
-except amulet.helpers.TimeoutError:
-    amulet.raise_status(amulet.SKIP, msg="Environment wasn't stood up in time")
-except:
-    raise
-
-
-##
-# Set relationship aliases
-##
-mysql_unit = d.sentry['mysql'][0]
-nagios_unit = d.sentry['nagios'][0]
-
-
-def test_nrpe_monitors_config():
-    # look for procrunning in nrpe config
-    try:
-        mysql_unit.file_contents('/etc/nagios/nrpe.d/'
-                                 'check_proc_mysqld_principle.cfg')
-    except IOError as e:
-        amulet.raise_status(amulet.ERROR,
-                            msg="procrunning config not found. Error:" +
-                            e.args[1])
-
-
-def test_nagios_monitors_response():
-    # look for mysql_database requests
-    nagpwd = nagios_unit.file_contents('/var/lib/juju/nagios.passwd').strip()
-    host_url = ("http://%s/cgi-bin/nagios3/status.cgi?"
-                "host=mysql-0")
-    r = requests.get(host_url % nagios_unit.info['public-address'],
-                     auth=('nagiosadmin', nagpwd))
-    if not r.text.find('mysql-0-basic'):
-        amulet.raise_status(amulet.ERROR,
-                            msg='Nagios is not monitoring the' +
-                            ' hosts it supposed to.')
-
-test_nrpe_monitors_config()
-test_nagios_monitors_response()

=== removed file 'tests/13-monitors-config'
--- tests/13-monitors-config	2016-02-03 22:17:17 +0000
+++ tests/13-monitors-config	1970-01-01 00:00:00 +0000
@@ -1,72 +0,0 @@
-#!/usr/bin/python3
-
-import amulet
-import requests
-
-seconds = 20000
-
-d = amulet.Deployment(series='trusty')
-
-d.add('nagios')
-d.add('mysql')
-d.add('nrpe')
-
-d.configure('mysql', {'dataset-size': '10%'})
-d.relate('nagios:monitors', 'mysql:monitors')
-d.relate('nrpe:monitors', 'nagios:monitors')
-d.relate('nrpe:local-monitors', 'mysql:local-monitors')
-
-d.configure('nrpe', {
-    'monitors': """
-    version: '0.3'
-    monitors:
-        remote: {}
-        local:
-            processcount:
-                all:
-                    max: 100
-    """
-})
-
-d.expose('nagios')
-
-try:
-    d.setup(timeout=seconds)
-except amulet.helpers.TimeoutError:
-    amulet.raise_status(amulet.SKIP, msg="Environment wasn't stood up in time")
-except:
-    raise
-
-
-##
-# Set relationship aliases
-##
-mysql_unit = d.sentry['mysql'][0]
-nagios_unit = d.sentry['nagios'][0]
-
-
-def test_nrpe_monitors_config():
-    # look for procrunning in nrpe config
-    try:
-        mysql_unit.file_contents('/etc/nagios/nrpe.d/'
-                                 'check_proc_mysqld_principle.cfg')
-    except IOError as e:
-        amulet.raise_status(amulet.ERROR,
-                            msg="procrunning config not found. Error:" +
-                            e.args[1])
-
-
-def test_nagios_monitors_response():
-    # look for mysql_database requests
-    nagpwd = nagios_unit.file_contents('/var/lib/juju/nagios.passwd').strip()
-    host_url = ("http://%s/cgi-bin/nagios3/status.cgi?"
-                "host=mysql-0")
-    r = requests.get(host_url % nagios_unit.info['public-address'],
-                     auth=('nagiosadmin', nagpwd))
-    if not r.text.find('processcount'):
-        amulet.raise_status(amulet.ERROR,
-                            msg='Nagios is not monitoring the' +
-                            ' hosts it supposed to.')
-
-test_nrpe_monitors_config()
-test_nagios_monitors_response()