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

[Stuart Bishop]

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.

[Review Queue (review-queue)]

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)

[lp:~stub/charms/trusty/cassandra/fix-diskchecks]

371. By Stuart Bishop on 2016-02-24

Fix unittests

372. By Stuart Bishop on 2016-03-17

Merge trunk, resolve conflicts

373. By Stuart Bishop on 2016-03-17

delint

[Cory Johns (johnsca)]

This is an excellent improvement, and has been merged.

review: Approve

=== modified file 'hooks/actions.py'
--- hooks/actions.py	2016-02-26 16:47:03 +0000
+++ hooks/actions.py	2016-03-17 11:13:01 +0000
@@ -825,7 +825,10 @@
     dirs = helpers.get_all_database_directories()
     dirs = set(dirs['data_file_directories'] +
                [dirs['commitlog_directory'], dirs['saved_caches_directory']])
-    for disk in dirs:
+    # We need to check the space on the mountpoint, not on the actual
+    # directory, as the nagios user won't have access to the actual directory.
+    mounts = set(helpers.mountpoint(d) for d in dirs)
+    for disk in mounts:
         check_name = re.sub('[^A-Za-z0-9_]', '_', disk)
         if cassandra_disk_warn and cassandra_disk_crit:
             shortname = "cassandra_disk{}".format(check_name)
 
=== modified file 'hooks/helpers.py'
--- hooks/helpers.py	2016-02-26 16:47:03 +0000
+++ hooks/helpers.py	2016-03-17 11:13:01 +0000
@@ -192,6 +192,14 @@
     return dirs
 
 
+def mountpoint(path):
+    '''Return the mountpoint that path exists on.'''
+    path = os.path.realpath(path)
+    while path != '/' and not os.path.ismount(path):
+        path = os.path.dirname(path)
+    return path
+
+
 # FOR CHARMHELPERS
 def is_lxc():
     '''Return True if we are running inside an LXC container.'''
 
=== modified file 'tests/test_actions.py'
--- tests/test_actions.py	2016-02-26 16:47:03 +0000
+++ tests/test_actions.py	2016-03-17 11:13:01 +0000
@@ -906,12 +906,15 @@
                                            call('1.1.0.2', 'any', 7002)],
                                           any_order=True)
 
+    @patch('helpers.mountpoint')
     @patch('helpers.get_cassandra_version')
     @patch('charmhelpers.core.host.write_file')
     @patch('charmhelpers.contrib.charmsupport.nrpe.NRPE')
     @patch('helpers.local_plugins_dir')
     def test_nrpe_external_master_relation(self, local_plugins_dir, nrpe,
-                                           write_file, cassandra_version):
+                                           write_file, cassandra_version,
+                                           mountpoint):
+        mountpoint.side_effect = os.path.dirname
         cassandra_version.return_value = '2.2'
         # The fake charm_dir() needs populating.
         plugin_src_dir = os.path.join(os.path.dirname(__file__),
@@ -936,16 +939,10 @@
                      description='Check Cassandra Heap',
                      check_cmd='check_cassandra_heap.sh localhost 80 90'),
                 call(description=('Check Cassandra Disk '
-                                  '/var/lib/cassandra/data'),
-                     shortname='cassandra_disk_var_lib_cassandra_data',
+                                  '/var/lib/cassandra'),
+                     shortname='cassandra_disk_var_lib_cassandra',
                      check_cmd=('check_disk -u GB -w 50% -c 25% -K 5% '
-                                '-p /var/lib/cassandra/data')),
-                call(description=ANY,
-                     shortname='cassandra_disk_var_lib_cassandra_saved_caches',
-                     check_cmd=ANY),
-                call(description=ANY,
-                     shortname='cassandra_disk_var_lib_cassandra_commitlog',
-                     check_cmd=ANY)],
+                                '-p /var/lib/cassandra'))],
                 any_order=True)
 
             nrpe().write.assert_called_once_with()
@@ -964,13 +961,15 @@
         actions.nrpe_external_master_relation('')
         self.assertFalse(write_file.called)
 
+    @patch('helpers.mountpoint')
     @patch('helpers.get_cassandra_version')
     @patch('os.path.exists')
     @patch('charmhelpers.contrib.charmsupport.nrpe.NRPE')
-    def test_nrpe_external_master_relation_disable_heapchk(self, nrpe,
-                                                           exists, ver):
+    def test_nrpe_external_master_relation_disable_heapchk(self, nrpe, exists,
+                                                           ver, mountpoint):
         ver.return_value = '2.2'
         exists.return_value = False
+        mountpoint.side_effect = os.path.dirname
 
         # Disable our checks
         config = hookenv.config()
@@ -981,11 +980,7 @@
         exists.assert_called_once_with(helpers.local_plugins_dir())
 
         nrpe().add_check.assert_has_calls([
-            call(shortname='cassandra_disk_var_lib_cassandra_data',
-                 description=ANY, check_cmd=ANY),
-            call(shortname='cassandra_disk_var_lib_cassandra_saved_caches',
-                 description=ANY, check_cmd=ANY),
-            call(shortname='cassandra_disk_var_lib_cassandra_commitlog',
+            call(shortname='cassandra_disk_var_lib_cassandra',
                  description=ANY, check_cmd=ANY)], any_order=True)
 
     @patch('helpers.get_cassandra_version')