openstack
/
manila
Code Issues Proposed changes

Implement shrink_share() method in Generic driver 

- Add implementation for shrink_share() method
- Add methods to generic driver helpers:
  - disable_access_for_maintenance()
  - restore_access_after_maintenance()
- Add appropriate unit tests
- Add appropriate tempest tests
- Update generic driver documentation

Partially implements bp shrink-share-in-generic-driver

Change-Id: I256db947b65f66dfe514e0c0b350c1949da7f7be
This commit is contained in:
Igor Malinovskiy 2015-07-20 18:11:08 +03:00
parent 1d00bfdfd2
commit cc97cfe8e0
7 changed files with 444 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
contrib/tempest/tempest/api/share/test_shares_actions.py
View File

@ -394,6 +394,22 @@ class SharesActionsTest(base.BaseSharesTest):
share = self.shares_client.get_share(share['id'])
self.assertEqual(new_size, share['size'])
@test.attr(type=["gate", ])
@testtools.skipUnless(
CONF.share.run_shrink_tests,
"Share shrink tests are disabled.")
def test_shrink_share(self):
share = self.create_share(size=2, cleanup_in_class=False)
new_size = 1
# shrink share and wait for active status
self.shares_client.shrink_share(share['id'], new_size)
self.shares_client.wait_for_share_status(share['id'], 'available')
# check state and new size
share = self.shares_client.get_share(share['id'])
self.assertEqual(new_size, share['size'])
class SharesRenameTest(base.BaseSharesTest):

44
contrib/tempest/tempest/api/share/test_shares_actions_negative.py
View File

@ -89,4 +89,48 @@ class SharesActionsNegativeTest(base.BaseSharesTest):
self.assertRaises(lib_exc.BadRequest,
self.shares_client.extend_share,
share['id'],
new_size)
@test.attr(type=["negative", ])
@testtools.skipUnless(
CONF.share.run_shrink_tests,
"Share shrink tests are disabled.")
def test_share_shrink_with_greater_size(self):
new_size = int(self.share['size']) + 1
# shrink share with invalid size and check result
self.assertRaises(lib_exc.BadRequest,
self.shares_client.shrink_share,
self.share['id'],
new_size)
@test.attr(type=["negative", ])
@testtools.skipUnless(
CONF.share.run_shrink_tests,
"Share shrink tests are disabled.")
def test_share_shrink_with_same_size(self):
new_size = int(self.share['size'])
# shrink share with invalid size and check result
self.assertRaises(lib_exc.BadRequest,
self.shares_client.shrink_share,
self.share['id'],
new_size)
@test.attr(type=["negative", ])
@testtools.skipUnless(
CONF.share.run_shrink_tests,
"Share shrink tests are disabled.")
def test_share_shrink_with_invalid_share_state(self):
share = self.create_share(size=2, cleanup_in_class=False)
new_size = int(share['size']) - 1
# set "error" state
admin_client = clients.AdminManager().shares_client
admin_client.reset_state(share['id'])
# run shrink operation on same share and check result
self.assertRaises(lib_exc.BadRequest,
self.shares_client.shrink_share,
share['id'],
new_size)

15
contrib/tempest/tempest/config_share.py
View File

@ -113,6 +113,16 @@ ShareGroup = [
help="Defines whether to run manage/unmanage tests or not. "
"These test may leave orphaned resources, so be careful "
"enabling this opt."),
cfg.BoolOpt("run_extend_tests",
default=True,
help="Defines whether to run share extend tests or not. "
"Disable this feature if used driver doesn't "
"support it."),
cfg.BoolOpt("run_shrink_tests",
default=True,
help="Defines whether to run share shrink tests or not. "
"Disable this feature if used driver doesn't "
"support it."),
cfg.StrOpt("image_with_share_tools",
default="manila-service-image",
help="Image name for vm booting with nfs/smb clients tool."),
@ -125,11 +135,6 @@ ShareGroup = [
cfg.StrOpt("client_vm_flavor_ref",
default="100",
help="Flavor used for client vm in scenario tests."),
cfg.BoolOpt("run_extend_tests",
default=True,
help="Defines whether to run share extend tests or not."
"Disable this feature if used driver doesn't "
"support it."),
]

11
contrib/tempest/tempest/services/share/json/shares_client.py
View File

@ -168,6 +168,17 @@ class SharesClient(rest_client.RestClient):
self.expected_success(202, resp.status)
return body
def shrink_share(self, share_id, new_size):
post_body = {
"os-shrink": {
"new_size": new_size,
}
}
body = json.dumps(post_body)
resp, body = self.post("shares/%s/action" % share_id, body)
self.expected_success(202, resp.status)
return body
def create_snapshot(self, share_id, name=None, description=None,
force=False):
if name is None:

4
doc/source/devref/generic_driver.rst
View File

@ -86,6 +86,10 @@ Known restrictions
- Liberty version adds a share extend capability. Share access will be briefly
interrupted during an extend operation.
- Liberty version adds a share shrink capability, but this capability is not
effective because generic driver shrinks only filesystem size and doesn't
shrink the size of Cinder volume.
The :mod:`manila.share.drivers.generic` Module
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

144
manila/share/drivers/generic.py
View File

@ -25,6 +25,7 @@ from oslo_log import log
from oslo_utils import excutils
from oslo_utils import importutils
from oslo_utils import strutils
from oslo_utils import units
import retrying
import six
@ -88,6 +89,10 @@ share_opts = [
CONF = cfg.CONF
CONF.register_opts(share_opts)
# NOTE(u_glide): These constants refer to the column number in the "df" output
BLOCK_DEVICE_SIZE_INDEX = 1
USED_SPACE_INDEX = 2
def ensure_server(f):
@ -578,12 +583,60 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
msg_error=msg_error, msg_timeout=msg_timeout
)
def _resize_filesystem(self, server_details, volume):
@ensure_server
def shrink_share(self, share, new_size, share_server=None):
server_details = share_server['backend_details']
helper = self._get_helper(share)
export_location = share['export_locations'][0]['path']
mount_path = helper.get_share_path_by_export_location(
server_details, export_location)
consumed_space = self._get_consumed_space(mount_path, server_details)
LOG.debug("Consumed space on share: %s", consumed_space)
if consumed_space >= new_size:
raise exception.ShareShrinkingPossibleDataLoss(
share_id=share['id'])
volume = self._get_volume(self.admin_context, share['id'])
helper.disable_access_for_maintenance(server_details, share['name'])
self._unmount_device(share, server_details)
try:
self._resize_filesystem(server_details, volume, new_size=new_size)
except exception.Invalid:
raise exception.ShareShrinkingPossibleDataLoss(
share_id=share['id'])
except Exception as e:
msg = _("Cannot shrink share: %s") % six.text_type(e)
raise exception.Invalid(msg)
finally:
self._mount_device(share, server_details, volume)
helper.restore_access_after_maintenance(server_details,
share['name'])
def _resize_filesystem(self, server_details, volume, new_size=None):
"""Resize filesystem of provided volume."""
check_command = ['sudo', 'fsck', '-pf', volume['mountpoint']]
self._ssh_exec(server_details, check_command)
command = ['sudo', 'resize2fs', volume['mountpoint']]
self._ssh_exec(server_details, command)
if new_size:
command.append("%sG" % six.text_type(new_size))
try:
self._ssh_exec(server_details, command)
except processutils.ProcessExecutionError as e:
if e.stderr.find('New size smaller than minimum') != -1:
msg = (_("Invalid 'new_size' provided: %s")
% six.text_type(new_size))
raise exception.Invalid(msg)
else:
msg = _("Cannot resize file-system: %s") % six.text_type(e)
raise exception.ManilaException(msg)
def _is_share_server_active(self, context, share_server):
"""Check if the share server is active."""
@ -818,13 +871,27 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
server_details, old_export_location)
return {'size': share_size, 'export_locations': export_locations}
def _get_mounted_share_size(self, mount_path, server_details):
share_size_cmd = ['df', '-PBG', mount_path]
def _get_mount_stats_by_index(self, mount_path, server_details, index,
block_size='G'):
"""Get mount stats using df shell command.
:param mount_path: Share path on share server
:param server_details: Share server connection details
:param index: Data index in df command output:
BLOCK_DEVICE_SIZE_INDEX - Size of block device
USED_SPACE_INDEX - Used space
:param block_size: size of block (example: G, M, Mib, etc)
:returns: value of provided index
"""
share_size_cmd = ['df', '-PB%s' % block_size, mount_path]
output, __ = self._ssh_exec(server_details, share_size_cmd)
lines = output.split('\n')
return int(lines[1].split()[index][:-1])
def _get_mounted_share_size(self, mount_path, server_details):
try:
size = int(lines[1].split()[1][:-1])
size = self._get_mount_stats_by_index(
mount_path, server_details, BLOCK_DEVICE_SIZE_INDEX)
except Exception as e:
msg = _("Cannot calculate size of share %(path)s : %(error)s") % {
'path': mount_path,
@ -834,6 +901,21 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
return size
def _get_consumed_space(self, mount_path, server_details):
try:
size = self._get_mount_stats_by_index(
mount_path, server_details, USED_SPACE_INDEX, block_size='M')
size /= float(units.Ki)
except Exception as e:
msg = _("Cannot calculate consumed space on share "
"%(path)s : %(error)s") % {
'path': mount_path,
'error': six.text_type(e)
}
raise exception.InvalidShare(reason=msg)
return size
class NASHelperBase(object):
"""Interface to work with share."""
@ -877,6 +959,16 @@ class NASHelperBase(object):
"""Returns share path by its export location."""
raise NotImplementedError()
def disable_access_for_maintenance(self, server, share_name):
"""Disables access to share to perform maintenance operations."""
def restore_access_after_maintenance(self, server, share_name):
"""Enables access to share after maintenance operations were done."""
def _get_maintenance_file_path(self, share_name):
return os.path.join(self.configuration.share_mount_path,
"%s.maintenance" % share_name)
def nfs_synchronized(f):
@ -968,6 +1060,32 @@ class NFSHelper(NASHelperBase):
def get_share_path_by_export_location(self, server, export_location):
return export_location.split(':')[-1]
@nfs_synchronized
def disable_access_for_maintenance(self, server, share_name):
maintenance_file = self._get_maintenance_file_path(share_name)
backup_exports = [
'cat', const.NFS_EXPORTS_FILE,
'| grep', share_name,
'| sudo tee', maintenance_file
]
self._ssh_exec(server, backup_exports)
local_path = os.path.join(self.configuration.share_mount_path,
share_name)
self._ssh_exec(server, ['sudo', 'exportfs', '-u', local_path])
self._sync_nfs_temp_and_perm_files(server)
@nfs_synchronized
def restore_access_after_maintenance(self, server, share_name):
maintenance_file = self._get_maintenance_file_path(share_name)
restore_exports = [
'cat', maintenance_file,
'| sudo tee -a', const.NFS_EXPORTS_FILE,
'&& sudo exportfs -r',
'&& sudo rm -f', maintenance_file
]
self._ssh_exec(server, restore_exports)
class CIFSHelper(NASHelperBase):
"""Manage shares in samba server by net conf tool.
@ -1109,3 +1227,19 @@ class CIFSHelper(NASHelperBase):
# Remove special symbols from response and return path
return out.strip()
def disable_access_for_maintenance(self, server, share_name):
maintenance_file = self._get_maintenance_file_path(share_name)
allowed_hosts = " ".join(self._get_allow_hosts(server, share_name))
backup_exports = [
'echo', "'%s'" % allowed_hosts, '| sudo tee', maintenance_file
]
self._ssh_exec(server, backup_exports)
self._set_allow_hosts(server, [], share_name)
def restore_access_after_maintenance(self, server, share_name):
maintenance_file = self._get_maintenance_file_path(share_name)
(exports, __) = self._ssh_exec(server, ['cat', maintenance_file])
self._set_allow_hosts(server, exports.split(), share_name)
self._ssh_exec(server, ['sudo rm -f', maintenance_file])

222
manila/tests/share/drivers/test_generic.py
View File

@ -1318,6 +1318,35 @@ class GenericShareDriverTestCase(test.TestCase):
self._driver._get_mounted_share_size,
'/fake/path', {})
def test_get_consumed_space(self):
mount_path = "fake_path"
server_details = {}
index = 2
valid_result = 1
self.mock_object(self._driver, '_get_mount_stats_by_index',
mock.Mock(return_value=valid_result * 1024))
actual_result = self._driver._get_consumed_space(
mount_path, server_details)
self.assertEqual(valid_result, actual_result)
self._driver._get_mount_stats_by_index.assert_called_once_with(
mount_path, server_details, index, block_size='M'
)
def test_get_consumed_space_invalid(self):
self.mock_object(
self._driver,
'_get_mount_stats_by_index',
mock.Mock(side_effect=exception.ManilaException("fake"))
)
self.assertRaises(
exception.InvalidShare,
self._driver._get_consumed_space,
"fake", "fake"
)
def test_extend_share(self):
fake_volume = "fake"
fake_share = {'id': 'fake'}
@ -1376,14 +1405,122 @@ class GenericShareDriverTestCase(test.TestCase):
fake_volume = {'mountpoint': '/dev/fake'}
self.mock_object(self._driver, '_ssh_exec')
self._driver._resize_filesystem(fake_server_details, fake_volume)
self._driver._resize_filesystem(
fake_server_details, fake_volume, new_size=123)
self._driver._ssh_exec.assert_any_call(
fake_server_details, ['sudo', 'fsck', '-pf', '/dev/fake'])
self._driver._ssh_exec.assert_any_call(
fake_server_details, ['sudo', 'resize2fs', '/dev/fake'])
fake_server_details,
['sudo', 'resize2fs', '/dev/fake', "%sG" % 123]
)
self.assertEqual(2, self._driver._ssh_exec.call_count)
@ddt.data(
{
'source': processutils.ProcessExecutionError(
stderr="resize2fs: New size smaller than minimum (123456)"),
'target': exception.Invalid
},
{
'source': processutils.ProcessExecutionError(stderr="fake_error"),
'target': exception.ManilaException
}
)
@ddt.unpack
def test_resize_filesystem_invalid_new_size(self, source, target):
fake_server_details = {'fake': 'fake'}
fake_volume = {'mountpoint': '/dev/fake'}
ssh_mock = mock.Mock(side_effect=["fake", source])
self.mock_object(self._driver, '_ssh_exec', ssh_mock)
self.assertRaises(
target,
self._driver._resize_filesystem,
fake_server_details, fake_volume, new_size=123
)
def test_shrink_share_invalid_size(self):
fake_share = {'id': 'fake', 'export_locations': [{'path': 'test'}]}
new_size = 123
self.mock_object(
self._driver.service_instance_manager,
'get_common_server',
mock.Mock(return_value=self.server)
)
self.mock_object(self._driver, '_get_helper')
self.mock_object(self._driver, '_get_consumed_space',
mock.Mock(return_value=200))
CONF.set_default('driver_handles_share_servers', False)
self.assertRaises(
exception.ShareShrinkingPossibleDataLoss,
self._driver.shrink_share,
fake_share,
new_size
)
self._driver._get_helper.assert_called_once_with(fake_share)
self._driver._get_consumed_space.assert_called_once_with(
mock.ANY, self.server['backend_details'])
def _setup_shrink_mocks(self):
share = {'id': 'fake', 'export_locations': [{'path': 'test'}],
'name': 'fake'}
volume = {'id': 'fake'}
new_size = 123
server_details = self.server['backend_details']
self.mock_object(
self._driver.service_instance_manager,
'get_common_server',
mock.Mock(return_value=self.server)
)
helper = mock.Mock()
self.mock_object(self._driver, '_get_helper',
mock.Mock(return_value=helper))
self.mock_object(self._driver, '_get_consumed_space',
mock.Mock(return_value=100))
self.mock_object(self._driver, '_get_volume',
mock.Mock(return_value=volume))
self.mock_object(self._driver, '_unmount_device')
self.mock_object(self._driver, '_mount_device')
CONF.set_default('driver_handles_share_servers', False)
return share, volume, new_size, server_details, helper
@ddt.data({'source': exception.Invalid("fake"),
'target': exception.ShareShrinkingPossibleDataLoss},
{'source': exception.ManilaException("fake"),
'target': exception.Invalid})
@ddt.unpack
def test_shrink_share_error_on_resize_fs(self, source, target):
share, vol, size, server_details, _ = self._setup_shrink_mocks()
resize_mock = mock.Mock(side_effect=source)
self.mock_object(self._driver, '_resize_filesystem', resize_mock)
self.assertRaises(target, self._driver.shrink_share, share, size)
resize_mock.assert_called_once_with(server_details, vol,
new_size=size)
def test_shrink_share(self):
share, vol, size, server_details, helper = self._setup_shrink_mocks()
self.mock_object(self._driver, '_resize_filesystem')
self._driver.shrink_share(share, size)
self._driver._get_helper.assert_called_once_with(share)
self._driver._get_consumed_space.assert_called_once_with(
mock.ANY, server_details)
self._driver._get_volume.assert_called_once_with(mock.ANY, share['id'])
self._driver._unmount_device.assert_called_once_with(share,
server_details)
self._driver._resize_filesystem(
server_details, vol, new_size=size)
self._driver._mount_device(share, server_details, vol)
self.assertTrue(helper.disable_access_for_maintenance.called)
self.assertTrue(helper.restore_access_after_maintenance.called)
@ddt.data({'share_servers': [], 'result': None},
{'share_servers': None, 'result': None},
{'share_servers': ['fake'], 'result': 'fake'},
@ -1554,6 +1691,49 @@ class NFSHelperTestCase(test.TestCase):
self.assertEqual('/foo/bar', result)
def test_disable_access_for_maintenance(self):
fake_maintenance_path = "fake.path"
share_mount_path = os.path.join(
self._helper.configuration.share_mount_path, self.share_name)
self.mock_object(self._helper, '_ssh_exec')
self.mock_object(self._helper, '_sync_nfs_temp_and_perm_files')
self.mock_object(self._helper, '_get_maintenance_file_path',
mock.Mock(return_value=fake_maintenance_path))
self._helper.disable_access_for_maintenance(
self.server, self.share_name)
self._helper._ssh_exec.assert_any_call(
self.server,
['cat', const.NFS_EXPORTS_FILE,
'| grep', self.share_name,
'| sudo tee', fake_maintenance_path]
)
self._helper._ssh_exec.assert_any_call(
self.server,
['sudo', 'exportfs', '-u', share_mount_path]
)
self._helper._sync_nfs_temp_and_perm_files.assert_called_once_with(
self.server
)
def test_restore_access_after_maintenance(self):
fake_maintenance_path = "fake.path"
self.mock_object(self._helper, '_get_maintenance_file_path',
mock.Mock(return_value=fake_maintenance_path))
self.mock_object(self._helper, '_ssh_exec')
self._helper.restore_access_after_maintenance(
self.server, self.share_name)
self._helper._ssh_exec.assert_called_once_with(
self.server,
['cat', fake_maintenance_path,
'| sudo tee -a', const.NFS_EXPORTS_FILE,
'&& sudo exportfs -r', '&& sudo rm -f',
fake_maintenance_path]
)
@ddt.ddt
class CIFSHelperTestCase(test.TestCase):
@ -1862,3 +2042,41 @@ class CIFSHelperTestCase(test.TestCase):
fake_server, ['sudo', 'net', 'conf', 'getparm', 'foo', 'path'])
self._helper._get_share_group_name_from_export_location.\
assert_called_once_with(export_location)
def test_disable_access_for_maintenance(self):
allowed_hosts = ['test', 'test2']
maintenance_path = os.path.join(
self._helper.configuration.share_mount_path,
"%s.maintenance" % self.share_name)
self.mock_object(self._helper, '_set_allow_hosts')
self.mock_object(self._helper, '_get_allow_hosts',
mock.Mock(return_value=allowed_hosts))
self._helper.disable_access_for_maintenance(
self.server_details, self.share_name)
self._helper._get_allow_hosts.assert_called_once_with(
self.server_details, self.share_name)
self._helper._set_allow_hosts.assert_called_once_with(
self.server_details, [], self.share_name)
valid_cmd = ['echo', "'test test2'", '| sudo tee', maintenance_path]
self._helper._ssh_exec.assert_called_once_with(
self.server_details, valid_cmd)
def test_restore_access_after_maintenance(self):
fake_maintenance_path = "test.path"
self.mock_object(self._helper, '_set_allow_hosts')
self.mock_object(self._helper, '_get_maintenance_file_path',
mock.Mock(return_value=fake_maintenance_path))
self.mock_object(self._helper, '_ssh_exec',
mock.Mock(side_effect=[("fake fake2", 0), "fake"]))
self._helper.restore_access_after_maintenance(
self.server_details, self.share_name)
self._helper._set_allow_hosts.assert_called_once_with(
self.server_details, ['fake', 'fake2'], self.share_name)
self._helper._ssh_exec.assert_any_call(
self.server_details, ['cat', fake_maintenance_path])
self._helper._ssh_exec.assert_any_call(
self.server_details, ['sudo rm -f', fake_maintenance_path])