diff --git a/cinder/tests/unit/volume/drivers/test_quobyte.py b/cinder/tests/unit/volume/drivers/test_quobyte.py index de5169cf9e5..2544b0389a8 100644 --- a/cinder/tests/unit/volume/drivers/test_quobyte.py +++ b/cinder/tests/unit/volume/drivers/test_quobyte.py @@ -17,6 +17,7 @@ import errno import os +import psutil import six import traceback @@ -100,6 +101,12 @@ class QuobyteDriverTestCase(test.TestCase): if not caught: self.fail('Expected raised exception but nothing caught.') + def get_mock_partitions(self): + mypart = mock.Mock() + mypart.device = "quobyte@" + mypart.mountpoint = self.TEST_MNT_POINT + return [mypart] + def test_local_path(self): """local_path common use case.""" drv = self._driver @@ -113,7 +120,9 @@ class QuobyteDriverTestCase(test.TestCase): def test_mount_quobyte_should_mount_correctly(self): with mock.patch.object(self._driver, '_execute') as mock_execute, \ mock.patch('cinder.volume.drivers.quobyte.QuobyteDriver' - '.read_proc_mount') as mock_open: + '.read_proc_mount') as mock_open, \ + mock.patch('cinder.volume.drivers.quobyte.QuobyteDriver' + '._validate_volume') as mock_validate: # Content of /proc/mount (not mounted yet). mock_open.return_value = six.StringIO( "/dev/sda5 / ext4 rw,relatime,data=ordered 0 0") @@ -127,17 +136,15 @@ class QuobyteDriverTestCase(test.TestCase): 'mount.quobyte', self.TEST_QUOBYTE_VOLUME, self.TEST_MNT_POINT, run_as_root=False) - getfattr_call = mock.call( - 'getfattr', '-n', 'quobyte.info', self.TEST_MNT_POINT, - run_as_root=False) - mock_execute.assert_has_calls( - [mkdir_call, mount_call, getfattr_call], any_order=False) + [mkdir_call, mount_call], any_order=False) + mock_validate.called_once_with(self.TEST_MNT_POINT) def test_mount_quobyte_already_mounted_detected_seen_in_proc_mount(self): - with mock.patch.object(self._driver, '_execute') as mock_execute, \ + with mock.patch('cinder.volume.drivers.quobyte.QuobyteDriver' + '.read_proc_mount') as mock_open, \ mock.patch('cinder.volume.drivers.quobyte.QuobyteDriver' - '.read_proc_mount') as mock_open: + '._validate_volume') as mock_validate: # Content of /proc/mount (already mounted). mock_open.return_value = six.StringIO( "quobyte@%s %s fuse rw,nosuid,nodev,noatime,user_id=1000" @@ -146,10 +153,7 @@ class QuobyteDriverTestCase(test.TestCase): self._driver._mount_quobyte(self.TEST_QUOBYTE_VOLUME, self.TEST_MNT_POINT) - - mock_execute.assert_called_once_with( - 'getfattr', '-n', 'quobyte.info', self.TEST_MNT_POINT, - run_as_root=False) + mock_validate.assert_called_once_with(self.TEST_MNT_POINT) def test_mount_quobyte_should_suppress_and_log_already_mounted_error(self): """test_mount_quobyte_should_suppress_and_log_already_mounted_error @@ -928,3 +932,82 @@ class QuobyteDriverTestCase(test.TestCase): self.assertEqual("true", drv.configuration.nas_secure_file_permissions) self.assertFalse(drv._execute_as_root) + + @mock.patch.object(psutil, "disk_partitions") + @mock.patch.object(os, "stat") + def test_validate_volume_all_good(self, stat_mock, part_mock): + part_mock.return_value = self.get_mock_partitions() + drv = self._driver + + def statMockCall(*args): + if args[0] == self.TEST_MNT_POINT: + stat_result = mock.Mock() + stat_result.st_size = 0 + return stat_result + return os.stat(args) + stat_mock.side_effect = statMockCall + + drv._validate_volume(self.TEST_MNT_POINT) + + stat_mock.assert_called_once_with(self.TEST_MNT_POINT) + part_mock.assert_called_once_with(all=True) + + @mock.patch.object(psutil, "disk_partitions") + @mock.patch.object(os, "stat") + def test_validate_volume_mount_not_working(self, stat_mock, part_mock): + part_mock.return_value = self.get_mock_partitions() + drv = self._driver + + def statMockCall(*args): + if args[0] == self.TEST_MNT_POINT: + raise exception.VolumeDriverException() + stat_mock.side_effect = [statMockCall, os.stat] + + self.assertRaises( + exception.VolumeDriverException, + drv._validate_volume, + self.TEST_MNT_POINT) + stat_mock.assert_called_once_with(self.TEST_MNT_POINT) + part_mock.assert_called_once_with(all=True) + + def test_validate_volume_no_mtab_entry(self): + msg = ("Volume driver reported an error: " + "No matching Quobyte mount entry for %(mpt)s" + " could be found for validation in partition list." + % {'mpt': self.TEST_MNT_POINT}) + + self.assertRaisesAndMessageMatches( + exception.VolumeDriverException, + msg, + self._driver._validate_volume, + self.TEST_MNT_POINT) + + @mock.patch.object(psutil, "disk_partitions") + def test_validate_volume_wrong_mount_type(self, part_mock): + mypart = mock.Mock() + mypart.device = "not-quobyte" + mypart.mountpoint = self.TEST_MNT_POINT + part_mock.return_value = [mypart] + msg = ("Volume driver reported an error: " + "The mount %(mpt)s is not a valid" + " Quobyte volume according to partition list." + % {'mpt': self.TEST_MNT_POINT}) + drv = self._driver + + self.assertRaisesAndMessageMatches( + exception.VolumeDriverException, + msg, + drv._validate_volume, + self.TEST_MNT_POINT) + part_mock.assert_called_once_with(all=True) + + @mock.patch.object(psutil, "disk_partitions") + def test_validate_volume_stale_mount(self, part_mock): + part_mock.return_value = self.get_mock_partitions() + drv = self._driver + + # As this uses a local fs dir size is >0, raising an exception + self.assertRaises( + exception.VolumeDriverException, + drv._validate_volume, + self.TEST_MNT_POINT) diff --git a/cinder/volume/drivers/quobyte.py b/cinder/volume/drivers/quobyte.py index 8d77e6544a5..c671709987e 100644 --- a/cinder/volume/drivers/quobyte.py +++ b/cinder/volume/drivers/quobyte.py @@ -16,6 +16,7 @@ import errno import os +import psutil from oslo_concurrency import processutils from oslo_config import cfg @@ -30,7 +31,7 @@ from cinder import interface from cinder import utils from cinder.volume.drivers import remotefs as remotefs_drv -VERSION = '1.1' +VERSION = '1.1.1' LOG = logging.getLogger(__name__) @@ -79,6 +80,8 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriverDistributed): Version history: 1.0 - Initial driver. 1.1 - Adds optional insecure NAS settings + 1.1.1 - Removes getfattr calls from driver + """ driver_volume_type = 'quobyte' @@ -465,16 +468,40 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriverDistributed): self._validate_volume(mount_path) def _validate_volume(self, mount_path): - """Wraps execute calls for checking validity of a Quobyte volume""" - command = ['getfattr', "-n", "quobyte.info", mount_path] - try: - self._execute(*command, run_as_root=self._execute_as_root) - except processutils.ProcessExecutionError as exc: - msg = (_("The mount %(mount_path)s is not a valid" - " Quobyte USP volume. Error: %(exc)s") - % {'mount_path': mount_path, 'exc': exc}) - raise exception.VolumeDriverException(msg) - - if not os.access(mount_path, os.W_OK | os.X_OK): - LOG.warning(_LW("Volume is not writable. Please broaden the file" - " permissions. Mount: %s"), mount_path) + """Runs a number of tests on the expect Quobyte mount""" + partitions = psutil.disk_partitions(all=True) + for p in partitions: + if mount_path == p.mountpoint: + if p.device.startswith("quobyte@"): + try: + statresult = os.stat(mount_path) + if statresult.st_size == 0: + # client looks healthy + if not os.access(mount_path, + os.W_OK | os.X_OK): + LOG.warning(_LW("Volume is not writable. " + "Please broaden the file" + " permissions." + " Mount: %s"), + mount_path) + return # we're happy here + else: + msg = (_("The mount %(mount_path)s is not a " + "valid Quobyte volume. Stale mount?") + % {'mount_path': mount_path}) + raise exception.VolumeDriverException(msg) + except Exception as exc: + msg = (_("The mount %(mount_path)s is not a valid" + " Quobyte volume. Error: %(exc)s . " + " Possibly a Quobyte client crash?") + % {'mount_path': mount_path, 'exc': exc}) + raise exception.VolumeDriverException(msg) + else: + msg = (_("The mount %(mount_path)s is not a valid" + " Quobyte volume according to partition list.") + % {'mount_path': mount_path}) + raise exception.VolumeDriverException(msg) + msg = (_("No matching Quobyte mount entry for %(mount_path)s" + " could be found for validation in partition list.") + % {'mount_path': mount_path}) + raise exception.VolumeDriverException(msg) diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters index 0451e6522ed..9d7acb9a2f5 100644 --- a/etc/cinder/rootwrap.d/volume.filters +++ b/etc/cinder/rootwrap.d/volume.filters @@ -224,6 +224,6 @@ ploop: CommandFilter, ploop, root drv_cfg: CommandFilter, /opt/emc/scaleio/sdc/bin/drv_cfg, root, /opt/emc/scaleio/sdc/bin/drv_cfg, --query_guid # cinder/volume/drivers/quobyte.py -getfattr: CommandFilter, getfattr, root -mount.quobyte: CommandFilter, getfattr, root +mount.quobyte: CommandFilter, mount.quobyte, root +umount.quobyte: CommandFilter, umount.quobyte, root