GlusterFS: Ensure Cinder can write to shares

Ensure the Cinder user can write to the GlusterFS share.  This
is required for snapshot functionality, and means the admin
does not have to set this permission manually.

Closes-Bug: #1236966
Change-Id: I4a9ea40df9681ca6931ad6b390aa21b09d6cfec9

cinder/tests/test_glusterfs.py

@@ -19,6 +19,7 @@
 import errno
 import json
 import os
+import tempfile
 
 import mox as mox_lib
 from mox import IgnoreArg
@@ -35,6 +36,7 @@
 from cinder import test
 from cinder.tests.compute import test_nova
 from cinder import units
+from cinder import utils
 from cinder.volume import configuration as conf
 from cinder.volume.drivers import glusterfs
 
@@ -311,6 +313,11 @@ def test_ensure_share_mounted(self):
         mox = self._mox
         drv = self._driver
 
+        mox.StubOutWithMock(utils, 'get_file_mode')
+        mox.StubOutWithMock(utils, 'get_file_gid')
+        mox.StubOutWithMock(drv, '_execute')
+        mox.StubOutWithMock(drv, '_ensure_share_writable')
+
         mox.StubOutWithMock(drv, '_get_mount_point_for_share')
         drv._get_mount_point_for_share(self.TEST_EXPORT1).\
             AndReturn(self.TEST_MNT_POINT)
@@ -319,6 +326,15 @@ def test_ensure_share_mounted(self):
         drv._mount_glusterfs(self.TEST_EXPORT1, self.TEST_MNT_POINT,
                              ensure=True)
 
+        utils.get_file_gid(self.TEST_MNT_POINT).AndReturn(333333)
+
+        utils.get_file_mode(self.TEST_MNT_POINT).AndReturn(0o777)
+
+        drv._ensure_share_writable(self.TEST_MNT_POINT)
+
+        drv._execute('chgrp', IgnoreArg(), self.TEST_MNT_POINT,
+                     run_as_root=True)
+
         mox.ReplayAll()
 
         drv._ensure_share_mounted(self.TEST_EXPORT1)
@@ -399,6 +415,52 @@ def test_setup_should_throw_exception_if_client_is_not_installed(self):
 
         mox.VerifyAll()
 
+    def _fake_load_shares_config(self, conf):
+        self._driver.shares = {'127.7.7.7:/gluster1': None}
+
+    def _fake_NamedTemporaryFile(self, prefix=None, dir=None):
+        raise OSError('Permission denied!')
+
+    def test_setup_set_share_permissions(self):
+        mox = self._mox
+        drv = self._driver
+
+        glusterfs.CONF.glusterfs_shares_config = self.TEST_SHARES_CONFIG_FILE
+
+        self.stubs.Set(drv, '_load_shares_config',
+                       self._fake_load_shares_config)
+        self.stubs.Set(tempfile, 'NamedTemporaryFile',
+                       self._fake_NamedTemporaryFile)
+        mox.StubOutWithMock(os.path, 'exists')
+        mox.StubOutWithMock(drv, '_execute')
+        mox.StubOutWithMock(utils, 'get_file_gid')
+        mox.StubOutWithMock(utils, 'get_file_mode')
+        mox.StubOutWithMock(os, 'getegid')
+
+        drv._execute('mount.glusterfs', check_exit_code=False)
+
+        drv._execute('mkdir', '-p', mox_lib.IgnoreArg())
+
+        os.path.exists(self.TEST_SHARES_CONFIG_FILE).AndReturn(True)
+
+        drv._execute('mount', '-t', 'glusterfs', '127.7.7.7:/gluster1',
+                     mox_lib.IgnoreArg(), run_as_root=True)
+
+        utils.get_file_gid(mox_lib.IgnoreArg()).AndReturn(33333)
+        # perms not writable
+        utils.get_file_mode(mox_lib.IgnoreArg()).AndReturn(0o000)
+
+        os.getegid().AndReturn(888)
+
+        drv._execute('chgrp', 888, mox_lib.IgnoreArg(), run_as_root=True)
+        drv._execute('chmod', 'g+w', mox_lib.IgnoreArg(), run_as_root=True)
+
+        mox.ReplayAll()
+
+        drv.do_setup(IsA(context.RequestContext))
+
+        mox.VerifyAll()
+
     def test_find_share_should_throw_error_if_there_is_no_mounted_shares(self):
         """_find_share should throw error if there is no mounted shares."""
         drv = self._driver
@@ -766,6 +828,7 @@ def test_delete_snapshot_bottom(self):
         (mox, drv) = self._mox, self._driver
 
         hashed = drv._get_hash_str(self.TEST_EXPORT1)
+        volume_dir = os.path.join(self.TEST_MNT_POINT_BASE, hashed)
         volume_path = '%s/%s/volume-%s' % (self.TEST_MNT_POINT_BASE,
                                            hashed,
                                            self.VOLUME_UUID)
@@ -790,8 +853,11 @@ def test_delete_snapshot_bottom(self):
         mox.StubOutWithMock(drv, '_get_backing_chain_for_path')
         mox.StubOutWithMock(drv, '_get_matching_backing_file')
         mox.StubOutWithMock(drv, '_write_info_file')
+        mox.StubOutWithMock(drv, '_ensure_share_writable')
         mox.StubOutWithMock(image_utils, 'qemu_img_info')
 
+        drv._ensure_share_writable(volume_dir)
+
         img_info = imageutils.QemuImgInfo(qemu_img_info_output)
         image_utils.qemu_img_info(snap_path_2).AndReturn(img_info)
 
@@ -851,6 +917,7 @@ def test_delete_snapshot_middle(self):
 
         hashed = drv._get_hash_str(self.TEST_EXPORT1)
         volume_file = 'volume-%s' % self.VOLUME_UUID
+        volume_dir = os.path.join(self.TEST_MNT_POINT_BASE, hashed)
         volume_path = '%s/%s/%s' % (self.TEST_MNT_POINT_BASE,
                                     hashed,
                                     volume_file)
@@ -888,13 +955,16 @@ def test_delete_snapshot_middle(self):
         mox.StubOutWithMock(drv, '_write_info_file')
         mox.StubOutWithMock(drv, '_get_backing_chain_for_path')
         mox.StubOutWithMock(drv, 'get_active_image_from_info')
+        mox.StubOutWithMock(drv, '_ensure_share_writable')
         mox.StubOutWithMock(image_utils, 'qemu_img_info')
 
         info_file_dict = {self.SNAP_UUID_2: 'volume-%s.%s' %
                           (self.VOLUME_UUID, self.SNAP_UUID_2),
                           self.SNAP_UUID: 'volume-%s.%s' %
                           (self.VOLUME_UUID, self.SNAP_UUID)}
 
+        drv._ensure_share_writable(volume_dir)
+
         info_path = drv._local_path_volume(volume) + '.info'
         drv._read_info_file(info_path).AndReturn(info_file_dict)
 
@@ -1129,6 +1199,7 @@ def test_delete_snapshot_online_1(self):
 
         hashed = drv._get_hash_str(self.TEST_EXPORT1)
         volume_file = 'volume-%s' % self.VOLUME_UUID
+        volume_dir = os.path.join(self.TEST_MNT_POINT_BASE, hashed)
         volume_path = '%s/%s/%s' % (self.TEST_MNT_POINT_BASE,
                                     hashed,
                                     volume_file)
@@ -1144,10 +1215,13 @@ def test_delete_snapshot_online_1(self):
         mox.StubOutWithMock(os.path, 'exists')
         mox.StubOutWithMock(db, 'snapshot_get')
         mox.StubOutWithMock(image_utils, 'qemu_img_info')
+        mox.StubOutWithMock(drv, '_ensure_share_writable')
 
         snap_info = {'active': snap_file,
                      self.SNAP_UUID: snap_file}
 
+        drv._ensure_share_writable(volume_dir)
+
         drv._read_info_file(info_path).AndReturn(snap_info)
 
         os.path.exists(snap_path).AndReturn(True)
@@ -1214,6 +1288,7 @@ def test_delete_snapshot_online_2(self):
 
         hashed = drv._get_hash_str(self.TEST_EXPORT1)
         volume_file = 'volume-%s' % self.VOLUME_UUID
+        volume_dir = os.path.join(self.TEST_MNT_POINT_BASE, hashed)
         volume_path = '%s/%s/%s' % (self.TEST_MNT_POINT_BASE,
                                     hashed,
                                     volume_file)
@@ -1231,11 +1306,14 @@ def test_delete_snapshot_online_2(self):
         mox.StubOutWithMock(os.path, 'exists')
         mox.StubOutWithMock(db, 'snapshot_get')
         mox.StubOutWithMock(image_utils, 'qemu_img_info')
+        mox.StubOutWithMock(drv, '_ensure_share_writable')
 
         snap_info = {'active': snap_file_2,
                      self.SNAP_UUID: snap_file,
                      self.SNAP_UUID_2: snap_file_2}
 
+        drv._ensure_share_writable(volume_dir)
+
         drv._read_info_file(info_path).AndReturn(snap_info)
 
         os.path.exists(snap_path).AndReturn(True)

cinder/tests/test_utils.py

@@ -513,6 +513,57 @@ def test_create_channel(self):
 
         self.mox.VerifyAll()
 
