Merge "Removes getfattr from Quobyte Cinder driver"

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)

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)

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