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

diff --git a/cinder/tests/test_glusterfs.py b/cinder/tests/test_glusterfs.py
index 58822be0477c31a21a743f31517c90859f716671..d2ac1fab982c91648ec33c990d05d456baba62f4 100644 (file)
--- a/cinder/tests/test_glusterfs.py
+++ b/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.openstack.common import processutils as putils
 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 @@ class GlusterFsDriverTestCase(test.TestCase):
         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 @@ class GlusterFsDriverTestCase(test.TestCase):
         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 @@ class GlusterFsDriverTestCase(test.TestCase):
 
         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 @@ class GlusterFsDriverTestCase(test.TestCase):
         (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 @@ class GlusterFsDriverTestCase(test.TestCase):
         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 @@ class GlusterFsDriverTestCase(test.TestCase):
 
         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,6 +955,7 @@ class GlusterFsDriverTestCase(test.TestCase):
         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' %
@@ -895,6 +963,8 @@ class GlusterFsDriverTestCase(test.TestCase):
                           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 @@ class GlusterFsDriverTestCase(test.TestCase):
 
         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 @@ class GlusterFsDriverTestCase(test.TestCase):
         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 @@ class GlusterFsDriverTestCase(test.TestCase):
 
         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 @@ class GlusterFsDriverTestCase(test.TestCase):
         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)

diff --git a/cinder/tests/test_utils.py b/cinder/tests/test_utils.py
index 7d77538eeaacb8d30b0aebfb2a650241353b15b9..cde2a96a5243c4ce0f3020b9ff932812b9370c1e 100644 (file)
--- a/cinder/tests/test_utils.py
+++ b/cinder/tests/test_utils.py
@@ -513,6 +513,57 @@ class GenericUtilsTestCase(test.TestCase):
 
         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()."""

diff --git a/cinder/utils.py b/cinder/utils.py
index de3d2481defad02cfde10919766a652602ed4a24..9e772890dfa62f5eadaa768b86a325c8da3c292b 100644 (file)
--- a/cinder/utils.py
+++ b/cinder/utils.py
@@ -30,6 +30,7 @@ import pyclbr
 import random
 import re
 import shutil
+import stat
 import sys
 import tempfile
 import time
@@ -819,3 +820,13 @@ def require_driver_initialized(func):
             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

diff --git a/cinder/volume/drivers/glusterfs.py b/cinder/volume/drivers/glusterfs.py
index 7f7497c37ed03b96f8e9799f5a2fc2d72d31de4a..eac70e106ebb0523e6e6c3546ec6df28775fbe02 100644 (file)
--- a/cinder/volume/drivers/glusterfs.py
+++ b/cinder/volume/drivers/glusterfs.py
@@ -20,6 +20,8 @@ import hashlib
 import json
 import os
 import re
+import stat
+import tempfile
 import time
 
 from oslo.config import cfg
@@ -108,6 +110,8 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
             else:
                 raise
 
+        self._ensure_shares_mounted()
+
     def check_for_setup_error(self):
         """Just to override parent behavior."""
         pass
@@ -552,6 +556,9 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
             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,6 +986,26 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
 
         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
@@ -986,6 +1013,21 @@ class GlusterfsDriver(nfs.RemoteFsDriver):
         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.

diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters
index c30aa089413205df13f6393f2608a7005aced4ab..11996250c9b4e1a2da59c8c69e4b581f94c5fb85 100644 (file)
--- a/etc/cinder/rootwrap.d/volume.filters
+++ b/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