Merge lp:~stub/charms/trusty/cassandra/fix-diskchecks into lp:charms/trusty/cassandra

Proposed by Stuart Bishop
Status: Merged
Merged at revision: 374
Proposed branch: lp:~stub/charms/trusty/cassandra/fix-diskchecks
Merge into: lp:charms/trusty/cassandra
Diff against target: 106 lines (+24/-18)
3 files modified
hooks/actions.py (+4/-1)
hooks/helpers.py (+8/-0)
tests/test_actions.py (+12/-17)
To merge this branch: bzr merge lp:~stub/charms/trusty/cassandra/fix-diskchecks
Reviewer Review Type Date Requested Status
Cory Johns Approve
Review Queue (community) automated testing Needs Fixing
Review via email: mp+283760@code.launchpad.net

Description of the change

Nagios disk space monitoring would fail if the various directories being monitored where not acessible to the nagios user.

This branch fixes this by instead determining the mount points of the interesting directories and checking available space on them. This also removes redundant checks, such as the common situation of the commitlog and saved_caches directories both being on the root volume.

To post a comment you must log in.
Revision history for this message
Review Queue (review-queue) wrote :

This item has failed automated testing! Results available here http://juju-ci.vapour.ws:8080/job/charm-bundle-test-lxc/2420/

review: Needs Fixing (automated testing)
371. By Stuart Bishop

Fix unittests

372. By Stuart Bishop

Merge trunk, resolve conflicts

373. By Stuart Bishop

delint

Revision history for this message
Cory Johns (johnsca) wrote :

This is an excellent improvement, and has been merged.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hooks/actions.py'
2--- hooks/actions.py 2016-02-26 16:47:03 +0000
3+++ hooks/actions.py 2016-03-17 11:13:01 +0000
4@@ -825,7 +825,10 @@
5 dirs = helpers.get_all_database_directories()
6 dirs = set(dirs['data_file_directories'] +
7 [dirs['commitlog_directory'], dirs['saved_caches_directory']])
8- for disk in dirs:
9+ # We need to check the space on the mountpoint, not on the actual
10+ # directory, as the nagios user won't have access to the actual directory.
11+ mounts = set(helpers.mountpoint(d) for d in dirs)
12+ for disk in mounts:
13 check_name = re.sub('[^A-Za-z0-9_]', '_', disk)
14 if cassandra_disk_warn and cassandra_disk_crit:
15 shortname = "cassandra_disk{}".format(check_name)
16
17=== modified file 'hooks/helpers.py'
18--- hooks/helpers.py 2016-02-26 16:47:03 +0000
19+++ hooks/helpers.py 2016-03-17 11:13:01 +0000
20@@ -192,6 +192,14 @@
21 return dirs
22
23
24+def mountpoint(path):
25+ '''Return the mountpoint that path exists on.'''
26+ path = os.path.realpath(path)
27+ while path != '/' and not os.path.ismount(path):
28+ path = os.path.dirname(path)
29+ return path
30+
31+
32 # FOR CHARMHELPERS
33 def is_lxc():
34 '''Return True if we are running inside an LXC container.'''
35
36=== modified file 'tests/test_actions.py'
37--- tests/test_actions.py 2016-02-26 16:47:03 +0000
38+++ tests/test_actions.py 2016-03-17 11:13:01 +0000
39@@ -906,12 +906,15 @@
40 call('1.1.0.2', 'any', 7002)],
41 any_order=True)
42
43+ @patch('helpers.mountpoint')
44 @patch('helpers.get_cassandra_version')
45 @patch('charmhelpers.core.host.write_file')
46 @patch('charmhelpers.contrib.charmsupport.nrpe.NRPE')
47 @patch('helpers.local_plugins_dir')
48 def test_nrpe_external_master_relation(self, local_plugins_dir, nrpe,
49- write_file, cassandra_version):
50+ write_file, cassandra_version,
51+ mountpoint):
52+ mountpoint.side_effect = os.path.dirname
53 cassandra_version.return_value = '2.2'
54 # The fake charm_dir() needs populating.
55 plugin_src_dir = os.path.join(os.path.dirname(__file__),
56@@ -936,16 +939,10 @@
57 description='Check Cassandra Heap',
58 check_cmd='check_cassandra_heap.sh localhost 80 90'),
59 call(description=('Check Cassandra Disk '
60- '/var/lib/cassandra/data'),
61- shortname='cassandra_disk_var_lib_cassandra_data',
62+ '/var/lib/cassandra'),
63+ shortname='cassandra_disk_var_lib_cassandra',
64 check_cmd=('check_disk -u GB -w 50% -c 25% -K 5% '
65- '-p /var/lib/cassandra/data')),
66- call(description=ANY,
67- shortname='cassandra_disk_var_lib_cassandra_saved_caches',
68- check_cmd=ANY),
69- call(description=ANY,
70- shortname='cassandra_disk_var_lib_cassandra_commitlog',
71- check_cmd=ANY)],
72+ '-p /var/lib/cassandra'))],
73 any_order=True)
74
75 nrpe().write.assert_called_once_with()
76@@ -964,13 +961,15 @@
77 actions.nrpe_external_master_relation('')
78 self.assertFalse(write_file.called)
79
80+ @patch('helpers.mountpoint')
81 @patch('helpers.get_cassandra_version')
82 @patch('os.path.exists')
83 @patch('charmhelpers.contrib.charmsupport.nrpe.NRPE')
84- def test_nrpe_external_master_relation_disable_heapchk(self, nrpe,
85- exists, ver):
86+ def test_nrpe_external_master_relation_disable_heapchk(self, nrpe, exists,
87+ ver, mountpoint):
88 ver.return_value = '2.2'
89 exists.return_value = False
90+ mountpoint.side_effect = os.path.dirname
91
92 # Disable our checks
93 config = hookenv.config()
94@@ -981,11 +980,7 @@
95 exists.assert_called_once_with(helpers.local_plugins_dir())
96
97 nrpe().add_check.assert_has_calls([
98- call(shortname='cassandra_disk_var_lib_cassandra_data',
99- description=ANY, check_cmd=ANY),
100- call(shortname='cassandra_disk_var_lib_cassandra_saved_caches',
101- description=ANY, check_cmd=ANY),
102- call(shortname='cassandra_disk_var_lib_cassandra_commitlog',
103+ call(shortname='cassandra_disk_var_lib_cassandra',
104 description=ANY, check_cmd=ANY)], any_order=True)
105
106 @patch('helpers.get_cassandra_version')

Subscribers

People subscribed via source and target branches

to all changes: