From 82a13da48e7451c2f7813bf2de6990625c05624c Mon Sep 17 00:00:00 2001 From: Adriano Rosso Date: Wed, 8 Nov 2017 15:51:36 -0200 Subject: [PATCH] NetApp ONTAP: Copy offload bugfix When 'NetApp NFS Copy Offload' tool is configured to download Glance images, these images are downloaded twice because the tool is doing the job after Cinder has already done it. This patch fixes the bug by executing the copy offload tool inside the clone_image function instead of using the copy_image_to_volume. Closes-bug: #1632333 Change-Id: I5c6ad150543213acfd0c78dbbdb1dc1584d22d26 (cherry-picked from commit c27173bad69da4889a5237cf2becc14bb6fc578a) --- .../netapp/dataontap/test_nfs_cmode.py | 176 ++++++++---------- .../drivers/netapp/dataontap/nfs_base.py | 27 ++- .../drivers/netapp/dataontap/nfs_cmode.py | 96 ++++------ ...s-glance-image-twice-08801d8c7b9eed2c.yaml | 5 + 4 files changed, 136 insertions(+), 168 deletions(-) create mode 100644 releasenotes/notes/bug-1632333-netapp-ontap-copyoffload-downloads-glance-image-twice-08801d8c7b9eed2c.yaml 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 9d02068fc99..5c1f1a53836 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 @@ -764,26 +764,25 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): self.driver.zapi_client.clone_file.assert_called_once_with( 'nfsvol', 'vol', 'clone', None, is_snapshot=True) - def test__copy_from_img_service_copyoffload_nonexistent_binary_path(self): + def test_copy_from_img_service_copyoffload_nonexistent_binary_path(self): self.mock_object(nfs_cmode.LOG, 'debug') drv = self.driver context = object() - volume = {'id': 'vol_id', 'name': 'name'} + volume = {'id': 'vol_id', 'name': 'name', + 'host': 'openstack@nfscmode#192.128.1.1:/mnt_point'} image_service = mock.Mock() image_service.get_location.return_value = (mock.Mock(), mock.Mock()) image_service.show.return_value = {'size': 0} image_id = 'image_id' drv._client = mock.Mock() drv._client.get_api_version = mock.Mock(return_value=(1, 20)) - drv._find_image_in_cache = mock.Mock(return_value=[]) + nfs_base.NetAppNfsDriver._find_image_in_cache = mock.Mock( + return_value=[]) drv._construct_image_nfs_url = mock.Mock(return_value=["nfs://1"]) drv._check_get_nfs_path_segs = mock.Mock( return_value=("test:test", "dr")) drv._get_ip_verify_on_cluster = mock.Mock(return_value="192.128.1.1") drv._get_mount_point_for_share = mock.Mock(return_value='mnt_point') - drv._get_host_ip = mock.Mock() - drv._get_provider_location = mock.Mock() - drv._get_export_path = mock.Mock(return_value="dr") drv._check_share_can_hold_size = mock.Mock() # Raise error as if the copyoffload file can not be found drv._clone_file_dst_exists = mock.Mock(side_effect=OSError()) @@ -796,10 +795,11 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): drv._discover_file_till_timeout.assert_not_called() @mock.patch.object(image_utils, 'qemu_img_info') - def test__copy_from_img_service_raw_copyoffload_workflow_success( + def test_copy_from_img_service_raw_copyoffload_workflow_success( self, mock_qemu_img_info): drv = self.driver - volume = {'id': 'vol_id', 'name': 'name', 'size': 1} + volume = {'id': 'vol_id', 'name': 'name', 'size': 1, + 'host': 'openstack@nfscmode#ip1:/mnt_point'} image_id = 'image_id' context = object() image_service = mock.Mock() @@ -828,17 +828,16 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): retval = drv._copy_from_img_service( context, volume, image_service, image_id) - self.assertIsNone(retval) + self.assertTrue(retval) drv._get_ip_verify_on_cluster.assert_any_call('ip1') - drv._get_export_path.assert_called_with('vol_id') - drv._check_share_can_hold_size.assert_called_with('share', 1) - drv._post_clone_image.assert_called_with(volume) + drv._check_share_can_hold_size.assert_called_with( + 'ip1:/mnt_point', 1) self.assertEqual(1, drv._execute.call_count) @mock.patch.object(image_utils, 'convert_image') @mock.patch.object(image_utils, 'qemu_img_info') @mock.patch('os.path.exists') - def test__copy_from_img_service_qcow2_copyoffload_workflow_success( + def test_copy_from_img_service_qcow2_copyoffload_workflow_success( self, mock_exists, mock_qemu_img_info, mock_cvrt_image): drv = self.driver cinder_mount_point_base = '/opt/stack/data/cinder/mnt/' @@ -849,7 +848,8 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): destination_copied_file = ( '/cinder-flexvol1/a155308c-0290-497b-b278-4cdd01de0253' ) - volume = {'id': 'vol_id', 'name': 'name', 'size': 1} + volume = {'id': 'vol_id', 'name': 'name', 'size': 1, + 'host': 'openstack@nfscmode#203.0.113.122:/cinder-flexvol1'} image_id = 'image_id' context = object() image_service = mock.Mock() @@ -862,10 +862,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): ) drv._get_ip_verify_on_cluster = mock.Mock(return_value='203.0.113.122') - drv._get_host_ip = mock.Mock(return_value='203.0.113.122') - drv._get_export_path = mock.Mock( - return_value='/cinder-flexvol1') - drv._get_provider_location = mock.Mock(return_value='share') drv._execute = mock.Mock() drv._execute_as_root = False drv._get_mount_point_for_share = mock.Mock( @@ -885,12 +881,10 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): retval = drv._copy_from_img_service( context, volume, image_service, image_id) - self.assertIsNone(retval) + self.assertTrue(retval) drv._get_ip_verify_on_cluster.assert_any_call('203.0.113.122') - drv._get_export_path.assert_called_with('vol_id') - drv._check_share_can_hold_size.assert_called_with('share', 1) - drv._post_clone_image.assert_called_with(volume) - self.assertEqual(1, mock_cvrt_image.call_count) + drv._check_share_can_hold_size.assert_called_with( + '203.0.113.122:/cinder-flexvol1', 1) # _execute must be called once for copy-offload and again to touch # the top directory to refresh cache @@ -909,30 +903,25 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): self.assertEqual(2, drv._delete_file_at_path.call_count) self.assertEqual(1, drv._clone_file_dst_exists.call_count) - def test__copy_from_cache_copyoffload_success(self): + def test_copy_from_cache_copyoffload_success(self): drv = self.driver - volume = {'id': 'vol_id', 'name': 'name', 'size': 1} + volume = {'id': 'vol_id', 'name': 'name', 'size': 1, + 'host': 'openstack@nfscmode#192.128.1.1:/exp_path'} 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( 'copyoffload_tool_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') def test_unmanage(self): mock_get_info = self.mock_object(na_utils, @@ -1081,40 +1070,38 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): (local_share, 'img-cache-imgid'), ('ip3:/openstack', 'img-cache-imgid'), ] - self.driver._get_provider_location = mock.Mock( - return_value=local_share) + + mock_extract_host = self.mock_object(volume_utils, 'extract_host') + mock_extract_host.return_value = local_share cache_copy, found_local_copy = self.driver._find_image_location( - cache_result, fake.VOLUME_ID) + cache_result, fake.VOLUME) 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') + + mock_extract_host = self.mock_object(volume_utils, 'extract_host') + mock_extract_host.return_value = '/share' cache_copy, found_local_copy = self.driver._find_image_location( - cache_result, fake.VOLUME_ID) + cache_result, fake.VOLUME) 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') + mock_extract_host = self.mock_object(volume_utils, 'extract_host') + mock_extract_host.return_value = '/share' cache_copy, found_local_copy = self.driver._find_image_location( - cache_result, fake.VOLUME_ID) + cache_result, fake.VOLUME) 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( @@ -1147,8 +1134,8 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): 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) + mock_extract_host = self.mock_object(volume_utils, 'extract_host') + mock_extract_host.return_value = fake.NFS_SHARE dest_ip, dest_path = self.driver._get_destination_ip_and_path( fake.VOLUME) @@ -1157,98 +1144,85 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): 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) + fake.SHARE_IP) - def test_copy_image_to_volume_copyoffload_non_cached_ssc_update(self): - mock_log = self.mock_object(nfs_cmode, 'LOG') + def test_clone_image_copyoffload_from_cache_success(self): drv = self.driver context = object() - volume = {'id': 'vol_id', 'name': 'name'} + volume = {'id': 'vol_id', 'name': 'name', + 'host': 'openstack@nfscmode#192.128.1.1:/mnt_point'} image_service = object() + image_location = 'img-loc' image_id = 'image_id' + image_meta = {'id': image_id} drv.zapi_client = mock.Mock() drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20)) - drv._copy_from_img_service = mock.Mock() - drv._get_provider_location = mock.Mock(return_value='share') - drv._get_vol_for_share = mock.Mock(return_value='vol') - - retval = drv.copy_image_to_volume( - context, volume, image_service, image_id) - - self.assertIsNone(retval) - drv._copy_from_img_service.assert_called_once_with( - context, volume, image_service, image_id) - self.assertEqual(1, mock_log.debug.call_count) - self.assertEqual(1, mock_log.info.call_count) - - def test_copy_image_to_volume_copyoffload_from_cache_success(self): - mock_info_log = self.mock_object(nfs_cmode.LOG, 'info') - drv = self.driver - context = object() - volume = {'id': 'vol_id', 'name': 'name'} - image_service = object() - image_id = 'image_id' - drv.zapi_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._find_image_in_cache = mock.Mock(return_value=[('share', 'img')]) + nfs_base.NetAppNfsDriver._find_image_in_cache = mock.Mock( + return_value=[('share', 'img')]) + nfs_base.NetAppNfsDriver._direct_nfs_clone = mock.Mock( + return_value=False) drv._copy_from_cache = mock.Mock(return_value=True) - drv.copy_image_to_volume(context, volume, image_service, image_id) + drv.clone_image(context, volume, image_location, image_meta, + image_service) drv._copy_from_cache.assert_called_once_with( volume, image_id, [('share', 'img')]) - self.assertEqual(1, mock_info_log.call_count) - def test_copy_image_to_volume_copyoffload_from_img_service(self): + def test_clone_image_copyoffload_from_img_service(self): drv = self.driver context = object() - volume = {'id': 'vol_id', 'name': 'name'} + volume = {'id': 'vol_id', 'name': 'name', + 'host': 'openstack@nfscmode#192.128.1.1:/mnt_point', + 'provider_location': '192.128.1.1:/mnt_point'} image_service = object() image_id = 'image_id' + image_meta = {'id': image_id} + image_location = 'img-loc' drv.zapi_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._find_image_in_cache = mock.Mock(return_value=False) - drv._copy_from_img_service = mock.Mock() + nfs_base.NetAppNfsDriver._find_image_in_cache = mock.Mock( + return_value=[]) + nfs_base.NetAppNfsDriver._direct_nfs_clone = mock.Mock( + return_value=False) + nfs_base.NetAppNfsDriver._post_clone_image = mock.Mock( + return_value=True) + drv._copy_from_img_service = mock.Mock(return_value=True) - retval = drv.copy_image_to_volume( - context, volume, image_service, image_id) + retval = drv.clone_image( + context, volume, image_location, image_meta, image_service) - self.assertIsNone(retval) + self.assertEqual(retval, ( + {'provider_location': '192.128.1.1:/mnt_point', + 'bootable': True}, True)) drv._copy_from_img_service.assert_called_once_with( context, volume, image_service, image_id) - def test_copy_image_to_volume_copyoffload_failure(self): + def test_clone_image_copyoffload_failure(self): mock_log = self.mock_object(nfs_cmode, 'LOG') drv = self.driver context = object() volume = {'id': 'vol_id', 'name': 'name'} image_service = object() image_id = 'image_id' + image_meta = {'id': image_id} + image_location = 'img-loc' drv.zapi_client = mock.Mock() drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20)) + nfs_base.NetAppNfsDriver._find_image_in_cache = mock.Mock( + return_value=[]) + nfs_base.NetAppNfsDriver._direct_nfs_clone = mock.Mock( + return_value=False) drv._copy_from_img_service = mock.Mock(side_effect=Exception()) - 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') - retval = drv.copy_image_to_volume( - context, volume, image_service, image_id) + retval = drv.clone_image( + context, volume, image_location, image_meta, image_service) - self.assertIsNone(retval) + self.assertEqual(retval, ({'bootable': False, + 'provider_location': None}, False)) drv._copy_from_img_service.assert_called_once_with( context, volume, image_service, image_id) - nfs_base.NetAppNfsDriver.copy_image_to_volume. \ - assert_called_once_with(context, volume, image_service, image_id) mock_log.info.assert_not_called() - self.assertEqual(1, mock_log.exception.call_count) def test_copy_from_remote_cache(self): source_ip = '192.0.1.1' @@ -1290,7 +1264,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): 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_remote_location_no_copyoffload(self): cache_result = [('ip1:/openstack', fake.IMAGE_FILE_ID), @@ -1328,7 +1301,6 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): 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 = [] diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py index a014e20c007..5eddc0b0586 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py @@ -514,6 +514,14 @@ class NetAppNfsDriver(driver.ManageableVD, LOG.warning('Exception during deleting %s', ex) return False + def _copy_from_cache(self, volume, image_id, cache_result): + """Try copying image file_name from cached file""" + raise NotImplementedError() + + def _copy_from_img_service(self, context, volume, image_service, + image_id): + raise NotImplementedError() + def clone_image(self, context, volume, image_location, image_meta, image_service): @@ -531,14 +539,22 @@ class NetAppNfsDriver(driver.ManageableVD, post_clone = False extra_specs = na_utils.get_volume_extra_specs(volume) + major, minor = self.zapi_client.get_ontapi_version() + col_path = self.configuration.netapp_copyoffload_tool_path try: cache_result = self._find_image_in_cache(image_id) if cache_result: - cloned = self._clone_from_cache(volume, image_id, cache_result) + cloned = self._copy_from_cache(volume, image_id, cache_result) else: cloned = self._direct_nfs_clone(volume, image_location, image_id) + + # Try to use the copy offload tool + if not cloned and col_path and major == 1 and minor >= 20: + cloned = self._copy_from_img_service(context, volume, + image_service, image_id) + if cloned: self._do_qos_for_volume(volume, extra_specs) post_clone = self._post_clone_image(volume) @@ -549,7 +565,8 @@ class NetAppNfsDriver(driver.ManageableVD, {'image_id': image_id, 'msg': msg}) finally: cloned = cloned and post_clone - share = volume['provider_location'] if cloned else None + share = (volume_utils.extract_host(volume['host'], level='pool') + if cloned else None) bootable = True if cloned else False return {'provider_location': share, 'bootable': bootable}, cloned @@ -586,7 +603,6 @@ class NetAppNfsDriver(driver.ManageableVD, share = self._is_cloneable_share(loc) if share and self._is_share_clone_compatible(volume, share): LOG.debug('Share is cloneable %s', share) - volume['provider_location'] = share (__, ___, img_file) = loc.rpartition('/') dir_path = self._get_mount_point_for_share(share) img_path = '%s/%s' % (dir_path, img_file) @@ -622,7 +638,10 @@ class NetAppNfsDriver(driver.ManageableVD, def _post_clone_image(self, volume): """Do operations post image cloning.""" LOG.info('Performing post clone for %s', volume['name']) - vol_path = self.local_path(volume) + + share = volume_utils.extract_host(volume['host'], level='pool') + vol_path = self._get_volume_path(share, volume['name']) + if self._discover_file_till_timeout(vol_path): self._set_rw_permissions(vol_path) self._resize_image_file(vol_path, volume['size']) diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py b/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py index 593c209294d..b5701a4d96c 100644 --- a/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py +++ b/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py @@ -464,39 +464,6 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver, LOG.exception('Exec of "rm" command on backing file for' ' %s was unsuccessful.', snapshot['id']) - @utils.trace_method - def copy_image_to_volume(self, context, volume, image_service, image_id): - """Fetch the image from image_service and write it to the volume.""" - copy_success = False - try: - major, minor = self.zapi_client.get_ontapi_version() - col_path = self.configuration.netapp_copyoffload_tool_path - # Search the local image cache before attempting copy offload - cache_result = self._find_image_in_cache(image_id) - if cache_result: - copy_success = self._copy_from_cache(volume, image_id, - cache_result) - if copy_success: - LOG.info('Copied image %(img)s to volume %(vol)s ' - '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): - LOG.debug('No result found in image cache') - self._copy_from_img_service(context, volume, image_service, - image_id) - LOG.info('Copied image %(img)s to volume %(vol)s using' - ' copy offload workflow.', - {'img': image_id, 'vol': volume['id']}) - copy_success = True - except Exception: - LOG.exception('Copy offload workflow unsuccessful.') - finally: - if not copy_success: - super(NetAppCmodeNfsDriver, self).copy_image_to_volume( - context, volume, image_service, image_id) - def _get_ip_verify_on_cluster(self, host): """Verifies if host on same cluster and returns ip.""" ip = na_utils.resolve_hostname(host) @@ -508,13 +475,13 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver, def _copy_from_cache(self, volume, image_id, cache_result): """Try copying image file_name from cached file_name.""" - LOG.debug("Trying copy from cache using copy offload.") copied = False cache_copy, found_local = self._find_image_location(cache_result, - volume['id']) + volume) try: if found_local: + LOG.debug("Trying copy from cache using cloning.") (nfs_share, file_name) = cache_copy self._clone_file_dst_exists( nfs_share, file_name, volume['name'], dest_exists=True) @@ -523,17 +490,14 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver, copied = True elif (cache_copy and self.configuration.netapp_copyoffload_tool_path): + LOG.debug("Trying copy from cache using copy offload.") self._copy_from_remote_cache(volume, image_id, cache_copy) copied = True - - if copied: - self._post_clone_image(volume) - except Exception: LOG.exception('Error in workflow copy from cache.') return copied - def _find_image_location(self, cache_result, volume_id): + def _find_image_location(self, cache_result, volume): """Finds the location of a cached image. Returns image location local to the NFS share, that matches the @@ -543,7 +507,10 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver, found_local_copy = False cache_copy = None - provider_location = self._get_provider_location(volume_id) + + provider_location = volume_utils.extract_host(volume['host'], + level='pool') + for res in cache_result: (share, file_name) = res if share == provider_location: @@ -581,10 +548,11 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver, 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']) + share = volume_utils.extract_host(volume['host'], level='pool') + share_ip_and_path = share.split(":") + dest_ip = self._get_ip_verify_on_cluster(share_ip_and_path[0]) + dest_path = os.path.join(share_ip_and_path[1], volume['name']) + return dest_ip, dest_path def _clone_file_dst_exists(self, share, src_name, dst_name, @@ -597,11 +565,14 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver, def _copy_from_img_service(self, context, volume, image_service, image_id): """Copies from the image service using copy offload.""" + LOG.debug("Trying copy from image service using copy offload.") image_loc = image_service.get_location(context, image_id) locations = self._construct_image_nfs_url(image_loc) src_ip = None selected_loc = None + cloned = False + # this will match the first location that has a valid IP on cluster for location in locations: conn, dr = self._check_get_nfs_path_segs(location) @@ -616,32 +587,31 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver, raise exception.NotFound(_("Source host details not found.")) (__, ___, img_file) = selected_loc.rpartition('/') src_path = os.path.join(dr, img_file) - dst_ip = self._get_ip_verify_on_cluster(self._get_host_ip( - volume['id'])) + + dst_ip, vol_path = self._get_destination_ip_and_path(volume) + share_path = vol_path.rsplit("/", 1)[0] + dst_share = dst_ip + ':' + share_path + # tmp file is required to deal with img formats tmp_img_file = six.text_type(uuid.uuid4()) col_path = self.configuration.netapp_copyoffload_tool_path img_info = image_service.show(context, image_id) - dst_share = self._get_provider_location(volume['id']) self._check_share_can_hold_size(dst_share, img_info['size']) run_as_root = self._execute_as_root dst_dir = self._get_mount_point_for_share(dst_share) dst_img_local = os.path.join(dst_dir, tmp_img_file) + try: - # If src and dst share not equal - if (('%s:%s' % (src_ip, dr)) != - ('%s:%s' % (dst_ip, self._get_export_path(volume['id'])))): - dst_img_serv_path = os.path.join( - self._get_export_path(volume['id']), tmp_img_file) - # 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_img_serv_path, run_as_root=False, - check_exit_code=0) - else: - self._clone_file_dst_exists(dst_share, img_file, tmp_img_file) + dst_img_serv_path = os.path.join( + share_path, tmp_img_file) + # 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_img_serv_path, run_as_root=False, + check_exit_code=0) + self._discover_file_till_timeout(dst_img_local, timeout=120) LOG.debug('Copied image %(img)s to tmp file %(tmp)s.', {'img': image_id, 'tmp': tmp_img_file}) @@ -683,11 +653,13 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver, finally: if os.path.exists(dst_img_conv_local): self._delete_file_at_path(dst_img_conv_local) - self._post_clone_image(volume) + cloned = True finally: if os.path.exists(dst_img_local): self._delete_file_at_path(dst_img_local) + return cloned + @utils.trace_method def unmanage(self, volume): """Removes the specified volume from Cinder management. diff --git a/releasenotes/notes/bug-1632333-netapp-ontap-copyoffload-downloads-glance-image-twice-08801d8c7b9eed2c.yaml b/releasenotes/notes/bug-1632333-netapp-ontap-copyoffload-downloads-glance-image-twice-08801d8c7b9eed2c.yaml new file mode 100644 index 00000000000..91db3b1fb69 --- /dev/null +++ b/releasenotes/notes/bug-1632333-netapp-ontap-copyoffload-downloads-glance-image-twice-08801d8c7b9eed2c.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixed bug 1632333 with the NetApp ONTAP Driver. Now the copy offload method is invoked + early to avoid downloading Glance images twice.