openstack
/
manila
Code Issues Proposed changes

Merge "[NetApp] Fix non-disruptive migration cifs shares"

This commit is contained in:
Zuul 2023-04-10 17:07:22 +00:00 committed by Gerrit Code Review
parent 4fcaff2b66 9ddddeeea3
commit 13923e6a72
8 changed files with 74 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
manila/db/sqlalchemy/api.py
View File

@ -1698,7 +1698,7 @@ def share_instance_get(context, share_instance_id, session=None,
).filter_by(
id=share_instance_id,
).options(
joinedload('export_locations'),
joinedload('export_locations').joinedload('_el_metadata_bare'),
joinedload('share_type'),
).first()
if result is None:

5
manila/share/drivers/netapp/dataontap/client/client_cmode.py
View File

@ -3434,9 +3434,8 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
self.send_request('cg-commit', api_args)
@na_utils.trace
def create_cifs_share(self, share_name):
share_path = '/%s' % share_name
api_args = {'path': share_path, 'share-name': share_name}
def create_cifs_share(self, share_name, path):
api_args = {'path': path, 'share-name': share_name}
self.send_request('cifs-share-create', api_args)
@na_utils.trace

27
manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py
View File

@ -3571,12 +3571,31 @@ class NetAppCmodeFileStorageLibrary(object):
}
LOG.info(msg, msg_args)
# NOTE(gouthamr): For nondisruptive migration, current export
# NOTE(gouthamr): For NFS nondisruptive migration, current export
# policy will not be cleared, the export policy will be renamed to
# match the name of the share.
export_locations = self._create_export(
destination_share, share_server, vserver, vserver_client,
clear_current_export_policy=False)
# NOTE (caiquemello): For CIFS nondisruptive migration, current CIFS
# share cannot be renamed, so keep the previous CIFS share.
share_protocol = source_share['share_proto'].lower()
if share_protocol != 'cifs':
export_locations = self._create_export(
destination_share, share_server, vserver, vserver_client,
clear_current_export_policy=False)
else:
export_locations = []
for item in source_share['export_locations']:
export_locations.append(
{
'path': item['path'],
'is_admin_only': item['is_admin_only'],
'metadata': item['el_metadata']
}
)
# Sort the export locations to report preferred paths first
export_locations = self._sort_export_locations_by_preferred_paths(
export_locations)
src_snaps_dict = {s['id']: s for s in source_snapshots}
snapshot_updates = {}

24
manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py
View File

