Merge lp:~brad-marshall/charms/trusty/nagios/add-livestatus-support into lp:charms/trusty/nagios

Proposed by Brad Marshall
Status: Merged
Merged at revision: 31
Proposed branch: lp:~brad-marshall/charms/trusty/nagios/add-livestatus-support
Merge into: lp:charms/trusty/nagios
Diff against target: 227 lines (+147/-2)
5 files modified
README.md (+4/-0)
config.yaml (+10/-0)
hooks/install (+18/-0)
hooks/upgrade-charm (+72/-2)
tests/23-livestatus-test (+43/-0)
To merge this branch: bzr merge lp:~brad-marshall/charms/trusty/nagios/add-livestatus-support
Reviewer Review Type Date Requested Status
Charles Butler (community) Approve
Review via email: mp+257992@code.launchpad.net

Description of the change

Add livestatus support, which can't be done via the extraconfig since the broker_module needs to be in the main nagios.cfg file.

To post a comment you must log in.
21. By Brad Marshall

[bradm] Merge from upstream, had started with lp:charms/nagios instead of lp:charms/trusty/nagios

22. By Brad Marshall

[bradm] Add test for livestatus config variable

Revision history for this message
Brad Marshall (brad-marshall) wrote :

Added tests for the livestatus path existing, although I'm not sure I did the config-get correctly - it feels like there should be a better way in amulet than a unit.run('config-get <var>') - please let me know if there is.

Revision history for this message
Adam Israel (aisrael) wrote :

Hi!

I'm doing a triage of reviews in the queue, in the spirit of fail-fast. This is a cursory check of the proposed merge to see if there are any "obvious" things that may prevent the review from being accepted.

The goal is to get you feedback sooner, before an in-depth review has been done. I'm happy to see the addition of a unit test for the new functionality. Could you also add something to the README to explains the usage of this new feature?

Thanks!

23. By Brad Marshall

[bradm] Add documentation about livestatus config options, tidy up comments

Revision history for this message
Brad Marshall (brad-marshall) wrote :

I've added some basic documentation about the config variables.

Revision history for this message
Charles Butler (lazypower) wrote :

Greetings Brad,

I took some time to review this and would like to thank you for the contribution. The test coverage + documentation updates.

In response to your earlier question about running config-get, that is one way of doing it or you can read directly from the config dictionary you supply to the charm in the test. However if you did not supply one - your method was correct. To obtain the data from juju-run.

I found some test failures that seem unrelated to your merge. I filed them under this existing bug about tests failing in CI here: https://bugs.launchpad.net/charms/+source/nagios/+bug/1403574

