diff --git a/cinder/tests/unit/test_netapp_nfs.py b/cinder/tests/unit/test_netapp_nfs.py index 97ea6cf48..225ee6300 100644 --- a/cinder/tests/unit/test_netapp_nfs.py +++ b/cinder/tests/unit/test_netapp_nfs.py @@ -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): diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py index 6a2f91082..c8c3f495e 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py @@ -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', diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py index 0aac910a9..4d62a32c0 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py @@ -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) diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py b/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py index 4f46fe4c5..2adc4c41f 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py @@ -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."""