@ -46,12 +46,13 @@ class NetAppCmodeCIFSHelper(base.NetAppBaseHelper):
"""
cifs_exist = self._client.cifs_share_exists(share_name)
export_path = self._client.get_volume_junction_path(share_name)
if ensure_share_already_exists and not cifs_exist:
msg = _("The expected CIFS share %(share_name)s was not found.")
msg_args = {'share_name': share_name}
raise exception.NetAppException(msg % msg_args)
elif not cifs_exist:
self._client.create_cifs_share(share_name)
self._client.create_cifs_share(share_name, export_path)
self._client.remove_cifs_share_access(share_name, 'Everyone')
# Ensure 'ntfs' security style for RW volume. DP volumes cannot set it.
@ -61,8 +62,8 @@ class NetAppCmodeCIFSHelper(base.NetAppBaseHelper):
# Return a callback that may be used for generating export paths
# for this share.
return (lambda export_address, share_name=share_name:
r'\\%s\%s' % (export_address, share_name))
return (lambda export_address, export_path=export_path:
r'\\%s%s' % (export_address, export_path.replace('/', '\\')))
@na_utils.trace
def delete_share(self, share, share_name):
@ -75,6 +76,7 @@ class NetAppCmodeCIFSHelper(base.NetAppBaseHelper):
def update_access(self, share, share_name, rules):
"""Replaces the list of access rules known to the backend storage."""
_, cifs_share_name = self._get_export_location(share)
# Ensure rules are valid
for rule in rules:
self._validate_access_rule(rule)
@ -82,13 +84,13 @@ class NetAppCmodeCIFSHelper(base.NetAppBaseHelper):
new_rules = {rule['access_to']: rule['access_level'] for rule in rules}
# Get rules from share
existing_rules = self._get_access_rules(share, share_name)
existing_rules = self._get_access_rules(share, cifs_share_name)
# Update rules in an order that will prevent transient disruptions
self._handle_added_rules(share_name, existing_rules, new_rules)
self._handle_ro_to_rw_rules(share_name, existing_rules, new_rules)
self._handle_rw_to_ro_rules(share_name, existing_rules, new_rules)
self._handle_deleted_rules(share_name, existing_rules, new_rules)
self._handle_added_rules(cifs_share_name, existing_rules, new_rules)
self._handle_ro_to_rw_rules(cifs_share_name, existing_rules, new_rules)
self._handle_rw_to_ro_rules(cifs_share_name, existing_rules, new_rules)
self._handle_deleted_rules(cifs_share_name, existing_rules, new_rules)
@na_utils.trace
def _validate_access_rule(self, rule):
@ -170,8 +172,10 @@ class NetAppCmodeCIFSHelper(base.NetAppBaseHelper):
@na_utils.trace
def get_share_name_for_share(self, share):
"""Returns the flexvol name that hosts a share."""
_, share_name = self._get_export_location(share)
return share_name
_, volume_junction_path = self._get_export_location(share)
volume = self._client.get_volume_at_junction_path(
f"/{volume_junction_path}")
return volume.get('name') if volume else None
@na_utils.trace
def _get_export_location(self, share):

5
manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py
View File

@ -5139,10 +5139,11 @@ class NetAppClientCmodeTestCase(test.TestCase):
self.mock_object(self.client, 'send_request')
self.client.create_cifs_share(fake.SHARE_NAME)
self.client.create_cifs_share(
fake.SHARE_NAME, fake.VOLUME_JUNCTION_PATH)
cifs_share_create_args = {
'path': '/%s' % fake.SHARE_NAME,
'path': fake.VOLUME_JUNCTION_PATH,
'share-name': fake.SHARE_NAME
}

4
manila/tests/share/drivers/netapp/dataontap/protocols/fakes.py
View File

@ -23,8 +23,10 @@ SHARE_ADDRESS_2 = '10.10.10.20'
CLIENT_ADDRESS_1 = '20.20.20.10'
CLIENT_ADDRESS_2 = '20.20.20.20'
CIFS_SHARE_PATH = '/%s' % SHARE_NAME
CIFS_SHARE_PATH_PARSED = '\\%s' % SHARE_NAME
CIFS_SHARE = {
'export_location': r'\\%s\%s' % (SHARE_ADDRESS_1, SHARE_NAME),
'export_location': r'\\%s%s' % (SHARE_ADDRESS_1, CIFS_SHARE_PATH_PARSED),
'id': SHARE_ID
}

23
manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py
View File

@ -46,6 +46,8 @@ class NetAppClusteredCIFSHelperTestCase(test.TestCase):
def test_create_share(self, replica, cifs_exist):
self.mock_client.cifs_share_exists.return_value = cifs_exist
self.mock_client.get_volume_junction_path.return_value = (
fake.CIFS_SHARE_PATH)
result = self.helper.create_share(
fake.CIFS_SHARE, fake.SHARE_NAME,
@ -54,8 +56,8 @@ class NetAppClusteredCIFSHelperTestCase(test.TestCase):
export_addresses = [fake.SHARE_ADDRESS_1, fake.SHARE_ADDRESS_2]
export_paths = [result(address) for address in export_addresses]
expected_paths = [
r'\\%s\%s' % (fake.SHARE_ADDRESS_1, fake.SHARE_NAME),
r'\\%s\%s' % (fake.SHARE_ADDRESS_2, fake.SHARE_NAME),
r'\\%s%s' % (fake.SHARE_ADDRESS_1, fake.CIFS_SHARE_PATH_PARSED),
r'\\%s%s' % (fake.SHARE_ADDRESS_2, fake.CIFS_SHARE_PATH_PARSED),
]
self.assertEqual(expected_paths, export_paths)
@ -66,7 +68,7 @@ class NetAppClusteredCIFSHelperTestCase(test.TestCase):
self.mock_client.remove_cifs_share.assert_not_called()
else:
self.mock_client.create_cifs_share.assert_called_once_with(
fake.SHARE_NAME)
fake.SHARE_NAME, fake.CIFS_SHARE_PATH)
self.mock_client.remove_cifs_share_access.assert_called_once_with(
fake.SHARE_NAME, 'Everyone')
@ -214,9 +216,24 @@ class NetAppClusteredCIFSHelperTestCase(test.TestCase):
def test_get_share_name_for_share(self):
self.mock_client.get_volume_at_junction_path.return_value = (
fake.VOLUME)
share_name = self.helper.get_share_name_for_share(fake.CIFS_SHARE)
self.assertEqual(fake.SHARE_NAME, share_name)
self.mock_client.get_volume_at_junction_path.assert_called_once_with(
fake.CIFS_SHARE_PATH)
def test_get_share_name_for_share_not_found(self):
self.mock_client.get_volume_at_junction_path.return_value = None
share_name = self.helper.get_share_name_for_share(fake.CIFS_SHARE)
self.assertIsNone(share_name)
self.mock_client.get_volume_at_junction_path.assert_called_once_with(
fake.CIFS_SHARE_PATH)
@ddt.data(
{

8
releasenotes/notes/bug-1920937-fixed-cifs-share-migration-752fde9631fb077a.yaml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
Fixed non-disruptive share migration of CIFS shares in the NetApp ONTAP
driver using ZAPI API. During the CIFS share migration the creation of a
new export path is skipped and the actual export path is taken from the
backend. For more details, please refer to
`launchpad bug #1920937 <https://bugs.launchpad.net/manila/+bug/1920937>`_.