Make generic driver update export location after manage operation
Recently was added support of manage/unmanage operations to deneric driver. But it does not update export locations according to server info. So, make driver update it and add appropriate change to share manager to expect export locations. Change-Id: I50726ce0b82b50363f5651ffced8b6cb9f0d853f Closes-Bug: #1434009
This commit is contained in:
Valeriy Ponomaryov
parent
62cf3256d5
commit
d47e03779c
|
|
@ -671,9 +671,9 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
share_server = self.service_instance_manager.get_common_server()
|
||||
server_details = share_server['backend_details']
|
||||
|
||||
old_export_location = share['export_locations'][0]['path']
|
||||
mount_path = helper.get_share_path_by_export_location(
|
||||
share_server['backend_details'],
|
||||
share['export_locations'][0]['path'])
|
||||
share_server['backend_details'], old_export_location)
|
||||
LOG.debug("Manage: mount path = %s", mount_path)
|
||||
|
||||
mounted = self._is_device_mounted(mount_path, server_details)
|
||||
|
|
@ -723,9 +723,9 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
share_size = self._get_mounted_share_size(
|
||||
mount_path, share_server['backend_details'])
|
||||
|
||||
# TODO(vponomaryov): need update export_locations too using driver's
|
||||
# information.
|
||||
return {'size': share_size}
|
||||
export_locations = helper.get_exports_for_share(
|
||||
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]
|
||||
|
|
@ -772,6 +772,16 @@ class NASHelperBase(object):
|
|||
"""Deny access to the host."""
|
||||
raise NotImplementedError()
|
||||
|
||||
@staticmethod
|
||||
def _verify_server_has_public_address(server):
|
||||
if 'public_address' not in server:
|
||||
raise exception.ManilaException(
|
||||
_("Can not get 'public_address' for generation of export."))
|
||||
|
||||
def get_exports_for_share(self, server, old_export_location):
|
||||
"""Returns list of exports based on server info."""
|
||||
raise NotImplementedError()
|
||||
|
||||
def get_share_path_by_export_location(self, server, export_location):
|
||||
"""Returns share path by its export location."""
|
||||
raise NotImplementedError()
|
||||
|
|
@ -859,6 +869,11 @@ class NFSHelper(NASHelperBase):
|
|||
]
|
||||
self._ssh_exec(server, sync_cmd)
|
||||
|
||||
def get_exports_for_share(self, server, old_export_location):
|
||||
self._verify_server_has_public_address(server)
|
||||
path = old_export_location.split(':')[-1]
|
||||
return [':'.join([server['public_address'], path])]
|
||||
|
||||
def get_share_path_by_export_location(self, server, export_location):
|
||||
return export_location.split(':')[-1]
|
||||
|
||||
|
|
@ -973,16 +988,29 @@ class CIFSHelper(NASHelperBase):
|
|||
self._ssh_exec(server, ['sudo', 'net', 'conf', 'setparm', share_name,
|
||||
'\"hosts allow\"', value])
|
||||
|
||||
@staticmethod
|
||||
def _get_share_group_name_from_export_location(export_location):
|
||||
if '/' in export_location and '\\' in export_location:
|
||||
pass
|
||||
elif export_location.startswith('\\\\'):
|
||||
return export_location.split('\\')[-1]
|
||||
elif export_location.startswith('//'):
|
||||
return export_location.split('/')[-1]
|
||||
|
||||
msg = _("Got incorrect CIFS export location '%s'.") % export_location
|
||||
raise exception.InvalidShare(reason=msg)
|
||||
|
||||
def get_exports_for_share(self, server, old_export_location):
|
||||
self._verify_server_has_public_address(server)
|
||||
group_name = self._get_share_group_name_from_export_location(
|
||||
old_export_location)
|
||||
data = dict(ip=server['public_address'], share=group_name)
|
||||
return ['\\\\%(ip)s\\%(share)s' % data]
|
||||
|
||||
def get_share_path_by_export_location(self, server, export_location):
|
||||
# Get name of group that contains share data on CIFS server
|
||||
if export_location.startswith('\\\\'):
|
||||
group_name = export_location.split('\\')[-1]
|
||||
elif export_location.startswith('//'):
|
||||
group_name = export_location.split('/')[-1]
|
||||
else:
|
||||
msg = _("Got incorrect CIFS export location "
|
||||
"'%s'.") % export_location
|
||||
raise exception.InvalidShare(reason=msg)
|
||||
group_name = self._get_share_group_name_from_export_location(
|
||||
export_location)
|
||||
|
||||
# Get parameter 'path' from group that belongs to current share
|
||||
(out, __) = self._ssh_exec(
|
||||
|
|
|
|||
|
|
@ -355,15 +355,24 @@ class ShareManager(manager.SchedulerDependentManager):
|
|||
'status': 'available',
|
||||
'launched_at': timeutils.utcnow(),
|
||||
})
|
||||
|
||||
# NOTE(vponomaryov): we should keep only those export locations
|
||||
# that driver has calculated to avoid incompatibilities with one
|
||||
# provided by user.
|
||||
if 'export_locations' in share_update:
|
||||
self.db.share_export_locations_update(
|
||||
context, share_id, share_update.pop('export_locations'),
|
||||
delete=True)
|
||||
|
||||
self.db.share_update(context, share_id, share_update)
|
||||
except Exception as e:
|
||||
LOG.error(_LW("Manage share failed: %s"), six.text_type(e))
|
||||
except Exception:
|
||||
# NOTE(vponomaryov): set size as 1 because design expects size
|
||||
# to be set, it also will allow us to handle delete/unmanage
|
||||
# operations properly with this errored share according to quotas.
|
||||
self.db.share_update(
|
||||
context, share_id,
|
||||
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1})
|
||||
raise
|
||||
|
||||
def _update_quota_usages(self, context, project_id, usages):
|
||||
user_id = context.user_id
|
||||
|
|
|
|||
|
|
@ -1115,15 +1115,35 @@ class GenericShareDriverTestCase(test.TestCase):
|
|||
def test_manage_share_not_attached_to_cinder_volume(self):
|
||||
share = get_fake_manage_share()
|
||||
share_size = "fake"
|
||||
fake_path = '/foo/bar'
|
||||
fake_exports = ['foo', 'bar']
|
||||
server_details = {}
|
||||
self._setup_manage_mocks(server_details=server_details)
|
||||
self.mock_object(self._driver, '_get_volume',
|
||||
mock.Mock(return_value=None))
|
||||
self.mock_object(self._driver, '_get_volume')
|
||||
self.mock_object(self._driver, '_get_mounted_share_size',
|
||||
mock.Mock(return_value=share_size))
|
||||
self.mock_object(
|
||||
self._driver._helpers[share['share_proto']],
|
||||
'get_share_path_by_export_location',
|
||||
mock.Mock(return_value=fake_path))
|
||||
self.mock_object(
|
||||
self._driver._helpers[share['share_proto']],
|
||||
'get_exports_for_share',
|
||||
mock.Mock(return_value=fake_exports))
|
||||
|
||||
actual_result = self._driver.manage_existing(share, {})
|
||||
self.assertEqual({'size': share_size}, actual_result)
|
||||
result = self._driver.manage_existing(share, {})
|
||||
|
||||
self.assertEqual(
|
||||
{'size': share_size, 'export_locations': fake_exports}, result)
|
||||
self._driver._helpers[share['share_proto']].get_exports_for_share.\
|
||||
assert_called_once_with(
|
||||
server_details, share['export_locations'][0]['path'])
|
||||
self._driver._helpers[share['share_proto']].\
|
||||
get_share_path_by_export_location.assert_called_once_with(
|
||||
server_details, share['export_locations'][0]['path'])
|
||||
self._driver._get_mounted_share_size.assert_called_once_with(
|
||||
fake_path, server_details)
|
||||
self.assertFalse(self._driver._get_volume.called)
|
||||
|
||||
def test_manage_share_attached_to_cinder_volume_not_found(self):
|
||||
share = get_fake_manage_share()
|
||||
|
|
@ -1162,9 +1182,11 @@ class GenericShareDriverTestCase(test.TestCase):
|
|||
|
||||
def test_manage_share_attached_to_cinder_volume(self):
|
||||
share = get_fake_manage_share()
|
||||
fake_size = 'foobar'
|
||||
fake_exports = ['foo', 'bar']
|
||||
server_details = {'instance_id': 'fake'}
|
||||
driver_options = {'volume_id': 'fake'}
|
||||
volume = {'id': 'fake', 'name': 'fake_volume_1', 'size': 'fake'}
|
||||
volume = {'id': 'fake', 'name': 'fake_volume_1', 'size': fake_size}
|
||||
self._setup_manage_mocks(server_details=server_details)
|
||||
self.mock_object(self._driver.volume_api, 'get',
|
||||
mock.Mock(return_value=volume))
|
||||
|
|
@ -1173,10 +1195,18 @@ class GenericShareDriverTestCase(test.TestCase):
|
|||
fake_volume.id = 'fake'
|
||||
self.mock_object(self._driver.compute_api, 'instance_volumes_list',
|
||||
mock.Mock(return_value=[fake_volume]))
|
||||
self.mock_object(
|
||||
self._driver._helpers[share['share_proto']],
|
||||
'get_exports_for_share',
|
||||
mock.Mock(return_value=fake_exports))
|
||||
|
||||
actual_result = self._driver.manage_existing(share, driver_options)
|
||||
result = self._driver.manage_existing(share, driver_options)
|
||||
|
||||
self.assertEqual({'size': 'fake'}, actual_result)
|
||||
self.assertEqual(
|
||||
{'size': fake_size, 'export_locations': fake_exports}, result)
|
||||
self._driver._helpers[share['share_proto']].get_exports_for_share.\
|
||||
assert_called_once_with(
|
||||
server_details, share['export_locations'][0]['path'])
|
||||
expected_volume_update = {
|
||||
'name': self._driver._get_volume_name(share['id'])
|
||||
}
|
||||
|
|
@ -1328,6 +1358,33 @@ class NFSHelperTestCase(test.TestCase):
|
|||
self._helper._sync_nfs_temp_and_perm_files(self.server)
|
||||
self._helper._ssh_exec.assert_called_once_with(self.server, mock.ANY)
|
||||
|
||||
@ddt.data('/foo/bar', '5.6.7.8:/bar/quuz', '5.6.7.88:/foo/quuz')
|
||||
def test_get_exports_for_share(self, export_location):
|
||||
server = dict(public_address='1.2.3.4')
|
||||
|
||||
result = self._helper.get_exports_for_share(server, export_location)
|
||||
|
||||
path = export_location.split(':')[-1]
|
||||
self.assertEqual([':'.join([server['public_address'], path])], result)
|
||||
|
||||
@ddt.data(
|
||||
{'public_address_with_suffix': 'foo'},
|
||||
{'with_prefix_public_address': 'bar'},
|
||||
{'with_prefix_public_address_and_with_suffix': 'quuz'}, {})
|
||||
def test_get_exports_for_share_with_error(self, server):
|
||||
export_location = '1.2.3.4:/foo/bar'
|
||||
|
||||
self.assertRaises(
|
||||
exception.ManilaException,
|
||||
self._helper.get_exports_for_share, server, export_location)
|
||||
|
||||
@ddt.data('/foo/bar', '5.6.7.8:/foo/bar', '5.6.7.88:fake:/foo/bar')
|
||||
def test_get_share_path_by_export_location(self, export_location):
|
||||
result = self._helper.get_share_path_by_export_location(
|
||||
dict(), export_location)
|
||||
|
||||
self.assertEqual('/foo/bar', result)
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class CIFSHelperTestCase(test.TestCase):
|
||||
|
|
@ -1579,3 +1636,60 @@ class CIFSHelperTestCase(test.TestCase):
|
|||
self._helper._get_allow_hosts.assert_called_once_with(
|
||||
self.server_details, self.share_name)
|
||||
self._helper._set_allow_hosts.assert_has_calls([])
|
||||
|
||||
@ddt.data(
|
||||
'', '1.2.3.4:/nfs/like/export', '/1.2.3.4/foo', '\\1.2.3.4\\foo',
|
||||
'//1.2.3.4\\mixed_slashes_and_backslashes_one',
|
||||
'\\\\1.2.3.4/mixed_slashes_and_backslashes_two')
|
||||
def test__get_share_group_name_from_export_location(self, export_location):
|
||||
self.assertRaises(
|
||||
exception.InvalidShare,
|
||||
self._helper._get_share_group_name_from_export_location,
|
||||
export_location)
|
||||
|
||||
@ddt.data('//5.6.7.8/foo', '\\\\5.6.7.8\\foo')
|
||||
def test_get_exports_for_share(self, export_location):
|
||||
server = dict(public_address='1.2.3.4')
|
||||
self.mock_object(
|
||||
self._helper, '_get_share_group_name_from_export_location',
|
||||
mock.Mock(side_effect=(
|
||||
self._helper._get_share_group_name_from_export_location)))
|
||||
|
||||
result = self._helper.get_exports_for_share(server, export_location)
|
||||
|
||||
expected_export_location = ['\\\\%s\\foo' % server['public_address']]
|
||||
self.assertEqual(expected_export_location, result)
|
||||
self._helper._get_share_group_name_from_export_location.\
|
||||
assert_called_once_with(export_location)
|
||||
|
||||
@ddt.data(
|
||||
{'public_address_with_suffix': 'foo'},
|
||||
{'with_prefix_public_address': 'bar'},
|
||||
{'with_prefix_public_address_and_with_suffix': 'quuz'}, {})
|
||||
def test_get_exports_for_share_with_exception(self, server):
|
||||
export_location = '1.2.3.4:/foo/bar'
|
||||
|
||||
self.assertRaises(
|
||||
exception.ManilaException,
|
||||
self._helper.get_exports_for_share, server, export_location)
|
||||
|
||||
@ddt.data('//5.6.7.8/foo', '\\\\5.6.7.8\\foo')
|
||||
def test_get_share_path_by_export_location(self, export_location):
|
||||
fake_path = ' /bar/quuz\n '
|
||||
fake_server = dict()
|
||||
self.mock_object(
|
||||
self._helper, '_ssh_exec',
|
||||
mock.Mock(return_value=(fake_path, 'fake')))
|
||||
self.mock_object(
|
||||
self._helper, '_get_share_group_name_from_export_location',
|
||||
mock.Mock(side_effect=(
|
||||
self._helper._get_share_group_name_from_export_location)))
|
||||
|
||||
result = self._helper.get_share_path_by_export_location(
|
||||
fake_server, export_location)
|
||||
|
||||
self.assertEqual('/bar/quuz', result)
|
||||
self._helper._ssh_exec.assert_called_once_with(
|
||||
fake_server, ['sudo', 'net', 'conf', 'getparm', 'foo', 'path'])
|
||||
self._helper._get_share_group_name_from_export_location.\
|
||||
assert_called_once_with(export_location)
|
||||
|
|
|
|||
|
|
@ -707,29 +707,36 @@ class ShareManagerTestCase(test.TestCase):
|
|||
share = self._create_share()
|
||||
share_id = share['id']
|
||||
|
||||
self.share_manager.manage_share(self.context, share_id, {})
|
||||
self.assertRaises(
|
||||
exception.InvalidShare,
|
||||
self.share_manager.manage_share, self.context, share_id, {})
|
||||
|
||||
self.share_manager.db.share_update.assert_called_once_with(
|
||||
mock.ANY, share_id,
|
||||
utils.IsAMatcher(context.RequestContext), share_id,
|
||||
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1})
|
||||
|
||||
def test_manage_share_driver_exception(self):
|
||||
CustomException = type('CustomException', (Exception,), dict())
|
||||
self.mock_object(self.share_manager, 'driver', mock.Mock())
|
||||
self.share_manager.driver.driver_handles_share_servers = False
|
||||
self.mock_object(self.share_manager.driver,
|
||||
"manage_existing", mock.Mock(side_effect=Exception()))
|
||||
self.mock_object(
|
||||
self.share_manager.driver,
|
||||
"manage_existing", mock.Mock(side_effect=CustomException))
|
||||
self.mock_object(self.share_manager.db, 'share_update', mock.Mock())
|
||||
share = self._create_share()
|
||||
share_id = share['id']
|
||||
driver_options = {'fake': 'fake'}
|
||||
|
||||
self.share_manager.manage_share(self.context, share_id, driver_options)
|
||||
self.assertRaises(
|
||||
CustomException,
|
||||
self.share_manager.manage_share,
|
||||
self.context, share_id, driver_options)
|
||||
|
||||
self.share_manager.driver.manage_existing.\
|
||||
assert_called_once_with(mock.ANY, driver_options)
|
||||
|
||||
self.share_manager.db.share_update.assert_called_once_with(
|
||||
mock.ANY, share_id,
|
||||
utils.IsAMatcher(context.RequestContext), share_id,
|
||||
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1})
|
||||
|
||||
def test_manage_share_invalid_size(self):
|
||||
|
|
@ -743,13 +750,15 @@ class ShareManagerTestCase(test.TestCase):
|
|||
share_id = share['id']
|
||||
driver_options = {'fake': 'fake'}
|
||||
|
||||
self.share_manager.manage_share(self.context, share_id, driver_options)
|
||||
self.assertRaises(
|
||||
exception.InvalidShare,
|
||||
self.share_manager.manage_share,
|
||||
self.context, share_id, driver_options)
|
||||
|
||||
self.share_manager.driver.manage_existing.\
|
||||
assert_called_once_with(mock.ANY, driver_options)
|
||||
|
||||
self.share_manager.db.share_update.assert_called_once_with(
|
||||
mock.ANY, share_id,
|
||||
utils.IsAMatcher(context.RequestContext), share_id,
|
||||
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1})
|
||||
|
||||
def test_manage_share_quota_error(self):
|
||||
|
|
@ -757,7 +766,7 @@ class ShareManagerTestCase(test.TestCase):
|
|||
self.share_manager.driver.driver_handles_share_servers = False
|
||||
self.mock_object(self.share_manager.driver,
|
||||
"manage_existing",
|
||||
mock.Mock(return_value={'size': 1}))
|
||||
mock.Mock(return_value={'size': 3}))
|
||||
self.mock_object(self.share_manager, '_update_quota_usages',
|
||||
mock.Mock(side_effect=exception.QuotaError))
|
||||
self.mock_object(self.share_manager.db, 'share_update', mock.Mock())
|
||||
|
|
@ -765,22 +774,35 @@ class ShareManagerTestCase(test.TestCase):
|
|||
share_id = share['id']
|
||||
driver_options = {'fake': 'fake'}
|
||||
|
||||
self.share_manager.manage_share(self.context, share_id, driver_options)
|
||||
self.assertRaises(
|
||||
exception.QuotaError,
|
||||
self.share_manager.manage_share,
|
||||
self.context, share_id, driver_options)
|
||||
|
||||
self.share_manager.driver.manage_existing.\
|
||||
assert_called_once_with(mock.ANY, driver_options)
|
||||
|
||||
self.share_manager.db.share_update.assert_called_once_with(
|
||||
mock.ANY, share_id,
|
||||
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1})
|
||||
self.share_manager._update_quota_usages.assert_called_once_with(
|
||||
utils.IsAMatcher(context.RequestContext),
|
||||
share['project_id'], {'shares': 1, 'gigabytes': 3})
|
||||
|
||||
@ddt.data({'size': 1},
|
||||
{'size': 1, 'name': 'fake'})
|
||||
@ddt.data(
|
||||
{'size': 1},
|
||||
{'size': 2, 'name': 'fake'},
|
||||
{'size': 3, 'export_locations': ['foo', 'bar', 'quuz']})
|
||||
def test_manage_share_valid_share(self, driver_data):
|
||||
export_locations = driver_data.get('export_locations')
|
||||
self.mock_object(self.share_manager.db, 'share_update', mock.Mock())
|
||||
self.mock_object(self.share_manager, 'driver', mock.Mock())
|
||||
self.mock_object(self.share_manager, '_update_quota_usages',
|
||||
mock.Mock())
|
||||
self.mock_object(
|
||||
self.share_manager.db,
|
||||
'share_export_locations_update',
|
||||
mock.Mock(side_effect=(
|
||||
self.share_manager.db.share_export_locations_update)))
|
||||
self.share_manager.driver.driver_handles_share_servers = False
|
||||
self.mock_object(self.share_manager.driver,
|
||||
"manage_existing",
|
||||
|
|
@ -793,12 +815,19 @@ class ShareManagerTestCase(test.TestCase):
|
|||
|
||||
self.share_manager.driver.manage_existing.\
|
||||
assert_called_once_with(mock.ANY, driver_options)
|
||||
|
||||
if export_locations:
|
||||
self.share_manager.db.share_export_locations_update.\
|
||||
assert_called_once_with(
|
||||
utils.IsAMatcher(context.RequestContext),
|
||||
share_id, export_locations, delete=True)
|
||||
else:
|
||||
self.assertFalse(
|
||||
self.share_manager.db.share_export_locations_update.called)
|
||||
valid_share_data = {'status': 'available', 'launched_at': mock.ANY}
|
||||
valid_share_data.update(driver_data)
|
||||
self.share_manager.db.share_update.assert_called_once_with(
|
||||
mock.ANY, share_id, valid_share_data
|
||||
)
|
||||
utils.IsAMatcher(context.RequestContext),
|
||||
share_id, valid_share_data)
|
||||
|
||||
def test_update_quota_usages_new(self):
|
||||
self.mock_object(self.share_manager.db, 'quota_usage_get',
|
||||
|
|
|
|||
Loading…
Reference in New Issue