+    def _make_fake_stat(self, test_file, orig_os_stat):
+        """Create a fake method to stub out os.stat().
+
+           Generate a function that will return a particular
+           stat object for a given file.
+
+           :param: test_file: file to spoof stat() for
+           :param: orig_os_stat: pointer to original os.stat()
+        """
+
+        def fake_stat(path):
+            if path == test_file:
+                class stat_result:
+                    st_mode = 0o777
+                    st_gid = 33333
+                return stat_result
+            else:
+                return orig_os_stat(path)
+
+        return fake_stat
+
+    def test_get_file_mode(self):
+        test_file = '/var/tmp/made_up_file'
+
+        orig_os_stat = os.stat
+        os.stat = self._make_fake_stat(test_file, orig_os_stat)
+
+        self.mox.ReplayAll()
+
+        mode = utils.get_file_mode(test_file)
+        self.assertEqual(mode, 0o777)
+
+        self.mox.VerifyAll()
+
+        os.stat = orig_os_stat
+
+    def test_get_file_gid(self):
+        test_file = '/var/tmp/made_up_file'
+
+        orig_os_stat = os.stat
+        os.stat = self._make_fake_stat(test_file, orig_os_stat)
+
+        self.mox.ReplayAll()
+
+        gid = utils.get_file_gid(test_file)
+        self.assertEqual(gid, 33333)
+
+        self.mox.VerifyAll()
+
+        os.stat = orig_os_stat
+
 
 class MonkeyPatchTestCase(test.TestCase):
     """Unit test for utils.monkey_patch()."""

cinder/utils.py

@@ -30,6 +30,7 @@
 import random
 import re
 import shutil
+import stat
 import sys
 import tempfile
 import time
@@ -819,3 +820,13 @@ def wrapper(self, *args, **kwargs):
             raise exception.DriverNotInitialized(driver=driver_name)
         return func(self, *args, **kwargs)
     return wrapper
+
+
+def get_file_mode(path):
+    """This primarily exists to make unit testing easier."""
+    return stat.S_IMODE(os.stat(path).st_mode)
+
+
+def get_file_gid(path):
+    """This primarily exists to make unit testing easier."""
+    return os.stat(path).st_gid

cinder/volume/drivers/glusterfs.py

@@ -20,6 +20,8 @@
 import json
 import os
 import re
+import stat
+import tempfile
 import time
 
 from oslo.config import cfg
@@ -108,6 +110,8 @@ def do_setup(self, context):
             else:
                 raise
 
+        self._ensure_shares_mounted()
+
     def check_for_setup_error(self):
         """Just to override parent behavior."""
         pass
@@ -552,6 +556,9 @@ def delete_snapshot(self, snapshot):
             msg = _('Volume status must be "available" or "in-use".')
             raise exception.InvalidVolume(msg)
 
+        self._ensure_share_writable(
+            self._local_volume_dir(snapshot['volume']))
+
         # Determine the true snapshot file for this snapshot
         #  based on the .info file
         info_path = self._local_path_volume(snapshot['volume']) + '.info'
@@ -979,13 +986,48 @@ def _ensure_shares_mounted(self):
 
         LOG.debug(_('Available shares: %s') % str(self._mounted_shares))
 
+    def _ensure_share_writable(self, path):
+        """Ensure that the Cinder user can write to the share.
+
+        If not, raise an exception.
+
+        :param path: path to test
+        :raises: GlusterfsException
+        :returns: None
+        """
+
+        prefix = '.cinder-write-test-' + str(os.getpid()) + '-'
+
+        try:
+            tempfile.NamedTemporaryFile(prefix=prefix, dir=path)
+        except OSError:
+            msg = _('GlusterFS share at %(dir)s is not writable by the '
+                    'Cinder volume service. Snapshot operations will not be '
+                    'supported.') % {'dir': path}
+            raise exception.GlusterfsException(msg)
+
     def _ensure_share_mounted(self, glusterfs_share):
         """Mount GlusterFS share.
         :param glusterfs_share: string
         """
         mount_path = self._get_mount_point_for_share(glusterfs_share)
         self._mount_glusterfs(glusterfs_share, mount_path, ensure=True)
 
+        # Ensure we can write to this share
+        group_id = os.getegid()
+        current_group_id = utils.get_file_gid(mount_path)
+        current_mode = utils.get_file_mode(mount_path)
+
+        if group_id != current_group_id:
+            cmd = ['chgrp', group_id, mount_path]
+            self._execute(*cmd, run_as_root=True)
+
+        if not (current_mode & stat.S_IWGRP):
+            cmd = ['chmod', 'g+w', mount_path]
+            self._execute(*cmd, run_as_root=True)
+
+        self._ensure_share_writable(mount_path)
+
     def _find_share(self, volume_size_for):
         """Choose GlusterFS share among available ones for given volume size.
         Current implementation looks for greatest capacity.

etc/cinder/rootwrap.d/volume.filters

@@ -67,6 +67,7 @@ find: CommandFilter, find, root
 
 # cinder/volume/drivers/glusterfs.py
 mv: CommandFilter, mv, root
+chgrp: CommandFilter, chgrp, root
 
 # cinder/volumes/drivers/hds/hds.py:
 hus-cmd: CommandFilter, hus-cmd, root