Local img-cache files ignored for image transfers

The cDOT NFS driver will only attempt to use the local img-cache-*
files if the copy offload path is defined in the cinder.conf file.
This should be attempted regardless of copy-offload's configuration.

Change-Id: I29e19da7c1e083e9c76cbcea1f397deefe30b016
Closes-Bug: #1490641

diff --git a/cinder/tests/unit/test_netapp_nfs.py b/cinder/tests/unit/test_netapp_nfs.py
index ac9567799ed8747e74741d9d1d46a7e98a1272b8..20b1a8678bb0e8ac27a174ef7fbe46fe2b0caad2 100644 (file)
--- a/cinder/tests/unit/test_netapp_nfs.py
+++ b/cinder/tests/unit/test_netapp_nfs.py
@@ -43,11 +43,11 @@ from cinder.volume.drivers.netapp.dataontap import nfs_base
 from cinder.volume.drivers.netapp.dataontap import ssc_cmode
 from cinder.volume.drivers.netapp import utils
 
-
 from oslo_config import cfg
-CONF = cfg.CONF
 
 
+CONF = cfg.CONF
+
 CONNECTION_INFO = {
     'hostname': 'fake_host',
     'transport_type': 'https',
@@ -392,10 +392,10 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase):
         mox = self.mox
         drv._mounted_shares = ['testshare']
         mox.StubOutWithMock(drv, '_get_mount_point_for_share')
-        mox.StubOutWithMock(os.path, 'exists')
+        mox.StubOutWithMock(os.path, 'isfile')
 
         drv._get_mount_point_for_share('testshare').AndReturn('/mnt')
-        os.path.exists('/mnt/img-cache-id').AndReturn(True)
+        os.path.isfile('/mnt/img-cache-id').AndReturn(True)
         mox.ReplayAll()
         result = drv._find_image_in_cache('id')
         (share, file_name) = result[0]
@@ -1300,15 +1300,15 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase):
         image_service = object()
         image_id = 'image_id'
         drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20))
-        drv._try_copyoffload = mock.Mock()
+        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')
         drv._update_stale_vols = mock.Mock()
 
         drv.copy_image_to_volume(context, volume, image_service, image_id)
-        drv._try_copyoffload.assert_called_once_with(context, volume,
-                                                     image_service,
-                                                     image_id)
+        drv._copy_from_img_service.assert_called_once_with(context, volume,
+                                                           image_service,
+                                                           image_id)
         drv._update_stale_vols.assert_called_once_with('vol')
 
     def test_copy_img_to_vol_copyoffload_failure(self):
@@ -1318,17 +1318,17 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase):
         image_service = object()
         image_id = 'image_id'
         drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20))
-        drv._try_copyoffload = mock.Mock(side_effect=Exception())
+        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')
         drv._update_stale_vols = mock.Mock()
 
         drv.copy_image_to_volume(context, volume, image_service, image_id)
-        drv._try_copyoffload.assert_called_once_with(context, volume,
-                                                     image_service,
-                                                     image_id)
-        nfs_base.NetAppNfsDriver.copy_image_to_volume.\
+        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)
         drv._update_stale_vols.assert_called_once_with('vol')
 
@@ -1356,7 +1356,7 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase):
         drv._clone_file_dst_exists = mock.Mock(side_effect=OSError())
 
         # Verify the original error is propagated
-        self.assertRaises(OSError, drv._try_copyoffload,
+        self.assertRaises(OSError, drv._copy_from_img_service,
                           context, volume, image_service, image_id)
 
     def test_copyoffload_frm_cache_success(self):
@@ -1365,10 +1365,15 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase):
         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._try_copyoffload(context, volume, image_service, image_id)
+        drv.copy_image_to_volume(context, volume, image_service, image_id)
         drv._copy_from_cache.assert_called_once_with(volume,
                                                      image_id,
                                                      [('share', 'img')])
@@ -1380,11 +1385,15 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase):
         image_service = object()
         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=[])
+        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._try_copyoffload(context, volume, image_service, image_id)
+        drv.copy_image_to_volume(context, volume, image_service, image_id)
         drv._copy_from_img_service.assert_called_once_with(context,
                                                            volume,
                                                            image_service,

diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py
index 37623ed86371d410929ab0b5644ce2a755d0c631..2cfc1feab5549763ede35e4b9356db4a913fcdf3 100644 (file)
--- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py
+++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py
@@ -324,7 +324,7 @@ class NetAppNfsDriver(driver.ManageableVD,
                 dir = self._get_mount_point_for_share(share)
                 file_name = 'img-cache-%s' % image_id
                 file_path = '%s/%s' % (dir, file_name)
-                if os.path.exists(file_path):
+                if os.path.isfile(file_path):
                     LOG.debug('Found cache file for image %(image_id)s'
                               ' on share %(share)s',
                               {'image_id': image_id, 'share': share})

diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py b/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py
index 0dcb6d9d9858feb26e1e1db5f844df14e36cd1ca..0e06045c2bd819b6c4829dfbedd24019e8482147 100644 (file)
--- a/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py
+++ b/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py
@@ -351,15 +351,24 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver):
         try:
             major, minor = self.zapi_client.get_ontapi_version()
             col_path = self.configuration.netapp_copyoffload_tool_path
-            if major == 1 and minor >= 20 and col_path:
-                self._try_copyoffload(context, volume, image_service, image_id)
-                copy_success = True
-                LOG.info(_LI('Copied image %(img)s to volume %(vol)s using '
-                             'copy offload workflow.'),
+            # 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(_LI('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(_LI('Copied image %(img)s to volume %(vol)s using'
+                             ' copy offload workflow.'),
                          {'img': image_id, 'vol': volume['id']})
-            else:
-                LOG.debug("Copy offload either not configured or"
-                          " unsupported.")
+                copy_success = True
         except Exception as e:
             LOG.exception(_LE('Copy offload workflow unsuccessful. %s'), e)
         finally:
@@ -370,16 +379,6 @@ class NetAppCmodeNfsDriver(nfs_base.NetAppNfsDriver):
                 sh = self._get_provider_location(volume['id'])
                 self._update_stale_vols(self._get_vol_for_share(sh))
 
-    def _try_copyoffload(self, context, volume, image_service, image_id):
-        """Tries server side file copy offload."""
-        copied = False
-        cache_result = self._find_image_in_cache(image_id)
-        if cache_result:
-            copied = self._copy_from_cache(volume, image_id, cache_result)
-        if not cache_result or not copied:
-            self._copy_from_img_service(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)