openstack/cinder
Code Issues Proposed changes

Merge "GlusterFS: Ensure Cinder can write to shares"

Browse Source
This commit is contained in:
Jenkins 2013-11-30 05:22:56 +00:00 committed by Gerrit Code Review
commit 7a31127bfb
5 changed files with 183 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
cinder/tests/test_glusterfs.py
View File

@ -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)

51
cinder/tests/test_utils.py
View File

@ -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()."""

11
cinder/utils.py
View File

@ -30,6 +30,7 @@ import pyclbr
import random
import re
import shutil
import stat
import sys
import tempfile
import time
@ -821,3 +822,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

42
cinder/volume/drivers/glusterfs.py
View File

@ -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
@ -554,6 +558,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'
@ -981,6 +988,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
@ -988,6 +1015,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.

1
etc/cinder/rootwrap.d/volume.filters
View File

@ -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