Merge lp:~chris-gondolin/charm-helpers/add-nagios-servicegroups-support into lp:charm-helpers

Proposed by Chris Stratford
Status: Superseded
Proposed branch: lp:~chris-gondolin/charm-helpers/add-nagios-servicegroups-support
Merge into: lp:charm-helpers
Diff against target: 142 lines (+28/-12)
2 files modified
charmhelpers/contrib/charmsupport/nrpe.py (+15/-5)
tests/contrib/charmsupport/test_nrpe.py (+13/-7)
To merge this branch: bzr merge lp:~chris-gondolin/charm-helpers/add-nagios-servicegroups-support
Reviewer Review Type Date Requested Status
Chris Glass (community) Needs Fixing
charmers Pending
Review via email: mp+230657@code.launchpad.net

This proposal has been superseded by a proposal from 2014-08-29.

Description of the change

Adds nagios_servicegroups support to nrpe.py, allowing alarms to be a bit more selective

To post a comment you must log in.
Revision history for this message
Chris Glass (tribaal) wrote :
Download full text (4.3 KiB)

This branch breaks unit tests unfortunately:

$ make test
[...]
ERROR: tests.contrib.charmsupport.test_nrpe.NRPECheckTestCase.test_check_write_nrpe_exportdir_not_accessible
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/home/tribaal/workspace/charm-helpers/trunk/tests/contrib/charmsupport/test_nrpe.py", line 193, in test_check_write_nrpe_exportdir_not_accessible
    self.assertEqual(None, check.write('testctx', 'hostname'))
TypeError: write() takes exactly 4 arguments (3 given)

======================================================================
ERROR: tests.contrib.charmsupport.test_nrpe.NRPECheckTestCase.test_write_removes_existing_config
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/home/tribaal/workspace/charm-helpers/trunk/tests/contrib/charmsupport/test_nrpe.py", line 183, in test_write_removes_existing_config
    self.assertEqual(None, check.write('testctx', 'hostname'))
TypeError: write() takes exactly 4 arguments (3 given)

======================================================================
ERROR: tests.contrib.charmsupport.test_nrpe.NRPETestCase.test_init_gets_config
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/home/tribaal/workspace/charm-helpers/trunk/tests/contrib/charmsupport/test_nrpe.py", line 55, in test_init_gets_config
    checker = nrpe.NRPE()
  File "/home/tribaal/workspace/charm-helpers/trunk/charmhelpers/contrib/charmsupport/nrpe.py", line 195, in __init__
    self.nagios_servicegroups = self.config['nagios_servicegroups']
KeyError: 'nagios_servicegroups'

======================================================================
ERROR: tests.contrib.charmsupport.test_nrpe.NRPETestCase.test_no_nagios_installed_bails
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/home/tribaal/workspace/charm-helpers/trunk/tests/contrib/charmsupport/test_nrpe.py", line 75, in test_no_nagios_installed_bails
    checker = nrpe.NRPE()
  File "/home/tribaal/workspace/charm-helpers/trunk/charmhelpers/contrib/charmsupport/nrpe.py", line 195, in __init__
    self.nagios_servicegroups = self.config['nagios_servicegroups']
KeyError: 'nagios_servicegroups'

======================================================================
ERROR: tests.contrib.charmsupport.test_nrpe.NRPETestCase.test_update_nrpe
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/home/tribaal/workspace/charm-helpers/trunk/tests/contrib/charmsupport/test_nrpe.py", line 109, in test_update_nrpe
    checker = nrpe.NRPE()
  File "/home/tribaal/workspace/charm-helpers/trunk/charmhelpers/contrib/charmsupport/nrpe.py", line 195, in __init__
    self.nagios_servicegroups = self.config['nagios_servicegroups']
KeyError: 'nagios_servicegroups'

======================================================================
ERROR: tests.contrib.charmsupport.test_nrpe.NRPE...

Read more...

review: Needs Fixing
196. By Chris Stratford

[chriss] Fix tests for nrpe with nagios_sericegroups

Revision history for this message
Chris Stratford (chris-gondolin) wrote :

