Merge "netapp: skip pcuser config for DP read-only volumes"

manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py

@@ -3089,9 +3089,14 @@ class NetAppCmodeFileStorageLibrary(object):
 
         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'])
 
@@ -3386,7 +3391,8 @@ class NetAppCmodeFileStorageLibrary(object):
                 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)
@@ -3922,7 +3928,8 @@ class NetAppCmodeFileStorageLibrary(object):
         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)
@@ -4032,7 +4039,7 @@ class NetAppCmodeFileStorageLibrary(object):
         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)

manila/share/drivers/netapp/dataontap/protocols/base.py

@@ -77,7 +77,7 @@ class NetAppBaseHelper(metaclass=abc.ABCMeta):
         """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

manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py

@@ -73,7 +73,7 @@ class NetAppCmodeCIFSHelper(base.NetAppBaseHelper):
 
     @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)

manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py

@@ -85,7 +85,7 @@ class NetAppCmodeNFSHelper(base.NetAppBaseHelper):
 
     @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
@@ -155,8 +155,12 @@ class NetAppCmodeNFSHelper(base.NetAppBaseHelper):
                                               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):

manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py

@@ -4004,7 +4004,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         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(),
@@ -4106,7 +4107,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         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):
@@ -4139,9 +4141,38 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         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,
@@ -4403,7 +4434,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
                 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()
@@ -5439,7 +5471,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
             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()
@@ -5757,7 +5790,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
 
         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',
@@ -5809,11 +5843,15 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         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):

manila/tests/share/drivers/netapp/dataontap/protocols/test_nfs_cmode.py

@@ -131,6 +131,30 @@ class NetAppClusteredNFSHelperTestCase(test.TestCase):
         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):
 

releasenotes/notes/netapp-skip-pcuser-dp-flexgroup-replica-a3f8b2c1d4e56789.yaml

@@ -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.