Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -3089,9 +3089,14 @@ def update_access(self, context, share, access_rules, add_rules,

share_name = self._get_backend_share_name(share['id'])
if self._share_exists(share_name, vserver_client):
is_dp_replica = (
replica_state is not None and
replica_state != constants.REPLICA_STATE_ACTIVE and
self._is_readable_replica(share))
helper = self._get_helper(share)
helper.set_client(vserver_client)
helper.update_access(share, share_name, access_rules)
helper.update_access(share, share_name, access_rules,
replica=is_dp_replica)
else:
raise exception.ShareResourceNotFound(share_id=share['id'])

Expand Down Expand Up @@ -3386,7 +3391,8 @@ def create_replica(self, context, replica_list, new_replica,
helper.set_client(vserver_client)
share_name = self._get_backend_share_name(new_replica_id)
try:
helper.update_access(new_replica, share_name, access_rules)
helper.update_access(new_replica, share_name, access_rules,
replica=True)
except Exception:
model_update['access_rules_status'] = (
constants.SHARE_INSTANCE_RULES_ERROR)
Expand Down Expand Up @@ -3922,7 +3928,8 @@ def _convert_destination_replica_to_independent(
helper = self._get_helper(replica)
helper.set_client(vserver_client)
try:
helper.update_access(replica, share_name, access_rules)
helper.update_access(replica, share_name, access_rules,
replica=False)
except Exception:
new_active_replica['access_rules_status'] = (
constants.SHARE_INSTANCE_RULES_SYNCING)
Expand Down Expand Up @@ -4032,7 +4039,7 @@ def _safe_change_replica_source(self, dm_session, replica,
helper.set_client(replica_client)
try:
helper.update_access(
replica, replica_volume_name, access_rules)
replica, replica_volume_name, access_rules, replica=True)
except Exception:
replica['access_rules_status'] = (
constants.SHARE_INSTANCE_RULES_ERROR)
Expand Down
2 changes: 1 addition & 1 deletion manila/share/drivers/netapp/dataontap/protocols/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def delete_share(self, share, share_name):
"""Deletes NAS share."""

@abc.abstractmethod
def update_access(self, share, share_name, rules):
def update_access(self, share, share_name, rules, replica=False):
"""Replaces the list of access rules known to the backend storage."""

@abc.abstractmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def delete_share(self, share, share_name):

@na_utils.trace
@base.access_rules_synchronized
def update_access(self, share, share_name, rules):
def update_access(self, share, share_name, rules, replica=False):
"""Replaces the list of access rules known to the backend storage."""

_, cifs_share_name = self._get_export_location(share)
Expand Down
10 changes: 7 additions & 3 deletions manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def delete_share(self, share, share_name):

@na_utils.trace
@base.access_rules_synchronized
def update_access(self, share, share_name, rules):
def update_access(self, share, share_name, rules, replica=False):
"""Replaces the list of access rules known to the backend storage."""

# Ensure rules are valid
Expand Down Expand Up @@ -155,8 +155,12 @@ def update_access(self, share, share_name, rules):
export_policy_name)

if set_volume_user_to_pcuser:
LOG.info('Setting user to pcuser for share %s.', share_name)
self._client.set_pcuser_for_volume(share_name)
if replica:
LOG.info('Skipping pcuser configuration for DP read-only '
'volume %s.', share_name)
else:
LOG.info('Setting user to pcuser for share %s.', share_name)
self._client.set_pcuser_for_volume(share_name)

@na_utils.trace
def _validate_access_rule(self, rule):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4004,7 +4004,8 @@ def test_update_access(self):
mock_share_exists.assert_called_once_with(share_name, vserver_client)
protocol_helper.set_client.assert_called_once_with(vserver_client)
protocol_helper.update_access.assert_called_once_with(
fake.SHARE, fake.SHARE_NAME, [fake.SHARE_ACCESS])
fake.SHARE, fake.SHARE_NAME, [fake.SHARE_ACCESS],
replica=False)

@ddt.data(exception.InvalidInput(reason='fake_reason'),
exception.VserverNotSpecified(),
Expand Down Expand Up @@ -4106,7 +4107,8 @@ def test_update_access_to_active_replica(self):
mock_share_exists.assert_called_once_with(share_name, vserver_client)
protocol_helper.set_client.assert_called_once_with(vserver_client)
protocol_helper.update_access.assert_called_once_with(
fake.SHARE, fake.SHARE_NAME, [fake.SHARE_ACCESS])
fake_share_copy, fake.SHARE_NAME, [fake.SHARE_ACCESS],
replica=False)

@ddt.data(True, False)
def test_update_access_to_in_sync_replica(self, is_readable):
Expand Down Expand Up @@ -4139,9 +4141,38 @@ def test_update_access_to_in_sync_replica(self, is_readable):
if is_readable:
mock_get_vserver.assert_called_once_with(
share_server=fake.SHARE_SERVER)
protocol_helper.update_access.assert_called_once_with(
fake_share_copy, fake.SHARE_NAME, [fake.SHARE_ACCESS],
replica=True)
else:
mock_get_vserver.assert_not_called()

def test_update_access_to_readable_dp_replica_skips_pcuser(self):
"""Verify replica=True is passed for non-active readable replicas."""
fake_share_copy = copy.deepcopy(fake.SHARE)
fake_share_copy['replica_state'] = constants.REPLICA_STATE_OUT_OF_SYNC
vserver_client = mock.Mock()
self.mock_object(
self.library, '_get_vserver',
mock.Mock(return_value=(fake.VSERVER1, vserver_client)))
self.mock_object(self.library, '_is_readable_replica',
mock.Mock(return_value=True))
protocol_helper = mock.Mock()
self.mock_object(self.library, '_get_helper',
mock.Mock(return_value=protocol_helper))
self.mock_object(self.library, '_share_exists',
mock.Mock(return_value=True))

self.library.update_access(self.context,
fake_share_copy,
[fake.SHARE_ACCESS],
[], [], [],
share_server=fake.SHARE_SERVER)

protocol_helper.update_access.assert_called_once_with(
fake_share_copy, fake.SHARE_NAME, [fake.SHARE_ACCESS],
replica=True)

def test_setup_server(self):
self.assertRaises(NotImplementedError,
self.library.setup_server,
Expand Down Expand Up @@ -4403,7 +4434,8 @@ def test_create_replica(self, is_readable, rules_status):
fake.SHARE, None, fake.VSERVER1, vserver_client, replica=True)
mock_get_helper.assert_called_once_with(fake.SHARE)
protocol_helper.update_access.assert_called_once_with(
fake.SHARE, fake.SHARE_NAME, [fake.SHARE_ACCESS])
fake.SHARE, fake.SHARE_NAME, [fake.SHARE_ACCESS],
replica=True)
else:
mock_create_export.assert_not_called()
mock_get_helper.assert_not_called()
Expand Down Expand Up @@ -5439,7 +5471,8 @@ def test_promote_replica_with_access_rules(self):
self.fake_replica_2['id'])
mock_helper.update_access.assert_called_once_with(self.fake_replica_2,
share_name,
[fake.SHARE_ACCESS])
[fake.SHARE_ACCESS],
replica=False)
self.library._unmount_orig_active_replica.assert_called_once_with(
self.fake_replica, fake.VSERVER1)
self.library._handle_qos_on_replication_change.assert_called_once()
Expand Down Expand Up @@ -5757,7 +5790,8 @@ def test_convert_destination_replica_to_independent_failed_access_rules(

fake_helper.assert_has_calls([
mock.call.set_client(mock.ANY),
mock.call.update_access(mock.ANY, mock.ANY, fake_access_rules),
mock.call.update_access(mock.ANY, mock.ANY, fake_access_rules,
replica=False),
])

self.assertEqual('fake_export_location',
Expand Down Expand Up @@ -5809,11 +5843,15 @@ def test_safe_change_replica_source(self, is_dr):
if is_dr:
self.assertEqual([], replica['export_locations'])
mock_dm_session.wait_for_mount_replica.assert_not_called()
protocol_helper.update_access.assert_not_called()
else:
self.assertEqual('fake_export_location',
replica['export_locations'])
mock_dm_session.wait_for_mount_replica.assert_called_once_with(
mock_client, fake.SHARE_NAME, timeout=30)
protocol_helper.update_access.assert_called_once_with(
self.fake_replica, fake.SHARE_NAME, [fake.SHARE_ACCESS],
replica=True)

@ddt.data(True, False)
def test_safe_change_replica_source_sync_policy(self, is_dr):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,30 @@ def test_update_access(self, all_squash_metadata):
if all_squash_metadata:
self.mock_client.set_pcuser_for_volume.assert_called_once_with(
fake.SHARE_NAME)
else:
self.mock_client.set_pcuser_for_volume.assert_not_called()

def test_update_access_all_squash_replica_skips_pcuser(self):

self.mock_object(self.helper, '_ensure_export_policy')
self.mock_object(self.helper,
'_get_export_policy_name',
mock.Mock(return_value='fake_export_policy'))
self.mock_object(self.helper,
'_get_temp_export_policy_name',
mock.Mock(side_effect=['fake_new_export_policy',
'fake_old_export_policy']))
self.mock_object(self.helper,
'_get_auth_methods',
mock.Mock(return_value='fake_auth_method'))

share = fake.NFS_SHARE.copy()
share.update({'metadata': {'all_squash': 'true'}})

self.helper.update_access(share, fake.SHARE_NAME, [fake.IP_ACCESS],
replica=True)

self.mock_client.set_pcuser_for_volume.assert_not_called()

def test_validate_access_rule(self):

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
fixes:
- |
Fixed an issue on the NetApp ONTAP driver during updating access rules for
shares having replicas in combination with ``all_squash=true`` metadata.
Loading