The tests pass now

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/contrib/charmsupport/nrpe.py'
2--- charmhelpers/contrib/charmsupport/nrpe.py 2014-05-02 13:19:21 +0000
3+++ charmhelpers/contrib/charmsupport/nrpe.py 2014-08-29 13:49:47 +0000
4@@ -54,6 +54,12 @@
5 # juju-myservice-0
6 # If you're running multiple environments with the same services in them
7 # this allows you to differentiate between them.
8+# nagios_servicegroups:
9+# default: ""
10+# type: string
11+# description: |
12+# A comma-separated list of nagios servicegroups.
13+# If left empty, the nagios_context will be used as the servicegroup
14 #
15 # 3. Add custom checks (Nagios plugins) to files/nrpe-external-master
16 #
17@@ -138,7 +144,7 @@
18 log('Check command not found: {}'.format(parts[0]))
19 return ''
20
21- def write(self, nagios_context, hostname):
22+ def write(self, nagios_context, hostname, nagios_servicegroups):
23 nrpe_check_file = '/etc/nagios/nrpe.d/{}.cfg'.format(
24 self.command)
25 with open(nrpe_check_file, 'w') as nrpe_check_config:
26@@ -150,16 +156,19 @@
27 log('Not writing service config as {} is not accessible'.format(
28 NRPE.nagios_exportdir))
29 else:
30- self.write_service_config(nagios_context, hostname)
31+ self.write_service_config(nagios_context, hostname, nagios_servicegroups)
32
33- def write_service_config(self, nagios_context, hostname):
34+ def write_service_config(self, nagios_context, hostname, nagios_servicegroups):
35 for f in os.listdir(NRPE.nagios_exportdir):
36 if re.search('.*{}.cfg'.format(self.command), f):
37 os.remove(os.path.join(NRPE.nagios_exportdir, f))
38
39+ if not nagios_servicegroups:
40+ nagios_servicegroups = nagios_context
41+
42 templ_vars = {
43 'nagios_hostname': hostname,
44- 'nagios_servicegroup': nagios_context,
45+ 'nagios_servicegroup': nagios_servicegroups,
46 'description': self.description,
47 'shortname': self.shortname,
48 'command': self.command,
49@@ -183,6 +192,7 @@
50 super(NRPE, self).__init__()
51 self.config = config()
52 self.nagios_context = self.config['nagios_context']
53+ self.nagios_servicegroups = self.config['nagios_servicegroups']
54 self.unit_name = local_unit().replace('/', '-')
55 if hostname:
56 self.hostname = hostname
57@@ -208,7 +218,7 @@
58 nrpe_monitors = {}
59 monitors = {"monitors": {"remote": {"nrpe": nrpe_monitors}}}
60 for nrpecheck in self.checks:
61- nrpecheck.write(self.nagios_context, self.hostname)
62+ nrpecheck.write(self.nagios_context, self.hostname, self.nagios_servicegroups)
63 nrpe_monitors[nrpecheck.shortname] = {
64 "command": nrpecheck.command,
65 }
66
67=== modified file 'tests/contrib/charmsupport/test_nrpe.py'
68--- tests/contrib/charmsupport/test_nrpe.py 2014-05-23 18:58:56 +0000
69+++ tests/contrib/charmsupport/test_nrpe.py 2014-08-29 13:49:47 +0000
70@@ -50,11 +50,13 @@
71 class NRPETestCase(NRPEBaseTestCase):
72
73 def test_init_gets_config(self):
74- self.patched['config'].return_value = {'nagios_context': 'testctx'}
75+ self.patched['config'].return_value = {'nagios_context': 'testctx',
76+ 'nagios_servicegroups': 'testsgrps'}
77
78 checker = nrpe.NRPE()
79
80 self.assertEqual('testctx', checker.nagios_context)
81+ self.assertEqual('testsgrps', checker.nagios_servicegroups)
82 self.assertEqual('testunit', checker.unit_name)
83 self.assertEqual('testctx-testunit', checker.hostname)
84 self.check_call_counts(config=1)
85@@ -70,7 +72,8 @@
86 self.assertEqual(checker.hostname, hostname)
87
88 def test_no_nagios_installed_bails(self):
89- self.patched['config'].return_value = {'nagios_context': 'test'}
90+ self.patched['config'].return_value = {'nagios_context': 'test',
91+ 'nagios_servicegroups': ''}
92 self.patched['getgrnam'].side_effect = KeyError
93 checker = nrpe.NRPE()
94
95@@ -81,7 +84,8 @@
96 self.check_call_counts(log=1, config=1, getpwnam=1, getgrnam=1)
97
98 def test_write_no_checker(self):
99- self.patched['config'].return_value = {'nagios_context': 'test'}
100+ self.patched['config'].return_value = {'nagios_context': 'test',
101+ 'nagios_servicegroups': ''}
102 self.patched['exists'].return_value = True
103 checker = nrpe.NRPE()
104
105@@ -90,7 +94,8 @@
106 self.check_call_counts(config=1, getpwnam=1, getgrnam=1, exists=1)
107
108 def test_write_restarts_service(self):
109- self.patched['config'].return_value = {'nagios_context': 'test'}
110+ self.patched['config'].return_value = {'nagios_context': 'test',
111+ 'nagios_servicegroups': ''}
112 self.patched['exists'].return_value = True
113 checker = nrpe.NRPE()
114
115@@ -102,7 +107,8 @@
116 exists=1, call=1)
117
118 def test_update_nrpe(self):
119- self.patched['config'].return_value = {'nagios_context': 'a'}
120+ self.patched['config'].return_value = {'nagios_context': 'a',
121+ 'nagios_servicegroups': ''}
122 self.patched['exists'].return_value = True
123 self.patched['relation_ids'].return_value = ['local-monitors:1']
124
125@@ -180,7 +186,7 @@
126 'foo', 'bar.cfg', 'check_shortname.cfg']
127 check = nrpe.Check('shortname', 'description', '/some/command')
128
129- self.assertEqual(None, check.write('testctx', 'hostname'))
130+ self.assertEqual(None, check.write('testctx', 'hostname', 'testsgrp'))
131
132 expected = '/var/lib/nagios/export/check_shortname.cfg'
133 self.patched['remove'].assert_called_once_with(expected)
134@@ -190,7 +196,7 @@
135 self.patched['exists'].return_value = False
136 check = nrpe.Check('shortname', 'description', '/some/command')
137
138- self.assertEqual(None, check.write('testctx', 'hostname'))
139+ self.assertEqual(None, check.write('testctx', 'hostname', 'testsgrps'))
140 expected = ('Not writing service config as '
141 '/var/lib/nagios/export is not accessible')
142 self.patched['log'].assert_has_calls(

Subscribers

People subscribed via source and target branches