From 5556bdb727f0f5df65f4861201992c22e38a93e7 Mon Sep 17 00:00:00 2001 From: Rodrigo Barbieri Date: Mon, 7 Jan 2019 15:03:04 -0200 Subject: [PATCH] [NetApp] Add manage/unmanage of share servers This patch implements the functionality of managing and unmanaging of share servers to the NetApp driver, allowing for shares and snapshots to be managed and unmanaged in DHSS=True driver mode. Implements: bp netapp-driver-manage-unmanage-with-share-servers Change-Id: I6051cf038dcf9f175e0610fff0adf360230dc23c Depends-On: I452c2a99b186f53d737cb7fbd7eabfcfd9b249d6 --- .../netapp/dataontap/client/client_cmode.py | 9 ++ .../dataontap/cluster_mode/drv_multi_svm.py | 28 +++++++ .../dataontap/cluster_mode/drv_single_svm.py | 24 ++++++ .../netapp/dataontap/cluster_mode/lib_base.py | 15 ++-- .../dataontap/cluster_mode/lib_multi_svm.py | 58 ++++++++++--- .../netapp/dataontap/protocols/nfs_cmode.py | 5 +- .../dataontap/client/test_client_cmode.py | 14 ++++ .../dataontap/cluster_mode/test_lib_base.py | 35 ++++---- .../cluster_mode/test_lib_multi_svm.py | 82 +++++++++++++++++++ .../share/drivers/netapp/dataontap/fakes.py | 3 + .../dataontap/protocols/test_nfs_cmode.py | 5 +- ...manage-share-servers-635496b46e306920.yaml | 6 ++ 12 files changed, 249 insertions(+), 35 deletions(-) create mode 100644 releasenotes/notes/netapp-manage-unmanage-share-servers-635496b46e306920.yaml diff --git a/manila/share/drivers/netapp/dataontap/client/client_cmode.py b/manila/share/drivers/netapp/dataontap/client/client_cmode.py index 0b910c516a..de87014f7c 100644 --- a/manila/share/drivers/netapp/dataontap/client/client_cmode.py +++ b/manila/share/drivers/netapp/dataontap/client/client_cmode.py @@ -1757,6 +1757,15 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): } self.send_request('volume-rename', api_args) + @na_utils.trace + def rename_vserver(self, vserver_name, new_vserver_name): + """Rename a vserver.""" + api_args = { + 'vserver-name': vserver_name, + 'new-name': new_vserver_name, + } + self.send_request('vserver-rename', api_args) + @na_utils.trace def modify_volume(self, aggregate_name, volume_name, thin_provisioned=False, snapshot_policy=None, diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py index 8d5238d48e..333f012d7f 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_multi_svm.py @@ -83,6 +83,22 @@ class NetAppCmodeMultiSvmShareDriver(driver.ShareDriver): def unmanage_snapshot(self, snapshot): raise NotImplementedError + def manage_existing_with_server( + self, share, driver_options, share_server=None): + return self.library.manage_existing( + share, driver_options, share_server=share_server) + + def unmanage_with_server(self, share, share_server=None): + self.library.unmanage(share, share_server=share_server) + + def manage_existing_snapshot_with_server( + self, snapshot, driver_options, share_server=None): + return self.library.manage_existing_snapshot( + snapshot, driver_options, share_server=share_server) + + def unmanage_snapshot_with_server(self, snapshot, share_server=None): + self.library.unmanage_snapshot(snapshot, share_server=share_server) + def update_access(self, context, share, access_rules, add_rules, delete_rules, **kwargs): self.library.update_access(context, share, access_rules, add_rules, @@ -240,3 +256,15 @@ class NetAppCmodeMultiSvmShareDriver(driver.ShareDriver): def ensure_shares(self, context, shares): return self.library.ensure_shares(context, shares) + + def get_share_server_network_info( + self, context, share_server, identifier, driver_options): + return self.library.get_share_server_network_info( + context, share_server, identifier, driver_options) + + def manage_server(self, context, share_server, identifier, driver_options): + return self.library.manage_server( + context, share_server, identifier, driver_options) + + def unmanage_server(self, server_details, security_services=None): + return self.library.unmanage_server(server_details, security_services) diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py index 42a0b1e9b0..bd5f04bf02 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/drv_single_svm.py @@ -83,6 +83,20 @@ class NetAppCmodeSingleSvmShareDriver(driver.ShareDriver): def unmanage_snapshot(self, snapshot): self.library.unmanage_snapshot(snapshot) + def manage_existing_with_server( + self, share, driver_options, share_server=None): + raise NotImplementedError + + def unmanage_with_server(self, share, share_server=None): + raise NotImplementedError + + def manage_existing_snapshot_with_server( + self, snapshot, driver_options, share_server=None): + raise NotImplementedError + + def unmanage_snapshot_with_server(self, snapshot, share_server=None): + raise NotImplementedError + def update_access(self, context, share, access_rules, add_rules, delete_rules, **kwargs): self.library.update_access(context, share, access_rules, add_rules, @@ -256,3 +270,13 @@ class NetAppCmodeSingleSvmShareDriver(driver.ShareDriver): def ensure_shares(self, context, shares): return self.library.ensure_shares(context, shares) + + def get_share_server_network_info( + self, context, share_server, identifier, driver_options): + raise NotImplementedError + + def manage_server(self, context, share_server, identifier, driver_options): + raise NotImplementedError + + def unmanage_server(self, server_details, security_services=None): + raise NotImplementedError 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 a7ac917ae9..ddd5e9a129 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -997,15 +997,15 @@ class NetAppCmodeFileStorageLibrary(object): raise exception.ShareSnapshotIsBusy(snapshot_name=snapshot_name) @na_utils.trace - def manage_existing(self, share, driver_options): - vserver, vserver_client = self._get_vserver(share_server=None) + def manage_existing(self, share, driver_options, share_server=None): + vserver, vserver_client = self._get_vserver(share_server=share_server) share_size = self._manage_container(share, vserver, vserver_client) - export_locations = self._create_export(share, None, vserver, + export_locations = self._create_export(share, share_server, vserver, vserver_client) return {'size': share_size, 'export_locations': export_locations} @na_utils.trace - def unmanage(self, share): + def unmanage(self, share, share_server=None): pass @na_utils.trace @@ -1110,9 +1110,10 @@ class NetAppCmodeFileStorageLibrary(object): raise exception.ManageInvalidShare(reason=msg % msg_args) @na_utils.trace - def manage_existing_snapshot(self, snapshot, driver_options): + def manage_existing_snapshot( + self, snapshot, driver_options, share_server=None): """Brings an existing snapshot under Manila management.""" - vserver, vserver_client = self._get_vserver(share_server=None) + vserver, vserver_client = self._get_vserver(share_server=share_server) share_name = self._get_backend_share_name(snapshot['share_id']) existing_snapshot_name = snapshot.get('provider_location') new_snapshot_name = self._get_backend_snapshot_name(snapshot['id']) @@ -1159,7 +1160,7 @@ class NetAppCmodeFileStorageLibrary(object): return {'size': size, 'provider_location': new_snapshot_name} @na_utils.trace - def unmanage_snapshot(self, snapshot): + def unmanage_snapshot(self, snapshot, share_server=None): """Removes the specified snapshot from Manila management.""" @na_utils.trace diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py index 188e6d6ba1..58c359c46c 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py @@ -67,21 +67,23 @@ class NetAppCmodeMultiSVMFileStorageLibrary( check_for_setup_error()) @na_utils.trace - def _get_vserver(self, share_server=None): + def _get_vserver(self, share_server=None, vserver_name=None): - if not share_server: + if share_server: + backend_details = share_server.get('backend_details') + vserver = backend_details.get( + 'vserver_name') if backend_details else None + + if not vserver: + msg = _('Vserver name is absent in backend details. Please ' + 'check whether Vserver was created properly.') + raise exception.VserverNotSpecified(msg) + elif vserver_name: + vserver = vserver_name + else: msg = _('Share server not provided') raise exception.InvalidInput(reason=msg) - backend_details = share_server.get('backend_details') - vserver = backend_details.get( - 'vserver_name') if backend_details else None - - if not vserver: - msg = _('Vserver name is absent in backend details. Please ' - 'check whether Vserver was created properly.') - raise exception.VserverNotSpecified(msg) - if not self._client.vserver_exists(vserver): raise exception.VserverNotFound(vserver=vserver) @@ -397,3 +399,37 @@ class NetAppCmodeMultiSVMFileStorageLibrary( if options['ipv6-enabled']: versions.append(6) return versions + + def manage_server(self, context, share_server, identifier, driver_options): + """Manages a vserver by renaming it and returning backend_details.""" + new_vserver_name = self._get_vserver_name(share_server['id']) + old_vserver_name = self._get_correct_vserver_old_name(identifier) + + if new_vserver_name != old_vserver_name: + self._client.rename_vserver(old_vserver_name, new_vserver_name) + + backend_details = {'vserver_name': new_vserver_name} + return new_vserver_name, backend_details + + def unmanage_server(self, server_details, security_services=None): + pass + + def get_share_server_network_info( + self, context, share_server, identifier, driver_options): + """Returns a list of IPs for each vserver network interface.""" + vserver_name = self._get_correct_vserver_old_name(identifier) + + vserver, vserver_client = self._get_vserver(vserver_name=vserver_name) + + interfaces = vserver_client.get_network_interfaces() + allocations = [] + for lif in interfaces: + allocations.append(lif['address']) + return allocations + + def _get_correct_vserver_old_name(self, identifier): + + # In case vserver_name includes the template, we check and add it here + if not self._client.vserver_exists(identifier): + return self._get_vserver_name(identifier) + return identifier diff --git a/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py b/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py index 5bb67ea644..4841969bb1 100644 --- a/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py +++ b/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py @@ -146,7 +146,10 @@ class NetAppCmodeNFSHelper(base.NetAppBaseHelper): def _get_export_location(share): """Returns IP address and export location of an NFS share.""" export_location = share['export_location'] or ':' - return export_location.rsplit(':', 1) + result = export_location.rsplit(':', 1) + if len(result) != 2: + return ['', ''] + return result @staticmethod def _get_temp_export_policy_name(): diff --git a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py index 91815befbb..58eef1206d 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py @@ -3067,6 +3067,20 @@ class NetAppClientCmodeTestCase(test.TestCase): self.client.send_request.assert_called_once_with( 'volume-rename', volume_rename_api_args) + def test_rename_vserver(self): + + vserver_api_args = { + 'vserver-name': fake.VSERVER_NAME, + 'new-name': fake.VSERVER_NAME_2, + } + self.mock_object(self.client, 'send_request') + + self.client.rename_vserver(fake.VSERVER_NAME, fake.VSERVER_NAME_2) + + self.client.send_request.assert_called_once_with( + 'vserver-rename', vserver_api_args + ) + def test_modify_volume_no_optional_args(self): self.mock_object(self.client, 'send_request') 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 6ac72bc8ea..baa12a5ff3 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 @@ -1534,13 +1534,13 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.assertFalse(vserver_client.split_volume_clone.called) self.assertFalse(vserver_client.soft_delete_snapshot.called) - def test_manage_existing(self): + @ddt.data(None, fake.VSERVER1) + def test_manage_existing(self, fake_vserver): vserver_client = mock.Mock() - self.mock_object(self.library, - '_get_vserver', - mock.Mock(return_value=(fake.VSERVER1, - vserver_client))) + mock__get_vserver = self.mock_object( + self.library, '_get_vserver', + mock.Mock(return_value=(fake.VSERVER1, vserver_client))) mock_manage_container = self.mock_object( self.library, '_manage_container', @@ -1550,24 +1550,29 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): '_create_export', mock.Mock(return_value=fake.NFS_EXPORTS)) - result = self.library.manage_existing(fake.SHARE, {}) + result = self.library.manage_existing(fake.SHARE, {}, + share_server=fake_vserver) expected = { 'size': fake.SHARE_SIZE, 'export_locations': fake.NFS_EXPORTS } + + mock__get_vserver.assert_called_once_with(share_server=fake_vserver) mock_manage_container.assert_called_once_with(fake.SHARE, fake.VSERVER1, vserver_client) + mock_create_export.assert_called_once_with(fake.SHARE, - None, + fake_vserver, fake.VSERVER1, vserver_client) self.assertDictEqual(expected, result) - def test_unmanage(self): + @ddt.data(None, fake.VSERVER1) + def test_unmanage(self, fake_vserver): - result = self.library.unmanage(fake.SHARE) + result = self.library.unmanage(fake.SHARE, share_server=fake_vserver) self.assertIsNone(result) @@ -1782,7 +1787,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): fake.FLEXVOL_TO_MANAGE, vserver_client) - def test_manage_existing_snapshot(self): + @ddt.data(None, fake.VSERVER1) + def test_manage_existing_snapshot(self, fake_vserver): vserver_client = mock.Mock() mock_get_vserver = self.mock_object( @@ -1791,13 +1797,13 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): vserver_client.get_volume.return_value = fake.FLEXVOL_TO_MANAGE vserver_client.volume_has_snapmirror_relationships.return_value = False result = self.library.manage_existing_snapshot( - fake.SNAPSHOT_TO_MANAGE, {}) + fake.SNAPSHOT_TO_MANAGE, {}, share_server=fake_vserver) share_name = self.library._get_backend_share_name( fake.SNAPSHOT['share_id']) new_snapshot_name = self.library._get_backend_snapshot_name( fake.SNAPSHOT['id']) - mock_get_vserver.assert_called_once_with(share_server=None) + mock_get_vserver.assert_called_once_with(share_server=fake_vserver) (vserver_client.volume_has_snapmirror_relationships. assert_called_once_with(fake.FLEXVOL_TO_MANAGE)) vserver_client.rename_snapshot.assert_called_once_with( @@ -1870,9 +1876,10 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.library.manage_existing_snapshot, fake.SNAPSHOT_TO_MANAGE, {}) - def test_unmanage_snapshot(self): + @ddt.data(None, fake.VSERVER1) + def test_unmanage_snapshot(self, fake_vserver): - result = self.library.unmanage_snapshot(fake.SNAPSHOT) + result = self.library.unmanage_snapshot(fake.SNAPSHOT, fake_vserver) self.assertIsNone(result) diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py index 3ebca55a18..b7d3284f4c 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py @@ -110,6 +110,25 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.assertRaises(exception.InvalidInput, self.library._get_vserver) + def test_get_vserver_no_share_server_with_vserver_name(self): + fake_vserver_client = 'fake_client' + + mock_vserver_exists = self.mock_object( + self.library._client, 'vserver_exists', + mock.Mock(return_value=True)) + self.mock_object(self.library, + '_get_api_client', + mock.Mock(return_value=fake_vserver_client)) + + result_vserver, result_vserver_client = self.library._get_vserver( + share_server=None, vserver_name=fake.VSERVER1) + + mock_vserver_exists.assert_called_once_with( + fake.VSERVER1 + ) + self.assertEqual(fake.VSERVER1, result_vserver) + self.assertEqual(fake_vserver_client, result_vserver_client) + def test_get_vserver_no_backend_details(self): fake_share_server = copy.deepcopy(fake.SHARE_SERVER) @@ -186,6 +205,69 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): } self.assertEqual(expected, result) + @ddt.data('fake', fake.IDENTIFIER) + def test_manage_server(self, fake_vserver_name): + + self.mock_object(context, + 'get_admin_context', + mock.Mock(return_value='fake_admin_context')) + mock_get_vserver_name = self.mock_object( + self.library, '_get_vserver_name', + mock.Mock(return_value=fake_vserver_name)) + + new_identifier, new_details = self.library.manage_server( + context, fake.SHARE_SERVER, fake.IDENTIFIER, {}) + + mock_get_vserver_name.assert_called_once_with(fake.SHARE_SERVER['id']) + self.assertEqual(fake_vserver_name, new_details['vserver_name']) + self.assertEqual(fake_vserver_name, new_identifier) + + def test_get_share_server_network_info(self): + + fake_vserver_client = mock.Mock() + + self.mock_object(context, + 'get_admin_context', + mock.Mock(return_value='fake_admin_context')) + mock_get_vserver = self.mock_object( + self.library, '_get_vserver', + mock.Mock(return_value=['fake', fake_vserver_client])) + + net_interfaces = copy.deepcopy(c_fake.NETWORK_INTERFACES_MULTIPLE) + + self.mock_object(fake_vserver_client, + 'get_network_interfaces', + mock.Mock(return_value=net_interfaces)) + + result = self.library.get_share_server_network_info(context, + fake.SHARE_SERVER, + fake.IDENTIFIER, + {}) + mock_get_vserver.assert_called_once_with( + vserver_name=fake.IDENTIFIER + ) + reference_allocations = [] + for lif in net_interfaces: + reference_allocations.append(lif['address']) + + self.assertEqual(reference_allocations, result) + + @ddt.data((True, fake.IDENTIFIER), + (False, fake.IDENTIFIER)) + @ddt.unpack + def test__verify_share_server_name(self, vserver_exists, identifier): + + mock_exists = self.mock_object(self.client, 'vserver_exists', + mock.Mock(return_value=vserver_exists)) + expected_result = identifier + if not vserver_exists: + expected_result = self.library._get_vserver_name(identifier) + + result = self.library._get_correct_vserver_old_name(identifier) + + self.assertEqual(result, expected_result) + mock_exists.assert_called_once_with(identifier) + def test_handle_housekeeping_tasks(self): self.mock_object(self.client, 'prune_deleted_nfs_export_policies') diff --git a/manila/tests/share/drivers/netapp/dataontap/fakes.py b/manila/tests/share/drivers/netapp/dataontap/fakes.py index 4fcd9f36bc..c7828ae777 100644 --- a/manila/tests/share/drivers/netapp/dataontap/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/fakes.py @@ -356,6 +356,7 @@ NETWORK_INFO = { NETWORK_INFO_NETMASK = '255.255.255.0' SHARE_SERVER = { + 'id': 'fake_id', 'share_network_id': 'c5b3a865-56d0-4d88-abe5-879965e099c9', 'backend_details': { 'vserver_name': VSERVER1 @@ -523,6 +524,8 @@ COLLATED_CGSNAPSHOT_INFO = [ }, ] +IDENTIFIER = 'c5b3a865-56d0-4d88-dke5-853465e099c9' + LIF_NAMES = [] LIF_ADDRESSES = ['10.10.10.10', '10.10.10.20'] LIFS = ( 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 a533fe318d..500b82aede 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 @@ -163,10 +163,11 @@ class NetAppClusteredNFSHelperTestCase(test.TestCase): self.assertEqual(fake.SHARE_ADDRESS_1, host_ip) self.assertEqual('/' + fake.SHARE_NAME, export_path) - def test_get_export_location_missing_location(self): + @ddt.data('', 'invalid') + def test_get_export_location_missing_location_invalid(self, export): fake_share = fake.NFS_SHARE.copy() - fake_share['export_location'] = '' + fake_share['export_location'] = export host_ip, export_path = self.helper._get_export_location(fake_share) diff --git a/releasenotes/notes/netapp-manage-unmanage-share-servers-635496b46e306920.yaml b/releasenotes/notes/netapp-manage-unmanage-share-servers-635496b46e306920.yaml new file mode 100644 index 0000000000..af9b7f4988 --- /dev/null +++ b/releasenotes/notes/netapp-manage-unmanage-share-servers-635496b46e306920.yaml @@ -0,0 +1,6 @@ +--- +features: + - Added managing and unmanaging of share servers + functionality to the NetApp driver, allowing for + shares and snapshots to be managed and unmanaged in + driver mode ``driver_handles_share_servers`` set to True.