openstack/manila
Code Issues Proposed changes

Merge "Migrate GlusterFS to privsep style"

Browse Source
This commit is contained in:
Zuul 2022-10-10 15:05:30 +00:00 committed by Gerrit Code Review
commit b8fdf9b9e7
7 changed files with 115 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
manila/privsep/os.py
View File

@ -28,6 +28,16 @@ def rmdir(dir_path):
processutils.execute('rmdir', dir_path)
@manila.privsep.sys_admin_pctxt.entrypoint
def mkdir(dir_path):
processutils.execute('mkdir', dir_path)
@manila.privsep.sys_admin_pctxt.entrypoint
def recursive_forced_rm(dir_path):
processutils.execute('rm', '-rf', dir_path)
@manila.privsep.sys_admin_pctxt.entrypoint
def is_data_definition_direct_io_supported(src_str, dest_str):
try:
@ -57,8 +67,9 @@ def umount(mount_path):
@manila.privsep.sys_admin_pctxt.entrypoint
def mount(device_name, mount_path):
processutils.execute('mount', device_name, mount_path)
def mount(device_name, mount_path, mount_type=None):
extra_args = ['-t', mount_type] if mount_type else []
processutils.execute('mount', device_name, mount_path, *extra_args)
@manila.privsep.sys_admin_pctxt.entrypoint
@ -70,3 +81,18 @@ def list_mounts():
@manila.privsep.sys_admin_pctxt.entrypoint
def chmod(permission_level_str, mount_path):
processutils.execute('chmod', permission_level_str, mount_path)
@manila.privsep.sys_admin_pctxt.entrypoint
def find(directory_to_find, min_depth='1', dirs_to_ignore=[], delete=False):
ignored_dirs = []
extra_args = []
for dir in dirs_to_ignore:
ignored_dirs += '!', '-path', dir
if delete:
extra_args.append('-delete')
processutils.execute(
'find', directory_to_find, '-mindepth', min_depth, *ignored_dirs,
*extra_args)

9
manila/share/drivers/glusterfs/common.py
View File

