Fix 'no actual-pathname' NetApp API error

Cinder volume logs sometimes show this error

      NetApp API failed. Reason - 13114:
      No actual-pathname for 10.1.0.9:/vol/whatever

with Kilo code and 7-mode DOT system.

The issue is due to our Cinder driver passing the
entire share to the relevant API instead of just
the export path portion of the share.  This only
happens when the NFS image cache is full, and cached
files need to be removed.

Change-Id: I0c40840dac975dd7fd2c62f1f9c0cd3f8c5c1252
Closes-Bug: #1468884

diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py
index 755ea2dd467e744dd03ef8bd4e3218d623a9be08..4f0f710e96ed0c61865da0fae147ac733ce92534 100644 (file)
--- a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py
+++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py
@@ -23,8 +23,9 @@ SIZE = 1024
 HOST_NAME = 'fake.host.name'
 BACKEND_NAME = 'fake_backend_name'
 POOL_NAME = 'aggr1'
+SHARE_IP = '192.168.99.24'
 EXPORT_PATH = '/fake/export/path'
-NFS_SHARE = '192.168.99.24:%s' % EXPORT_PATH
+NFS_SHARE = '%s:%s' % (SHARE_IP, EXPORT_PATH)
 HOST_STRING = '%s@%s#%s' % (HOST_NAME, BACKEND_NAME, POOL_NAME)
 NFS_HOST_STRING = '%s@%s#%s' % (HOST_NAME, BACKEND_NAME, NFS_SHARE)
 FLEXVOL = 'openstack-flexvol'
@@ -206,3 +207,5 @@ SNAPSHOT = {
 }
 
 VOLUME_REF = {'name': 'fake_vref_name', 'size': 42}
+
+FILE_LIST = ['file1', 'file2', 'file3']

diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_7mode.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_7mode.py
index 2ff1d54b3d7dda22b67ce650407f7193b60b62b3..d15a09061a19e58238d77c178bdff2ed82cc6082 100644 (file)
--- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_7mode.py
+++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_7mode.py
@@ -12,7 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 """
-Mock unit tests for the NetApp 7mode nfs storage driver
+Unit tests for the NetApp 7mode NFS storage driver
 """
 
 import mock
@@ -40,6 +40,7 @@ class NetApp7modeNfsDriverTestCase(test.TestCase):
                 self.driver = nfs_7mode.NetApp7modeNfsDriver(**kwargs)
                 self.driver._mounted_shares = [fake.NFS_SHARE]
                 self.driver.ssc_vols = True
+                self.driver.zapi_client = mock.Mock()
 
     def get_config_7mode(self):
         config = na_fakes.create_configuration_cmode()
@@ -73,3 +74,34 @@ class NetApp7modeNfsDriverTestCase(test.TestCase):
                          result[0]['reserved_percentage'])
         self.assertEqual(total_capacity_gb, result[0]['total_capacity_gb'])
         self.assertEqual(free_capacity_gb, result[0]['free_capacity_gb'])
+
+    def test_shortlist_del_eligible_files(self):
+        mock_get_path_for_export = self.mock_object(
+            self.driver.zapi_client, 'get_actual_path_for_export')
+        mock_get_path_for_export.return_value = fake.FLEXVOL
+
+        mock_get_file_usage = self.mock_object(
+            self.driver.zapi_client, 'get_file_usage')
+        mock_get_file_usage.return_value = fake.CAPACITY_VALUES[0]
+
+        expected = [(old_file, fake.CAPACITY_VALUES[0]) for old_file
+                    in fake.FILE_LIST]
+
+        result = self.driver._shortlist_del_eligible_files(
+            fake.NFS_SHARE, fake.FILE_LIST)
+
+        self.assertEqual(expected, result)
+
+    def test_shortlist_del_eligible_files_empty_list(self):
+        mock_get_export_ip_path = self.mock_object(
+            self.driver, '_get_export_ip_path')
+        mock_get_export_ip_path.return_value = ('', '/export_path')
+
+        mock_get_path_for_export = self.mock_object(
+            self.driver.zapi_client, 'get_actual_path_for_export')
+        mock_get_path_for_export.return_value = fake.FLEXVOL
+
+        result = self.driver._shortlist_del_eligible_files(
+            fake.NFS_SHARE, [])
+
+        self.assertEqual([], result)

diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py
index 44d032f5a9d0350801cb97c70dc04ce03627f3af..794bb4337eef7c7ffa4a2181fd12fcbaf2a49cb5 100644 (file)
--- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py
+++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py
@@ -14,7 +14,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 """
-Mock unit tests for the NetApp nfs storage driver
+Unit tests for the NetApp NFS storage driver
 """
 
 import os
@@ -49,6 +49,7 @@ class NetAppNfsDriverTestCase(test.TestCase):
                                    return_value=mock.Mock()):
                 self.driver = nfs_base.NetAppNfsDriver(**kwargs)
                 self.driver.ssc_enabled = False
+                self.driver.db = mock.Mock()
 
     @mock.patch.object(nfs.NfsDriver, 'do_setup')
     @mock.patch.object(na_utils, 'check_flags')
