openstack/deb-cinder
Code Issues Proposed changes

NetApp: Use Local Cache to Copy an Available Image

Browse Source 
This fixes a defect where a locally cached image is not
always used by the Copy Offload feature. By using an available
image, local to the same backend as the share, performance can be
improved as a clone operation is executed instead of a remote copy.

Also, moved and refactored tests from test_netapp_nfs.py to the
correct test class under the netapp driver test directory. This is
part of the ongoing refactoring of test_netapp_nfs.py to meet
current code standards.

Change-Id: Ia4d4435d81d2f7dabd12582553b89f9b9d5ab38a
Closes-Bug: 1568998
This commit is contained in:
Chuck Fouts 2016-04-11 14:27:00 -04:00
parent 2f0e3fc45e
commit fdca451cbd
4 changed files with 256 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
cinder/tests/unit/test_netapp_nfs.py
View File

@ -1314,71 +1314,6 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase):
self.assertRaises(OSError, drv._copy_from_img_service,
context, volume, image_service, image_id)
def test_copyoffload_frm_cache_success(self):
drv = self._driver
context = object()
volume = {'id': 'vol_id', 'name': 'name'}
image_service = object()
image_id = 'image_id'
drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20))
nfs_base.NetAppNfsDriver.copy_image_to_volume = mock.Mock()
drv._get_provider_location = mock.Mock(return_value='share')
drv._get_vol_for_share = mock.Mock(return_value='vol')
drv._update_stale_vols = mock.Mock()
drv._find_image_in_cache = mock.Mock(return_value=[('share', 'img')])
drv._copy_from_cache = mock.Mock(return_value=True)
drv.copy_image_to_volume(context, volume, image_service, image_id)
drv._copy_from_cache.assert_called_once_with(volume,
image_id,
[('share', 'img')])
def test_copyoffload_frm_img_service_success(self):
drv = self._driver
context = object()
volume = {'id': 'vol_id', 'name': 'name'}
image_service = object()
image_id = 'image_id'
drv._client = mock.Mock()
drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20))
nfs_base.NetAppNfsDriver.copy_image_to_volume = mock.Mock()
drv._get_provider_location = mock.Mock(return_value='share')
drv._get_vol_for_share = mock.Mock(return_value='vol')
drv._update_stale_vols = mock.Mock()
drv._find_image_in_cache = mock.Mock(return_value=False)
drv._copy_from_img_service = mock.Mock()
drv.copy_image_to_volume(context, volume, image_service, image_id)
drv._copy_from_img_service.assert_called_once_with(context,
volume,
image_service,
image_id)
def test_cache_copyoffload_workflow_success(self):
drv = self._driver
volume = {'id': 'vol_id', 'name': 'name', 'size': 1}
image_id = 'image_id'
cache_result = [('ip1:/openstack', 'img-cache-imgid')]
drv._get_ip_verify_on_cluster = mock.Mock(return_value='ip1')
drv._get_host_ip = mock.Mock(return_value='ip2')
drv._get_export_path = mock.Mock(return_value='/exp_path')
drv._execute = mock.Mock()
drv._register_image_in_cache = mock.Mock()
drv._get_provider_location = mock.Mock(return_value='/share')
drv._post_clone_image = mock.Mock()
copied = drv._copy_from_cache(volume, image_id, cache_result)
self.assertTrue(copied)
drv._get_ip_verify_on_cluster.assert_any_call('ip1')
drv._get_export_path.assert_called_with('vol_id')
drv._execute.assert_called_once_with('cof_path', 'ip1', 'ip1',
'/openstack/img-cache-imgid',
'/exp_path/name',
run_as_root=False,
check_exit_code=0)
drv._post_clone_image.assert_called_with(volume)
drv._get_provider_location.assert_called_with('vol_id')
@mock.patch.object(image_utils, 'qemu_img_info')
def test_img_service_raw_copyoffload_workflow_success(self,
mock_qemu_img_info):

1
cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py
View File