@ -24,6 +24,7 @@ from oslo_log import log
from manila import exception
from manila.i18n import _
from manila.privsep import os as privsep_os
from manila.share.drivers.ganesha import utils as ganesha_utils
from manila import utils
@ -387,9 +388,8 @@ def _mount_gluster_vol(execute, gluster_export, mount_path, ensure=False):
:param ensure: boolean to allow remounting a volume with a warning
"""
execute('mkdir', '-p', mount_path)
command = ['mount', '-t', 'glusterfs', gluster_export, mount_path]
try:
execute(*command, run_as_root=True)
privsep_os.mount(gluster_export, mount_path, mount_type='glusterfs')
except exception.ProcessExecutionError as exc:
if ensure and 'already mounted' in exc.stderr:
LOG.warning("%s is already mounted.", gluster_export)
@ -399,15 +399,14 @@ def _mount_gluster_vol(execute, gluster_export, mount_path, ensure=False):
)
def _umount_gluster_vol(execute, mount_path):
def _umount_gluster_vol(mount_path):
"""Unmount a GlusterFS volume at the specified mount path.
:param execute: command execution function
:param mount_path: path where volume is mounted
"""
try:
execute('umount', mount_path, run_as_root=True)
privsep_os.umount(mount_path)
except exception.ProcessExecutionError as exc:
msg = (_("Unable to unmount gluster volume. "
"mount_dir: %(mount_path)s, Error: %(error)s") %

10
manila/share/drivers/glusterfs/layout_directory.py
View File

@ -24,6 +24,7 @@ import xml.etree.cElementTree as etree
from manila import exception
from manila.i18n import _
from manila.privsep import os as privsep_os
from manila.share.drivers.glusterfs import common
from manila.share.drivers.glusterfs import layout
from manila import utils
@ -140,10 +141,9 @@ class GlusterfsDirectoryMappedLayout(layout.GlusterfsShareLayoutBase):
# probe into getting a NAS protocol helper for the share in order
# to facilitate early detection of unsupported protocol type
local_share_path = self._get_local_share_path(share)
cmd = ['mkdir', local_share_path]
try:
self.driver._execute(*cmd, run_as_root=True)
privsep_os.mkdir(local_share_path)
self._set_directory_quota(share, share['size'])
except Exception as exc:
if isinstance(exc, exception.ProcessExecutionError):
@ -164,9 +164,8 @@ class GlusterfsDirectoryMappedLayout(layout.GlusterfsShareLayoutBase):
def _cleanup_create_share(self, share_path, share_name):
"""Cleanup share that errored out during its creation."""
if os.path.exists(share_path):
cmd = ['rm', '-rf', share_path]
try:
self.driver._execute(*cmd, run_as_root=True)
privsep_os.recursive_forced_rm(share_path)
except exception.ProcessExecutionError as exc:
LOG.error('Cannot cleanup share, %s, that errored out '
'during its creation, but exists in GlusterFS '
@ -176,9 +175,8 @@ class GlusterfsDirectoryMappedLayout(layout.GlusterfsShareLayoutBase):
def delete_share(self, context, share, share_server=None):
"""Remove a sub-directory/share from the GlusterFS volume."""
local_share_path = self._get_local_share_path(share)
cmd = ['rm', '-rf', local_share_path]
try:
self.driver._execute(*cmd, run_as_root=True)
privsep_os.recursive_forced_rm(local_share_path)
except exception.ProcessExecutionError:
LOG.exception('Unable to delete share %s', share['name'])
raise

14
manila/share/drivers/glusterfs/layout_volume.py
View File

@ -28,6 +28,7 @@ from oslo_log import log
from manila import exception
from manila.i18n import _
from manila.privsep import os as privsep_os
from manila.share.drivers.glusterfs import common
from manila.share.drivers.glusterfs import layout
from manila import utils
@ -352,17 +353,16 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase):
# delete the paths of the two directories, but delete their contents
# along with the rest of the contents of the volume.
srvaddr = gluster_mgr.host_access
if common.numreduct(self.glusterfs_versions[srvaddr]) < (3, 7):
cmd = ['find', tmpdir, '-mindepth', '1', '-delete']
else:
ignored_dirs = []
if common.numreduct(self.glusterfs_versions[srvaddr]) > (3, 6):
ignored_dirs = map(lambda x: os.path.join(tmpdir, *x),
[('.trashcan', ), ('.trashcan', 'internal_op')])
ignored_dirs = list(ignored_dirs)
cmd = ['find', tmpdir, '-mindepth', '1', '!', '-path',
ignored_dirs[0], '!', '-path', ignored_dirs[1], '-delete']
ignored_dirs = [ignored_dirs[0], ignored_dirs[1]]
try:
self.driver._execute(*cmd, run_as_root=True)
privsep_os.find(
tmpdir, dirs_to_ignore=ignored_dirs, delete=True)
except exception.ProcessExecutionError as exc:
msg = (_("Error trying to wipe gluster volume. "
"gluster_export: %(export)s, Error: %(error)s") %
@ -371,7 +371,7 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase):
raise exception.GlusterfsException(msg)
finally:
# Unmount.
common._umount_gluster_vol(self.driver._execute, tmpdir)
common._umount_gluster_vol(tmpdir)
shutil.rmtree(tmpdir, ignore_errors=True)
def create_share(self, context, share, share_server=None):

48
manila/tests/share/drivers/glusterfs/test_common.py
View File

@ -21,6 +21,7 @@ import ddt
from oslo_config import cfg
from manila import exception
from manila.privsep import os as privsep_os
from manila.share.drivers.glusterfs import common
from manila import test
from manila.tests import fake_utils
@ -764,14 +765,15 @@ class GlusterFSCommonTestCase(test.TestCase):
@staticmethod
def _mount_exec(vol, mnt):
return ['mkdir -p %s' % mnt,
'mount -t glusterfs %(exp)s %(mnt)s' % {'exp': vol,
'mnt': mnt}]
return ['mkdir -p %s' % mnt]
def test_mount_gluster_vol(self):
expected_exec = self._mount_exec(fakeexport, fakemnt)
self.mock_object(privsep_os, 'mount')
ret = common._mount_gluster_vol(self._execute, fakeexport, fakemnt,
False)
privsep_os.mount.assert_called_once_with(
fakeexport, fakemnt, mount_type='glusterfs')
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
self.assertIsNone(ret)
@ -779,10 +781,13 @@ class GlusterFSCommonTestCase(test.TestCase):
def exec_runner(*ignore_args, **ignore_kwargs):
raise exception.ProcessExecutionError(stderr='already mounted')
expected_exec = self._mount_exec(fakeexport, fakemnt)
fake_utils.fake_execute_set_repliers([('mount', exec_runner)])
self.mock_object(
privsep_os, 'mount', mock.Mock(side_effect=exec_runner))
self.assertRaises(exception.GlusterfsException,
common._mount_gluster_vol,
self._execute, fakeexport, fakemnt, False)
privsep_os.mount.assert_called_once_with(
fakeexport, fakemnt, mount_type='glusterfs')
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
def test_mount_gluster_vol_mounted_ensure(self):
@ -790,11 +795,14 @@ class GlusterFSCommonTestCase(test.TestCase):
raise exception.ProcessExecutionError(stderr='already mounted')
expected_exec = self._mount_exec(fakeexport, fakemnt)
common.LOG.warning = mock.Mock()
fake_utils.fake_execute_set_repliers([('mount', exec_runner)])
self.mock_object(
privsep_os, 'mount', mock.Mock(side_effect=exec_runner))
ret = common._mount_gluster_vol(self._execute, fakeexport, fakemnt,
True)
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
self.assertIsNone(ret)
self.mock_object(
privsep_os, 'mount', mock.Mock(side_effect=exec_runner))
common.LOG.warning.assert_called_with(
"%s is already mounted.", fakeexport)
@ -803,15 +811,24 @@ class GlusterFSCommonTestCase(test.TestCase):
def exec_runner(*ignore_args, **ignore_kwargs):
raise RuntimeError('fake error')
expected_exec = self._mount_exec(fakeexport, fakemnt)
fake_utils.fake_execute_set_repliers([('mount', exec_runner)])
self.assertRaises(RuntimeError, common._mount_gluster_vol,
self._execute, fakeexport, fakemnt, ensure)
self.mock_object(
privsep_os, 'mount', mock.Mock(side_effect=exec_runner))
self.assertRaises(
RuntimeError,
common._mount_gluster_vol,
self._execute,
fakeexport,
fakemnt,
ensure)
privsep_os.mount.assert_called_once_with(
fakeexport, fakemnt, mount_type='glusterfs')
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
def test_umount_gluster_vol(self):
expected_exec = ['umount %s' % fakemnt]
ret = common._umount_gluster_vol(self._execute, fakemnt)
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
self.mock_object(privsep_os, 'umount')
ret = common._umount_gluster_vol(fakemnt)
privsep_os.umount.assert_called_once_with(fakemnt)
self.assertIsNone(ret)
@ddt.data({'in_exc': exception.ProcessExecutionError,
@ -821,11 +838,10 @@ class GlusterFSCommonTestCase(test.TestCase):
def test_umount_gluster_vol_fail(self, in_exc, out_exc):
def exec_runner(*ignore_args, **ignore_kwargs):
raise in_exc('fake error')
expected_exec = ['umount %s' % fakemnt]
fake_utils.fake_execute_set_repliers([('umount', exec_runner)])
self.assertRaises(out_exc, common._umount_gluster_vol,
self._execute, fakemnt)
self.assertEqual(fake_utils.fake_execute_get_log(), expected_exec)
self.mock_object(privsep_os, 'umount',
mock.Mock(side_effect=exec_runner))
self.assertRaises(out_exc, common._umount_gluster_vol, fakemnt)
privsep_os.umount.assert_called_once_with(fakemnt)
def test_restart_gluster_vol(self):
gmgr = common.GlusterManager(fakeexport, self._execute, None, None)

55
manila/tests/share/drivers/glusterfs/test_layout_directory.py
View File

@ -21,6 +21,7 @@ from oslo_config import cfg
from manila import context
from manila import exception
from manila.privsep import os as privsep_os
from manila.share import configuration as config
from manila.share.drivers.glusterfs import common
from manila.share.drivers.glusterfs import layout_directory
@ -234,8 +235,6 @@ class GlusterfsDirectoryMappedLayoutTestCase(test.TestCase):
@ddt.data((), (None,))
def test_create_share(self, extra_args):
exec_cmd1 = 'mkdir %s' % fake_local_share_path
expected_exec = [exec_cmd1, ]
expected_ret = 'testuser@127.0.0.1:/testvol/fakename'
self.mock_object(
self._layout, '_get_local_share_path',
@ -246,13 +245,14 @@ class GlusterfsDirectoryMappedLayoutTestCase(test.TestCase):
self.mock_object(
self._layout.driver, '_setup_via_manager',
mock.Mock(return_value=expected_ret))
self.mock_object(privsep_os, 'mkdir')
ret = self._layout.create_share(self._context, self.share, *extra_args)
self._layout._get_local_share_path.called_once_with(self.share)
self._layout.gluster_manager.gluster_call.assert_called_once_with(
'volume', 'quota', 'testvol', 'limit-usage', '/fakename', '1GB')
self.assertEqual(expected_exec, fake_utils.fake_execute_get_log())
privsep_os.mkdir.assert_called_once_with(fake_local_share_path)
self._layout._glustermanager.assert_called_once_with(
{'user': 'testuser', 'host': '127.0.0.1',
'volume': 'testvol', 'path': '/fakename'})
@ -262,23 +262,20 @@ class GlusterfsDirectoryMappedLayoutTestCase(test.TestCase):
@ddt.data(exception.ProcessExecutionError, exception.GlusterfsException)
def test_create_share_unable_to_create_share(self, trouble):
def exec_runner(*ignore_args, **ignore_kw):
raise trouble
self.mock_object(
self._layout, '_get_local_share_path',
mock.Mock(return_value=fake_local_share_path))
self.mock_object(privsep_os, 'mkdir', mock.Mock(side_effect=trouble))
self.mock_object(self._layout, '_cleanup_create_share')
self.mock_object(layout_directory.LOG, 'error')
expected_exec = ['mkdir %s' % fake_local_share_path]
fake_utils.fake_execute_set_repliers([(expected_exec[0],
exec_runner)])
self.assertRaises(
exception.GlusterfsException, self._layout.create_share,
self._context, self.share)
self._layout._get_local_share_path.called_once_with(self.share)
privsep_os.mkdir.assert_called_once_with(fake_local_share_path)
self._layout._cleanup_create_share.assert_called_once_with(
fake_local_share_path, self.share['name'])
layout_directory.LOG.error.assert_called_once_with(
@ -292,6 +289,8 @@ class GlusterfsDirectoryMappedLayoutTestCase(test.TestCase):
self._layout, '_get_local_share_path',
mock.Mock(return_value=fake_local_share_path))
self.mock_object(self._layout, '_cleanup_create_share')
self.mock_object(
privsep_os, 'mkdir', mock.Mock(side_effect=exec_runner))
self.mock_object(layout_directory.LOG, 'error')
expected_exec = ['mkdir %s' % fake_local_share_path]
fake_utils.fake_execute_set_repliers([(expected_exec[0],
@ -302,26 +301,26 @@ class GlusterfsDirectoryMappedLayoutTestCase(test.TestCase):
self._context, self.share)
self._layout._get_local_share_path.called_once_with(self.share)
privsep_os.mkdir.assert_called_once_with(fake_local_share_path)
self.assertFalse(self._layout._cleanup_create_share.called)
def test_cleanup_create_share_local_share_path_exists(self):
expected_exec = ['rm -rf %s' % fake_local_share_path]
self.mock_object(privsep_os, 'recursive_forced_rm')
self.mock_object(os.path, 'exists', mock.Mock(return_value=True))
ret = self._layout._cleanup_create_share(fake_local_share_path,
self.share['name'])
os.path.exists.assert_called_once_with(fake_local_share_path)
self.assertEqual(expected_exec, fake_utils.fake_execute_get_log())
privsep_os.recursive_forced_rm.assert_called_once_with(
fake_local_share_path)
self.assertIsNone(ret)
def test_cleanup_create_share_cannot_cleanup_unusable_share(self):
def exec_runner(*ignore_args, **ignore_kw):
raise exception.ProcessExecutionError
expected_exec = ['rm -rf %s' % fake_local_share_path]
fake_utils.fake_execute_set_repliers([(expected_exec[0],
exec_runner)])
self.mock_object(privsep_os, 'recursive_forced_rm',
mock.Mock(side_effect=exec_runner))
self.mock_object(layout_directory.LOG, 'error')
self.mock_object(os.path, 'exists', mock.Mock(return_value=True))
@ -342,31 +341,34 @@ class GlusterfsDirectoryMappedLayoutTestCase(test.TestCase):
self.assertIsNone(ret)
def test_delete_share(self):
local_share_path = '/mnt/nfs/testvol/fakename'
self._layout._get_local_share_path = (
mock.Mock(return_value='/mnt/nfs/testvol/fakename'))
mock.Mock(return_value=local_share_path))
mock_force_rm = self.mock_object(privsep_os, 'recursive_forced_rm')
self._layout.delete_share(self._context, self.share)
self.assertEqual(['rm -rf /mnt/nfs/testvol/fakename'],
fake_utils.fake_execute_get_log())
mock_force_rm.assert_called_once_with(
local_share_path)
def test_cannot_delete_share(self):
local_share_path = '/mnt/nfs/testvol/fakename'
self._layout._get_local_share_path = (
mock.Mock(return_value='/mnt/nfs/testvol/fakename'))
def exec_runner(*ignore_args, **ignore_kw):
raise exception.ProcessExecutionError
expected_exec = ['rm -rf %s' % (self._layout._get_local_share_path())]
fake_utils.fake_execute_set_repliers([(expected_exec[0], exec_runner)])
mock.Mock(return_value=local_share_path))
self.mock_object(
privsep_os, 'recursive_forced_rm', mock.Mock(
side_effect=exception.ProcessExecutionError))
self.assertRaises(exception.ProcessExecutionError,
self._layout.delete_share, self._context, self.share)
self.assertEqual(expected_exec, fake_utils.fake_execute_get_log())
privsep_os.mkdir.assert_called_once_with = local_share_path
def test_delete_share_can_be_called_with_extra_arg_share_server(self):
self._layout._get_local_share_path = mock.Mock()
local_share_path = '/mnt/nfs/testvol/fakename'
self._layout._get_local_share_path = mock.Mock(
return_value=local_share_path)
mock_force_rm = self.mock_object(privsep_os, 'recursive_forced_rm')
share_server = None
ret = self._layout.delete_share(self._context, self.share,
@ -374,6 +376,7 @@ class GlusterfsDirectoryMappedLayoutTestCase(test.TestCase):
self.assertIsNone(ret)
self._layout._get_local_share_path.assert_called_once_with(self.share)
mock_force_rm.assert_called_once_with(local_share_path)
def test_ensure_share(self):
self.assertIsNone(self._layout.ensure_share(self._context, self.share))

28
manila/tests/share/drivers/glusterfs/test_layout_volume.py
View File

@ -27,6 +27,7 @@ from oslo_config import cfg
from manila.common import constants
from manila import context
from manila import exception
from manila.privsep import os as privsep_os
from manila.share import configuration as config
from manila.share.drivers.glusterfs import common
from manila.share.drivers.glusterfs import layout_volume
@ -449,14 +450,12 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
self.glusterfs_target2)
@ddt.data({'vers_minor': '6',
'cmd': ['find', '/tmp/tmpKGHKJ', '-mindepth', '1',
'-delete']},
'dirs_to_ignore': []},
{'vers_minor': '7',
'cmd': ['find', '/tmp/tmpKGHKJ', '-mindepth', '1', '!',
'-path', '/tmp/tmpKGHKJ/.trashcan', '!', '-path',
'/tmp/tmpKGHKJ/.trashcan/internal_op', '-delete']})
'dirs_to_ignore': ['/tmp/tmpKGHKJ/.trashcan',
'/tmp/tmpKGHKJ/.trashcan/internal_op']})
@ddt.unpack
def test_wipe_gluster_vol(self, vers_minor, cmd):
def test_wipe_gluster_vol(self, vers_minor, dirs_to_ignore):
tmpdir = '/tmp/tmpKGHKJ'
gmgr = common.GlusterManager
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
@ -466,6 +465,7 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
self.mock_object(tempfile, 'mkdtemp',
mock.Mock(return_value=tmpdir))
self.mock_object(self.fake_driver, '_execute', mock.Mock())
self.mock_object(privsep_os, 'find')
self.mock_object(common, '_mount_gluster_vol', mock.Mock())
self.mock_object(common, '_umount_gluster_vol', mock.Mock())
self.mock_object(shutil, 'rmtree', mock.Mock())
@ -476,11 +476,10 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
common._mount_gluster_vol.assert_called_once_with(
self.fake_driver._execute, gmgr1.export,
tmpdir)
kwargs = {'run_as_root': True}
self.fake_driver._execute.assert_called_once_with(
*cmd, **kwargs)
privsep_os.find.assert_called_once_with(
tmpdir, dirs_to_ignore=dirs_to_ignore, delete=True)
common._umount_gluster_vol.assert_called_once_with(
self.fake_driver._execute, tmpdir)
tmpdir)
kwargs = {'ignore_errors': True}
shutil.rmtree.assert_called_once_with(tmpdir,
**kwargs)
@ -519,11 +518,10 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
self._layout.glusterfs_versions = {
self.glusterfs_server1: ('3', '6')}
cmd = ['find', '/tmp/tmpKGHKJ', '-mindepth', '1', '-delete']
self.mock_object(tempfile, 'mkdtemp',
mock.Mock(return_value=tmpdir))
self.mock_object(
self.fake_driver, '_execute',
privsep_os, 'find',
mock.Mock(side_effect=exception.ProcessExecutionError))
self.mock_object(common, '_mount_gluster_vol', mock.Mock())
self.mock_object(common, '_umount_gluster_vol', mock.Mock())
@ -537,11 +535,7 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
common._mount_gluster_vol.assert_called_once_with(
self.fake_driver._execute, gmgr1.export,
tmpdir)
kwargs = {'run_as_root': True}
self.fake_driver._execute.assert_called_once_with(
*cmd, **kwargs)
common._umount_gluster_vol.assert_called_once_with(
self.fake_driver._execute, tmpdir)
common._umount_gluster_vol.assert_called_once_with(tmpdir)
kwargs = {'ignore_errors': True}
shutil.rmtree.assert_called_once_with(tmpdir,
**kwargs)