Merge lp:~paulgear/charms/trusty/memcached/allow-rsync-firewall into lp:charms/trusty/memcached

[Paul Gear]

When fixing a couple of other bugs I realised that there is explicit support in the hooks for nrpe-external-master. Thus, a less complex fix for https://bugs.launchpad.net/charms/+source/memcached/+bug/1423439 is to allow rsync through the firewall when the relation is joined.

[Review Queue (review-queue)]

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

review: Needs Fixing (automated testing)

[Adam Israel (aisrael)]

Hi Paul,

Thanks again (and again) for your work on improving the memcached charm. This is a minor addition, but I'd like to see this new functionality added to the unit tests, and have the functionality documented in the README. It'll be good to see this added nagios work landed.

review: Needs Fixing

[Brad Marshall (brad-marshall)]

What new functionality are you talking about here? This is a simple addition of a firewall rule to make the existing functionality work after it was broken in the python rewrite. Without it, the existing code for nrpe-external-master is useless.

[Adam Israel (aisrael)]

Hi Brad,

There are two other related merge proposals from Paul that I tested, along with this one. I wasn't able to get any of the nrpe functionality to work. There's nothing in the README about its usage, nor any unit tests to verify functionality across substrates. At the very least, I'd like to see some instructions on using nrpe with the memcached charm, so that I can fully test that functionality.

[Brad Marshall (brad-marshall)]

Ah, I see. This is functionality that has been in the charm since 2013, hence my surprise that you were regarding it as new. And I believe it was working until someone did a python rewrite, and broke it. I'll look at writing up some quick instructions on how to use nrpe-external-master and getting it included.

[Adam Israel (aisrael)]

Hey Paul,

Thanks for the updated docs. Unfortunately, I'm still running into an issue.

`juju deploy nrpe-external-master` fails because there isn't a version of nrpe-external-master for Trusty in the charm store, and because it's a subordinate, the series must match.

Where are you deploying nrpe-external-master from?

[Adam Israel (aisrael)]

Hi Paul,

Following up on my previous comment. There is a rewrite of the trusty/npre charm that includes the nrpe-external-master relation, since nrpe-external-master isn't available in the charm store for trusty. If you can update the doc to reference nrpe instead of nrpe-external-master, we'll re-review this once the nrpe charm lands.

Thanks again for your work on this!

[Brad Marshall (brad-marshall)]

Adam,

> Where are you deploying nrpe-external-master from?

We are using a local copy of the charm from precise, checked out into a trusty directory.

> If you can update the doc to reference nrpe instead of nrpe-external-master,
> we'll re-review this once the nrpe charm lands.

We have production instances of memcached (and other charms) using the nrpe-external-master
relationship - both internally to Canonical and for customers. We need a working
nrpe-external-master interface to continue to support our customers, and these fixes address
shortcomings of the current code introduced in the python rewrite.

I have been talking to Liam about the upcoming nrpe rewrite and will be working with him to
get the charms on which we rely (at least) updated to support the new interface once it lands,
but it will probably be a phased approach to get rid of the nrpe-external-master interface.

Is there any reason not to fix the broken code now? Without this merge, we'll end up maintaining yet another separate branch for customers, which as you know is extra maintenance overhead.

Thanks,
Brad

[Adam Israel (aisrael)]

Hi Brad,

The problem I see is by promoting it to the charm store, we're expecting users to know that they need to manually fork the precise version of the nrpe-external-master charm to trusty and deploy from local. It's not an intuitive process.

If you could expand the README to demystify the process, I'd be happy to approve this in the interim.

[lp:~paulgear/charms/trusty/memcached/allow-rsync-firewall]

69. By Paul Gear on 2015-03-26

[bradm] Briefly explain how to use a local copy of the n-e-m charm.

[Brad Marshall (brad-marshall)]

Adam,

> The problem I see is by promoting it to the charm store, we're expecting
> users to know that they need to manually fork the precise version of the
> nrpe-external-master charm to trusty and deploy from local. It's not an
> intuitive process.

I agree, the whole n-e-m interface is one huge hack, since you have to have a copy of nagios running outside of Juju to use it. Its all working around not having cross environment relations. Its not pretty, but its what we have to work with for now.

> If you could expand the README to demystify the process, I'd be happy
> to approve this in the interim.

I've just added a quick description of how to use it - its a bit hacky, but its how we're using the charm for now. Once the nrpe rewrite gets landed we can update things to be neater.

Thanks,
Brad

[Adam Israel (aisrael)]

Thanks for all of the work, Paul and Brad! Looks good to me. +1

review: Approve

=== modified file 'README.md'
--- README.md	2015-02-17 20:45:19 +0000
+++ README.md	2015-03-26 01:05:18 +0000
@@ -71,6 +71,16 @@
 
 To add more units. Memcached doesn't share load, it's very simple and the clients have the intelligence to know which server to pick.
 
+## Nagios Monitoring
+
+To use this charm with nrpe-external-master:
+
+    juju deploy memcached
+    juju deploy local:trusty/nrpe-external-master
+    juju add-relation memcached nrpe-external-master
+
+For more details see the [nrpe-external-master charm](https://jujucharms.com/q/?text=nrpe-external-master).  Until there is a version of nrpe-external-master in trusty, you will need to use a local copy of the precise charm branched into an appropriate trusty directory.
+
 ## Known Limitations and Issues
 
 # Configuration

=== modified file 'hooks/memcached_hooks.py'
--- hooks/memcached_hooks.py	2015-03-11 20:13:57 +0000
+++ hooks/memcached_hooks.py	2015-03-26 01:05:18 +0000
@@ -278,6 +278,9 @@
     apt_install(['nagios-nrpe-server'], fatal=True)
     ufw.service('5666', 'open')
 
+    # make sure rsync port is open for check collection by external master
+    ufw.service('873/tcp', 'open')
+
     if not os.path.isdir(LOCAL_NAGIOS_PLUGINS):
         os.makedirs(LOCAL_NAGIOS_PLUGINS)