Merge lp:~paulgear/charms/trusty/memcached/correct-nagios-service into lp:charms/trusty/memcached

[Paul Gear]

[Review Queue (review-queue)]

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-11063-results

review: Needs Fixing (automated testing)

[Adam Israel (aisrael)]

Hi Paul,

Thanks for your work towards improving the memcached charm! I had the opportunity to review this merge proposal today. While the proposed change is minor, I've been unable to test it properly. I added the relation between nagios, nrpe, and memcached but the check_memcached.cfg isn't being rendered.

Ideally, this functionality should be documented in the README and have an amulet test added to demonstrate the nagios relation.

When you're ready, set the status of this MP back to 'Needs Review' and we'll be happy to retest!

review: Needs Fixing

[Paul Gear (paulgear)]

Hi Adam, Brad has provided documentation about this in one of the other MPs for memcached. If you add a relation to nrpe-external-master, it should work.

I'm no longer working on this project, so I don't have the ability to dedicate time to writing tests, but if you review the two nrpe* templates in http://bazaar.launchpad.net/~charmers/charms/trusty/memcached/trunk/files/head:/templates/, you'll see that this MP is critical for the nrpe-external-master relation to function at all (the variables simply don't match what the template is expecting). IMO, it is better for it to be working without a unit test than not working without a unit test.

[Adam Israel (aisrael)]

Hi Paul,

Documentation received and commented on. nrpe-external-master isn't currently packaged in trusty, but there is a rewrite of the nrpe charm that includes the nrpe-external-master relation pending some test fixes. Once that has been released, we can finish testing this merge proposal.

Thanks again for your work, and your patience!

[Adam Israel (aisrael)]

Hey Paul,

Thanks again for all of your work here. The documentation with regard to nrpe-external-master has been added via lp:250717. You have my +1.

review: Approve

[Paul Gear (paulgear)]

Thanks Adam. Do you need us to merge, or do you guys handle that?

[Adam Israel (aisrael)]

No worries! One of the ~charmers will be along in the morning, US time, to do the merge.

=== modified file 'hooks/memcached_hooks.py'
--- hooks/memcached_hooks.py	2015-02-20 16:15:18 +0000
+++ hooks/memcached_hooks.py	2015-02-24 07:03:47 +0000
@@ -290,9 +290,9 @@
 
     nagios_hostname = "{}-{}".format(config('nagios_context'),
                                      local_unit().replace('/', '-'))
-    configs = {'tcp_port': config('tcp_port'),
-               'servicegroup': config('nagios-context'),
-               'hostname': nagios_hostname}
+    configs = {'tcp_port': config('tcp-port'),
+               'NAGIOS_SERVICEGROUP': config('nagios_context'),
+               'NAGIOS_HOSTNAME': nagios_hostname}
 
     templating.render('nrpe_check_memcached.cfg.tmpl',
                       '/etc/nagios/nrpe.d/check_memcached.cfg',