From 371fa540600b20b97eae389e1f976145866cadae Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Tue, 19 Nov 2013 18:01:55 -0500 Subject: [PATCH] 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 | 78 ++++++++++++++++++++++++++++ cinder/tests/test_utils.py | 51 ++++++++++++++++++ cinder/utils.py | 11 ++++ cinder/volume/drivers/glusterfs.py | 42 +++++++++++++++ etc/cinder/rootwrap.d/volume.filters | 1 + 5 files changed, 183 insertions(+) diff --git a/cinder/tests/test_glusterfs.py b/cinder/tests/test_glusterfs.py index 58822be04..d2ac1fab9 100644 --- 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 7d77538ee..cde2a96a5 100644 --- 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 de3d2481d..9e772890d 100644 --- 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 7f7497c37..eac70e106 100644 --- 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 c30aa0894..11996250c 100644 --- 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 -- 2.45.2