@ -38,6 +38,7 @@ NFS_HOST_STRING = '%s@%s#%s' % (HOST_NAME, BACKEND_NAME, NFS_SHARE)
FLEXVOL = 'openstack-flexvol'
NFS_FILE_PATH = 'nfsvol'
PATH = '/vol/%s/%s' % (POOL_NAME, LUN_NAME)
IMAGE_FILE_ID = 'img-cache-imgid'
LUN_METADATA = {
'OsType': None,
'SpaceReserved': 'true',

192
cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py
View File

@ -55,6 +55,7 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
self.driver.vserver = fake.VSERVER_NAME
self.driver.ssc_enabled = True
self.driver.perf_library = mock.Mock()
self.driver.zapi_client = mock.Mock()
def get_config_cmode(self):
config = na_fakes.create_configuration_cmode()
@ -65,6 +66,7 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
config.netapp_transport_type = 'http'
config.netapp_server_port = '80'
config.netapp_vserver = fake.VSERVER_NAME
config.netapp_copyoffload_tool_path = 'copyoffload_tool_path'
return config
@mock.patch.object(perf_cmode, 'PerformanceCmodeLibrary', mock.Mock())
@ -174,7 +176,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
ssc_cmode, 'check_ssc_api_permissions')
mock_start_periodic_tasks = self.mock_object(
self.driver, '_start_periodic_tasks')
self.driver.zapi_client = mock.Mock()
self.driver.check_for_setup_error()
@ -188,7 +189,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
fake_volume = {'provider_location': fake_provider_location}
self.mock_object(self.driver, '_delete_backing_file_for_volume')
self.mock_object(na_utils, 'get_valid_qos_policy_group_info')
self.driver.zapi_client = mock.Mock()
mock_prov_deprov = self.mock_object(self.driver,
'_post_prov_deprov_in_ssc')
@ -235,7 +235,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
def test_delete_volume_on_filer(self):
mock_get_vs_ip = self.mock_object(self.driver, '_get_export_ip_path')
mock_get_vs_ip.return_value = (fake.VSERVER_NAME, '/%s' % fake.FLEXVOL)
self.driver.zapi_client = mock.Mock()
mock_zapi_delete = self.driver.zapi_client.delete_file
self.driver._delete_volume_on_filer(fake.NFS_VOLUME)
@ -285,7 +284,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
def test_delete_snapshot_on_filer(self):
mock_get_vs_ip = self.mock_object(self.driver, '_get_export_ip_path')
mock_get_vs_ip.return_value = (fake.VSERVER_NAME, '/%s' % fake.FLEXVOL)
self.driver.zapi_client = mock.Mock()
mock_zapi_delete = self.driver.zapi_client.delete_file
self.driver._delete_snapshot_on_filer(fake.test_snapshot)
@ -298,7 +296,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
mock_get_info = self.mock_object(na_utils,
'get_valid_qos_policy_group_info')
mock_get_info.return_value = fake.QOS_POLICY_GROUP_INFO
self.driver.zapi_client = mock.Mock()
mock_provision_qos = self.driver.zapi_client.provision_qos_policy_group
mock_set_policy = self.mock_object(self.driver,
'_set_qos_policy_group_on_volume')
@ -323,7 +320,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
mock_get_info = self.mock_object(na_utils,
'get_valid_qos_policy_group_info')
mock_get_info.return_value = fake.QOS_POLICY_GROUP_INFO
self.driver.zapi_client = mock.Mock()
mock_provision_qos = self.driver.zapi_client.provision_qos_policy_group
mock_set_policy = self.mock_object(self.driver,
'_set_qos_policy_group_on_volume')
@ -354,7 +350,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
mock_get_info = self.mock_object(na_utils,
'get_valid_qos_policy_group_info')
mock_get_info.side_effect = exception.Invalid
self.driver.zapi_client = mock.Mock()
mock_provision_qos = self.driver.zapi_client.provision_qos_policy_group
mock_set_policy = self.mock_object(self.driver,
'_set_qos_policy_group_on_volume')
@ -383,7 +378,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
mock_extract_host = self.mock_object(volume_utils, 'extract_host')
mock_extract_host.return_value = fake.NFS_SHARE
self.driver.zapi_client = mock.Mock()
mock_get_flex_vol_name =\
self.driver.zapi_client.get_vol_by_junc_vserver
mock_get_flex_vol_name.return_value = fake.FLEXVOL
@ -410,7 +404,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
mock_extract_host = self.mock_object(volume_utils, 'extract_host')
self.driver.zapi_client = mock.Mock()
mock_get_flex_vol_name =\
self.driver.zapi_client.get_vol_by_junc_vserver
@ -432,7 +425,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
mock_extract_host = self.mock_object(volume_utils, 'extract_host')
self.driver.zapi_client = mock.Mock()
mock_get_flex_vol_name =\
self.driver.zapi_client.get_vol_by_junc_vserver
@ -452,7 +444,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
'get_valid_qos_policy_group_info')
mock_get_info.return_value = fake.QOS_POLICY_GROUP_INFO
self.driver.zapi_client = mock.Mock()
mock_mark_for_deletion =\
self.driver.zapi_client.mark_qos_policy_group_for_deletion
@ -507,7 +498,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
def test_start_periodic_tasks(self):
self.driver.zapi_client = mock.Mock()
mock_remove_unused_qos_policy_groups = self.mock_object(
self.driver.zapi_client,
'remove_unused_qos_policy_groups')
@ -618,3 +608,181 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
def test_get_vol_for_share_no_ssc_vols(self):
with mock.patch.object(self.driver, 'ssc_vols', None):
self.assertIsNone(self.driver._get_vol_for_share(fake.NFS_SHARE))
def test_find_image_location_with_local_copy(self):
local_share = '/share'
cache_result = [
('ip1:/openstack', 'img-cache-imgid'),
('ip2:/openstack', 'img-cache-imgid'),
(local_share, 'img-cache-imgid'),
('ip3:/openstack', 'img-cache-imgid'),
]
self.driver._get_provider_location = mock.Mock(
return_value=local_share)
cache_copy, found_local_copy = self.driver._find_image_location(
cache_result, fake.VOLUME_ID)
self.assertEqual(cache_result[2], cache_copy)
self.assertTrue(found_local_copy)
self.driver._get_provider_location.assert_called_once_with(
fake.VOLUME_ID)
def test_find_image_location_with_remote_copy(self):
cache_result = [('ip1:/openstack', 'img-cache-imgid')]
self.driver._get_provider_location = mock.Mock(return_value='/share')
cache_copy, found_local_copy = self.driver._find_image_location(
cache_result, fake.VOLUME_ID)
self.assertEqual(cache_result[0], cache_copy)
self.assertFalse(found_local_copy)
self.driver._get_provider_location.assert_called_once_with(
fake.VOLUME_ID)
def test_find_image_location_without_cache_copy(self):
cache_result = []
self.driver._get_provider_location = mock.Mock(return_value='/share')
cache_copy, found_local_copy = self.driver._find_image_location(
cache_result, fake.VOLUME_ID)
self.assertIsNone(cache_copy)
self.assertFalse(found_local_copy)
self.driver._get_provider_location.assert_called_once_with(
fake.VOLUME_ID)
def test_clone_file_dest_exists(self):
self.driver._get_vserver_and_exp_vol = mock.Mock(
return_value=(fake.VSERVER_NAME, fake.EXPORT_PATH))
self.driver.zapi_client.clone_file = mock.Mock()
self.driver._clone_file_dst_exists(
fake.NFS_SHARE, fake.IMAGE_FILE_ID, fake.VOLUME['name'],
dest_exists=True)
self.driver._get_vserver_and_exp_vol.assert_called_once_with(
share=fake.NFS_SHARE)
self.driver.zapi_client.clone_file.assert_called_once_with(
fake.EXPORT_PATH, fake.IMAGE_FILE_ID, fake.VOLUME['name'],
fake.VSERVER_NAME, dest_exists=True)
def test_get_source_ip_and_path(self):
self.driver._get_ip_verify_on_cluster = mock.Mock(
return_value=fake.SHARE_IP)
src_ip, src_path = self.driver._get_source_ip_and_path(
fake.NFS_SHARE, fake.IMAGE_FILE_ID)
self.assertEqual(fake.SHARE_IP, src_ip)
assert_path = fake.EXPORT_PATH + '/' + fake.IMAGE_FILE_ID
self.assertEqual(assert_path, src_path)
self.driver._get_ip_verify_on_cluster.assert_called_once_with(
fake.SHARE_IP)
def test_get_destination_ip_and_path(self):
self.driver._get_ip_verify_on_cluster = mock.Mock(
return_value=fake.SHARE_IP)
self.driver._get_host_ip = mock.Mock(return_value='host.ip')
self.driver._get_export_path = mock.Mock(return_value=fake.EXPORT_PATH)
dest_ip, dest_path = self.driver._get_destination_ip_and_path(
fake.VOLUME)
self.assertEqual(fake.SHARE_IP, dest_ip)
assert_path = fake.EXPORT_PATH + '/' + fake.LUN_NAME
self.assertEqual(assert_path, dest_path)
self.driver._get_ip_verify_on_cluster.assert_called_once_with(
'host.ip')
self.driver._get_host_ip.assert_called_once_with(fake.VOLUME_ID)
self.driver._get_export_path.assert_called_once_with(fake.VOLUME_ID)
def test_copy_from_remote_cache(self):
source_ip = '192.0.1.1'
source_path = '/openstack/img-cache-imgid'
cache_copy = ('192.0.1.1:/openstack', fake.IMAGE_FILE_ID)
dest_path = fake.EXPORT_PATH + '/' + fake.VOLUME['name']
self.driver._execute = mock.Mock()
self.driver._get_source_ip_and_path = mock.Mock(
return_value=(source_ip, source_path))
self.driver._get_destination_ip_and_path = mock.Mock(
return_value=(fake.SHARE_IP, dest_path))
self.driver._register_image_in_cache = mock.Mock()
self.driver._copy_from_remote_cache(
fake.VOLUME, fake.IMAGE_FILE_ID, cache_copy)
self.driver._execute.assert_called_once_with(
'copyoffload_tool_path', source_ip, fake.SHARE_IP,
source_path, dest_path, run_as_root=False, check_exit_code=0)
self.driver._get_source_ip_and_path.assert_called_once_with(
cache_copy[0], fake.IMAGE_FILE_ID)
self.driver._get_destination_ip_and_path.assert_called_once_with(
fake.VOLUME)
self.driver._register_image_in_cache.assert_called_once_with(
fake.VOLUME, fake.IMAGE_FILE_ID)
def test_copy_from_cache_workflow_remote_location(self):
cache_result = [('ip1:/openstack', fake.IMAGE_FILE_ID),
('ip2:/openstack', fake.IMAGE_FILE_ID),
('ip3:/openstack', fake.IMAGE_FILE_ID)]
self.driver._find_image_location = mock.Mock(return_value=[
cache_result[0], False])
self.driver._copy_from_remote_cache = mock.Mock()
self.driver._post_clone_image = mock.Mock()
copied = self.driver._copy_from_cache(
fake.VOLUME, fake.IMAGE_FILE_ID, cache_result)
self.assertTrue(copied)
self.driver._copy_from_remote_cache.assert_called_once_with(
fake.VOLUME, fake.IMAGE_FILE_ID, cache_result[0])
self.driver._post_clone_image.assert_called_once_with(fake.VOLUME)
def test_copy_from_cache_workflow_local_location(self):
local_share = '/share'
cache_result = [
('ip1:/openstack', 'img-cache-imgid'),
('ip2:/openstack', 'img-cache-imgid'),
(local_share, 'img-cache-imgid'),
('ip3:/openstack', 'img-cache-imgid'),
]
self.driver._find_image_location = mock.Mock(return_value=[
cache_result[2], True])
self.driver._clone_file_dst_exists = mock.Mock()
self.driver._post_clone_image = mock.Mock()
copied = self.driver._copy_from_cache(
fake.VOLUME, fake.IMAGE_FILE_ID, cache_result)
self.assertTrue(copied)
self.driver._clone_file_dst_exists.assert_called_once_with(
local_share, fake.IMAGE_FILE_ID, fake.VOLUME['name'],
dest_exists=True)
self.driver._post_clone_image.assert_called_once_with(fake.VOLUME)
def test_copy_from_cache_workflow_no_location(self):
cache_result = []
self.driver._find_image_location = mock.Mock(
return_value=(None, False))
copied = self.driver._copy_from_cache(
fake.VOLUME, fake.IMAGE_FILE_ID, cache_result)
self.assertFalse(copied)
def test_copy_from_cache_workflow_exception(self):
cache_result = [('ip1:/openstack', fake.IMAGE_FILE_ID)]
self.driver._find_image_location = mock.Mock(return_value=[
cache_result[0], False])
self.driver._copy_from_remote_cache = mock.Mock(
side_effect=Exception)
self.driver._post_clone_image = mock.Mock()
copied = self.driver._copy_from_cache(
fake.VOLUME, fake.IMAGE_FILE_ID, cache_result)
self.assertFalse(copied)
self.driver._copy_from_remote_cache.assert_called_once_with(
fake.VOLUME, fake.IMAGE_FILE_ID, cache_result[0])
self.assertFalse(self.driver._post_clone_image.called)

