From e46e2a6e415e5817ac498fead6581564d2da37b1 Mon Sep 17 00:00:00 2001
From: Kiran Pawar <kinpaa@gmail.com>
Date: Thu, 24 Nov 2022 20:41:07 +0000
Subject: [PATCH] Ignore replicas in error state during allow/deny access.

Allow/deny access will be blocked only if share or its replica will be
in invalid state i.e. transitional states. Error state will be ignored.

Related-bug: #1965561
Change-Id: I180061a50fec8a9e76ef7ceb2787239ff82a2e12
Depends-on: Id23d04f8d18a0e30b511c4d501a1910f2f2b4ca6
---
 manila_tempest_tests/config.py                |  2 +-
 .../tests/api/test_replication.py             | 24 +++++++++++++++++++
 .../tests/api/test_replication_negative.py    | 16 +++++++++----
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/manila_tempest_tests/config.py b/manila_tempest_tests/config.py
index 2c1b3755..56a5b5a8 100644
--- a/manila_tempest_tests/config.py
+++ b/manila_tempest_tests/config.py
@@ -40,7 +40,7 @@ ShareGroup = [
                     "This value is only used to validate the versions "
                     "response from Manila."),
     cfg.StrOpt("max_api_microversion",
-               default="2.73",
+               default="2.74",
                help="The maximum api microversion is configured to be the "
                     "value of the latest microversion supported by Manila."),
     cfg.StrOpt("region",
diff --git a/manila_tempest_tests/tests/api/test_replication.py b/manila_tempest_tests/tests/api/test_replication.py
index 9a9a4783..14c4d457 100644
--- a/manila_tempest_tests/tests/api/test_replication.py
+++ b/manila_tempest_tests/tests/api/test_replication.py
@@ -360,6 +360,30 @@ class ReplicationTest(base.BaseSharesMixedTest):
         # Delete the replica
         self.delete_share_replica(share_replica["id"])
 
+    @decorators.idempotent_id('600a13d2-5cf0-482e-97af-9f598b55a406')
+    @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND)
+    @utils.skip_if_microversion_not_supported("2.74")
+    def test_add_access_rule_share_replica_error_status(self):
+        '''From 2.74, we can add rules even if replicas are in error state.'''
+        access_type, access_to = utils.get_access_rule_data_from_config(
+            self.shares_v2_client.share_protocol)
+        # Create the replica
+        share_replica = self._verify_create_replica()
+
+        # Reset the replica status to error
+        self.admin_client.reset_share_replica_status(
+            share_replica['id'], constants.STATUS_ERROR)
+
+        # Verify access rule will be added in error state
+        self.shares_v2_client.create_access_rule(
+            self.shares[0]["id"], access_type=access_type, access_to=access_to,
+            access_level='ro')
+
+        # Verify access_rules_status transitions to 'active' state.
+        waiters.wait_for_resource_status(
+            self.shares_v2_client, self.shares[0]["id"],
+            constants.RULE_STATE_ACTIVE, status_attr='access_rules_status')
+
     @decorators.idempotent_id('a542c179-ea41-4bc0-bd80-e06eaddf5253')
     @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND)
     @testtools.skipUnless(CONF.share.run_multiple_share_replicas_tests,
diff --git a/manila_tempest_tests/tests/api/test_replication_negative.py b/manila_tempest_tests/tests/api/test_replication_negative.py
index 0262142e..5025bc4f 100644
--- a/manila_tempest_tests/tests/api/test_replication_negative.py
+++ b/manila_tempest_tests/tests/api/test_replication_negative.py
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import ddt
 from tempest import config
 from tempest.lib.common.utils import data_utils
 from tempest.lib import decorators
@@ -83,6 +84,7 @@ class ReplicationNegativeBase(base.BaseSharesMixedTest):
         return share, instance_id
 
 
+@ddt.ddt
 class ReplicationNegativeTest(ReplicationNegativeBase):
 
     def _is_replication_type_promotable(self):
@@ -189,7 +191,9 @@ class ReplicationNegativeTest(ReplicationNegativeBase):
 
     @decorators.idempotent_id('600a13d2-5cf0-482e-97af-9f598b55a407')
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    def test_add_access_rule_share_replica_error_status(self):
+    @ddt.data('2.11', '2.73')
+    def test_add_access_rule_share_replica_error_status(self, version):
+        '''From 2.74, we can add rules even if replicas are in error state.'''
         access_type, access_to = utils.get_access_rule_data_from_config(
             self.shares_v2_client.share_protocol)
         # Create the replica
@@ -200,10 +204,12 @@ class ReplicationNegativeTest(ReplicationNegativeBase):
         self.admin_client.reset_share_replica_status(
             share_replica['id'], constants.STATUS_ERROR)
 
-        # Verify access rule cannot be added
-        self.assertRaises(lib_exc.BadRequest,
-                          self.admin_client.create_access_rule,
-                          self.share1["id"], access_type, access_to, 'ro')
+        if utils.is_microversion_lt(version, '2.74'):
+            # Verify access rule cannot be added
+            self.assertRaises(lib_exc.BadRequest,
+                              self.shares_v2_client.create_access_rule,
+                              self.share1["id"], access_type, access_to, 'ro',
+                              version)
 
     @decorators.idempotent_id('91b93b71-4048-412b-bb42-0fe88edfb987')
     @testtools.skipUnless(CONF.share.run_host_assisted_migration_tests or