From 725a9ced5551e326ef0004dc2205263b375c3f70 Mon Sep 17 00:00:00 2001 From: Clinton Knight Date: Fri, 13 Mar 2015 14:33:33 -0400 Subject: [PATCH] cDOT driver should report all share export locations Manila has the ability for shares to report multiple export locations. This is an important capability for drivers that manage storage backends that export shares on multiple network interfaces. This commit adds this capability to the cDOT driver. This commit also updates the cDOT driver to use backslashes in CIFS export paths. Change-Id: I772caaad2871a072bb5bc4a505678d9fbf0c579c Closes-Bug: #1429246 --- .../netapp/dataontap/cluster_mode/lib_base.py | 6 ++-- .../netapp/dataontap/protocols/base.py | 2 +- .../netapp/dataontap/protocols/cifs_cmode.py | 17 ++++++--- .../netapp/dataontap/protocols/nfs_cmode.py | 6 ++-- .../dataontap/cluster_mode/test_lib_base.py | 7 ++-- .../share/drivers/netapp/dataontap/fakes.py | 5 ++- .../netapp/dataontap/protocols/fakes.py | 7 ++-- .../dataontap/protocols/test_cifs_cmode.py | 35 ++++++++++++++++--- .../dataontap/protocols/test_nfs_cmode.py | 24 ++++++++++--- 9 files changed, 79 insertions(+), 30 deletions(-) diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py index 63779f5195..d4c47105e6 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -488,9 +488,9 @@ class NetAppCmodeFileStorageLibrary(object): msg_args = {'vserver': vserver, 'proto': share['share_proto']} raise exception.NetAppException(msg % msg_args) - ip_address = interfaces[0]['address'] - export_location = helper.create_share(share_name, ip_address) - return export_location + export_addresses = [interface['address'] for interface in interfaces] + export_locations = helper.create_share(share_name, export_addresses) + return export_locations @na_utils.trace def _remove_export(self, share, vserver_client): diff --git a/manila/share/drivers/netapp/dataontap/protocols/base.py b/manila/share/drivers/netapp/dataontap/protocols/base.py index 3634ceab68..51aaa64e20 100644 --- a/manila/share/drivers/netapp/dataontap/protocols/base.py +++ b/manila/share/drivers/netapp/dataontap/protocols/base.py @@ -29,7 +29,7 @@ class NetAppBaseHelper(object): self._client = client @abc.abstractmethod - def create_share(self, share, export_ip): + def create_share(self, share, export_addresses): """Creates NAS share.""" @abc.abstractmethod diff --git a/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py b/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py index e80c53f75f..af87e2dbbd 100644 --- a/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py +++ b/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py @@ -15,6 +15,8 @@ NetApp CIFS protocol helper class. """ +import re + from oslo_log import log from manila import exception @@ -31,11 +33,12 @@ class NetAppCmodeCIFSHelper(base.NetAppBaseHelper): """Netapp specific cluster-mode CIFS sharing driver.""" @na_utils.trace - def create_share(self, share_name, export_ip): + def create_share(self, share_name, export_addresses): """Creates CIFS share on Data ONTAP Vserver.""" self._client.create_cifs_share(share_name) self._client.remove_cifs_share_access(share_name, 'Everyone') - return "//%s/%s" % (export_ip, share_name) + return [r'\\%s\%s' % (export_address, share_name) + for export_address in export_addresses] @na_utils.trace def delete_share(self, share): @@ -84,6 +87,10 @@ class NetAppCmodeCIFSHelper(base.NetAppBaseHelper): @staticmethod def _get_export_location(share): """Returns host ip and share name for a given CIFS share.""" - export_location = share['export_location'] or '///' - _x, _x, host_ip, share_name = export_location.split('/') - return host_ip, share_name + export_location = share['export_location'] or '\\\\\\' + regex = r'^(?:\\\\|//)(?P.*)(?:\\|/)(?P.*)$' + match = re.match(regex, export_location) + if match: + return match.group('host_ip'), match.group('share_name') + else: + return '', '' diff --git a/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py b/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py index d86c0527f2..54e324c710 100644 --- a/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py +++ b/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py @@ -29,11 +29,11 @@ class NetAppCmodeNFSHelper(base.NetAppBaseHelper): """Netapp specific cluster-mode NFS sharing driver.""" @na_utils.trace - def create_share(self, share_name, export_ip): + def create_share(self, share_name, export_addresses): """Creates NFS share.""" export_path = self._client.get_volume_junction_path(share_name) - export_location = ':'.join([export_ip, export_path]) - return export_location + return [':'.join([export_address, export_path]) + for export_address in export_addresses] @na_utils.trace def delete_share(self, share): diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py index d83c77055b..cc2fffaa8b 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py @@ -790,7 +790,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): def test_create_export(self): protocol_helper = mock.Mock() - protocol_helper.create_share.return_value = 'fake_export_location' + protocol_helper.create_share.return_value = fake.NFS_EXPORTS self.mock_object(self.library, '_get_helper', mock.Mock(return_value=protocol_helper)) @@ -802,10 +802,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): vserver_client) share_name = self.library._get_valid_share_name(fake.SHARE['id']) - self.assertEqual('fake_export_location', result) + self.assertEqual(fake.NFS_EXPORTS, result) protocol_helper.create_share.assert_called_once_with( - share_name, - fake.LIFS[0]['address']) + share_name, fake.LIF_ADDRESSES) def test_create_export_lifs_not_found(self): diff --git a/manila/tests/share/drivers/netapp/dataontap/fakes.py b/manila/tests/share/drivers/netapp/dataontap/fakes.py index 82aa3b1f8b..58309c4980 100644 --- a/manila/tests/share/drivers/netapp/dataontap/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/fakes.py @@ -188,7 +188,7 @@ SNAPSHOT = { } LIF_NAMES = [] -LIF_ADDRESSES = ('10.10.10.10', '10.10.10.20') +LIF_ADDRESSES = ['10.10.10.10', '10.10.10.20'] LIFS = ( {'address': LIF_ADDRESSES[0], 'home-node': CLUSTER_NODES[0], @@ -208,6 +208,9 @@ LIFS = ( }, ) +NFS_EXPORTS = [':'.join([LIF_ADDRESSES[0], 'fake_export_path']), + ':'.join([LIF_ADDRESSES[1], 'fake_export_path'])] + SHARE_ACCESS = { 'access_type': 'user', 'access_to': [LIF_ADDRESSES[0]] diff --git a/manila/tests/share/drivers/netapp/dataontap/protocols/fakes.py b/manila/tests/share/drivers/netapp/dataontap/protocols/fakes.py index 1993d3764e..be84bf26d6 100644 --- a/manila/tests/share/drivers/netapp/dataontap/protocols/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/protocols/fakes.py @@ -15,18 +15,19 @@ SHARE_NAME = 'fake_share' SHARE_ID = '9dba208c-9aa7-11e4-89d3-123b93f75cba' -SHARE_ADDRESS = '10.10.10.10' +SHARE_ADDRESS_1 = '10.10.10.10' +SHARE_ADDRESS_2 = '10.10.10.20' CLIENT_ADDRESS_1 = '20.20.20.10' CLIENT_ADDRESS_2 = '20.20.20.20' CIFS_SHARE = { - 'export_location': '//%s/%s' % (SHARE_ADDRESS, SHARE_NAME), + 'export_location': r'\\%s\%s' % (SHARE_ADDRESS_1, SHARE_NAME), 'id': SHARE_ID } NFS_SHARE_PATH = '/%s' % SHARE_NAME NFS_SHARE = { - 'export_location': '%s:%s' % (SHARE_ADDRESS, NFS_SHARE_PATH), + 'export_location': '%s:%s' % (SHARE_ADDRESS_1, NFS_SHARE_PATH), 'id': SHARE_ID } diff --git a/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py b/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py index 87b7d6fd05..5d48241a1e 100644 --- a/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py +++ b/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py @@ -45,14 +45,29 @@ class NetAppClusteredCIFSHelperTestCase(test.TestCase): def test_create_share(self): - result = self.helper.create_share(fake.SHARE_NAME, fake.SHARE_ADDRESS) + result = self.helper.create_share(fake.SHARE_NAME, + [fake.SHARE_ADDRESS_1]) + expected = [r'\\%s\%s' % (fake.SHARE_ADDRESS_1, fake.SHARE_NAME)] + self.assertEqual(expected, result) + self.mock_client.create_cifs_share.assert_called_once_with( + fake.SHARE_NAME) + self.mock_client.remove_cifs_share_access.assert_called_once_with( + fake.SHARE_NAME, 'Everyone') + + def test_create_share_multiple(self): + + result = self.helper.create_share(fake.SHARE_NAME, + [fake.SHARE_ADDRESS_1, + fake.SHARE_ADDRESS_2]) + + expected = [r'\\%s\%s' % (fake.SHARE_ADDRESS_1, fake.SHARE_NAME), + r'\\%s\%s' % (fake.SHARE_ADDRESS_2, fake.SHARE_NAME)] + self.assertEqual(expected, result) self.mock_client.create_cifs_share.assert_called_once_with( fake.SHARE_NAME) self.mock_client.remove_cifs_share_access.assert_called_once_with( fake.SHARE_NAME, 'Everyone') - self.assertEqual('//%s/%s' % (fake.SHARE_ADDRESS, fake.SHARE_NAME), - result) def test_delete_share(self): @@ -147,7 +162,7 @@ class NetAppClusteredCIFSHelperTestCase(test.TestCase): def test_get_target(self): target = self.helper.get_target(fake.CIFS_SHARE) - self.assertEqual(fake.SHARE_ADDRESS, target) + self.assertEqual(fake.SHARE_ADDRESS_1, target) def test_get_target_missing_location(self): @@ -157,7 +172,17 @@ class NetAppClusteredCIFSHelperTestCase(test.TestCase): def test_get_export_location(self): host_ip, share_name = self.helper._get_export_location(fake.CIFS_SHARE) - self.assertEqual(fake.SHARE_ADDRESS, host_ip) + self.assertEqual(fake.SHARE_ADDRESS_1, host_ip) + self.assertEqual(fake.SHARE_NAME, share_name) + + def test_get_export_location_legacy_forward_slashes(self): + + fake_share = fake.CIFS_SHARE.copy() + fake_share['export_location'] = fake_share['export_location'].replace( + '\\', '/') + + host_ip, share_name = self.helper._get_export_location(fake_share) + self.assertEqual(fake.SHARE_ADDRESS_1, host_ip) self.assertEqual(fake.SHARE_NAME, share_name) def test_get_export_location_missing_location(self): diff --git a/manila/tests/share/drivers/netapp/dataontap/protocols/test_nfs_cmode.py b/manila/tests/share/drivers/netapp/dataontap/protocols/test_nfs_cmode.py index badd64cec4..68c960b400 100644 --- a/manila/tests/share/drivers/netapp/dataontap/protocols/test_nfs_cmode.py +++ b/manila/tests/share/drivers/netapp/dataontap/protocols/test_nfs_cmode.py @@ -41,10 +41,24 @@ class NetAppClusteredNFSHelperTestCase(test.TestCase): self.mock_client.get_volume_junction_path.return_value = ( fake.NFS_SHARE_PATH) - result = self.helper.create_share(fake.SHARE_NAME, fake.SHARE_ADDRESS) + result = self.helper.create_share(fake.SHARE_NAME, + [fake.SHARE_ADDRESS_1]) - self.assertEqual(':'.join([fake.SHARE_ADDRESS, fake.NFS_SHARE_PATH]), - result) + expected = [':'.join([fake.SHARE_ADDRESS_1, fake.NFS_SHARE_PATH])] + self.assertEqual(expected, result) + + def test_create_share_multiple(self): + + self.mock_client.get_volume_junction_path.return_value = ( + fake.NFS_SHARE_PATH) + + result = self.helper.create_share(fake.SHARE_NAME, + [fake.SHARE_ADDRESS_1, + fake.SHARE_ADDRESS_2]) + + expected = [':'.join([fake.SHARE_ADDRESS_1, fake.NFS_SHARE_PATH]), + ':'.join([fake.SHARE_ADDRESS_2, fake.NFS_SHARE_PATH])] + self.assertEqual(expected, result) def test_delete_share(self): @@ -123,7 +137,7 @@ class NetAppClusteredNFSHelperTestCase(test.TestCase): def test_get_target(self): target = self.helper.get_target(fake.NFS_SHARE) - self.assertEqual(fake.SHARE_ADDRESS, target) + self.assertEqual(fake.SHARE_ADDRESS_1, target) def test_get_target_missing_location(self): @@ -154,7 +168,7 @@ class NetAppClusteredNFSHelperTestCase(test.TestCase): host_ip, export_path = self.helper._get_export_location( fake.NFS_SHARE) - self.assertEqual(fake.SHARE_ADDRESS, host_ip) + self.assertEqual(fake.SHARE_ADDRESS_1, host_ip) self.assertEqual('/' + fake.SHARE_NAME, export_path) def test_get_export_location_missing_location(self):