Handle access rules on share replicas.

Currently rules that are copied from source share would hang in
'queued_to_apply' forever. To fix this check access_rule_status of
the created replica and accordingly update access rule and state.

Change-Id: Ie7ddd9f631510ba97e92a1eb0eb9a5d944ec1b3b
Closes-Bug: #2000253

manila/share/manager.py

@@ -2411,14 +2411,37 @@ class ShareManager(manager.SchedulerDependentManager):
                  'replica_state': replica_ref.get('replica_state'),
                  'progress': '100%'})
 
-        if replica_ref.get('access_rules_status'):
-            self._update_share_instance_access_rules_state(
-                context, share_replica['id'],
-                replica_ref.get('access_rules_status'))
-        else:
+        reported_access_rules_status = replica_ref.get('access_rules_status')
+        if reported_access_rules_status in (None, "active"):
+            # update all rules to "active"
+            conditionally_change = {'queued_to_apply': 'active'}
+            self.access_helper.get_and_update_share_instance_access_rules(
+                context, share_instance_id=share_replica['id'],
+                conditionally_change=conditionally_change)
+            # update "access_rules_status" on the replica
             self._update_share_instance_access_rules_state(
                 context, share_replica['id'],
                 constants.STATUS_ACTIVE)
+        elif replica_ref.get('share_access_rules'):
+            # driver would like to update individual access rules
+            share_access_rules_dict = {
+                rule['id']: rule for rule in share_access_rules}
+            for rule_update in replica_ref.get('share_access_rules'):
+                self.access_helper.get_and_update_share_instance_access_rule(
+                    context,
+                    rule_update['id'],
+                    {'state': rule_update['state']},
+                    share_instance_id=share_replica['id'])
+                share_access_rules_dict.pop(rule_update['id'])
+            for rule_id in share_access_rules_dict:
+                self.access_helper.get_and_update_share_instance_access_rule(
+                    context,
+                    rule_id,
+                    {'state': 'active'},
+                    share_instance_id=share_replica['id'])
+            self._update_share_instance_access_rules_state(
+                context, share_replica['id'],
+                replica_ref.get('access_rules_status'))
 
         LOG.info("Share replica %s created successfully.",
                  share_replica['id'])

manila/tests/share/test_manager.py

@@ -1084,7 +1084,8 @@ class ShareManagerTestCase(test.TestCase):
     def test_create_share_replica_no_availability_zone(self):
         replica = fake_replica(
             availability_zone=None, share_network='',
-            replica_state=constants.REPLICA_STATE_OUT_OF_SYNC)
+            replica_state=constants.REPLICA_STATE_OUT_OF_SYNC,
+            access_rules_status=None)
         replica_2 = fake_replica(id='fake2')
         self.mock_object(db, 'share_replicas_get_all_by_share',
                          mock.Mock(return_value=[replica, replica_2]))
@@ -1119,8 +1120,12 @@ class ShareManagerTestCase(test.TestCase):
         mock_log_error = self.mock_object(manager.LOG, 'warning')
         self.mock_object(db, 'share_instance_access_get',
                          mock.Mock(return_value=fake_access_rules[0]))
-        mock_share_replica_access_update = self.mock_object(
-            self.share_manager, '_update_share_instance_access_rules_state')
+        mock_share_replica_access_rule_update = self.mock_object(
+            self.share_manager.access_helper,
+            'get_and_update_share_instance_access_rules')
+        mock_share_replica_access_state_update = self.mock_object(
+            self.share_manager,
+            '_update_share_instance_access_rules_state')
         driver_call = self.mock_object(
             self.share_manager.driver, 'create_replica',
             mock.Mock(return_value=replica))
@@ -1129,8 +1134,13 @@ class ShareManagerTestCase(test.TestCase):
         self.share_manager.create_share_replica(self.context, replica)
 
         mock_replica_update_call.assert_has_calls(mock_calls, any_order=False)
-        mock_share_replica_access_update.assert_called_once_with(
-            mock.ANY, replica['id'], replica['access_rules_status'])
+        mock_share_replica_access_rule_update.assert_called_once_with(
+            utils.IsAMatcher(context.RequestContext),
+            share_instance_id=replica['id'],
+            conditionally_change={'queued_to_apply': 'active'})
+        mock_share_replica_access_state_update.assert_called_once_with(
+            utils.IsAMatcher(context.RequestContext),
+            replica['id'], constants.STATUS_ACTIVE)
         self.assertTrue(mock_export_locs_update_call.called)
         self.assertTrue(mock_log_info.called)
         self.assertFalse(mock_log_warning.called)
@@ -1140,7 +1150,9 @@ class ShareManagerTestCase(test.TestCase):
     @ddt.data(True, False)
     def test_create_share_replica(self, has_snapshots):
         replica = fake_replica(
-            share_network='', replica_state=constants.REPLICA_STATE_IN_SYNC)
+            share_network='',
+            replica_state=constants.REPLICA_STATE_IN_SYNC,
+            access_rules_status='active')
         replica_2 = fake_replica(id='fake2')
         snapshots = ([fakes.fake_snapshot(create_instance=True)]
                      if has_snapshots else [])
@@ -1174,9 +1186,13 @@ class ShareManagerTestCase(test.TestCase):
         mock_log_error = self.mock_object(manager.LOG, 'warning')
         self.mock_object(db, 'share_instance_access_get',
                          mock.Mock(return_value=fake_access_rules[0]))
-        mock_share_replica_access_update = self.mock_object(
+        mock_share_replica_access_rule_update = self.mock_object(
             self.share_manager.access_helper,
-            'get_and_update_share_instance_access_rules_status')
+            'get_and_update_share_instance_access_rules')
+        mock_share_replica_access_state_update = self.mock_object(
+            self.share_manager,
+            '_update_share_instance_access_rules_state')
+
         driver_call = self.mock_object(
             self.share_manager.driver, 'create_replica',
             mock.Mock(return_value=replica))
@@ -1189,10 +1205,13 @@ class ShareManagerTestCase(test.TestCase):
             {'status': constants.STATUS_AVAILABLE,
              'replica_state': constants.REPLICA_STATE_IN_SYNC,
              'progress': '100%'})
-        mock_share_replica_access_update.assert_called_once_with(
+        mock_share_replica_access_rule_update.assert_called_once_with(
             utils.IsAMatcher(context.RequestContext),
             share_instance_id=replica['id'],
-            status=replica['access_rules_status'])
+            conditionally_change={'queued_to_apply': 'active'})
+        mock_share_replica_access_state_update.assert_called_once_with(
+            utils.IsAMatcher(context.RequestContext),
+            replica['id'], constants.STATUS_ACTIVE)
         self.assertTrue(mock_export_locs_update_call.called)
         self.assertTrue(mock_log_info.called)
         self.assertFalse(mock_log_warning.called)

releasenotes/notes/bug-2000253-handle-access-rules-on-replica-c7304ae55c68857f.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    While creating share replicas, the rules that are copied from source share
+    would hang in 'queued_to_apply' forever. Fixed it by checking status of
+    access_rule of the created replica and conditionally changed from
+    'queued_to_apply' to 'active'. For more details check
+    `Launchpad bug 2000253 <https://bugs.launchpad.net/manila/+bug/2000253>`_