Fix error'ed access rules being sent to driver

Error'ed snapshot access rules should not be sent down to drivers,
as drivers that only look at 'access_rules' parameter will always
try to apply them, causing every subsequent call to result in
error. They should only be sent to drivers when being removed.

Change-Id: Icf7e87d82eb0d51d441cd813da146cc0965fa334
Closes-bug: #1660425

manila/share/snapshot_access.py

@@ -79,15 +79,17 @@ class ShareSnapshotInstanceAccess(object):
         else:
 
             # NOTE(ganso): error'ed rules are to be left alone until
-            # reset back to "queued_to_deny" by API. Some drivers may
-            # attempt to reapply these rules, and later get deleted when
-            # requested.
+            # reset back to "queued_to_deny" by API.
             rules_to_be_on_snapshot = [
                 r for r in rules if r['state'] not in (
                     constants.ACCESS_STATE_QUEUED_TO_DENY,
                     # NOTE(ganso): We select denying rules as a recovery
                     # mechanism for invalid rules during a restart.
-                    constants.ACCESS_STATE_DENYING)
+                    constants.ACCESS_STATE_DENYING,
+                    # NOTE(ganso): We do not re-send error-ed access rules to
+                    # drivers.
+                    constants.ACCESS_STATE_ERROR
+                )
             ]
 
             # NOTE(ganso): Process queued rules

manila/tests/share/test_snapshot_access.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import copy
 import ddt
 import mock
 
@@ -52,6 +53,12 @@ class SnapshotAccessTestCase(test.TestCase):
                 'state': state,
                 'access_id': 'rule_id%s' % i
             })
+        all_rules = copy.deepcopy(rules)
+        all_rules.append({
+            'id': 'id-3',
+            'state': constants.ACCESS_STATE_ERROR,
+            'access_id': 'rule_id3'
+        })
 
         snapshot_instance_get = self.mock_object(
             db, 'share_snapshot_instance_get',
@@ -59,7 +66,7 @@ class SnapshotAccessTestCase(test.TestCase):
 
         snap_get_all_for_snap_instance = self.mock_object(
             db, 'share_snapshot_access_get_all_for_snapshot_instance',
-            mock.Mock(return_value=rules))
+            mock.Mock(return_value=all_rules))
 
         self.mock_object(db, 'share_snapshot_instance_access_update')
         self.mock_object(self.driver, 'snapshot_update_access')

releasenotes/notes/bug-1660425-snapshot-access-in-error-bce279ee310060f5.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - Snapshot access rules in error state no longer cause other
+    rules to go into error state as well.