Merge ~xavpaice/charm-thruk-external-agent:feature/max_check_attempts into charm-thruk-external-agent:master

[Xav Paice]

[Linda Guo (lihuiguo)]

LGTM

review: Approve

[Peter Sabaini (peter-sabaini)]

Hey,

similarly to the comment on https://code.launchpad.net/~xavpaice/charm-graylog/+git/graylog-charm/+merge/397950 --

in general lgtm, but I believe charmhelpers.contrib.charmsupport.nrpe currently don't set a default (also cf. https://github.com/juju/charm-helpers/issues/564 ).

We'd need to handle defaults either here or in charmhelpers.

Personally I'd lean (explicit is better than implicit) to configure an explicit default=4 and make it type=int for the max_check_attempts in config.yaml but either here or in charmhelpers works

[Xav Paice (xavpaice)]

> Hey,
>
> similarly to the comment on https://code.launchpad.net/~xavpaice/charm-
> graylog/+git/graylog-charm/+merge/397950 --
>
> in general lgtm, but I believe charmhelpers.contrib.charmsupport.nrpe
> currently don't set a default (also cf. https://github.com/juju/charm-
> helpers/issues/564 ).
>
> We'd need to handle defaults either here or in charmhelpers.
>
> Personally I'd lean (explicit is better than implicit) to configure an
> explicit default=4 and make it type=int for the max_check_attempts in
> config.yaml but either here or in charmhelpers works

Worth checking the pattern with NTP, what we have done there is the same as here, the Nagios charm itself doesn't set the parameter at all if it's not listed and therefore uses the default in the service definition template.

[Celia Wang (ziyiwang)]

+1

review: Approve

diff --git a/config.yaml b/config.yaml
index 7818881..7c340fc 100644
--- a/config.yaml
+++ b/config.yaml
@@ -41,3 +41,10 @@ options:
         Name and location of the remotely trusty ssl cert.  Basename of this file
         will be used as the basename of the chain file rooted at
         /etc/ssl/certs.
+  max_check_attempts:
+    default: ""
+    type: string
+    description: >
+        The maxiumum number of attempts to check the remote agent service before switching an
+        alert to HARD status rather than SOFT.  Default for Nagios is 4, if this is set,
+        the default will be overwritten for the remote Thruk agent check only.
diff --git a/hooks/actions.py b/hooks/actions.py
index f3efdd7..d09c35d 100644
--- a/hooks/actions.py
+++ b/hooks/actions.py
@@ -56,6 +56,7 @@ def update_nrpe_config(service_name):
         shortname=shortname,
         description="Check Nagios PID via thruk agent",
         check_cmd=check_cmd,
+        max_check_attempts=hookenv.config("max_check_attempts"),
         )
     nrpe_compat.write()