diff --git a/manila/privsep/os.py b/manila/privsep/os.py index ce86be2c17..2d6e7a5758 100644 --- a/manila/privsep/os.py +++ b/manila/privsep/os.py @@ -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) diff --git a/manila/share/drivers/glusterfs/common.py b/manila/share/drivers/glusterfs/common.py index 911c7d1455..cd155a007d 100644 --- a/manila/share/drivers/glusterfs/common.py +++ b/manila/share/drivers/glusterfs/common.py @@ -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") % diff --git a/manila/share/drivers/glusterfs/layout_directory.py b/manila/share/drivers/glusterfs/layout_directory.py index 54d1e9365e..5251f3e08f 100644 --- a/manila/share/drivers/glusterfs/layout_directory.py +++ b/manila/share/drivers/glusterfs/layout_directory.py @@ -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 diff --git a/manila/share/drivers/glusterfs/layout_volume.py b/manila/share/drivers/glusterfs/layout_volume.py index 32da9b9099..3550774531 100644 --- a/manila/share/drivers/glusterfs/layout_volume.py +++ b/manila/share/drivers/glusterfs/layout_volume.py @@ -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): diff --git a/manila/tests/share/drivers/glusterfs/test_common.py b/manila/tests/share/drivers/glusterfs/test_common.py index 2b0c3328b7..888dc2a842 100644 --- a/manila/tests/share/drivers/glusterfs/test_common.py +++ b/manila/tests/share/drivers/glusterfs/test_common.py @@ -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) diff --git a/manila/tests/share/drivers/glusterfs/test_layout_directory.py b/manila/tests/share/drivers/glusterfs/test_layout_directory.py index a3e1dadc1a..4973a1a652 100644 --- a/manila/tests/share/drivers/glusterfs/test_layout_directory.py +++ b/manila/tests/share/drivers/glusterfs/test_layout_directory.py @@ -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)) diff --git a/manila/tests/share/drivers/glusterfs/test_layout_volume.py b/manila/tests/share/drivers/glusterfs/test_layout_volume.py index 06857c5a05..a27eaea5ef 100644 --- a/manila/tests/share/drivers/glusterfs/test_layout_volume.py +++ b/manila/tests/share/drivers/glusterfs/test_layout_volume.py @@ -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)