diff --git a/manila/privsep/common.py b/manila/privsep/common.py new file mode 100644 index 0000000000..14d3d4895e --- /dev/null +++ b/manila/privsep/common.py @@ -0,0 +1,39 @@ +# Copyright 2021 Red Hat, Inc +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from manila import exception +from manila import utils as manila_utils +from oslo_log import log + +LOG = log.getLogger(__name__) + + +def execute_with_retries(action, action_args, max_retries): + + @manila_utils.retry( + retry_param=exception.ProcessExecutionError, backoff_rate=2, + retries=max_retries) + def execute(): + try: + action(*action_args) + return True + except exception.ProcessExecutionError: + LOG.exception("Recovering from a failed execute.") + raise + + try: + execute() + except exception.ProcessExecutionError: + LOG.exception("Failed to run command. Tries exhausted.") + raise diff --git a/manila/privsep/filesystem.py b/manila/privsep/filesystem.py new file mode 100644 index 0000000000..518dc017c7 --- /dev/null +++ b/manila/privsep/filesystem.py @@ -0,0 +1,36 @@ +# Copyright 2021 Red Hat, Inc +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +Helpers for filesystem commands +""" + +from oslo_concurrency import processutils + +import manila.privsep + + +@manila.privsep.sys_admin_pctxt.entrypoint +def e2fsck(device_path): + return processutils.execute('e2fsck', '-y', '-f', device_path) + + +@manila.privsep.sys_admin_pctxt.entrypoint +def tune2fs(device_path): + return processutils.execute('tune2fs', '-U', 'random', device_path) + + +@manila.privsep.sys_admin_pctxt.entrypoint +def make_filesystem(ext_version, device_name): + return processutils.execute(f'mkfs.{ext_version}', device_name) diff --git a/manila/privsep/lvm.py b/manila/privsep/lvm.py new file mode 100644 index 0000000000..d30d639dee --- /dev/null +++ b/manila/privsep/lvm.py @@ -0,0 +1,78 @@ +# Copyright 2021 Red Hat, Inc +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +Helpers for lvm related routines +""" + +from oslo_concurrency import processutils + +import manila.privsep + + +@manila.privsep.sys_admin_pctxt.entrypoint +def lvremove(vg_name, lv_name): + processutils.execute('lvremove', '-f', f'{vg_name}/{lv_name}') + + +@manila.privsep.sys_admin_pctxt.entrypoint +def lvcreate(lv_size, lv_name, vg_name, mirrors=0, region_size=0): + extra_params = [] + + if mirrors: + extra_params += ['-m', mirrors, '--nosync'] + if region_size: + extra_params += ['-R', region_size] + + processutils.execute( + 'lvcreate', '-Wy', '--yes', '-L', f'{lv_size}G', '-n', lv_name, + vg_name, *extra_params) + + +@manila.privsep.sys_admin_pctxt.entrypoint +def lv_snapshot_create(snapshot_size, snap_name, orig_lv_name): + size_str = '%sG' % snapshot_size + processutils.execute( + 'lvcreate', '-L', size_str, '--name', snap_name, + '--snapshot', orig_lv_name) + + +@manila.privsep.sys_admin_pctxt.entrypoint +def get_vgs(vg_name): + out, err = processutils.execute( + 'vgs', vg_name, '--rows', '--units', 'g',) + return out, err + + +@manila.privsep.sys_admin_pctxt.entrypoint +def list_vgs_get_name(): + out, err = processutils.execute('vgs', '--noheadings', '-o', 'name') + return out, err + + +@manila.privsep.sys_admin_pctxt.entrypoint +def lvconvert(vg_name, snapshot_name): + processutils.execute( + 'lvconvert', '--merge', f'{vg_name}/{snapshot_name}') + + +@manila.privsep.sys_admin_pctxt.entrypoint +def lvrename(vg_name, lv_name, new_name): + processutils.execute( + 'lvrename', vg_name, lv_name, new_name) + + +@manila.privsep.sys_admin_pctxt.entrypoint +def lvextend(lv_name, new_size): + processutils.execute('lvextend', '-L', '%sG' % new_size, '-r', lv_name) diff --git a/manila/privsep/os.py b/manila/privsep/os.py new file mode 100644 index 0000000000..ce86be2c17 --- /dev/null +++ b/manila/privsep/os.py @@ -0,0 +1,72 @@ +# Copyright 2021 Red Hat, Inc +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +Helpers for os basic commands +""" + +from oslo_concurrency import processutils + +from manila import exception + +import manila.privsep + + +@manila.privsep.sys_admin_pctxt.entrypoint +def rmdir(dir_path): + processutils.execute('rmdir', dir_path) + + +@manila.privsep.sys_admin_pctxt.entrypoint +def is_data_definition_direct_io_supported(src_str, dest_str): + try: + processutils.execute( + 'dd', 'count=0', f'if={src_str}', f'of={dest_str}', + 'iflag=direct', 'oflag=direct') + is_direct_io_supported = True + except exception.ProcessExecutionError: + is_direct_io_supported = False + + return is_direct_io_supported + + +@manila.privsep.sys_admin_pctxt.entrypoint +def data_definition(src_str, dest_str, size_in_g, use_direct_io=False): + extra_flags = [] + if use_direct_io: + extra_flags += ['iflag=direct', 'oflag=direct'] + processutils.execute( + 'dd', 'if=%s' % src_str, 'of=%s' % dest_str, 'count=%d' % size_in_g, + 'bs=1M', *extra_flags) + + +@manila.privsep.sys_admin_pctxt.entrypoint +def umount(mount_path): + processutils.execute('umount', '-f', mount_path) + + +@manila.privsep.sys_admin_pctxt.entrypoint +def mount(device_name, mount_path): + processutils.execute('mount', device_name, mount_path) + + +@manila.privsep.sys_admin_pctxt.entrypoint +def list_mounts(): + out, err = processutils.execute('mount', '-l') + return out, err + + +@manila.privsep.sys_admin_pctxt.entrypoint +def chmod(permission_level_str, mount_path): + processutils.execute('chmod', permission_level_str, mount_path) diff --git a/manila/share/drivers/lvm.py b/manila/share/drivers/lvm.py index a51e829c5d..fcda90e3e9 100644 --- a/manila/share/drivers/lvm.py +++ b/manila/share/drivers/lvm.py @@ -23,6 +23,7 @@ import math import os import re +from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log from oslo_utils import importutils @@ -30,6 +31,10 @@ from oslo_utils import timeutils from manila import exception from manila.i18n import _ +from manila.privsep import common as privsep_common +from manila.privsep import filesystem as privsep_filesystem +from manila.privsep import lvm as privsep_lvm +from manila.privsep import os as privsep_os from manila.share import driver from manila.share.drivers import generic from manila.share import utils as share_utils @@ -67,8 +72,11 @@ CONF.register_opts(generic.share_opts) class LVMMixin(driver.ExecuteMixin): def check_for_setup_error(self): """Returns an error if prerequisites aren't met.""" - out, err = self._execute('vgs', '--noheadings', '-o', 'name', - run_as_root=True) + try: + out, err = privsep_lvm.list_vgs_get_name() + except processutils.ProcessExecutionError: + msg = _("Failed to get LVM volume group names.") + raise exception.ShareBackendException(msg=msg) volume_groups = out.split() if self.configuration.lvm_share_volume_group not in volume_groups: msg = (_("Share volume group %s doesn't exist.") @@ -81,32 +89,46 @@ class LVMMixin(driver.ExecuteMixin): def _allocate_container(self, share): sizestr = '%sG' % share['size'] - cmd = ['lvcreate', '-Wy', '--yes', '-L', sizestr, '-n', share['name'], - self.configuration.lvm_share_volume_group] + mirrors = 0 + region_size = 0 if self.configuration.lvm_share_mirrors: - cmd += ['-m', self.configuration.lvm_share_mirrors, '--nosync'] + mirrors = self.configuration.lvm_share_mirrors terras = int(sizestr[:-1]) / 1024.0 if terras >= 1.5: rsize = int(2 ** math.ceil(math.log(terras) / math.log(2))) # NOTE(vish): Next power of two for region size. See: # http://red.ht/U2BPOD - cmd += ['-R', str(rsize)] - - self._try_execute(*cmd, run_as_root=True) + region_size = str(rsize) + action_args = [ + share['size'], + share['name'], + self.configuration.lvm_share_volume_group, + mirrors, + region_size + ] + privsep_common.execute_with_retries( + privsep_lvm.lvcreate, action_args, + self.configuration.num_shell_tries) device_name = self._get_local_path(share) - self._execute('mkfs.%s' % self.configuration.share_volume_fstype, - device_name, run_as_root=True) + try: + privsep_filesystem.make_filesystem( + self.configuration.share_volume_fstype, device_name) + except processutils.ProcessExecutionError: + raise def _extend_container(self, share, device_name, size): - cmd = ['lvextend', '-L', '%sG' % size, '-r', device_name] - self._try_execute(*cmd, run_as_root=True) + privsep_common.execute_with_retries( + privsep_lvm.lvextend, [device_name, size], + self.configuration.num_shell_tries) def _deallocate_container(self, share_name): """Deletes a logical volume for share.""" try: - self._try_execute('lvremove', '-f', "%s/%s" % - (self.configuration.lvm_share_volume_group, - share_name), run_as_root=True) + action_args = [ + self.configuration.lvm_share_volume_group, share_name] + privsep_common.execute_with_retries( + privsep_lvm.lvremove, action_args, + self.configuration.num_shell_tries) except exception.ProcessExecutionError as exc: err_pattern = re.compile(".*failed to find.*|.*not found.*", re.IGNORECASE) @@ -119,10 +141,11 @@ class LVMMixin(driver.ExecuteMixin): """Creates a snapshot.""" orig_lv_name = "%s/%s" % (self.configuration.lvm_share_volume_group, snapshot['share_name']) - self._try_execute( - 'lvcreate', '-L', - '%sG' % snapshot['share']['size'], '--name', snapshot['name'], - '--snapshot', orig_lv_name, run_as_root=True) + action_args = [ + snapshot['share']['size'], snapshot['name'], orig_lv_name] + privsep_common.execute_with_retries( + privsep_lvm.lv_snapshot_create, action_args, + self.configuration.num_shell_tries) self._set_random_uuid_to_device(snapshot) @@ -135,10 +158,12 @@ class LVMMixin(driver.ExecuteMixin): # a recently checked filesystem. # See: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=857336 device_path = self._get_local_path(share_or_snapshot) - self._execute('e2fsck', '-y', '-f', device_path, run_as_root=True) - self._execute( - 'tune2fs', '-U', 'random', device_path, run_as_root=True, - ) + try: + privsep_filesystem.e2fsck(device_path) + privsep_filesystem.tune2fs(device_path) + except processutils.ProcessExecutionError: + msg = _("Failed to check or modify filesystems.") + raise exception.ShareBackendException(msg=msg) def create_snapshot(self, context, snapshot, share_server=None): self._create_snapshot(context, snapshot) @@ -224,10 +249,12 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): super(LVMShareDriver, self)._update_share_stats(data) def get_share_server_pools(self, share_server=None): - out, err = self._execute('vgs', - self.configuration.lvm_share_volume_group, - '--rows', '--units', 'g', - run_as_root=True) + try: + out, err = privsep_lvm.get_vgs( + self.configuration.lvm_share_volume_group) + except processutils.ProcessExecutionError: + msg = _("Failed to list LVM Volume Groups.") + raise exception.ShareBackendException(msg=msg) total_size = re.findall(r"VSize\s[0-9.]+g", out)[0][6:-1] free_size = re.findall(r"VFree\s[0-9.]+g", out)[0][6:-1] return [{ @@ -279,7 +306,7 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): retries=retries) def _unmount_device_with_retry(): try: - self._execute('umount', '-f', mount_path, run_as_root=True) + privsep_os.umount(mount_path) except exception.ProcessExecutionError as exc: if 'is busy' in exc.stderr.lower(): raise exception.ShareBusyException( @@ -294,7 +321,11 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): _unmount_device_with_retry() # remove dir - self._execute('rmdir', mount_path, run_as_root=True) + try: + privsep_os.rmdir(mount_path) + except exception.ProcessExecutionError: + msg = _("Failed to remove the directory.") + raise exception.ShareBackendException(msg=msg) def ensure_shares(self, context, shares): updates = {} @@ -362,12 +393,10 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): mount_path = self._get_mount_path(share_or_snapshot) self._execute('mkdir', '-p', mount_path) try: - self._execute('mount', device_name, mount_path, - run_as_root=True, check_exit_code=True) - self._execute('chmod', '777', mount_path, - run_as_root=True, check_exit_code=True) + privsep_os.mount(device_name, mount_path) + privsep_os.chmod('777', mount_path) except exception.ProcessExecutionError: - out, err = self._execute('mount', '-l', run_as_root=True) + out, err = privsep_os.list_mounts() if device_name in out: LOG.warning("%s is already mounted", device_name) else: @@ -381,19 +410,18 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): def _copy_volume(self, srcstr, deststr, size_in_g): # Use O_DIRECT to avoid thrashing the system buffer cache - extra_flags = ['iflag=direct', 'oflag=direct'] - # Check whether O_DIRECT is supported - try: - self._execute('dd', 'count=0', 'if=%s' % srcstr, 'of=%s' % deststr, - *extra_flags, run_as_root=True) - except exception.ProcessExecutionError: - extra_flags = [] + use_direct_io = ( + privsep_os.is_data_definition_direct_io_supported(srcstr, deststr)) # Perform the copy - self._execute('dd', 'if=%s' % srcstr, 'of=%s' % deststr, - 'count=%d' % (size_in_g * 1024), 'bs=1M', - *extra_flags, run_as_root=True) + try: + privsep_os.data_definition( + srcstr, deststr, (size_in_g * 1024), + use_direct_io=use_direct_io) + except exception.ProcessExecutionError: + msg = _("Failed while copying from the snapshot to the share.") + raise exception.ShareBackendException(msg=msg) def extend_share(self, share, new_size, share_server=None): device_name = self._get_local_path(share) @@ -412,9 +440,12 @@ class LVMShareDriver(LVMMixin, driver.ShareDriver): # Unmount the share filesystem self._unmount_device(share) # Merge the snapshot LV back into the share, reverting it - snap_lv_name = "%s/%s" % (self.configuration.lvm_share_volume_group, + try: + privsep_lvm.lvconvert(self.configuration.lvm_share_volume_group, snapshot['name']) - self._execute('lvconvert', '--merge', snap_lv_name, run_as_root=True) + except exception.ProcessExecutionError: + msg = _('Failed to revert the share to the given snapshot.') + raise exception.ShareBackendException(msg=msg) # Now recreate the snapshot that was destroyed by the merge self._create_snapshot(context, snapshot) diff --git a/manila/tests/share/drivers/test_lvm.py b/manila/tests/share/drivers/test_lvm.py index 01b86014e2..ac101d7942 100644 --- a/manila/tests/share/drivers/test_lvm.py +++ b/manila/tests/share/drivers/test_lvm.py @@ -25,6 +25,10 @@ from oslo_utils import timeutils from manila.common import constants as const from manila import context from manila import exception +from manila.privsep import common as privsep_common +from manila.privsep import filesystem +from manila.privsep import lvm as privsep_lvm +from manila.privsep import os as os_routines from manila.share import configuration from manila.share.drivers import lvm from manila import test @@ -134,29 +138,28 @@ class LVMShareDriverTestCase(test.TestCase): ]) def test_check_for_setup_error(self): - def exec_runner(*ignore_args, **ignore_kwargs): - return '\n fake1\n fakevg\n fake2\n', '' + out, err = '\n fake1\n fakevg\n fake2\n', '' + self.mock_object(privsep_lvm, 'list_vgs_get_name', + mock.Mock(return_value=(out, err))) - expected_exec = ['vgs --noheadings -o name'] - fake_utils.fake_execute_set_repliers([(expected_exec[0], exec_runner)]) self._driver.check_for_setup_error() - self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) + + privsep_lvm.list_vgs_get_name.assert_called_once() def test_check_for_setup_error_no_vg(self): - def exec_runner(*ignore_args, **ignore_kwargs): - return '\n fake0\n fake1\n fake2\n', '' - - fake_utils.fake_execute_set_repliers([('vgs --noheadings -o name', - exec_runner)]) + out = '\n fake0\n fake1\n fake2\n' + err = '' + self.mock_object(privsep_lvm, 'list_vgs_get_name', + mock.Mock(return_value=(out, err))) self.assertRaises(exception.InvalidParameterValue, self._driver.check_for_setup_error) def test_check_for_setup_error_no_export_ips(self): - def exec_runner(*ignore_args, **ignore_kwargs): - return '\n fake1\n fakevg\n fake2\n', '' + out = '\n fake1\n fakevg\n fake2\n' + err = '' + self.mock_object(privsep_lvm, 'list_vgs_get_name', + mock.Mock(return_value=(out, err))) - fake_utils.fake_execute_set_repliers([('vgs --noheadings -o name', - exec_runner)]) CONF.set_default('lvm_share_export_ips', None) self.assertRaises(exception.InvalidParameterValue, self._driver.check_for_setup_error) @@ -176,17 +179,22 @@ class LVMShareDriverTestCase(test.TestCase): def test_create_share(self): CONF.set_default('lvm_share_mirrors', 0) self._driver._mount_device = mock.Mock() + lv_create_mock = privsep_lvm.lvcreate = mock.Mock() + lv_create_args = [ + self.share['size'], self.share['name'], + CONF.lvm_share_volume_group, 0, 0] + self.mock_object(privsep_common, 'execute_with_retries') + self.mock_object(filesystem, 'make_filesystem') ret = self._driver.create_share(self._context, self.share, self.share_server) self._driver._mount_device.assert_called_with( self.share, '/dev/mapper/fakevg-fakename') - expected_exec = [ - 'lvcreate -Wy --yes -L 1G -n fakename fakevg', - 'mkfs.ext4 /dev/mapper/fakevg-fakename', - ] - self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) + privsep_common.execute_with_retries.assert_called_once_with( + lv_create_mock, lv_create_args, CONF.num_shell_tries) + filesystem.make_filesystem.assert_called_once_with( + 'ext4', '/dev/mapper/fakevg-fakename') self.assertEqual(self._helper_nfs.create_exports.return_value, ret) def test_create_share_from_snapshot(self): @@ -199,6 +207,21 @@ class LVMShareDriverTestCase(test.TestCase): mount_share = '/dev/mapper/fakevg-fakename' mount_snapshot = '/dev/mapper/fakevg-fakename' self._helper_nfs.create_export.return_value = 'fakelocation' + lv_create_mock = privsep_lvm.lvcreate = mock.Mock() + lv_create_args = [ + self.share['size'], self.share['name'], + CONF.lvm_share_volume_group, 0, 0] + + self.mock_object(privsep_common, 'execute_with_retries') + self.mock_object(os_routines, 'is_data_definition_direct_io_supported', + mock.Mock(return_value=True)) + self.mock_object(os_routines, 'data_definition') + self.mock_object(os_routines, 'mount') + self.mock_object(os_routines, 'chmod') + self.mock_object(filesystem, 'make_filesystem') + self.mock_object(filesystem, 'e2fsck') + self.mock_object(filesystem, 'tune2fs') + self._driver.create_share_from_snapshot(self._context, self.share, snapshot_instance, @@ -206,44 +229,59 @@ class LVMShareDriverTestCase(test.TestCase): self._driver._mount_device.assert_called_with(self.share, mount_snapshot) - expected_exec = [ - 'lvcreate -Wy --yes -L 1G -n fakename fakevg', - 'mkfs.ext4 /dev/mapper/fakevg-fakename', - 'e2fsck -y -f %s' % mount_share, - 'tune2fs -U random %s' % mount_share, - ("dd count=0 if=%s of=%s iflag=direct oflag=direct" % - (mount_snapshot, mount_share)), - ("dd if=%s of=%s count=1024 bs=1M iflag=direct oflag=direct" % - (mount_snapshot, mount_share)), - ] - self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) + privsep_common.execute_with_retries.assert_called_once_with( + lv_create_mock, lv_create_args, 3) + filesystem.make_filesystem.assert_called_once_with( + 'ext4', '/dev/mapper/fakevg-fakename') + filesystem.e2fsck.assert_called_once_with( + mount_share) + filesystem.tune2fs.assert_called_once_with( + mount_share) + (os_routines.is_data_definition_direct_io_supported + .assert_called_once_with( + mount_snapshot, mount_share)) + os_routines.data_definition.assert_called_once_with( + mount_snapshot, mount_share, (self.share['size'] * 1024), + use_direct_io=True) def test_create_share_mirrors(self): share = fake_share(size='2048') CONF.set_default('lvm_share_mirrors', 2) + lv_create_mock = privsep_lvm.lvcreate = mock.Mock() + lv_create_args = [ + '2048', self.share['name'], + CONF.lvm_share_volume_group, 2, '2'] + self._driver._mount_device = mock.Mock() + self.mock_object(privsep_common, 'execute_with_retries') + self.mock_object(filesystem, 'make_filesystem') ret = self._driver.create_share(self._context, share, self.share_server) self._driver._mount_device.assert_called_with( share, '/dev/mapper/fakevg-fakename') - expected_exec = [ - 'lvcreate -Wy --yes -L 2048G -n fakename fakevg -m 2 --nosync' - ' -R 2', 'mkfs.ext4 /dev/mapper/fakevg-fakename'] - self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) + privsep_common.execute_with_retries.assert_called_once_with( + lv_create_mock, lv_create_args, 3) + filesystem.make_filesystem.assert_called_once_with( + 'ext4', '/dev/mapper/fakevg-fakename') self.assertEqual(self._helper_nfs.create_exports.return_value, ret) def test_deallocate_container(self): - expected_exec = ['lvremove -f fakevg/fakename'] + mock_lvremove = privsep_lvm.lvremove = mock.Mock() + self.mock_object(privsep_common, 'execute_with_retries') + self._driver._deallocate_container(self.share['name']) - self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) + + privsep_common.execute_with_retries.assert_called_once_with( + mock_lvremove, [CONF.lvm_share_volume_group, self.share['name']], 3 + ) def test_deallocate_container_error(self): def _fake_exec(*args, **kwargs): raise exception.ProcessExecutionError(stderr="error") - self.mock_object(self._driver, '_try_execute', _fake_exec) + self.mock_object(privsep_common, 'execute_with_retries', _fake_exec) self.assertRaises(exception.ProcessExecutionError, self._driver._deallocate_container, self.share['name']) @@ -255,7 +293,7 @@ class LVMShareDriverTestCase(test.TestCase): def _fake_exec(*args, **kwargs): raise exception.ProcessExecutionError(stderr=error_msg) - self.mock_object(self._driver, '_try_execute', _fake_exec) + self.mock_object(privsep_common, 'execute_with_retries', _fake_exec) self._driver._deallocate_container(self.share['name']) @mock.patch.object(lvm.LVMShareDriver, '_update_share_stats', mock.Mock()) @@ -272,74 +310,94 @@ class LVMShareDriverTestCase(test.TestCase): self._driver._update_share_stats.assert_called_once_with() def test__unmount_device_not_mounted(self): - def exec_runner(*ignore_args, **ignore_kwargs): - umount_msg = ( - "umount: /opt/stack/data/manila/mnt/share-fake-share: not " - "mounted.\n" - ) - raise exception.ProcessExecutionError(stderr=umount_msg) - self._os.path.exists.return_value = True mount_path = self._get_mount_path(self.share) - expected_exec = "umount -f %s" % (mount_path) - fake_utils.fake_execute_set_repliers([(expected_exec, exec_runner)]) + error_msg = ( + "umount: /opt/stack/data/manila/mnt/share-fake-share: not " + "mounted.\n") + umount_exception = exception.ProcessExecutionError(stderr=error_msg) + self.mock_object( + os_routines, 'umount', mock.Mock(side_effect=umount_exception)) + self.mock_object(os_routines, 'rmdir') + self._os.path.exists.return_value = True self._driver._unmount_device(self.share, raise_if_missing=False) self._os.path.exists.assert_called_with(mount_path) + os_routines.umount.assert_called_once_with(mount_path) def test__unmount_device_is_busy_error(self): - def exec_runner(*ignore_args, **ignore_kwargs): - raise exception.ProcessExecutionError(stderr='device is busy') + error_msg = 'device is busy' + umount_exception = exception.ProcessExecutionError(stderr=error_msg) + self.mock_object( + os_routines, 'umount', mock.Mock(side_effect=umount_exception)) self._os.path.exists.return_value = True mount_path = self._get_mount_path(self.share) - expected_exec = [ - "umount -f %s" % (mount_path), - ] - fake_utils.fake_execute_set_repliers([(expected_exec[0], exec_runner)]) self.assertRaises(exception.ShareBusyException, self._driver._unmount_device, self.share) - self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) + os_routines.umount.assert_called_once_with(mount_path) def test__unmount_device_error(self): - def exec_runner(*ignore_args, **ignore_kwargs): - raise exception.ProcessExecutionError(stderr='fake error') + error_msg = 'fake error' mount_path = self._get_mount_path(self.share) + umount_exception = exception.ProcessExecutionError(stderr=error_msg) + self.mock_object( + os_routines, 'umount', mock.Mock(side_effect=umount_exception)) self._os.path.exists.return_value = True - cmd = "umount -f %s" % (mount_path) - fake_utils.fake_execute_set_repliers([(cmd, exec_runner)]) + self.assertRaises(processutils.ProcessExecutionError, self._driver._unmount_device, self.share) self._os.path.exists.assert_called_with(mount_path) + os_routines.umount.assert_called_once_with(mount_path) def test__unmount_device_rmdir_error(self): - def exec_runner(*ignore_args, **ignore_kwargs): - raise exception.ProcessExecutionError(stderr='fake error') + error_msg = 'fake error' mount_path = self._get_mount_path(self.share) + umount_exception = exception.ProcessExecutionError(stderr=error_msg) + self.mock_object(os_routines, 'umount') + self.mock_object(os_routines, 'rmdir', + mock.Mock(side_effect=umount_exception)) self._os.path.exists.return_value = True - cmd = "rmdir %s" % (mount_path) - fake_utils.fake_execute_set_repliers([(cmd, exec_runner)]) - self.assertRaises(processutils.ProcessExecutionError, + + self.assertRaises(exception.ShareBackendException, self._driver._unmount_device, self.share) self._os.path.exists.assert_called_with(mount_path) + os_routines.umount.assert_called_once_with(mount_path) + os_routines.rmdir.assert_called_once_with(mount_path) def test_create_snapshot(self): + mock_lv_create = privsep_lvm.lvcreate = mock.Mock() + orig_lv_name = "%s/%s" % (CONF.lvm_share_volume_group, + self.snapshot['share_name']) + device_path = '/dev/mapper/fakevg-%s' % self.snapshot['name'] + lv_create_args = [ + self.snapshot['share']['size'], self.snapshot['share']['name'], + orig_lv_name] + + self.mock_object(privsep_common, 'execute_with_retries') + self.mock_object(filesystem, 'e2fsck') + self.mock_object(filesystem, 'tune2fs') + self.mock_object(os_routines, 'mount') + self.mock_object(os_routines, 'chmod') + self._driver.create_snapshot(self._context, self.snapshot, self.share_server) mount_path = self._get_mount_path(self.snapshot) expected_exec = [ - ("lvcreate -L 1G --name fakesnapshotname --snapshot " - "%s/fakename" % (CONF.lvm_share_volume_group,)), - "e2fsck -y -f /dev/mapper/fakevg-%s" % self.snapshot['name'], - "tune2fs -U random /dev/mapper/fakevg-%s" % self.snapshot['name'], "mkdir -p " + mount_path, - "mount /dev/mapper/fakevg-fakesnapshotname " + mount_path, - "chmod 777 " + mount_path, ] self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) + privsep_common.execute_with_retries( + mock_lv_create, lv_create_args, CONF.num_shell_tries) + filesystem.e2fsck.assert_called_once_with(device_path) + filesystem.tune2fs.assert_called_once_with(device_path) + os_routines.mount.assert_called_once_with( + "/dev/mapper/fakevg-fakesnapshotname", mount_path) + os_routines.chmod.assert_called_once_with( + '777', mount_path) def test_ensure_share(self): device_name = '/dev/mapper/fakevg-fakename' @@ -360,20 +418,32 @@ class LVMShareDriverTestCase(test.TestCase): def test_delete_snapshot(self): mount_path = self._get_mount_path(self.snapshot) - expected_exec = [ - 'umount -f %s' % mount_path, - 'rmdir %s' % mount_path, - 'lvremove -f fakevg/fakesnapshotname', - ] + self.mock_object(os_routines, 'umount') + self.mock_object(os_routines, 'rmdir') + self.mock_object(privsep_common, 'execute_with_retries') + self.mock_object(self._driver, '_deallocate_container') + self._driver.delete_snapshot(self._context, self.snapshot, self.share_server) - self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) + + os_routines.umount.assert_called_once_with(mount_path) + os_routines.rmdir.assert_called_once_with(mount_path) + self._driver._deallocate_container.assert_called_once_with( + self.snapshot['name']) def test_delete_share_invalid_share(self): + self.mock_object(self._driver, '_unmount_device') + self.mock_object(self._driver, '_deallocate_container') self._driver._get_helper = mock.Mock( side_effect=exception.InvalidShare(reason='fake')) + self._driver.delete_share(self._context, self.share, self.share_server) + self._driver._unmount_device.assert_called_once_with( + self.share, raise_if_missing=False, retry_busy_device=True) + self._driver._deallocate_container.assert_called_once_with( + self.share['name']) + def test_delete_share_process_execution_error(self): self.mock_object( self._helper_nfs, @@ -421,13 +491,18 @@ class LVMShareDriverTestCase(test.TestCase): def test_mount_device(self): mount_path = self._get_mount_path(self.share) - ret = self._driver._mount_device(self.share, 'fakedevice') + self.mock_object(os_routines, 'mount') + self.mock_object(os_routines, 'chmod') expected_exec = [ "mkdir -p %s" % (mount_path,), - "mount fakedevice %s" % (mount_path,), - "chmod 777 %s" % (mount_path,), ] + device_name = 'fakedevice' + + ret = self._driver._mount_device(self.share, device_name) + self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) + os_routines.mount.assert_called_once_with(device_name, mount_path) + os_routines.chmod.assert_called_once_with('777', mount_path) self.assertEqual(mount_path, ret) def test_mount_device_already(self): @@ -438,19 +513,22 @@ class LVMShareDriverTestCase(test.TestCase): return 'fakedevice', '' self.mock_object(self._driver, '_execute', exec_runner) + self.mock_object(os_routines, 'mount') + self.mock_object(os_routines, 'chmod') mount_path = self._get_mount_path(self.share) ret = self._driver._mount_device(self.share, 'fakedevice') self.assertEqual(mount_path, ret) def test_mount_device_error(self): - def exec_runner(*args, **kwargs): - if 'mount' in args and '-l' not in args: - raise exception.ProcessExecutionError() - else: - return 'fake', '' + self.mock_object(self._driver, '_execute') + self.mock_object( + os_routines, 'mount', + mock.Mock(side_effect=exception.ProcessExecutionError)) + self.mock_object( + os_routines, 'list_mounts', + mock.Mock(return_value=('fake', ''))) - self.mock_object(self._driver, '_execute', exec_runner) self.assertRaises(exception.ProcessExecutionError, self._driver._mount_device, self.share, 'fakedevice') @@ -477,8 +555,9 @@ class LVMShareDriverTestCase(test.TestCase): ] if retry_busy_device else [None, None] mount_path = self._get_mount_path(self.share) self._os.path.exists.return_value = True - self.mock_object(self._driver, '_execute', mock.Mock( + self.mock_object(os_routines, 'umount', mock.Mock( side_effect=execute_sideeffects)) + self.mock_object(os_routines, 'rmdir') self._driver._unmount_device(self.share, retry_busy_device=retry_busy_device) @@ -486,11 +565,9 @@ class LVMShareDriverTestCase(test.TestCase): num_of_times_umount_is_called = 3 if retry_busy_device else 1 self._os.path.exists.assert_called_with(mount_path) - self._driver._execute.assert_has_calls([ - mock.call('umount', '-f', mount_path, run_as_root=True), - ] * num_of_times_umount_is_called + [ - mock.call('rmdir', mount_path, run_as_root=True) - ]) + os_routines.umount.assert_has_calls([ + mock.call(mount_path)] * num_of_times_umount_is_called) + os_routines.rmdir.assert_called_once_with(mount_path) def test_extend_share(self): local_path = self._driver._get_local_path(self.share) @@ -515,15 +592,13 @@ class LVMShareDriverTestCase(test.TestCase): 'fake_command', run_as_root=True, check_exit_code=True) def test_extend_container(self): - self.mock_object(self._driver, '_try_execute') + mock_lvextend = privsep_lvm.lvextend = mock.Mock() + + self.mock_object(privsep_common, 'execute_with_retries') self._driver._extend_container(self.share, 'device_name', 3) - self._driver._try_execute.assert_called_once_with( - 'lvextend', - '-L', - '3G', - '-r', - 'device_name', - run_as_root=True) + + privsep_common.execute_with_retries.assert_called_once_with( + mock_lvextend, ['device_name', 3], CONF.num_shell_tries) def test_get_share_server_pools(self): expected_result = [{ @@ -533,30 +608,32 @@ class LVMShareDriverTestCase(test.TestCase): 'reserved_percentage': 0, 'reserved_snapshot_percentage': 0, }, ] + out, err = "VSize 33g VFree 22g", None self.mock_object( - self._driver, - '_execute', - mock.Mock(return_value=("VSize 33g VFree 22g", None))) + privsep_lvm, 'get_vgs', mock.Mock(return_value=(out, err))) self.assertEqual(expected_result, self._driver.get_share_server_pools()) - self._driver._execute.assert_called_once_with( - 'vgs', 'fakevg', '--rows', '--units', 'g', run_as_root=True) - def test_copy_volume_error(self): - def _fake_exec(*args, **kwargs): - if 'count=0' in args: - raise exception.ProcessExecutionError() + @ddt.data(True, False) + def test_copy_volume_error(self, use_direct_io): + src_str = 'src' + dest_str = 'dest' + self.mock_object( + os_routines, 'is_data_definition_direct_io_supported', + mock.Mock(return_value=use_direct_io)) + self.mock_object( + os_routines, 'data_definition', + mock.Mock(side_effect=exception.ProcessExecutionError)) - self.mock_object(self._driver, '_execute', - mock.Mock(side_effect=_fake_exec)) - self._driver._copy_volume('src', 'dest', 1) - self._driver._execute.assert_any_call('dd', 'count=0', 'if=src', - 'of=dest', 'iflag=direct', - 'oflag=direct', run_as_root=True) - self._driver._execute.assert_any_call('dd', 'if=src', 'of=dest', - 'count=1024', 'bs=1M', - run_as_root=True) + self.assertRaises( + exception.ShareBackendException, + self._driver._copy_volume, src_str, dest_str, 1) + (os_routines.is_data_definition_direct_io_supported + .assert_called_once_with( + src_str, dest_str)) + os_routines.data_definition.assert_called_once_with( + src_str, dest_str, (1 * 1024), use_direct_io=use_direct_io) @ddt.data((['1.1.1.1'], 4), (['1001::1001'], 6)) @ddt.unpack @@ -576,37 +653,38 @@ class LVMShareDriverTestCase(test.TestCase): self.assertEqual(version == 6, self._driver._stats['ipv6_support']) def test_revert_to_snapshot(self): - mock_update_access = self.mock_object(self._helper_nfs, - 'update_access') + share_local_path = '/dev/mapper/fakevg-fakename' + snapshot_local_path = '/dev/mapper/fakevg-fakesnapshotname' + mock_update_access = self.mock_object( + self._helper_nfs, 'update_access') + mock__unmount_device = self.mock_object( + self._driver, '_unmount_device') + mock_lvconvert = self.mock_object(privsep_lvm, 'lvconvert') + mock_create_snapshot = self.mock_object( + self._driver, '_create_snapshot') + mock_mount_device = self.mock_object( + self._driver, '_mount_device') + mock_get_local_path = self.mock_object( + self._driver, '_get_local_path', + mock.Mock(side_effect=[share_local_path, snapshot_local_path])) + snapshot_parent_share = self.snapshot['share'] + self._driver.revert_to_snapshot(self._context, self.snapshot, [], [], self.share_server) - snap_lv = "%s/fakesnapshotname" % (CONF.lvm_share_volume_group) - share_lv = "%s/fakename" % (CONF.lvm_share_volume_group) - share_mount_path = self._get_mount_path(self.snapshot['share']) - snapshot_mount_path = self._get_mount_path(self.snapshot) - expected_exec = [ - ('umount -f %s' % snapshot_mount_path), - ("rmdir %s" % snapshot_mount_path), - ("umount -f %s" % share_mount_path), - ("rmdir %s" % share_mount_path), - ("lvconvert --merge %s" % snap_lv), - ("lvcreate -L 1G --name fakesnapshotname --snapshot %s" % - share_lv), - ("e2fsck -y -f /dev/mapper/%s-fakesnapshotname" % - CONF.lvm_share_volume_group), - ("tune2fs -U random /dev/mapper/%s-fakesnapshotname" % - CONF.lvm_share_volume_group), - ("mkdir -p %s" % share_mount_path), - ("mount /dev/mapper/%s-fakename %s" % - (CONF.lvm_share_volume_group, share_mount_path)), - ("chmod 777 %s" % share_mount_path), - ("mkdir -p %s" % snapshot_mount_path), - ("mount /dev/mapper/fakevg-fakesnapshotname " - "%s" % snapshot_mount_path), - ("chmod 777 %s" % snapshot_mount_path), - ] - self.assertEqual(expected_exec, fake_utils.fake_execute_get_log()) self.assertEqual(4, mock_update_access.call_count) + mock__unmount_device.assert_has_calls( + [mock.call(self.snapshot), mock.call(self.snapshot['share'])]) + mock_lvconvert.assert_called_once_with( + CONF.lvm_share_volume_group, self.snapshot['name']) + mock_create_snapshot.assert_called_once_with( + self._context, self.snapshot) + mock_mount_device.assert_has_calls( + [mock.call(snapshot_parent_share, share_local_path), + mock.call(self.snapshot, snapshot_local_path)] + ) + mock_get_local_path.assert_has_calls( + [mock.call(snapshot_parent_share), + mock.call(self.snapshot)]) def test_snapshot_update_access(self): access_rules = [{