Merge ~paulgear/charm-nrpe/+git/nrpe-charm:master into ~nrpe-charmers/charm-nrpe:master

Proposed by Paul Gear
Status: Merged
Merged at revision: 10198d38461b550b406ca945947533318fd71c8b
Proposed branch: ~paulgear/charm-nrpe/+git/nrpe-charm:master
Merge into: ~nrpe-charmers/charm-nrpe:master
Diff against target: 87 lines (+30/-9)
2 files modified
hooks/nrpe_utils.py (+28/-2)
hooks/services.py (+2/-7)
Reviewer Review Type Date Requested Status
Stuart Bishop (community) Approve
William Grant Approve
Review via email: mp+323818@code.launchpad.net

Description of the change

I spent a sizable chunk of a large ticket a few months back debugging why adding a relation made the nrpe charm generate incorrect hostnames when it had previously had correct ones. I think this is due to a timing issue because of when the nagios hostname is obtained vs. when it is used, and this change seems to help, but I'm not familiar enough with the services framework to know whether this is just luck.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) wrote :

This seems like a reasonable change. I'm not sure why the host definition would need a rewrite on nrpe-config, but it empirically has seemed to be the case in the past, and neither change can make things worse.

review: Approve
Revision history for this message
Paul Gear (paulgear) wrote :

This branch is still buggy; I've tested it and it causes host files to be removed on upgrade - still trying to work out why.

Revision history for this message
Paul Gear (paulgear) wrote :

I've pushed a new revision which fixes the host export file removal issue, and tested deploy on xenial and upgrade-charm on trusty & xenial.

Revision history for this message
William Grant (wgrant) :
review: Approve
Revision history for this message
Stuart Bishop (stub) wrote :

'ick. I'd forgotten service manager.

review: Approve
Revision history for this message
Paul Gear (paulgear) wrote :

This might fix lp:1671707

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/hooks/nrpe_utils.py b/hooks/nrpe_utils.py
index 89f8092..ae83158 100644
--- a/hooks/nrpe_utils.py
+++ b/hooks/nrpe_utils.py
@@ -4,8 +4,10 @@ import glob
44
5from charmhelpers import fetch5from charmhelpers import fetch
6from charmhelpers.core import host6from charmhelpers.core import host
7from charmhelpers.core.templating import render
8from charmhelpers.core import hookenv7from charmhelpers.core import hookenv
8from charmhelpers.core.services import helpers
9from charmhelpers.core.services.base import ManagerCallback
10from charmhelpers.core.templating import render
911
10import nrpe_helpers12import nrpe_helpers
1113
@@ -115,7 +117,7 @@ def process_local_monitors():
115117
116def update_nrpe_external_master_relation(service_name):118def update_nrpe_external_master_relation(service_name):
117 """119 """
118 Send updated nagsios_hostname to charms attached to nrpe_external_master120 Send updated nagios_hostname to charms attached to nrpe_external_master
119 relation.121 relation.
120 """122 """
121 principle_relation = nrpe_helpers.PrincipleRelation()123 principle_relation = nrpe_helpers.PrincipleRelation()
@@ -134,3 +136,27 @@ def update_monitor_relation(service_name):
134 relation_id=rid,136 relation_id=rid,
135 relation_settings=monitor_relation.provide_data()137 relation_settings=monitor_relation.provide_data()
136 )138 )
139
140
141class ExportManagerCallback(ManagerCallback):
142
143 """
144 This class exists in order to defer lookup of nagios_hostname()
145 until the template is ready to be rendered. This should reduce the
146 incidence of incorrectly-rendered hostnames in /var/lib/nagios/exports.
147 See charmhelpers.core.services.base.ManagerCallback and
148 charmhelpers.core.services.helpers.TemplateCallback for more background.
149 """
150
151 def __call__(self, manager, service_name, event_name):
152 nag_hostname = nrpe_helpers.PrincipleRelation().nagios_hostname()
153 target = '/var/lib/nagios/export/host__{}.cfg'.format(nag_hostname)
154 renderer = helpers.render_template(
155 source='export_host.cfg.tmpl',
156 target=target,
157 perms=0o644,
158 )
159 renderer(manager, service_name, event_name)
160
161
162create_host_export_fragment = ExportManagerCallback()
diff --git a/hooks/services.py b/hooks/services.py
index 9034981..543bcc6 100644
--- a/hooks/services.py
+++ b/hooks/services.py
@@ -8,7 +8,6 @@ import nrpe_helpers
88
9def manage():9def manage():
10 config = hookenv.config()10 config = hookenv.config()
11 nag_hostname = nrpe_helpers.PrincipleRelation().nagios_hostname()
12 manager = ServiceManager([11 manager = ServiceManager([
13 {12 {
14 'service': 'nrpe-install',13 'service': 'nrpe-install',
@@ -28,6 +27,7 @@ def manage():
28 'data_ready': [27 'data_ready': [
29 nrpe_utils.update_nrpe_external_master_relation,28 nrpe_utils.update_nrpe_external_master_relation,
30 nrpe_utils.update_monitor_relation,29 nrpe_utils.update_monitor_relation,
30 nrpe_utils.create_host_export_fragment,
31 nrpe_utils.render_nrped_files,31 nrpe_utils.render_nrped_files,
32 helpers.render_template(32 helpers.render_template(
33 source='nrpe.tmpl',33 source='nrpe.tmpl',
@@ -51,12 +51,7 @@ def manage():
51 source='rsync-juju.d.tmpl',51 source='rsync-juju.d.tmpl',
52 target='/etc/rsync-juju.d/010-nrpe-external-master.conf'52 target='/etc/rsync-juju.d/010-nrpe-external-master.conf'
53 ),53 ),
54 helpers.render_template(54 nrpe_utils.create_host_export_fragment,
55 source='export_host.cfg.tmpl',
56 target='/var/lib/nagios/export/'
57 'host__{}.cfg'.format(nag_hostname),
58 perms=0o644,
59 ),
60 ],55 ],
61 'start': [nrpe_utils.restart_rsync],56 'start': [nrpe_utils.restart_rsync],
62 },57 },

Subscribers

People subscribed via source and target branches