@@ -290,3 +291,63 @@ class NetAppNfsDriverTestCase(test.TestCase):
         self.assertRaises(NotImplementedError,
                           self.driver._get_vol_for_share,
                           fake.NFS_SHARE)
+
+    def test_get_export_ip_path_volume_id_provided(self):
+        mock_get_host_ip = self.mock_object(self.driver, '_get_host_ip')
+        mock_get_host_ip.return_value = fake.IPV4_ADDRESS
+
+        mock_get_export_path = self.mock_object(
+            self.driver, '_get_export_path')
+        mock_get_export_path.return_value = fake.EXPORT_PATH
+
+        expected = (fake.IPV4_ADDRESS, fake.EXPORT_PATH)
+
+        result = self.driver._get_export_ip_path(fake.VOLUME_ID)
+
+        self.assertEqual(expected, result)
+
+    def test_get_export_ip_path_share_provided(self):
+        expected = (fake.SHARE_IP, fake.EXPORT_PATH)
+
+        result = self.driver._get_export_ip_path(share=fake.NFS_SHARE)
+
+        self.assertEqual(expected, result)
+
+    def test_get_export_ip_path_volume_id_and_share_provided(self):
+        mock_get_host_ip = self.mock_object(self.driver, '_get_host_ip')
+        mock_get_host_ip.return_value = fake.IPV4_ADDRESS
+
+        mock_get_export_path = self.mock_object(
+            self.driver, '_get_export_path')
+        mock_get_export_path.return_value = fake.EXPORT_PATH
+
+        expected = (fake.IPV4_ADDRESS, fake.EXPORT_PATH)
+
+        result = self.driver._get_export_ip_path(
+            fake.VOLUME_ID, fake.NFS_SHARE)
+
+        self.assertEqual(expected, result)
+
+    def test_get_export_ip_path_no_args(self):
+        self.assertRaises(exception.InvalidInput,
+                          self.driver._get_export_ip_path)
+
+    def test_get_host_ip(self):
+        mock_get_provider_location = self.mock_object(
+            self.driver, '_get_provider_location')
+        mock_get_provider_location.return_value = fake.NFS_SHARE
+        expected = fake.SHARE_IP
+
+        result = self.driver._get_host_ip(fake.VOLUME_ID)
+
+        self.assertEqual(expected, result)
+
+    def test_get_export_path(self):
+        mock_get_provider_location = self.mock_object(
+            self.driver, '_get_provider_location')
+        mock_get_provider_location.return_value = fake.NFS_SHARE
+        expected = fake.EXPORT_PATH
+
+        result = self.driver._get_export_path(fake.VOLUME_ID)
+
+        self.assertEqual(expected, result)

diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_7mode.py b/cinder/volume/drivers/netapp/dataontap/nfs_7mode.py
index a3c4e5008dd9e22caa406ebe0dff5e15e3ee6651..09dc9ab888d058f3be02ffb9cae209e724518b99 100644 (file)
--- a/cinder/volume/drivers/netapp/dataontap/nfs_7mode.py
+++ b/cinder/volume/drivers/netapp/dataontap/nfs_7mode.py
@@ -21,6 +21,8 @@
 Volume driver for NetApp NFS storage.
 """
 
+import os
+
 from oslo_log import log as logging
 
 from cinder import exception
@@ -120,11 +122,13 @@ class NetApp7modeNfsDriver(nfs_base.NetAppNfsDriver):
     def _shortlist_del_eligible_files(self, share, old_files):
         """Prepares list of eligible files to be deleted from cache."""
         file_list = []
-        exp_volume = self.zapi_client.get_actual_path_for_export(share)
-        for file in old_files:
-            path = '/vol/%s/%s' % (exp_volume, file)
+        (_, export_path) = self._get_export_ip_path(share=share)
+        exported_volume = self.zapi_client.get_actual_path_for_export(
+            export_path)
+        for old_file in old_files:
+            path = os.path.join(exported_volume, old_file)
             u_bytes = self.zapi_client.get_file_usage(path)
-            file_list.append((file, u_bytes))
+            file_list.append((old_file, u_bytes))
         LOG.debug('Shortlisted files eligible for deletion: %s', file_list)
         return file_list
 

diff --git a/cinder/volume/drivers/netapp/dataontap/nfs_base.py b/cinder/volume/drivers/netapp/dataontap/nfs_base.py
index 51debdae16eb485b0cf9c4b47e5d858c46cfcd4a..4ac9538ce127b95d17cb895c50860a63aa91c21a 100644 (file)
--- a/cinder/volume/drivers/netapp/dataontap/nfs_base.py
+++ b/cinder/volume/drivers/netapp/dataontap/nfs_base.py
@@ -242,11 +242,11 @@ class NetAppNfsDriver(nfs.NfsDriver):
 
     def _get_host_ip(self, volume_id):
         """Returns IP address for the given volume."""
-        return self._get_provider_location(volume_id).split(':')[0]
+        return self._get_provider_location(volume_id).rsplit(':')[0]
 
     def _get_export_path(self, volume_id):
         """Returns NFS export path for the given volume."""
-        return self._get_provider_location(volume_id).split(':')[1]
+        return self._get_provider_location(volume_id).rsplit(':')[1]
 
     def _volume_not_present(self, nfs_mount, volume_name):
         """Check if volume exists."""