From 6a65422c69bd5f2fc363238b2383b66aab575272 Mon Sep 17 00:00:00 2001 From: Tom Barron 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 755ea2dd467..4f0f710e96e 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 2ff1d54b3d7..d15a09061a1 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 44d032f5a9d..794bb4337ee 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 a3c4e5008dd..09dc9ab888d 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 51debdae16e..4ac9538ce12 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."""