+1 LGTM. Thank you for taking the time to submit this feature for the nagios charm. We appreciate your work. I've merged this branch and it should be available in the charm store after the next ingestion.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'README.md'
2--- README.md 2014-04-11 14:06:48 +0000
3+++ README.md 2015-05-07 07:07:58 +0000
4@@ -28,6 +28,10 @@
5
6 # Configuration
7
8+- `enable_livestatus` - Setting to enable the [livestatus module](https://mathias-kettner.de/checkmk_livestatus.html). This is an easy interface to get data out of Nagios.
9+
10+- `livestatus_path` - Configuration of where the livestatus module is stored - defaults to /var/lib/nagios3/livestatus/socket.
11+
12 ### SSL Configuration
13
14 - `ssl` - Determinant configuration for enabling SSL. Valid options are "on", "off", "only". The "only" option disables HTTP traffic on Apache in favor of HTTPS. This setting may cause unexpected behavior with existing nagios charm deployments.
15
16=== modified file 'config.yaml'
17--- config.yaml 2014-03-04 21:33:40 +0000
18+++ config.yaml 2015-05-07 07:07:58 +0000
19@@ -34,3 +34,13 @@
20 description: |
21 base64 encoded chain certificates file. If ssl_cert is
22 blank, this will be ignored.
23+ enable_livestatus:
24+ type: boolean
25+ default: false
26+ description: |
27+ Config variable to enable livestatus module or not.
28+ livestatus_path:
29+ type: string
30+ default: "/var/lib/nagios3/livestatus/socket"
31+ description: |
32+ Default path to livestatus socket, if enabled via enable_livestatus
33
34=== modified file 'hooks/install'
35--- hooks/install 2014-10-13 23:19:09 +0000
36+++ hooks/install 2015-05-07 07:07:58 +0000
37@@ -37,6 +37,24 @@
38 rm -v /etc/nagios3/conf.d/extinfo_nagios2.cfg
39 fi
40
41+enable_livestatus=$(config-get enable_livestatus)
42+livestatus_path=$(config-get livestatus_path)
43+livestatus_dir=$(dirname $livestatus_path)
44+
45+if [ "$enable_livestatus" ]; then
46+ # install check-mk-livestatus
47+ DEBIAN_FRONTEND=noninteractive apt-get -qy install check-mk-livestatus
48+ # enable livestatus broker
49+ if ! grep '^broker_module=' /etc/nagios3/nagios.cfg ; then
50+ echo "broker_module=/usr/lib/check_mk/livestatus.o $livestatus_path" >> /etc/nagios3/nagios.cfg
51+ fi
52+ # fix permissions on the livestatus directory
53+ mkdir -p $livestatus_dir
54+ chown nagios:www-data $livestatus_dir
55+ chmod g+w $livestatus_dir
56+ chmod g+s $livestatus_dir
57+fi
58+
59 if [ -f $CHARM_DIR/files/index.html ]; then
60 # Replace the default index.html file to redirect to nagios3/
61 cp -v $CHARM_DIR/files/index.html /var/www/html/index.html
62
63=== modified file 'hooks/upgrade-charm'
64--- hooks/upgrade-charm 2014-10-14 17:18:28 +0000
65+++ hooks/upgrade-charm 2015-05-07 07:07:58 +0000
66@@ -5,19 +5,28 @@
67 import base64
68 from jinja2 import Template
69 import os
70-import shutil
71+import re
72+import pwd
73+import grp
74+import stat
75+import errno
76+# import shutil
77 import subprocess
78 from charmhelpers.contrib import ssl
79 from charmhelpers.core import hookenv, host
80+from charmhelpers import fetch
81
82 from common import update_localhost
83
84 # Gather facts
85 legacy_relations = hookenv.config('legacy')
86 extra_config = hookenv.config('extraconfig')
87+enable_livestatus = hookenv.config('enable_livestatus')
88+livestatus_path = hookenv.config('livestatus_path')
89 ssl_config = hookenv.config('ssl')
90 charm_dir = os.environ['CHARM_DIR']
91 cert_domain = hookenv.unit_get('public-address')
92+nagios_cfg = "/etc/nagios3/nagios.cfg"
93
94
95 # Checks the charm relations for legacy relations
96@@ -44,6 +53,66 @@
97 host.write_file('/etc/nagios3/conf.d/extra.cfg', extra_config)
98
99
100+# Equivalent of mkdir -p, since we can't rely on
101+# python 3.2 os.makedirs exist_ok argument
102+def mkdir_p(path):
103+ try:
104+ os.makedirs(path)
105+ except OSError as exc: # Python >2.5
106+ if exc.errno == errno.EEXIST and os.path.isdir(path):
107+ pass
108+ else:
109+ raise
110+
111+
112+# Fix the path to be world executable
113+def fixpath(path):
114+ if os.path.isdir(path):
115+ st = os.stat(path)
116+ os.chmod(path, st.st_mode | stat.S_IXOTH)
117+ if path != "/":
118+ fixpath(os.path.split(path)[0])
119+
120+
121+def enable_livestatus_config():
122+ if enable_livestatus:
123+ hookenv.log("Livestatus is enabled")
124+ fetch.apt_update()
125+ fetch.apt_install('check-mk-livestatus')
126+ broker = re.compile("^broker_module=")
127+ broker_found = False
128+ for line in open(nagios_cfg):
129+ if broker.match(line):
130+ broker_found = True
131+ hookenv.log("broker_module line exists, not adding..")
132+ break
133+
134+ if not broker_found:
135+ with open(nagios_cfg, "a") as nagiosfile:
136+ broker_str = "broker_module=/usr/lib/check_mk/livestatus.o " . livestatus_path
137+ nagiosfile.write(broker_str)
138+ nagiosfile.close()
139+
140+ # Make the directory and fix perms on it
141+ hookenv.log("Fixing perms on livestatus_path")
142+ livestatus_path = hookenv.config('livestatus_path')
143+ livestatus_dir = os.path.dirname(livestatus_path)
144+ if not os.path.isdir(livestatus_dir):
145+ hookenv.log("Making path for livestatus_dir")
146+ mkdir_p(livestatus_dir)
147+ fixpath(livestatus_dir)
148+
149+ # Fix the perms on the socket
150+ hookenv.log("Fixing perms on the socket")
151+ uid = pwd.getpwnam("nagios").pw_uid
152+ gid = grp.getgrnam("www-data").gr_gid
153+ os.chown(livestatus_path, uid, gid)
154+ os.chown(livestatus_dir, uid, gid)
155+ st = os.stat(livestatus_path)
156+ os.chmod(livestatus_path, st.st_mode | stat.S_IRGRP)
157+ os.chmod(livestatus_dir, st.st_mode | stat.S_IRGRP | stat.S_ISGID)
158+
159+
160 def ssl_configured():
161 allowed_options = ["on", "only"]
162 if str(ssl_config).lower() in allowed_options:
163@@ -89,7 +158,7 @@
164 def enable_ssl():
165 # Set the basename of all ssl files
166
167- #Validate that we have configs, and generate a self signed certificate.
168+ # Validate that we have configs, and generate a self signed certificate.
169 if not hookenv.config('ssl_cert'):
170 # bail if keys already exist
171 if os.path.exists(cert_file):
172@@ -143,6 +212,7 @@
173
174 warn_legacy_relations()
175 write_extra_config()
176+enable_livestatus_config()
177 if ssl_configured():
178 enable_ssl()
179 update_apache()
180
181=== added file 'tests/23-livestatus-test'
182--- tests/23-livestatus-test 1970-01-01 00:00:00 +0000
183+++ tests/23-livestatus-test 2015-05-07 07:07:58 +0000
184@@ -0,0 +1,43 @@
185+#!/usr/bin/python3
186+
187+import amulet
188+# import requests
189+
190+seconds = 20000
191+
192+d = amulet.Deployment(series='trusty')
193+
194+d.add('nagios')
195+
196+d.expose('nagios')
197+
198+try:
199+ d.setup(timeout=seconds)
200+ d.sentry.wait()
201+except amulet.helpers.TimeoutError:
202+ amulet.raise_status(amulet.SKIP, msg="Environment wasn't stood up in time")
203+except:
204+ raise
205+
206+
207+##
208+# Set relationship aliases
209+##
210+nagios_unit = d.sentry.unit['nagios/0']
211+
212+d.configure('nagios', {
213+ 'enable_livestatus': True
214+})
215+
216+d.sentry.wait()
217+
218+
219+def test_livestatus_file_exists():
220+ livestatus_path = nagios_unit.run('config-get livestatus_path')
221+ try:
222+ livestatus_file = nagios_unit.file(livestatus_path[0])
223+ except OSError:
224+ message = "Can't find livestatus file"
225+ amulet.raise_status(amulet.FAIL, msg=message)
226+
227+test_livestatus_file_exists()

Subscribers

People subscribed via source and target branches

to all changes: