From 6a65422c69bd5f2fc363238b2383b66aab575272 Mon Sep 17 00:00:00 2001
From: Tom Barron <tpb@dyncloud.net>
Date: Wed, 17 Jun 2015 14:54:01 -0400
Subject: [PATCH] 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
---
.../volume/drivers/netapp/dataontap/fakes.py | 5 +-
.../netapp/dataontap/test_nfs_7mode.py | 34 +++++++++-
.../drivers/netapp/dataontap/test_nfs_base.py | 63 ++++++++++++++++++-
.../drivers/netapp/dataontap/nfs_7mode.py | 12 ++--
.../drivers/netapp/dataontap/nfs_base.py | 4 +-
5 files changed, 109 insertions(+), 9 deletions(-)
diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py
index 755ea2dd4..4f0f710e9 100644
--- 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 2ff1d54b3..d15a09061 100644
--- 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 44d032f5a..794bb4337 100644
--- 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 a3c4e5008..09dc9ab88 100644
--- 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 51debdae1..4ac9538ce 100644
--- 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."""
--
2.45.2