109
cinder/volume/drivers/netapp/dataontap/nfs_cmode.py
View File

@ -434,7 +434,8 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver):
'using local image cache.'),
{'img': image_id, 'vol': volume['id']})
# Image cache was not present, attempt copy offload workflow
if not copy_success and col_path and major == 1 and minor >= 20:
if (not copy_success and col_path and
major == 1 and minor >= 20):
LOG.debug('No result found in image cache')
self._copy_from_img_service(context, volume, image_service,
image_id)
@ -465,42 +466,82 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver):
"""Try copying image file_name from cached file_name."""
LOG.debug("Trying copy from cache using copy offload.")
copied = False
for res in cache_result:
try:
(share, file_name) = res
LOG.debug("Found cache file_name on share %s.", share)
if share != self._get_provider_location(volume['id']):
col_path = self.configuration.netapp_copyoffload_tool_path
src_ip = self._get_ip_verify_on_cluster(
share.split(':')[0])
src_path = os.path.join(share.split(':')[1], file_name)
dst_ip = self._get_ip_verify_on_cluster(self._get_host_ip(
volume['id']))
dst_path = os.path.join(
self._get_export_path(volume['id']), volume['name'])
# Always run copy offload as regular user, it's sufficient
# and rootwrap doesn't allow copy offload to run as root
# anyways.
self._execute(col_path, src_ip, dst_ip,
src_path, dst_path,
run_as_root=False,
check_exit_code=0)
self._register_image_in_cache(volume, image_id)
LOG.debug("Copied image from cache to volume %s using"
" copy offload.", volume['id'])
else:
self._clone_file_dst_exists(share, file_name,
volume['name'],
dest_exists=True)
LOG.debug("Copied image from cache to volume %s using"
" cloning.", volume['id'])
self._post_clone_image(volume)
cache_copy, found_local = self._find_image_location(cache_result,
volume['id'])
try:
if found_local:
(nfs_share, file_name) = cache_copy
self._clone_file_dst_exists(
nfs_share, file_name, volume['name'], dest_exists=True)
LOG.debug("Copied image from cache to volume %s using "
"cloning.", volume['id'])
copied = True
break
except Exception as e:
LOG.exception(_LE('Error in workflow copy from cache. %s.'), e)
elif cache_copy:
self._copy_from_remote_cache(volume, image_id, cache_copy)
copied = True
if copied:
self._post_clone_image(volume)
except Exception as e:
LOG.exception(_LE('Error in workflow copy from cache. %s.'), e)
return copied
def _find_image_location(self, cache_result, volume_id):
"""Finds the location of a cached image.
Returns image location local to the NFS share, that matches the
volume_id, if it exists. Otherwise returns the last entry in
cache_result or None if cache_result is empty.
"""
found_local_copy = False
cache_copy = None
provider_location = self._get_provider_location(volume_id)
for res in cache_result:
(share, file_name) = res
if share == provider_location:
cache_copy = res
found_local_copy = True
break
else:
cache_copy = res
return cache_copy, found_local_copy
def _copy_from_remote_cache(self, volume, image_id, cache_copy):
"""Copies the remote cached image to the provided volume.
Executes the copy offload binary which copies the cached image to
the destination path of the provided volume. Also registers the new
copy of the image as a cached image.
"""
(nfs_share, file_name) = cache_copy
col_path = self.configuration.netapp_copyoffload_tool_path
src_ip, src_path = self._get_source_ip_and_path(nfs_share, file_name)
dest_ip, dest_path = self._get_destination_ip_and_path(volume)
# Always run copy offload as regular user, it's sufficient
# and rootwrap doesn't allow copy offload to run as root anyways.
self._execute(col_path, src_ip, dest_ip, src_path, dest_path,
run_as_root=False, check_exit_code=0)
self._register_image_in_cache(volume, image_id)
LOG.debug("Copied image from cache to volume %s using copy offload.",
volume['id'])
def _get_source_ip_and_path(self, nfs_share, file_name):
src_ip = self._get_ip_verify_on_cluster(nfs_share.split(':')[0])
src_path = os.path.join(nfs_share.split(':')[1], file_name)
return src_ip, src_path
def _get_destination_ip_and_path(self, volume):
dest_ip = self._get_ip_verify_on_cluster(
self._get_host_ip(volume['id']))
dest_path = os.path.join(self._get_export_path(
volume['id']), volume['name'])
return dest_ip, dest_path
def _clone_file_dst_exists(self, share, src_name, dst_name,
dest_exists=False):
"""Clone file even if dest exists."""