Merge lp:~thedac/charm-helpers/set-primary-charm-on-nrpe-relation into lp:charm-helpers

[David Ames]

In IS we have scenarios where a unit may have its primary charm and multiple subordinate charms related to nrpe{,-external-master}.

The nrpe-external-master and nrpe charms determine the hostname to use by the charm service name when using nagios_hostname_type unit. This process is non-deterministic and the ordering of which charm is related to nrpe is significant.

This leads to situations where the hostname generated uses the subordinate charm's service name rather than the primary charm.

The only way to guarantee that the primary charm is used, is to set the information on the relation in the charm and then test for it in the nrpe-external-master charm.

This MP is one half of the solution. Primary charms will default to True and subordinate charms can set primary=False.

Explicitly set when a charm is the primary charm (vs a subordinate charm) in the npre-external-master relation.
For use when multiple charms primary and subordinate are related to nrpe{,-external-master} on the same unit.
The default is set to True as this is the case 99% of the time.
Only subordinate charms which are related to nrpe{,-external-master} need set primary=False on the relation.

[Stuart Bishop (stub)]

The code looks good. The test changes are more dubious - you have added a relation_set call, but the new code path is not being tested. relation_set is already mocked in the test harness, so something like the following:

    def test_init_primary(self):
        from unittest.mock import sentinel
        checker = nrpe.NRPE(primary=sentinel.primary)
        self.assertEqual(checker.primary, sentinel.primary)
        self.patched['relation_set'].assert_called_once_with(relation_settings={'primary': sentinel.primary})

[Stuart Bishop (stub)]

No longer needed with the new hookenv.principal_unit() call and modern Juju

review: Disapprove

Unmerged revisions

351. By David Ames on 2015-04-13

Document the primary setting on NRPE

350. By David Ames on 2015-04-13

Explicitly set when a charm is the primary charm (vs a subordinate charm) in the npre-external-master relation.
For use when multiple charms primary and subordinate are related to nrpe{,-external-master} on the same unit.
The default is set to True as this is the case 99% of the time.
Only subordinate charms which are related to nrpe{,-external-master} need set primary=False on the relation.

=== modified file 'charmhelpers/contrib/charmsupport/nrpe.py'
--- charmhelpers/contrib/charmsupport/nrpe.py	2015-02-19 07:49:41 +0000
+++ charmhelpers/contrib/charmsupport/nrpe.py	2015-04-13 23:10:18 +0000
@@ -110,6 +110,13 @@
 #    def local_monitors_relation_changed():
 #        update_nrpe_config()
 #
+# 4.a If your charm is a subordinate charm set primary=False
+#
+#    from charmsupport.nrpe import NRPE
+#    (...)
+#    def update_nrpe_config():
+#        nrpe_compat = NRPE(primary=False)
+#
 # 5. ln -s hooks.py nrpe-external-master-relation-changed
 #    ln -s hooks.py local-monitors-relation-changed
 
@@ -206,9 +213,10 @@
     nagios_exportdir = '/var/lib/nagios/export'
     nrpe_confdir = '/etc/nagios/nrpe.d'
 
-    def __init__(self, hostname=None):
+    def __init__(self, hostname=None, primary=True):
         super(NRPE, self).__init__()
         self.config = config()
+        self.primary = primary
         self.nagios_context = self.config['nagios_context']
         if 'nagios_servicegroups' in self.config and self.config['nagios_servicegroups']:
             self.nagios_servicegroups = self.config['nagios_servicegroups']
@@ -220,6 +228,12 @@
         else:
             self.hostname = "{}-{}".format(self.nagios_context, self.unit_name)
         self.checks = []
+        # Iff in an nrpe-external-master relation hook set primary status
+        relation = relation_ids()
+        if relation:
+            if relation[0].startswith('nrpe-external-master'):
+                log("Setting charm primary status {}".format(primary))
+                relation_set(relation_settings={'primary': self.primary})
 
     def add_check(self, *args, **kwargs):
         self.checks.append(Check(*args, **kwargs))

=== modified file 'tests/contrib/charmsupport/test_nrpe.py'
--- tests/contrib/charmsupport/test_nrpe.py	2015-02-19 08:41:54 +0000
+++ tests/contrib/charmsupport/test_nrpe.py	2015-04-13 23:10:18 +0000
@@ -87,8 +87,8 @@
         self.assertEqual(None, checker.write())
 
         expected = 'Nagios user not set up, nrpe checks not updated'
-        self.patched['log'].assert_called_once_with(expected)
-        self.check_call_counts(log=1, config=1, getpwnam=1, getgrnam=1)
+        self.patched['log'].assert_called_with(expected)
+        self.check_call_counts(log=2, config=1, getpwnam=1, getgrnam=1)
 
     def test_write_no_checker(self):
         self.patched['config'].return_value = {'nagios_context': 'test',
@@ -163,7 +163,7 @@
             relation_id="local-monitors:1", monitors=monitors)
         self.check_call_counts(config=1, getpwnam=1, getgrnam=1,
                                exists=3, open=2, listdir=1,
-                               relation_ids=1, relation_set=1)
+                               relation_ids=2, relation_set=1)
 
 
 class NRPECheckTestCase(NRPEBaseTestCase):