Add update_access() method to driver interface

- Add update_access() method to driver interface
- Move all code related to access operations to ShareInstanceAccess
class
- Statuses from individual access rules are now mapped to
share_instance's access_rules_status
- Add 'access_rules_status' field to share instance, which indicates
current status of applying access rules

APIImpact
Co-Authored-By: Rodrigo Barbieri <rodrigo.barbieri@fit-tecnologia.org.br>
Co-Authored-By: Tiago Pasqualini da Silva <tiago.pasqualini@gmail.com>
Implements: bp new-share-access-driver-interface

Change-Id: Iff1ec2e3176a46e9f6bd383b38ffc5d838aa8bb8

contrib/share_driver_hooks/zaqar_notification.py

@@ -30,37 +30,46 @@ class ZaqarNotification(hook.HookBase):
     share_api = api.API()
 
     def _access_changed_trigger(self, context, func_name,
-                                access_id, share_instance_id):
-        access = self.share_api.access_get(context, access_id=access_id)
-        share = self.share_api.get(context, share_id=access.share_id)
+                                access_rules_ids, share_instance_id):
 
-        for ins in share.instances:
-            if ins.id == share_instance_id:
-                share_instance = ins
-                break
-        else:
-            raise exception.InstanceNotFound(instance_id=share_instance_id)
+        access = [self.db.share_access_get(context, rule_id)
+                  for rule_id in access_rules_ids]
 
-        for ins in access.instance_mappings:
-            if ins.share_instance_id == share_instance_id:
-                access_instance = ins
-                break
-        else:
-            raise exception.InstanceNotFound(instance_id=share_instance_id)
+        share_instance = self.db.share_instance_get(context, share_instance_id)
+
+        share = self.share_api.get(context, share_id=share_instance.share_id)
+
+        def rules_view(rules):
+            result = []
+
+            for rule in rules:
+                access_instance = None
+
+                for ins in rule.instance_mappings:
+                    if ins.share_instance_id == share_instance_id:
+                        access_instance = ins
+                        break
+                    else:
+                        raise exception.InstanceNotFound(
+                            instance_id=share_instance_id)
+
+                result.append({
+                    'access_id': rule.id,
+                    'access_instance_id': access_instance.id,
+                    'access_type': rule.access_type,
+                    'access_to': rule.access_to,
+                    'access_level': rule.access_level,
+                })
+            return result
 
         is_allow_operation = 'allow' in func_name
         results = {
-            'share_id': access.share_id,
+            'share_id': share.share_id,
             'share_instance_id': share_instance_id,
             'export_locations': [
                 el.path for el in share_instance.export_locations],
             'share_proto': share.share_proto,
-            'access_id': access.id,
-            'access_instance_id': access_instance.id,
-            'access_type': access.access_type,
-            'access_to': access.access_to,
-            'access_level': access.access_level,
-            'access_state': access_instance.state,
+            'access_rules': rules_view(access),
             'is_allow_operation': is_allow_operation,
             'availability_zone': share_instance.availability_zone,
         }
@@ -75,7 +84,7 @@ class ZaqarNotification(hook.HookBase):
             data = self._access_changed_trigger(
                 context,
                 func_name,
-                kwargs.get('access_id'),
+                kwargs.get('access_rules'),
                 kwargs.get('share_instance_id'),
             )
             self._send_notification(data)
@@ -89,7 +98,7 @@ class ZaqarNotification(hook.HookBase):
             data = self._access_changed_trigger(
                 context,
                 func_name,
-                kwargs.get('access_id'),
+                kwargs.get('access_rules'),
                 kwargs.get('share_instance_id'),
             )
             self._send_notification(data)

contrib/share_driver_hooks/zaqar_notification_example_consumer.py

@@ -105,12 +105,17 @@ def handle_message(data):
 
     Expected structure of a message is following:
         {'data': {
-             'access_id': u'b28268b9-36c6-40d3-a485-22534077328f',
-             'access_instance_id': u'd137b2cb-f549-4141-9dd7-36b2789fb973',
-             'access_level': u'rw',
-             'access_state': u'active',
-             'access_to': u'7.7.7.7',
-             'access_type': u'ip',
+            'access_rules': [
+                 {
+                    'access_id': u'b28268b9-36c6-40d3-a485-22534077328f',
+                    'access_instance_id':
+                        u'd137b2cb-f549-4141-9dd7-36b2789fb973',
+                    'access_level': u'rw',
+                    'access_state': u'active',
+                    'access_to': u'7.7.7.7',
+                    'access_type': u'ip',
+                }
+              ],
              'availability_zone': u'nova',
              'export_locations': [u'127.0.0.1:/path/to/nfs/share'],
              'is_allow_operation': True,
@@ -121,10 +126,14 @@ def handle_message(data):
     """
     if 'data' in data.keys():
         data = data['data']
-    if (data.get('access_type', '?').lower() == 'ip' and
-            'access_state' in data.keys() and
-            'error' not in data.get('access_state', '?').lower() and
-            data.get('share_proto', '?').lower() == 'nfs'):
+
+    valid_access = (
+        'access_rules' in data and len(data['access_rules']) == 1 and
+        data['access_rules'][0].get('access_type', '?').lower() == 'ip' and
+        data.get('share_proto', '?').lower() == 'nfs'
+    )
+
+    if valid_access:
         is_allow_operation = data['is_allow_operation']
         export_location = data['export_locations'][0]
         if is_allow_operation:

manila/api/openstack/api_version_request.py

@@ -44,24 +44,27 @@ REST_API_VERSION_HISTORY = """
 
     REST API Version History:
 
-    * 1.0 - Initial version. Includes all V1 APIs and extensions in Kilo.
-    * 2.0 - Versions API updated to reflect beginning of microversions epoch.
-    * 2.1 - Share create() doesn't ignore availability_zone field of share.
-    * 2.2 - Snapshots become optional feature.
-    * 2.3 - Share instances admin API
-    * 2.4 - Consistency Group support
-    * 2.5 - Share Migration admin API
-    * 2.6 - Return share_type UUID instead of name in Share API
-    * 2.7 - Rename old extension-like API URLs to core-API-like
-    * 2.8 - Attr "is_public" can be set for share using API "manage"
-    * 2.9 - Add export locations API
+    * 1.0  - Initial version. Includes all V1 APIs and extensions in Kilo.
+    * 2.0  - Versions API updated to reflect beginning of microversions epoch.
+    * 2.1  - Share create() doesn't ignore availability_zone field of share.
+    * 2.2  - Snapshots become optional feature.
+    * 2.3  - Share instances admin API
+    * 2.4  - Consistency Group support
+    * 2.5  - Share Migration admin API
+    * 2.6  - Return share_type UUID instead of name in Share API
+    * 2.7  - Rename old extension-like API URLs to core-API-like
+    * 2.8  - Attr "is_public" can be set for share using API "manage"
+    * 2.9  - Add export locations API
+    * 2.10 - Field 'access_rules_status' was added to shares and share
+            instances.
+
 """
 
 # The minimum and maximum versions of the API supported
 # The default api version request is defined to be the
 # the minimum version of the API supported.
 _MIN_API_VERSION = "2.0"
-_MAX_API_VERSION = "2.9"
+_MAX_API_VERSION = "2.10"
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 

manila/api/openstack/rest_api_version_history.rst

@@ -74,3 +74,7 @@ user documentation.
 ---
   Add export locations API. Remove export locations from "shares" and
   "share instances" APIs.
+
+2.10
+----
+  Field 'access_rules_status' was added to shares and share instances.

manila/api/views/share_instance.py

@@ -20,6 +20,7 @@ class ViewBuilder(common.ViewBuilder):
 
     _detail_version_modifiers = [
         "remove_export_locations",
+        "add_access_rules_status_field",
     ]
 
     def detail_list(self, request, instances):
@@ -64,3 +65,9 @@ class ViewBuilder(common.ViewBuilder):
     def remove_export_locations(self, share_instance_dict, share_instance):
         share_instance_dict.pop('export_location')
         share_instance_dict.pop('export_locations')
+
+    @common.ViewBuilder.versioned_method("2.10")
+    def add_access_rules_status_field(self, instance_dict, share_instance):
+        instance_dict['access_rules_status'] = (
+            share_instance.get('access_rules_status')
+        )

manila/api/views/shares.py

@@ -26,6 +26,7 @@ class ViewBuilder(common.ViewBuilder):
         "add_task_state_field",
         "modify_share_type_field",
         "remove_export_locations",
+        "add_access_rules_status_field",
     ]
 
     def summary_list(self, request, shares):
@@ -123,6 +124,10 @@ class ViewBuilder(common.ViewBuilder):
         share_dict.pop('export_location')
         share_dict.pop('export_locations')
 
+    @common.ViewBuilder.versioned_method("2.10")
+    def add_access_rules_status_field(self, share_dict, share):
+        share_dict['access_rules_status'] = share.get('access_rules_status')
+
     def _list_view(self, func, request, shares):
         """Provide a view for a list of shares."""
         shares_list = [func(request, share)['share'] for share in shares]

manila/common/constants.py

@@ -22,6 +22,7 @@ STATUS_ERROR_DELETING = 'error_deleting'
 STATUS_AVAILABLE = 'available'
 STATUS_ACTIVE = 'active'
 STATUS_INACTIVE = 'inactive'
+STATUS_OUT_OF_SYNC = 'out_of_sync'
 STATUS_MANAGING = 'manage_starting'
 STATUS_MANAGE_ERROR = 'manage_error'
 STATUS_UNMANAGING = 'unmanage_starting'

manila/db/api.py

@@ -435,6 +435,12 @@ def share_instance_access_get_all(context, access_id, session=None):
     return IMPL.share_instance_access_get_all(context, access_id, session=None)
 
 
+def share_access_get_all_for_instance(context, instance_id, session=None):
+    """Get all access rules related to a certain share instance."""
+    return IMPL.share_access_get_all_for_instance(
+        context, instance_id, session=None)
+
+
 def share_access_get_all_by_type_and_access(context, share_id, access_type,
                                             access):
     """Returns share access by given type and access."""
@@ -452,9 +458,10 @@ def share_instance_access_delete(context, mapping_id):
     return IMPL.share_instance_access_delete(context, mapping_id)
 
 
-def share_instance_access_update_state(context, mapping_id, state):
-    """Update state of access rule mapping."""
-    return IMPL.share_instance_access_update_state(context, mapping_id, state)
+def share_instance_update_access_status(context, share_instance_id, status):
+    """Update access rules status of share instance."""
+    return IMPL.share_instance_update_access_status(context, share_instance_id,
+                                                    status)
 
 
 ####################

manila/db/migrations/alembic/versions/344c1ac4747f_add_share_instance_access_rules_status.py

@@ -0,0 +1,133 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+"""Remove access rules status and add access_rule_status to share_instance
+model
+
+Revision ID: 344c1ac4747f
+Revises: dda6de06349
+Create Date: 2015-11-18 14:58:55.806396
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '344c1ac4747f'
+down_revision = 'dda6de06349'
+
+from alembic import op
+from sqlalchemy import Column, String
+
+from manila.common import constants
+from manila.db.migrations import utils
+
+
+priorities = {
+    'active': 0,
+    'new': 1,
+    'error': 2
+}
+
+upgrade_data_mapping = {
+    'active': 'active',
+    'new': 'out_of_sync',
+    'error': 'error',
+}
+
+downgrade_data_mapping = {
+    'active': 'active',
+    # NOTE(u_glide): We cannot determine is it applied rule or not in Manila,
+    # so administrator should manually handle such access rules.
+    'out_of_sync': 'error',
+    'error': 'error',
+}
+
+
+def upgrade():
+    """Transform individual access rules states to 'access_rules_status'.
+
+    WARNING: This method performs lossy converting of existing data in DB.
+    """
+    op.add_column(
+        'share_instances',
+        Column('access_rules_status', String(length=255))
+    )
+
+    connection = op.get_bind()
+    share_instances_table = utils.load_table('share_instances', connection)
+    instance_access_table = utils.load_table('share_instance_access_map',
+                                             connection)
+
+    # NOTE(u_glide): Data migrations shouldn't be performed on live clouds
+    # because it will lead to unpredictable behaviour of running operations
+    # like migration.
+    instances_query = (
+        share_instances_table.select()
+        .where(share_instances_table.c.status == constants.STATUS_AVAILABLE)
+        .where(share_instances_table.c.deleted == 'False')
+    )
+
+    for instance in connection.execute(instances_query):
+
+        access_mappings_query = instance_access_table.select().where(
+            instance_access_table.c.share_instance_id == instance['id']
+        ).where(instance_access_table.c.deleted == 'False')
+
+        status = constants.STATUS_ACTIVE
+
+        for access_rule in connection.execute(access_mappings_query):
+
+            if (access_rule['state'] == constants.STATUS_DELETING or
+                    access_rule['state'] not in priorities):
+                continue
+
+            if priorities[access_rule['state']] > priorities[status]:
+                status = access_rule['state']
+
+        op.execute(
+            share_instances_table.update().where(
+                share_instances_table.c.id == instance['id']
+            ).values({'access_rules_status': upgrade_data_mapping[status]})
+        )
+
+    op.drop_column('share_instance_access_map', 'state')
+
+
+def downgrade():
+    op.add_column(
+        'share_instance_access_map',
+        Column('state', String(length=255))
+    )
+
+    connection = op.get_bind()
+    share_instances_table = utils.load_table('share_instances', connection)
+    instance_access_table = utils.load_table('share_instance_access_map',
+                                             connection)
+
+    instances_query = (
+        share_instances_table.select()
+        .where(share_instances_table.c.status == constants.STATUS_AVAILABLE)
+        .where(share_instances_table.c.deleted == 'False')
+    )
+
+    for instance in connection.execute(instances_query):
+
+        state = downgrade_data_mapping[instance['access_rules_status']]
+
+        op.execute(
+            instance_access_table.update().where(
+                instance_access_table.c.share_instance_id == instance['id']
+            ).where(instance_access_table.c.deleted == 'False').values(
+                {'state': state}
+            )
+        )
+
+    op.drop_column('share_instances', 'access_rules_status')

manila/db/sqlalchemy/api.py

@@ -1538,9 +1538,12 @@ def _share_access_get_query(context, session, values, read_deleted='no'):
     return query.filter_by(**values)
 
 
-def _share_instance_access_query(context, session, access_id,
+def _share_instance_access_query(context, session, access_id=None,
                                  instance_id=None):
-    filters = {'access_id': access_id}
+    filters = {}
+
+    if access_id is not None:
+        filters.update({'access_id': access_id})
 
     if instance_id is not None:
         filters.update({'share_instance_id': instance_id})
@@ -1555,7 +1558,6 @@ def share_access_create(context, values):
     session = get_session()
     with session.begin():
         access_ref = models.ShareAccessMapping()
-        state = values.pop('state', None)
         access_ref.update(values)
         access_ref.save(session=session)
 
@@ -1566,8 +1568,6 @@ def share_access_create(context, values):
                 'share_instance_id': instance['id'],
                 'access_id': access_ref['id'],
             }
-            if state is not None:
-                vals.update({'state': state})
 
             _share_instance_access_create(vals, session)
 
@@ -1614,6 +1614,18 @@ def share_access_get_all_for_share(context, share_id):
                                    {'share_id': share_id}).all()
 
 
+@require_context
+def share_access_get_all_for_instance(context, instance_id, session=None):
+    """Get all access rules related to a certain share instance."""
+    session = get_session()
+    return _share_access_get_query(context, session, {}).join(
+        models.ShareInstanceAccessMapping,
+        models.ShareInstanceAccessMapping.access_id ==
+        models.ShareAccessMapping.id).filter(
+        models.ShareInstanceAccessMapping.share_instance_id ==
+        instance_id).all()
+
+
 @require_context
 def share_instance_access_get_all(context, access_id, session=None):
     if not session:
@@ -1644,8 +1656,7 @@ def share_access_delete(context, access_id):
             raise exception.InvalidShareAccess(msg)
 
         session.query(models.ShareAccessMapping).\
-            filter_by(id=access_id).soft_delete(update_status=True,
-                                                status_field_name='state')
+            filter_by(id=access_id).soft_delete()
 
 
 @require_context
@@ -1653,8 +1664,7 @@ def share_access_delete_all_by_share(context, share_id):
     session = get_session()
     with session.begin():
         session.query(models.ShareAccessMapping). \
-            filter_by(share_id=share_id).soft_delete(update_status=True,
-                                                     status_field_name='state')
+            filter_by(share_id=share_id).soft_delete()
 
 
 @require_context
@@ -1668,8 +1678,7 @@ def share_instance_access_delete(context, mapping_id):
         if not mapping:
             exception.NotFound()
 
-        mapping.soft_delete(session, update_status=True,
-                            status_field_name='state')
+        mapping.soft_delete(session)
 
         other_mappings = share_instance_access_get_all(
             context, mapping['access_id'], session)
@@ -1679,18 +1688,18 @@ def share_instance_access_delete(context, mapping_id):
             (
                 session.query(models.ShareAccessMapping)
                 .filter_by(id=mapping['access_id'])
-                .soft_delete(update_status=True, status_field_name='state')
+                .soft_delete()
             )
 
 
 @require_context
 @oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True)
-def share_instance_access_update_state(context, mapping_id, state):
+def share_instance_update_access_status(context, share_instance_id, status):
     session = get_session()
     with session.begin():
-        mapping = session.query(models.ShareInstanceAccessMapping).\
-            filter_by(id=mapping_id).first()
-        mapping.update({'state': state})
+        mapping = session.query(models.ShareInstance).\
+            filter_by(id=share_instance_id).first()
+        mapping.update({'access_rules_status': status})
         mapping.save(session=session)
         return mapping
 

manila/db/sqlalchemy/models.py

@@ -189,7 +189,7 @@ class Share(BASE, ManilaBase):
     __tablename__ = 'shares'
     _extra_keys = ['name', 'export_location', 'export_locations', 'status',
                    'host', 'share_server_id', 'share_network_id',
-                   'availability_zone']
+                   'availability_zone', 'access_rules_status']
 
     @property
     def name(self):
@@ -253,6 +253,10 @@ class Share(BASE, ManilaBase):
                     result = self.instances[0]
         return result
 
+    @property
+    def access_rules_status(self):
+        return get_access_rules_status(self.instances)
+
     id = Column(String(36), primary_key=True)
     deleted = Column(String(36), default='False')
     user_id = Column(String(255))
@@ -326,6 +330,18 @@ class ShareInstance(BASE, ManilaBase):
     deleted = Column(String(36), default='False')
     host = Column(String(255))
     status = Column(String(255))
+
+    ACCESS_STATUS_PRIORITIES = {
+        constants.STATUS_ACTIVE: 0,
+        constants.STATUS_OUT_OF_SYNC: 1,
+        constants.STATUS_ERROR: 2,
+    }
+
+    access_rules_status = Column(Enum(constants.STATUS_ACTIVE,
+                                      constants.STATUS_OUT_OF_SYNC,
+                                      constants.STATUS_ERROR),
+                                 default=constants.STATUS_ACTIVE)
+
     scheduled_at = Column(DateTime)
     launched_at = Column(DateTime)
     terminated_at = Column(DateTime)
@@ -476,26 +492,6 @@ class ShareAccessMapping(BASE, ManilaBase):
     """Represents access to share."""
     __tablename__ = 'share_access_map'
 
-    @property
-    def state(self):
-        state = ShareInstanceAccessMapping.STATE_NEW
-
-        if len(self.instance_mappings) > 0:
-            state = ShareInstanceAccessMapping.STATE_ACTIVE
-            priorities = ShareInstanceAccessMapping.STATE_PRIORITIES
-
-            for mapping in self.instance_mappings:
-                priority = priorities.get(
-                    mapping['state'], ShareInstanceAccessMapping.STATE_ERROR)
-
-                if priority > priorities.get(state):
-                    state = mapping['state']
-
-                if state == ShareInstanceAccessMapping.STATE_ERROR:
-                    break
-
-        return state
-
     id = Column(String(36), primary_key=True)
     deleted = Column(String(36), default='False')
     share_id = Column(String(36), ForeignKey('shares.id'))
@@ -516,33 +512,36 @@ class ShareAccessMapping(BASE, ManilaBase):
         )
     )
 
+    @property
+    def state(self):
+        instances = [im.instance for im in self.instance_mappings]
+        access_rules_status = get_access_rules_status(instances)
+
+        if access_rules_status == constants.STATUS_OUT_OF_SYNC:
+            return constants.STATUS_NEW
+        else:
+            return access_rules_status
+
 
 class ShareInstanceAccessMapping(BASE, ManilaBase):
     """Represents access to individual share instances."""
-    STATE_NEW = constants.STATUS_NEW
-    STATE_ACTIVE = constants.STATUS_ACTIVE
-    STATE_DELETING = constants.STATUS_DELETING
-    STATE_DELETED = constants.STATUS_DELETED
-    STATE_ERROR = constants.STATUS_ERROR
-
-    # NOTE(u_glide): State with greatest priority becomes a state of access
-    # rule
-    STATE_PRIORITIES = {
-        STATE_ACTIVE: 0,
-        STATE_NEW: 1,
-        STATE_DELETED: 2,
-        STATE_DELETING: 3,
-        STATE_ERROR: 4
-    }
 
     __tablename__ = 'share_instance_access_map'
     id = Column(String(36), primary_key=True)
     deleted = Column(String(36), default='False')
     share_instance_id = Column(String(36), ForeignKey('share_instances.id'))
     access_id = Column(String(36), ForeignKey('share_access_map.id'))
-    state = Column(Enum(STATE_NEW, STATE_ACTIVE,
-                        STATE_DELETING, STATE_DELETED, STATE_ERROR),
-                   default=STATE_NEW)
+
+    instance = orm.relationship(
+        "ShareInstance",
+        lazy='immediate',
+        primaryjoin=(
+            'and_('
+            'ShareInstanceAccessMapping.share_instance_id == '
+            'ShareInstance.id, '
+            'ShareInstanceAccessMapping.deleted == "False")'
+        )
+    )
 
 
 class ShareSnapshot(BASE, ManilaBase):
@@ -902,3 +901,27 @@ def register_models():
     engine = create_engine(CONF.database.connection, echo=False)
     for model in models:
         model.metadata.create_all(engine)
+
+
+def get_access_rules_status(instances):
+    share_access_status = constants.STATUS_ACTIVE
+
+    if len(instances) == 0:
+        return share_access_status
+
+    priorities = ShareInstance.ACCESS_STATUS_PRIORITIES
+
+    for instance in instances:
+        if instance['status'] != constants.STATUS_AVAILABLE:
+            continue
+
+        instance_access_status = instance['access_rules_status']
+
+        if priorities.get(instance_access_status) > priorities.get(
+                share_access_status):
+            share_access_status = instance_access_status
+
+        if share_access_status == constants.STATUS_ERROR:
+            break
+
+    return share_access_status

manila/exception.py

@@ -411,7 +411,7 @@ class ShareAccessExists(ManilaException):
 
 
 class InvalidShareAccess(Invalid):
-    message = _("Invalid access_rule: %(reason)s.")
+    message = _("Invalid access rule: %(reason)s")
 
 
 class InvalidShareAccessLevel(Invalid):

manila/share/access.py

@@ -0,0 +1,155 @@
+# Copyright (c) 2015 Mirantis Inc.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from oslo_log import log
+import six
+
+from manila.common import constants
+from manila import exception
+from manila.i18n import _LI
+
+LOG = log.getLogger(__name__)
+
+
+class ShareInstanceAccess(object):
+
+    def __init__(self, db, driver):
+        self.db = db
+        self.driver = driver
+
+    def update_access_rules(self, context, share_instance_id, add_rules=None,
+                            delete_rules=None, share_server=None):
+        """Update access rules in driver and database for given share instance.
+
+        :param context: current context
+        :param share_instance_id: Id of the share instance model
+        :param add_rules: list with ShareAccessMapping models or None - rules
+        which should be added
+        :param delete_rules: list with ShareAccessMapping models, "all", None
+        - rules which should be deleted. If "all" is provided - all rules will
+        be deleted.
+        :param share_server: Share server model or None
+        """
+        share_instance = self.db.share_instance_get(
+            context, share_instance_id, with_share_data=True)
+
+        add_rules = add_rules or []
+        delete_rules = delete_rules or []
+        remove_rules = None
+
+        if six.text_type(delete_rules).lower() == "all":
+            # NOTE(ganso): if we are deleting an instance or clearing all
+            # the rules, we want to remove only the ones related
+            # to this instance.
+            delete_rules = self.db.share_access_get_all_for_instance(
+                context, share_instance['id'])
+            rules = []
+        else:
+            rules = self.db.share_access_get_all_for_share(
+                context, share_instance['share_id'])
+            if delete_rules:
+                delete_ids = [rule['id'] for rule in delete_rules]
+                rules = list(filter(lambda r: r['id'] not in delete_ids,
+                                    rules))
+                # NOTE(ganso): trigger maintenance mode
+                if share_instance['access_rules_status'] == (
+                        constants.STATUS_ERROR):
+                    remove_rules = delete_rules
+                    delete_rules = []
+
+        try:
+            try:
+                self.driver.update_access(
+                    context,
+                    share_instance,
+                    rules,
+                    add_rules=add_rules,
+                    delete_rules=delete_rules,
+                    share_server=share_server
+                )
+            except NotImplementedError:
+                # NOTE(u_glide): Fallback to legacy allow_access/deny_access
+                # for drivers without update_access() method support
+
+                self._update_access_fallback(add_rules, context, delete_rules,
+                                             remove_rules, share_instance,
+                                             share_server)
+        except Exception as e:
+            error = six.text_type(e)
+            LOG.exception(error)
+            self.db.share_instance_update_access_status(
+                context,
+                share_instance['id'],
+                constants.STATUS_ERROR)
+            raise exception.ManilaException(message=error)
+
+        # NOTE(ganso): remove rules after maintenance is complete
+        if remove_rules:
+            delete_rules = remove_rules
+
+        self._remove_access_rules(context, delete_rules, share_instance['id'])
+
+        self.db.share_instance_update_access_status(
+            context,
+            share_instance['id'],
+            constants.STATUS_ACTIVE
+        )
+
+        LOG.info(_LI("Access rules were successfully applied for "
+                     "share instance: %s"),
+                 share_instance['id'])
+
+    def _update_access_fallback(self, add_rules, context, delete_rules,
+                                remove_rules, share_instance, share_server):
+        for rule in add_rules:
+            LOG.info(
+                _LI("Applying access rule '%(rule)s' for share "
+                    "instance '%(instance)s'"),
+                {'rule': rule['id'], 'instance': share_instance['id']}
+            )
+
+            self.driver.allow_access(
+                context,
+                share_instance,
+                rule,
+                share_server=share_server
+            )
+
+        # NOTE(ganso): Fallback mode temporary compatibility workaround
+        if remove_rules:
+            delete_rules = remove_rules
+        for rule in delete_rules:
+            LOG.info(
+                _LI("Denying access rule '%(rule)s' from share "
+                    "instance '%(instance)s'"),
+                {'rule': rule['id'], 'instance': share_instance['id']}
+            )
+
+            self.driver.deny_access(
+                context,
+                share_instance,
+                rule,
+                share_server=share_server
+            )
+
+    def _remove_access_rules(self, context, access_rules, share_instance_id):
+        if not access_rules:
+            return
+
+        for rule in access_rules:
+            access_mapping = self.db.share_instance_access_get(
+                context, rule['id'], share_instance_id)
+
+            self.db.share_instance_access_delete(context, access_mapping['id'])

manila/share/api.py

@@ -818,11 +818,14 @@ class API(base.Base):
             'access_to': access_to,
             'access_level': access_level,
         }
-        for access in self.db.share_access_get_all_by_type_and_access(
-                ctx, share['id'], access_type, access_to):
-            if access['state'] != constants.STATUS_ERROR:
+
+        share_access_list = self.db.share_access_get_all_by_type_and_access(
+            ctx, share['id'], access_type, access_to)
+
+        if len(share_access_list) > 0:
                 raise exception.ShareAccessExists(access_type=access_type,
                                                   access=access_to)
+
         if access_level not in constants.ACCESS_LEVELS + (None, ):
             msg = _("Invalid share access level: %s.") % access_level
             raise exception.InvalidShareAccess(reason=msg)
@@ -830,6 +833,10 @@ class API(base.Base):
 
         for share_instance in share.instances:
             self.allow_access_to_instance(ctx, share_instance, access)
+
+        # NOTE(tpsilva): refreshing share_access model
+        access = self.db.share_access_get(ctx, access['id'])
+
         return {
             'id': access['id'],
             'share_id': access['share_id'],
@@ -846,6 +853,22 @@ class API(base.Base):
             msg = _("Invalid share instance host: %s") % share_instance['host']
             raise exception.InvalidShareInstance(reason=msg)
 
+        if share_instance['access_rules_status'] != constants.STATUS_ACTIVE:
+            status = share_instance['access_rules_status']
+            msg = _("Share instance should have '%(valid_status)s' "
+                    "access rules status, but current status is: "
+                    "%(status)s.") % {
+                'valid_status': constants.STATUS_ACTIVE,
+                'status': status,
+            }
+
+            raise exception.InvalidShareInstance(reason=msg)
+
+        self.db.share_instance_update_access_status(
+            context, share_instance['id'],
+            constants.STATUS_OUT_OF_SYNC
+        )
+
         self.share_rpcapi.allow_access(context, share_instance, access)
 
     def deny_access(self, ctx, share, access):
@@ -860,27 +883,14 @@ class API(base.Base):
             msg = _("Share status must be %s") % constants.STATUS_AVAILABLE
             raise exception.InvalidShare(reason=msg)
 
-        # Then check state of the access rule
-        if (access['state'] == constants.STATUS_ERROR and not
-                self.db.share_instance_access_get_all(ctx, access['id'])):
-            self.db.share_access_delete(ctx, access["id"])
-
-        elif access['state'] in [constants.STATUS_ACTIVE,
-                                 constants.STATUS_ERROR]:
-            for share_instance in share.instances:
-                try:
-                    self.deny_access_to_instance(ctx, share_instance, access)
-                except exception.NotFound:
-                    LOG.warning(_LW("Access rule %(access_id)s not found "
-                                    "for instance %(instance_id)s.") % {
-                                'access_id': access['id'],
-                                'instance_id': share_instance['id']})
-        else:
-            msg = _("Access policy should be %(active)s or in %(error)s "
-                    "state") % {"active": constants.STATUS_ACTIVE,
-                                "error": constants.STATUS_ERROR}
-            raise exception.InvalidShareAccess(reason=msg)
-            # update share state and send message to manager
+        for share_instance in share.instances:
+            try:
+                self.deny_access_to_instance(ctx, share_instance, access)
+            except exception.NotFound:
+                LOG.warning(_LW("Access rule %(access_id)s not found "
+                                "for instance %(instance_id)s.") % {
+                    'access_id': access['id'],
+                    'instance_id': share_instance['id']})
 
     def deny_access_to_instance(self, context, share_instance, access):
         policy.check_policy(context, 'share', 'deny_access')
@@ -889,11 +899,10 @@ class API(base.Base):
             msg = _("Invalid share instance host: %s") % share_instance['host']
             raise exception.InvalidShareInstance(reason=msg)
 
-        access_mapping = self.db.share_instance_access_get(
-            context, access['id'], share_instance['id'])
-        self.db.share_instance_access_update_state(
-            context, access_mapping['id'],
-            access_mapping.STATE_DELETING)
+        if share_instance['access_rules_status'] != constants.STATUS_ERROR:
+            self.db.share_instance_update_access_status(
+                context, share_instance['id'],
+                constants.STATUS_OUT_OF_SYNC)
 
         self.share_rpcapi.deny_access(context, share_instance, access)
 
@@ -905,7 +914,8 @@ class API(base.Base):
                  'access_type': rule.access_type,
                  'access_to': rule.access_to,
                  'access_level': rule.access_level,
-                 'state': rule.state} for rule in rules]
+                 'state': rule.state,
+                 } for rule in rules]
 
     def access_get(self, context, access_id):
         """Returns access rule with the id."""

manila/share/driver.py

@@ -603,6 +603,32 @@ class ShareDriver(object):
         """Deny access to the share."""
         raise NotImplementedError()
 
+    def update_access(self, context, share, access_rules, add_rules=None,
+                      delete_rules=None, share_server=None):
+        """Update access rules for given share.
+
+        Drivers should support 2 different cases in this method:
+        1. Recovery after error - 'access_rules' contains all access_rules,
+        'add_rules' and 'delete_rules' are None. Driver should clear any
+        existent access rules and apply all access rules for given share.
+        This recovery is made at driver start up.
+
+        2. Adding/Deleting of several access rules - 'access_rules' contains
+        all access_rules, 'add_rules' and 'delete_rules' contain rules which
+        should be added/deleted. Driver can ignore rules in 'access_rules' and
+        apply only rules from 'add_rules' and 'delete_rules'.
+
+        :param context: Current context
+        :param share: Share model with share data.
+        :param access_rules: All access rules for given share
+        :param add_rules: None or List of access rules which should be added
+               access_rules already contains these rules.
+        :param delete_rules: None or List of access rules which should be
+               removed. access_rules doesn't contain these rules.
+        :param share_server: None or Share server model
+        """
+        raise NotImplementedError()
+
     def check_for_setup_error(self):
         """Check for setup error."""
         max_ratio = self.configuration.safe_get('max_over_subscription_ratio')

manila/share/manager.py

@@ -41,6 +41,7 @@ from manila.i18n import _LI
 from manila.i18n import _LW
 from manila import manager
 from manila import quota
+from manila.share import access
 import manila.share.configuration
 from manila.share import drivers_private_data
 from manila.share import migration
@@ -136,7 +137,7 @@ def add_hooks(f):
 class ShareManager(manager.SchedulerDependentManager):
     """Manages NAS storages."""
 
-    RPC_API_VERSION = '1.6'
+    RPC_API_VERSION = '1.7'
 
     def __init__(self, share_driver=None, service_name=None, *args, **kwargs):
         """Load the driver from args, or from flags."""
@@ -167,6 +168,8 @@ class ShareManager(manager.SchedulerDependentManager):
             configuration=self.configuration
         )
 
+        self.access_helper = access.ShareInstanceAccess(self.db, self.driver)
+
         self.hooks = []
         self._init_hook_drivers()
 
@@ -271,28 +274,18 @@ class ShareManager(manager.SchedulerDependentManager):
                 self.db.share_export_locations_update(
                     ctxt, share_instance['id'], export_locations)
 
-            rules = self.db.share_access_get_all_for_share(
-                ctxt, share_instance['share_id'])
-            for access_ref in rules:
-                if access_ref['state'] != constants.STATUS_ACTIVE:
-                    continue
+            if share_instance['access_rules_status'] == (
+                    constants.STATUS_OUT_OF_SYNC):
 
                 try:
-                    self.driver.allow_access(ctxt, share_instance, access_ref,
-                                             share_server=share_server)
-                except exception.ShareAccessExists:
-                    pass
+                    self.access_helper.update_access_rules(
+                        ctxt, share_instance['id'], share_server=share_server)
                 except Exception as e:
                     LOG.error(
-                        _LE("Unexpected exception during share access"
-                            " allow operation. Share id is '%(s_id)s'"
-                            ", access rule type is '%(ar_type)s', "
-                            "access rule id is '%(ar_id)s', exception"
-                            " is '%(e)s'."),
-                        {'s_id': share_instance['id'],
-                         'ar_type': access_ref['access_type'],
-                         'ar_id': access_ref['id'],
-                         'e': six.text_type(e)},
+                        _LE("Unexpected error occurred while updating access "
+                            "rules for share instance %(s_id)s. "
+                            "Exception: \n%(e)s."),
+                        {'s_id': share_instance['id'], 'e': six.text_type(e)},
                     )
 
         self.publish_service_capabilities(ctxt)
@@ -946,8 +939,12 @@ class ShareManager(manager.SchedulerDependentManager):
 
         if self.configuration.safe_get('unmanage_remove_access_rules'):
             try:
-                self._remove_share_access_rules(context, share_ref,
-                                                share_instance, share_server)
+                self.access_helper.update_access_rules(
+                    context,
+                    share_instance['id'],
+                    delete_rules="all",
+                    share_server=share_server
+                )
             except Exception as e:
                 share_manage_set_error_status(
                     _LE("Can not remove access rules of share: %s."), e)
@@ -962,12 +959,15 @@ class ShareManager(manager.SchedulerDependentManager):
         """Delete a share instance."""
         context = context.elevated()
         share_instance = self._get_share_instance(context, share_instance_id)
-        share = self.db.share_get(context, share_instance['share_id'])
         share_server = self._get_share_server(context, share_instance)
 
         try:
-            self._remove_share_access_rules(context, share, share_instance,
-                                            share_server)
+            self.access_helper.update_access_rules(
+                context,
+                share_instance_id,
+                delete_rules="all",
+                share_server=share_server
+            )
             self.driver.delete_share(context, share_instance,
                                      share_server=share_server)
         except Exception:
@@ -1004,15 +1004,6 @@ class ShareManager(manager.SchedulerDependentManager):
         for server in servers:
             self.delete_share_server(ctxt, server)
 
-    def _remove_share_access_rules(self, context, share_ref, share_instance,
-                                   share_server):
-        rules = self.db.share_access_get_all_for_share(
-            context, share_ref['id'])
-
-        for access_ref in rules:
-            self._deny_access(context, access_ref,
-                              share_instance, share_server)
-
     @add_hooks
     @utils.require_driver_initialized
     def create_snapshot(self, context, share_id, snapshot_id):
@@ -1097,61 +1088,37 @@ class ShareManager(manager.SchedulerDependentManager):
 
     @add_hooks
     @utils.require_driver_initialized
-    def allow_access(self, context, share_instance_id, access_id):
+    def allow_access(self, context, share_instance_id, access_rules):
         """Allow access to some share instance."""
-        access_mapping = self.db.share_instance_access_get(context, access_id,
-                                                           share_instance_id)
+        add_rules = [self.db.share_access_get(context, rule_id)
+                     for rule_id in access_rules]
 
-        if access_mapping['state'] != access_mapping.STATE_NEW:
-            return
+        share_instance = self._get_share_instance(context, share_instance_id)
+        share_server = self._get_share_server(context, share_instance)
 
-        try:
-            access_ref = self.db.share_access_get(context, access_id)
-            share_instance = self.db.share_instance_get(
-                context, share_instance_id, with_share_data=True)
-            share_server = self._get_share_server(context, share_instance)
-            self.driver.allow_access(context, share_instance, access_ref,
-                                     share_server=share_server)
-            self.db.share_instance_access_update_state(
-                context, access_mapping['id'], access_mapping.STATE_ACTIVE)
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                self.db.share_instance_access_update_state(
-                    context, access_mapping['id'], access_mapping.STATE_ERROR)
-
-        LOG.info(_LI("'%(access_to)s' has been successfully allowed "
-                     "'%(access_level)s' access on share instance "
-                     "%(share_instance_id)s."),
-                 {'access_to': access_ref['access_to'],
-                  'access_level': access_ref['access_level'],
-                  'share_instance_id': share_instance_id})
+        return self.access_helper.update_access_rules(
+            context,
+            share_instance_id,
+            add_rules=add_rules,
+            share_server=share_server
+        )
 
     @add_hooks
     @utils.require_driver_initialized
-    def deny_access(self, context, share_instance_id, access_id):
+    def deny_access(self, context, share_instance_id, access_rules):
         """Deny access to some share."""
-        access_ref = self.db.share_access_get(context, access_id)
-        share_instance = self.db.share_instance_get(
-            context, share_instance_id, with_share_data=True)
+        delete_rules = [self.db.share_access_get(context, rule_id)
+                        for rule_id in access_rules]
+
+        share_instance = self._get_share_instance(context, share_instance_id)
         share_server = self._get_share_server(context, share_instance)
-        self._deny_access(context, access_ref, share_instance, share_server)
 
-        LOG.info(_LI("'(access_to)s' has been successfully denied access to "
-                     "share instance %(share_instance_id)s."),
-                 {'access_to': access_ref['access_to'],
-                  'share_instance_id': share_instance_id})
-
-    def _deny_access(self, context, access_ref, share_instance, share_server):
-        access_mapping = self.db.share_instance_access_get(
-            context, access_ref['id'], share_instance['id'])
-        try:
-            self.driver.deny_access(context, share_instance, access_ref,
-                                    share_server=share_server)
-            self.db.share_instance_access_delete(context, access_mapping['id'])
-        except Exception:
-            with excutils.save_and_reraise_exception():
-                self.db.share_instance_access_update_state(
-                    context, access_mapping['id'], access_mapping.STATE_ERROR)
+        return self.access_helper.update_access_rules(
+            context,
+            share_instance_id,
+            delete_rules=delete_rules,
+            share_server=share_server
+        )
 
     @periodic_task.periodic_task(spacing=CONF.periodic_interval)
     @utils.require_driver_initialized

manila/share/migration.py

@@ -104,115 +104,52 @@ class ShareMigrationHelper(object):
     def deny_rules_and_wait(self, context, share, saved_rules):
 
         api = share_api.API()
+        api.deny_access_to_instance(context, share.instance, saved_rules)
 
-        # Deny each one.
-        for instance in share.instances:
-            for access in saved_rules:
-                api.deny_access_to_instance(context, instance, access)
-
-        # Wait for all rules to be cleared.
-        starttime = time.time()
-        deadline = starttime + self.migration_wait_access_rules_timeout
-        tries = 0
-        rules = self.db.share_access_get_all_for_share(context, share['id'])
-        while len(rules) > 0:
-            tries += 1
-            now = time.time()
-            if now > deadline:
-                msg = _("Timeout removing access rules from share "
-                        "%(share_id)s. Timeout was %(timeout)s seconds.") % {
-                    'share_id': share['id'],
-                    'timeout': self.migration_wait_access_rules_timeout}
-                raise exception.ShareMigrationFailed(reason=msg)
-            else:
-                time.sleep(tries ** 2)
-            rules = self.db.share_access_get_all_for_share(
-                context, share['id'])
+        self.wait_for_access_update(share.instance)
 
     def add_rules_and_wait(self, context, share, saved_rules,
                            access_level=None):
-
+        rules = []
         for access in saved_rules:
-            if access_level:
-                level = access_level
-            else:
-                level = access['access_level']
-            self.api.allow_access(context, share, access['access_type'],
-                                  access['access_to'], level)
+            values = {
+                'share_id': share['id'],
+                'access_type': access['access_type'],
+                'access_level': access_level or access['access_level'],
+                'access_to': access['access_to'],
+            }
+            rules.append(self.db.share_access_create(context, values))
 
+        self.api.allow_access_to_instance(context, share.instance, rules)
+        self.wait_for_access_update(share.instance)
+
+    def wait_for_access_update(self, share_instance):
         starttime = time.time()
         deadline = starttime + self.migration_wait_access_rules_timeout
-        rules_added = False
         tries = 0
-        rules = self.db.share_access_get_all_for_share(context, share['id'])
-        while not rules_added:
-            rules_added = True
-            tries += 1
-            now = time.time()
-            for access in rules:
-                if access['state'] != constants.STATUS_ACTIVE:
-                    rules_added = False
-            if rules_added:
+
+        while True:
+            instance = self.db.share_instance_get(
+                self.context, share_instance['id'])
+
+            if instance['access_rules_status'] == constants.STATUS_ACTIVE:
                 break
-            if now > deadline:
-                msg = _("Timeout adding access rules for share "
-                        "%(share_id)s. Timeout was %(timeout)s seconds.") % {
-                    'share_id': share['id'],
-                    'timeout': self.migration_wait_access_rules_timeout}
-                raise exception.ShareMigrationFailed(reason=msg)
-            else:
-                time.sleep(tries ** 2)
-            rules = self.db.share_access_get_all_for_share(
-                context, share['id'])
 
-    def wait_for_allow_access(self, access_ref):
-
-        starttime = time.time()
-        deadline = starttime + self.migration_wait_access_rules_timeout
-        tries = 0
-        rule = self.api.access_get(self.context, access_ref['id'])
-        while rule['state'] != constants.STATUS_ACTIVE:
             tries += 1
             now = time.time()
-            if rule['state'] == constants.STATUS_ERROR:
-                msg = _("Failed to allow access"
-                        " on share %s") % self.share['id']
+            if instance['access_rules_status'] == constants.STATUS_ERROR:
+                msg = _("Failed to update access rules"
+                        " on share instance %s") % share_instance['id']
                 raise exception.ShareMigrationFailed(reason=msg)
             elif now > deadline:
-                msg = _("Timeout trying to allow access"
-                        " on share %(share_id)s. Timeout "
+                msg = _("Timeout trying to update access rules"
+                        " on share instance %(share_id)s. Timeout "
                         "was %(timeout)s seconds.") % {
-                    'share_id': self.share['id'],
+                    'share_id': share_instance['id'],
                     'timeout': self.migration_wait_access_rules_timeout}
                 raise exception.ShareMigrationFailed(reason=msg)
             else:
                 time.sleep(tries ** 2)
-            rule = self.api.access_get(self.context, access_ref['id'])
-
-        return rule
-
-    def wait_for_deny_access(self, access_ref):
-
-        starttime = time.time()
-        deadline = starttime + self.migration_wait_access_rules_timeout
-        tries = -1
-        rule = "Something not None"
-        while rule:
-            try:
-                rule = self.api.access_get(self.context, access_ref['id'])
-                tries += 1
-                now = time.time()
-                if now > deadline:
-                    msg = _("Timeout trying to deny access"
-                            " on share %(share_id)s. Timeout "
-                            "was %(timeout)s seconds.") % {
-                        'share_id': self.share['id'],
-                        'timeout': self.migration_wait_access_rules_timeout}
-                    raise exception.ShareMigrationFailed(reason=msg)
-            except exception.NotFound:
-                rule = None
-            else:
-                time.sleep(tries ** 2)
 
     def allow_migration_access(self, access):
         allowed = False
@@ -234,7 +171,7 @@ class ShareMigrationHelper(object):
                     access_ref = access_item
 
         if access_ref and allowed:
-            access_ref = self.wait_for_allow_access(access_ref)
+            self.wait_for_access_update(self.share.instance)
 
         return access_ref
 
@@ -273,7 +210,7 @@ class ShareMigrationHelper(object):
                     raise
 
             if denied:
-                self.wait_for_deny_access(access_ref)
+                self.wait_for_access_update(self.share.instance)
 
     # NOTE(ganso): Cleanup methods do not throw exception, since the
     # exceptions that should be thrown are the ones that call the cleanup

manila/share/rpcapi.py

@@ -45,6 +45,7 @@ class ShareAPI(object):
             migrate_share()
             get_migration_info()
             get_driver_migration_info()
+        1.7 - Update target call API in allow/deny access methods
     """
 
     BASE_RPC_API_VERSION = '1.0'
@@ -53,7 +54,7 @@ class ShareAPI(object):
         super(ShareAPI, self).__init__()
         target = messaging.Target(topic=CONF.share_topic,
                                   version=self.BASE_RPC_API_VERSION)
-        self.client = rpc.get_client(target, version_cap='1.6')
+        self.client = rpc.get_client(target, version_cap='1.7')
 
     def create_share_instance(self, ctxt, share_instance, host,
                               request_spec, filter_properties,
@@ -131,19 +132,26 @@ class ShareAPI(object):
         cctxt = self.client.prepare(server=new_host)
         cctxt.cast(ctxt, 'delete_snapshot', snapshot_id=snapshot['id'])
 
+    @staticmethod
+    def _get_access_rules(access):
+        if isinstance(access, list):
+            return [rule['id'] for rule in access]
+        else:
+            return [access['id']]
+
     def allow_access(self, ctxt, share_instance, access):
         host = utils.extract_host(share_instance['host'])
-        cctxt = self.client.prepare(server=host, version='1.4')
+        cctxt = self.client.prepare(server=host, version='1.7')
         cctxt.cast(ctxt, 'allow_access',
                    share_instance_id=share_instance['id'],
-                   access_id=access['id'])
+                   access_rules=self._get_access_rules(access))
 
     def deny_access(self, ctxt, share_instance, access):
         host = utils.extract_host(share_instance['host'])
-        cctxt = self.client.prepare(server=host, version='1.4')
+        cctxt = self.client.prepare(server=host, version='1.7')
         cctxt.cast(ctxt, 'deny_access',
                    share_instance_id=share_instance['id'],
-                   access_id=access['id'])
+                   access_rules=self._get_access_rules(access))
 
     def publish_service_capabilities(self, ctxt):
         cctxt = self.client.prepare(fanout=True, version='1.0')

manila/tests/api/contrib/stubs.py

@@ -32,6 +32,7 @@ def stub_share(id, **kwargs):
         'host': 'fakehost',
         'size': 1,
         'availability_zone': 'fakeaz',
+        'access_rules_status': 'active',
         'status': 'fakestatus',
         'display_name': 'displayname',
         'display_description': 'displaydesc',

manila/tests/db/migrations/alembic/migrations_data_checks.py

@@ -253,3 +253,101 @@ class ShareInstanceExportLocationMetadataChecks(BaseMigrationChecks):
         self.test_case.assertRaises(
             sa_exc.NoSuchTableError,
             utils.load_table, self.elm_table_name, engine)
+
+
+@map_to_migration('344c1ac4747f')
+class AccessRulesStatusMigrationChecks(BaseMigrationChecks):
+
+    def _get_instance_data(self, data):
+        base_dict = {}
+        base_dict.update(data)
+        return base_dict
+
+    def setup_upgrade_data(self, engine):
+
+        share_table = utils.load_table('shares', engine)
+
+        share = {
+            'id': 1,
+            'share_proto': "NFS",
+            'size': 0,
+            'snapshot_id': None,
+            'user_id': 'fake',
+            'project_id': 'fake',
+        }
+
+        engine.execute(share_table.insert(share))
+
+        rules1 = [
+            {'id': 'r1', 'share_instance_id': 1, 'state': 'active',
+             'deleted': 'False'},
+            {'id': 'r2', 'share_instance_id': 1, 'state': 'active',
+             'deleted': 'False'},
+            {'id': 'r3', 'share_instance_id': 1, 'state': 'deleting',
+             'deleted': 'False'},
+        ]
+        rules2 = [
+            {'id': 'r4', 'share_instance_id': 2, 'state': 'active',
+             'deleted': 'False'},
+            {'id': 'r5', 'share_instance_id': 2, 'state': 'error',
+             'deleted': 'False'},
+        ]
+
+        rules3 = [
+            {'id': 'r6', 'share_instance_id': 3, 'state': 'new',
+             'deleted': 'False'},
+        ]
+
+        instance_fixtures = [
+            {'id': 1, 'deleted': 'False', 'host': 'fake1', 'share_id': 1,
+             'status': 'available', 'rules': rules1},
+            {'id': 2, 'deleted': 'False', 'host': 'fake2', 'share_id': 1,
+             'status': 'available', 'rules': rules2},
+            {'id': 3, 'deleted': 'False', 'host': 'fake3', 'share_id': 1,
+             'status': 'available', 'rules': rules3},
+            {'id': 4, 'deleted': 'False', 'host': 'fake4', 'share_id': 1,
+             'status': 'deleting', 'rules': []},
+        ]
+
+        share_instances_table = utils.load_table('share_instances', engine)
+        share_instances_rules_table = utils.load_table(
+            'share_instance_access_map', engine)
+
+        for fixture in instance_fixtures:
+            rules = fixture.pop('rules')
+            engine.execute(share_instances_table.insert(fixture))
+
+            for rule in rules:
+                engine.execute(share_instances_rules_table.insert(rule))
+
+    def check_upgrade(self, engine, _):
+        instances_table = utils.load_table('share_instances', engine)
+
+        valid_statuses = {
+            '1': 'active',
+            '2': 'error',
+            '3': 'out_of_sync',
+            '4': None,
+        }
+
+        instances = engine.execute(instances_table.select().where(
+            instances_table.c.id in valid_statuses.keys()))
+
+        for instance in instances:
+            self.test_case.assertEqual(valid_statuses[instance['id']],
+                                       instance['access_rules_status'])
+
+    def check_downgrade(self, engine):
+        share_instances_rules_table = utils.load_table(
+            'share_instance_access_map', engine)
+
+        valid_statuses = {
+            '1': 'active',
+            '2': 'error',
+            '3': 'error',
+            '4': None,
+        }
+
+        for rule in engine.execute(share_instances_rules_table.select()):
+            valid_state = valid_statuses[rule['share_instance_id']]
+            self.test_case.assertEqual(valid_state, rule['state'])

manila/tests/db/sqlalchemy/test_api.py

@@ -92,43 +92,33 @@ class ShareAccessDatabaseAPITestCase(test.TestCase):
         super(ShareAccessDatabaseAPITestCase, self).setUp()
         self.ctxt = context.get_admin_context()
 
-    @ddt.data(
-        {'statuses': (constants.STATUS_ACTIVE, constants.STATUS_ACTIVE,
-                      constants.STATUS_ACTIVE),
-         'valid': constants.STATUS_ACTIVE},
-        {'statuses': (constants.STATUS_ACTIVE, constants.STATUS_ACTIVE,
-                      constants.STATUS_NEW),
-         'valid': constants.STATUS_NEW},
-        {'statuses': (constants.STATUS_ACTIVE, constants.STATUS_ACTIVE,
-                      constants.STATUS_ERROR),
-         'valid': constants.STATUS_ERROR},
-        {'statuses': (constants.STATUS_DELETING, constants.STATUS_DELETED,
-                      constants.STATUS_ERROR),
-         'valid': constants.STATUS_ERROR},
-        {'statuses': (constants.STATUS_DELETING, constants.STATUS_DELETED,
-                      constants.STATUS_ACTIVE),
-         'valid': constants.STATUS_DELETING},
-        {'statuses': (constants.STATUS_DELETED, constants.STATUS_DELETED,
-                      constants.STATUS_DELETED),
-         'valid': constants.STATUS_DELETED},
-    )
-    @ddt.unpack
-    def test_share_access_state(self, statuses, valid):
+    def test_share_instance_update_access_status(self):
         share = db_utils.create_share()
-        db_utils.create_share_instance(share_id=share['id'])
-        db_utils.create_share_instance(share_id=share['id'])
+        share_instance = db_utils.create_share_instance(share_id=share['id'])
+        db_utils.create_access(share_id=share_instance['share_id'])
 
-        share = db_api.share_get(self.ctxt, share['id'])
-        access = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                        share_id=share['id'])
+        db_api.share_instance_update_access_status(
+            self.ctxt,
+            share_instance['id'],
+            constants.STATUS_ACTIVE
+        )
 
-        for index, mapping in enumerate(access.instance_mappings):
-            db_api.share_instance_access_update_state(
-                self.ctxt, mapping['id'], statuses[index])
+        result = db_api.share_instance_get(self.ctxt, share_instance['id'])
 
-        access = db_api.share_access_get(self.ctxt, access['id'])
+        self.assertEqual(constants.STATUS_ACTIVE,
+                         result['access_rules_status'])
 
-        self.assertEqual(valid, access.state)
+    def test_share_instance_update_access_status_invalid(self):
+        share = db_utils.create_share()
+        share_instance = db_utils.create_share_instance(share_id=share['id'])
+        db_utils.create_access(share_id=share_instance['share_id'])
+
+        self.assertRaises(
+            db_exception.DBError,
+            db_api.share_instance_update_access_status,
+            self.ctxt, share_instance['id'],
+            "fake_status"
+        )
 
 
 @ddt.ddt

manila/tests/db/sqlalchemy/test_models.py

@@ -75,3 +75,27 @@ class ShareTestCase(test.TestCase):
         share = db_utils.create_share(status=constants.STATUS_CREATING)
 
         self.assertEqual(constants.STATUS_CREATING, share.instance['status'])
+
+    def test_access_rules_status_no_instances(self):
+        share = db_utils.create_share(instances=[])
+
+        self.assertEqual(constants.STATUS_ACTIVE, share.access_rules_status)
+
+    @ddt.data(constants.STATUS_ACTIVE, constants.STATUS_OUT_OF_SYNC,
+              constants.STATUS_ERROR)
+    def test_access_rules_status(self, access_status):
+        instances = [
+            db_utils.create_share_instance(
+                share_id='fake_id', status=constants.STATUS_ERROR,
+                access_rules_status=constants.STATUS_ACTIVE),
+            db_utils.create_share_instance(
+                share_id='fake_id', status=constants.STATUS_AVAILABLE,
+                access_rules_status=constants.STATUS_ACTIVE),
+            db_utils.create_share_instance(
+                share_id='fake_id', status=constants.STATUS_AVAILABLE,
+                access_rules_status=access_status),
+        ]
+
+        share = db_utils.create_share(instances=instances)
+
+        self.assertEqual(access_status, share.access_rules_status)

manila/tests/share/test_access.py

@@ -0,0 +1,119 @@
+# Copyright 2016 Hitachi Data Systems inc.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import mock
+
+from manila.common import constants
+from manila import context
+from manila import db
+from manila import exception
+from manila.share import access
+from manila import test
+from manila.tests import db_utils
+
+
+class ShareInstanceAccessTestCase(test.TestCase):
+    def setUp(self):
+        super(ShareInstanceAccessTestCase, self).setUp()
+        self.driver = self.mock_class("manila.share.driver.ShareDriver",
+                                      mock.Mock())
+        self.share_access_helper = access.ShareInstanceAccess(db, self.driver)
+        self.context = context.get_admin_context()
+        self.share = db_utils.create_share()
+        self.share_instance = db_utils.create_share_instance(
+            share_id=self.share['id'],
+            access_rules_status=constants.STATUS_ERROR)
+
+    def test_update_access_rules(self):
+        original_rules = []
+
+        self.mock_object(db, "share_instance_get", mock.Mock(
+            return_value=self.share_instance))
+        self.mock_object(db, "share_access_get_all_for_share",
+                         mock.Mock(return_value=original_rules))
+        self.mock_object(db, "share_instance_update_access_status",
+                         mock.Mock())
+        self.mock_object(self.driver, "update_access", mock.Mock())
+
+        self.share_access_helper.update_access_rules(self.context,
+                                                     self.share_instance['id'])
+
+        self.driver.update_access.assert_called_with(
+            self.context, self.share_instance, original_rules, add_rules=[],
+            delete_rules=[], share_server=None)
+        db.share_instance_update_access_status.assert_called_with(
+            self.context, self.share_instance['id'], constants.STATUS_ACTIVE)
+
+    def test_update_access_rules_fallback(self):
+        add_rules = [db_utils.create_access(share_id=self.share['id'])]
+        delete_rules = [db_utils.create_access(share_id=self.share['id'])]
+        original_rules = [db_utils.create_access(share_id=self.share['id'])]
+
+        self.mock_object(db, "share_instance_get", mock.Mock(
+            return_value=self.share_instance))
+        self.mock_object(db, "share_access_get_all_for_share",
+                         mock.Mock(return_value=original_rules))
+        self.mock_object(db, "share_instance_update_access_status",
+                         mock.Mock())
+        self.mock_object(self.driver, "update_access",
+                         mock.Mock(side_effect=NotImplementedError))
+        self.mock_object(self.driver, "allow_access",
+                         mock.Mock())
+        self.mock_object(self.driver, "deny_access",
+                         mock.Mock())
+
+        self.share_access_helper.update_access_rules(self.context,
+                                                     self.share_instance['id'],
+                                                     add_rules, delete_rules)
+
+        self.driver.update_access.assert_called_with(
+            self.context, self.share_instance, original_rules,
+            add_rules=add_rules, delete_rules=[], share_server=None)
+        self.driver.allow_access.assert_called_with(self.context,
+                                                    self.share_instance,
+                                                    add_rules[0],
+                                                    share_server=None)
+        self.driver.deny_access.assert_called_with(self.context,
+                                                   self.share_instance,
+                                                   delete_rules[0],
+                                                   share_server=None)
+        db.share_instance_update_access_status.assert_called_with(
+            self.context, self.share_instance['id'], constants.STATUS_ACTIVE)
+
+    def test_update_access_rules_exception(self):
+        original_rules = []
+        add_rules = [db_utils.create_access(share_id=self.share['id'])]
+        delete_rules = 'all'
+
+        self.mock_object(db, "share_instance_get", mock.Mock(
+            return_value=self.share_instance))
+        self.mock_object(db, "share_access_get_all_for_instance",
+                         mock.Mock(return_value=original_rules))
+        self.mock_object(db, "share_instance_update_access_status",
+                         mock.Mock())
+        self.mock_object(self.driver, "update_access",
+                         mock.Mock(side_effect=exception.ManilaException))
+
+        self.assertRaises(exception.ManilaException,
+                          self.share_access_helper.update_access_rules,
+                          self.context, self.share_instance['id'], add_rules,
+                          delete_rules)
+
+        self.driver.update_access.assert_called_with(
+            self.context, self.share_instance, [], add_rules=add_rules,
+            delete_rules=original_rules, share_server=None)
+
+        db.share_instance_update_access_status.assert_called_with(
+            self.context, self.share_instance['id'], constants.STATUS_ERROR)

manila/tests/share/test_api.py

@@ -1293,7 +1293,7 @@ class ShareAPITestCase(test.TestCase):
         fake_access_expected = copy.deepcopy(values)
         fake_access_expected.update({
             'id': 'fake_access_id',
-            'state': 'fake_state',
+            'state': constants.STATUS_ACTIVE,
         })
         fake_access = copy.deepcopy(fake_access_expected)
         fake_access.update({
@@ -1303,6 +1303,8 @@ class ShareAPITestCase(test.TestCase):
         })
         self.mock_object(db_api, 'share_access_create',
                          mock.Mock(return_value=fake_access))
+        self.mock_object(db_api, 'share_access_get',
+                         mock.Mock(return_value=fake_access))
 
         access = self.api.allow_access(
             self.context, share, fake_access['access_type'],
@@ -1317,6 +1319,15 @@ class ShareAPITestCase(test.TestCase):
         share_api.policy.check_policy.assert_called_with(
             self.context, 'share', 'allow_access')
 
+    def test_allow_access_existent_access(self):
+        share = db_utils.create_share(status=constants.STATUS_AVAILABLE)
+        fake_access = db_utils.create_access(share_id=share['id'])
+
+        self.assertRaises(exception.ShareAccessExists, self.api.allow_access,
+                          self.context, share, fake_access['access_type'],
+                          fake_access['access_to'], fake_access['access_level']
+                          )
+
     def test_allow_access_invalid_access_level(self):
         share = db_utils.create_share(status=constants.STATUS_AVAILABLE)
         self.assertRaises(exception.InvalidShareAccess, self.api.allow_access,
@@ -1335,8 +1346,7 @@ class ShareAPITestCase(test.TestCase):
 
     def test_allow_access_to_instance(self):
         share = db_utils.create_share(host='fake')
-        access = db_utils.create_access(share_id=share['id'],
-                                        state=constants.STATUS_ACTIVE)
+        access = db_utils.create_access(share_id=share['id'])
         rpc_method = self.mock_object(self.api.share_rpcapi, 'allow_access')
 
         self.api.allow_access_to_instance(self.context, share.instance, access)
@@ -1344,25 +1354,32 @@ class ShareAPITestCase(test.TestCase):
         rpc_method.assert_called_once_with(
             self.context, share.instance, access)
 
+    def test_allow_access_to_instance_exception(self):
+        share = db_utils.create_share(host='fake')
+        access = db_utils.create_access(share_id=share['id'])
+
+        share.instance['access_rules_status'] = constants.STATUS_ERROR
+
+        self.assertRaises(exception.InvalidShareInstance,
+                          self.api.allow_access_to_instance, self.context,
+                          share.instance, access)
+
     def test_deny_access_to_instance(self):
         share = db_utils.create_share(host='fake')
-        access = db_utils.create_access(share_id=share['id'],
-                                        state=constants.STATUS_ACTIVE)
+        access = db_utils.create_access(share_id=share['id'])
         rpc_method = self.mock_object(self.api.share_rpcapi, 'deny_access')
         self.mock_object(db_api, 'share_instance_access_get',
                          mock.Mock(return_value=access.instance_mappings[0]))
-        self.mock_object(db_api, 'share_instance_access_update_state')
+        self.mock_object(db_api, 'share_instance_update_access_status')
 
         self.api.deny_access_to_instance(self.context, share.instance, access)
 
         rpc_method.assert_called_once_with(
             self.context, share.instance, access)
-        db_api.share_instance_access_get.assert_called_once_with(
-            self.context, access['id'], share.instance['id'])
-        db_api.share_instance_access_update_state.assert_called_once_with(
+        db_api.share_instance_update_access_status.assert_called_once_with(
             self.context,
-            access.instance_mappings[0]['id'],
-            constants.STATUS_DELETING
+            share.instance['id'],
+            constants.STATUS_OUT_OF_SYNC
         )
 
     @ddt.data('allow_access_to_instance', 'deny_access_to_instance')
@@ -1377,12 +1394,12 @@ class ShareAPITestCase(test.TestCase):
 
     @mock.patch.object(db_api, 'share_get', mock.Mock())
     @mock.patch.object(share_api.API, 'deny_access_to_instance', mock.Mock())
-    @mock.patch.object(db_api, 'share_instance_access_get_all', mock.Mock())
+    @mock.patch.object(db_api, 'share_instance_update_access_status',
+                       mock.Mock())
     def test_deny_access_error(self):
         share = db_utils.create_share(status=constants.STATUS_AVAILABLE)
         db_api.share_get.return_value = share
-        access = db_utils.create_access(state=constants.STATUS_ERROR,
-                                        share_id=share['id'])
+        access = db_utils.create_access(share_id=share['id'])
         share_instance = share.instances[0]
         db_api.share_instance_access_get_all.return_value = [share_instance, ]
         self.api.deny_access(self.context, share, access)
@@ -1391,8 +1408,6 @@ class ShareAPITestCase(test.TestCase):
             self.context, 'share', 'deny_access')
         share_api.API.deny_access_to_instance.assert_called_once_with(
             self.context, share_instance, access)
-        db_api.share_instance_access_get_all.assert_called_once_with(
-            self.context, access['id'])
 
     @mock.patch.object(db_api, 'share_get', mock.Mock())
     @mock.patch.object(db_api, 'share_instance_access_get_all', mock.Mock())
@@ -1400,29 +1415,24 @@ class ShareAPITestCase(test.TestCase):
     def test_deny_access_error_no_share_instance_mapping(self):
         share = db_utils.create_share(status=constants.STATUS_AVAILABLE)
         db_api.share_get.return_value = share
-        access = db_utils.create_access(state=constants.STATUS_ERROR,
-                                        share_id=share['id'])
+        access = db_utils.create_access(share_id=share['id'])
         db_api.share_instance_access_get_all.return_value = []
-        self.api.deny_access(self.context, share, access)
-        db_api.share_get.assert_called_once_with(self.context, share['id'])
-        share_api.policy.check_policy.assert_called_once_with(
-            self.context, 'share', 'deny_access')
-        db_api.share_access_delete.assert_called_once_with(
-            self.context, access['id'])
-        db_api.share_instance_access_get_all.assert_called_once_with(
-            self.context, access['id'])
 
-    @mock.patch.object(db_api, 'share_instance_access_update_state',
+        self.api.deny_access(self.context, share, access)
+
+        db_api.share_get.assert_called_once_with(self.context, share['id'])
+        self.assertTrue(share_api.policy.check_policy.called)
+
+    @mock.patch.object(db_api, 'share_instance_update_access_status',
                        mock.Mock())
     def test_deny_access_active(self):
         share = db_utils.create_share(status=constants.STATUS_AVAILABLE)
-        access = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                        share_id=share['id'])
+        access = db_utils.create_access(share_id=share['id'])
         self.api.deny_access(self.context, share, access)
-        db_api.share_instance_access_update_state.assert_called_once_with(
+        db_api.share_instance_update_access_status.assert_called_once_with(
             self.context,
-            access.instance_mappings[0]['id'],
-            constants.STATUS_DELETING
+            share.instance['id'],
+            constants.STATUS_OUT_OF_SYNC
         )
         share_api.policy.check_policy.assert_called_with(
             self.context, 'share', 'deny_access')
@@ -1431,22 +1441,13 @@ class ShareAPITestCase(test.TestCase):
 
     def test_deny_access_not_found(self):
         share = db_utils.create_share(status=constants.STATUS_AVAILABLE)
-        access = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                        share_id=share['id'])
+        access = db_utils.create_access(share_id=share['id'])
         self.mock_object(db_api, 'share_instance_access_get',
                          mock.Mock(side_effect=[exception.NotFound('fake')]))
         self.api.deny_access(self.context, share, access)
         share_api.policy.check_policy.assert_called_with(
             self.context, 'share', 'deny_access')
 
-    def test_deny_access_not_active_not_error(self):
-        share = db_utils.create_share(status=constants.STATUS_AVAILABLE)
-        access = db_utils.create_access(share_id=share['id'])
-        self.assertRaises(exception.InvalidShareAccess, self.api.deny_access,
-                          self.context, share, access)
-        share_api.policy.check_policy.assert_called_once_with(
-            self.context, 'share', 'deny_access')
-
     def test_deny_access_status_not_available(self):
         share = db_utils.create_share(status=constants.STATUS_ERROR)
         self.assertRaises(exception.InvalidShare, self.api.deny_access,
@@ -1475,13 +1476,12 @@ class ShareAPITestCase(test.TestCase):
     def test_access_get_all(self):
         share = db_utils.create_share(id='fakeid')
 
-        expected = {
+        values = {
             'fakeacc0id': {
                 'id': 'fakeacc0id',
                 'access_type': 'fakeacctype',
                 'access_to': 'fakeaccto',
                 'access_level': 'rw',
-                'state': constants.STATUS_ACTIVE,
                 'share_id': share['id'],
             },
             'fakeacc1id': {
@@ -1489,20 +1489,23 @@ class ShareAPITestCase(test.TestCase):
                 'access_type': 'fakeacctype',
                 'access_to': 'fakeaccto',
                 'access_level': 'rw',
-                'state': constants.STATUS_DELETING,
                 'share_id': share['id'],
             },
         }
         rules = [
-            db_utils.create_access(**expected['fakeacc0id']),
-            db_utils.create_access(**expected['fakeacc1id']),
+            db_utils.create_access(**values['fakeacc0id']),
+            db_utils.create_access(**values['fakeacc1id']),
         ]
 
+        # add state property
+        values['fakeacc0id']['state'] = constants.STATUS_ACTIVE
+        values['fakeacc1id']['state'] = constants.STATUS_ACTIVE
+
         self.mock_object(db_api, 'share_access_get_all_for_share',
                          mock.Mock(return_value=rules))
         actual = self.api.access_get_all(self.context, share)
         for access in actual:
-            expected_access = expected[access['id']]
+            expected_access = values[access['id']]
             expected_access.pop('share_id')
             self.assertEqual(expected_access, access)
 

manila/tests/share/test_driver.py

@@ -689,3 +689,13 @@ class ShareDriverTestCase(test.TestCase):
         self.assertRaises(exception.ShareMigrationFailed,
                           share_driver.copy_share_data, 'ctx', None,
                           fake_share, None, None, None, None, local, remote)
+
+    def test_update_access(self):
+        share_driver = driver.ShareDriver(True, configuration=None)
+        self.assertRaises(
+            NotImplementedError,
+            share_driver.update_access,
+            'ctx',
+            'fake_share',
+            'fake_access_rules'
+        )

manila/tests/share/test_manager.py

@@ -29,6 +29,7 @@ from manila import db
 from manila.db.sqlalchemy import models
 from manila import exception
 from manila import quota
+from manila.share import access as share_access
 from manila.share import drivers_private_data
 from manila.share import manager
 from manila.share import migration
@@ -203,15 +204,19 @@ class ShareManagerTestCase(test.TestCase):
                 status=constants.STATUS_AVAILABLE,
                 task_state=constants.STATUS_TASK_STATE_MIGRATION_IN_PROGRESS,
                 display_name='fake_name_4').instance,
+            db_utils.create_share(id='fake_id_5',
+                                  status=constants.STATUS_AVAILABLE,
+                                  display_name='fake_name_5').instance,
         ]
+
+        instances[4]['access_rules_status'] = constants.STATUS_OUT_OF_SYNC
+
         if not setup_access_rules:
             return instances
 
         rules = [
-            db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                   share_id='fake_id_1'),
-            db_utils.create_access(state=constants.STATUS_ERROR,
-                                   share_id='fake_id_3'),
+            db_utils.create_access(share_id='fake_id_1'),
+            db_utils.create_access(share_id='fake_id_3'),
         ]
 
         return instances, rules
@@ -231,7 +236,7 @@ class ShareManagerTestCase(test.TestCase):
                          mock.Mock(return_value=instances))
         self.mock_object(self.share_manager.db, 'share_instance_get',
                          mock.Mock(side_effect=[instances[0], instances[2],
-                                                instances[3]]))
+                                                instances[4]]))
         self.mock_object(self.share_manager.db,
                          'share_export_locations_update')
         self.mock_object(self.share_manager.driver, 'ensure_share',
@@ -244,8 +249,11 @@ class ShareManagerTestCase(test.TestCase):
         self.mock_object(self.share_manager.db,
                          'share_access_get_all_for_share',
                          mock.Mock(return_value=rules))
-        self.mock_object(self.share_manager.driver, 'allow_access',
-                         mock.Mock(side_effect=raise_share_access_exists))
+        self.mock_object(
+            self.share_manager.access_helper,
+            'update_access_rules',
+            mock.Mock(side_effect=raise_share_access_exists)
+        )
 
         # call of 'init_host' method
         self.share_manager.init_host()
@@ -277,20 +285,11 @@ class ShareManagerTestCase(test.TestCase):
             mock.call(utils.IsAMatcher(context.RequestContext), instances[2],
                       share_server=share_server),
         ])
-        self.share_manager.db.share_access_get_all_for_share.assert_has_calls([
-            mock.call(utils.IsAMatcher(context.RequestContext),
-                      instances[0]['share_id']),
-            mock.call(utils.IsAMatcher(context.RequestContext),
-                      instances[2]['share_id']),
-        ])
         self.share_manager.publish_service_capabilities.\
             assert_called_once_with(
                 utils.IsAMatcher(context.RequestContext))
-        self.share_manager.driver.allow_access.assert_has_calls([
-            mock.call(mock.ANY, instances[0], rules[0],
-                      share_server=share_server),
-            mock.call(mock.ANY, instances[2], rules[0],
-                      share_server=share_server),
+        self.share_manager.access_helper.update_access_rules.assert_has_calls([
+            mock.call(mock.ANY, instances[4]['id'], share_server=share_server),
         ])
 
     def test_init_host_with_exception_on_ensure_share(self):
@@ -351,51 +350,50 @@ class ShareManagerTestCase(test.TestCase):
             {'id': instances[1]['id'], 'status': instances[1]['status']},
         )
 
-    def test_init_host_with_exception_on_rule_access_allow(self):
+    def test_init_host_with_exception_on_update_access_rules(self):
         def raise_exception(*args, **kwargs):
             raise exception.ManilaException(message="Fake raise")
 
         instances, rules = self._setup_init_mocks()
         share_server = 'fake_share_server_type_does_not_matter'
-        self.mock_object(self.share_manager.db,
-                         'share_instances_get_all_by_host',
+        smanager = self.share_manager
+        self.mock_object(smanager.db, 'share_instances_get_all_by_host',
                          mock.Mock(return_value=instances))
         self.mock_object(self.share_manager.db, 'share_instance_get',
                          mock.Mock(side_effect=[instances[0], instances[2],
-                                                instances[3]]))
+                                                instances[4]]))
         self.mock_object(self.share_manager.driver, 'ensure_share',
                          mock.Mock(return_value=None))
-        self.mock_object(self.share_manager, '_ensure_share_instance_has_pool')
-        self.mock_object(self.share_manager, '_get_share_server',
+        self.mock_object(smanager, '_ensure_share_instance_has_pool')
+        self.mock_object(smanager, '_get_share_server',
                          mock.Mock(return_value=share_server))
-        self.mock_object(self.share_manager, 'publish_service_capabilities')
+        self.mock_object(smanager, 'publish_service_capabilities')
         self.mock_object(manager.LOG, 'error')
         self.mock_object(manager.LOG, 'info')
-        self.mock_object(self.share_manager.db,
-                         'share_access_get_all_for_share',
+        self.mock_object(smanager.db, 'share_access_get_all_for_share',
                          mock.Mock(return_value=rules))
-        self.mock_object(self.share_manager.driver, 'allow_access',
+        self.mock_object(smanager.access_helper, 'update_access_rules',
                          mock.Mock(side_effect=raise_exception))
 
         # call of 'init_host' method
-        self.share_manager.init_host()
+        smanager.init_host()
 
         # verification of call
-        self.share_manager.db.share_instances_get_all_by_host.\
+        smanager.db.share_instances_get_all_by_host.\
             assert_called_once_with(utils.IsAMatcher(context.RequestContext),
-                                    self.share_manager.host)
-        self.share_manager.driver.do_setup.assert_called_once_with(
+                                    smanager.host)
+        smanager.driver.do_setup.assert_called_once_with(
             utils.IsAMatcher(context.RequestContext))
-        self.share_manager.driver.check_for_setup_error.assert_called_with()
-        self.share_manager._ensure_share_instance_has_pool.assert_has_calls([
+        smanager.driver.check_for_setup_error.assert_called_with()
+        smanager._ensure_share_instance_has_pool.assert_has_calls([
             mock.call(utils.IsAMatcher(context.RequestContext), instances[0]),
             mock.call(utils.IsAMatcher(context.RequestContext), instances[2]),
         ])
-        self.share_manager._get_share_server.assert_has_calls([
+        smanager._get_share_server.assert_has_calls([
             mock.call(utils.IsAMatcher(context.RequestContext), instances[0]),
             mock.call(utils.IsAMatcher(context.RequestContext), instances[2]),
         ])
-        self.share_manager.driver.ensure_share.assert_has_calls([
+        smanager.driver.ensure_share.assert_has_calls([
             mock.call(utils.IsAMatcher(context.RequestContext), instances[0],
                       share_server=share_server),
             mock.call(utils.IsAMatcher(context.RequestContext), instances[2],
@@ -413,15 +411,12 @@ class ShareManagerTestCase(test.TestCase):
             mock.ANY,
             {'id': instances[1]['id'], 'status': instances[1]['status']},
         )
-        self.share_manager.driver.allow_access.assert_has_calls([
-            mock.call(utils.IsAMatcher(context.RequestContext), instances[0],
-                      rules[0], share_server=share_server),
-            mock.call(utils.IsAMatcher(context.RequestContext), instances[2],
-                      rules[0], share_server=share_server),
+        smanager.access_helper.update_access_rules.assert_has_calls([
+            mock.call(utils.IsAMatcher(context.RequestContext),
+                      instances[4]['id'], share_server=share_server),
         ])
         manager.LOG.error.assert_has_calls([
             mock.call(mock.ANY, mock.ANY),
-            mock.call(mock.ANY, mock.ANY),
         ])
 
     def test_create_share_instance_from_snapshot_with_server(self):
@@ -1181,8 +1176,11 @@ class ShareManagerTestCase(test.TestCase):
         manager.CONF.unmanage_remove_access_rules = True
         self._setup_unmanage_mocks(mock_driver=False,
                                    mock_unmanage=mock.Mock())
-        self.mock_object(self.share_manager, '_remove_share_access_rules',
-                         mock.Mock(side_effect=Exception()))
+        self.mock_object(
+            self.share_manager.access_helper,
+            'update_access_rules',
+            mock.Mock(side_effect=Exception())
+        )
         self.mock_object(quota.QUOTAS, 'reserve', mock.Mock(return_value=[]))
         share = db_utils.create_share()
 
@@ -1196,37 +1194,22 @@ class ShareManagerTestCase(test.TestCase):
         manager.CONF.unmanage_remove_access_rules = True
         self._setup_unmanage_mocks(mock_driver=False,
                                    mock_unmanage=mock.Mock())
-        self.mock_object(self.share_manager, '_remove_share_access_rules')
+        smanager = self.share_manager
+        self.mock_object(smanager.access_helper, 'update_access_rules')
         self.mock_object(quota.QUOTAS, 'reserve', mock.Mock(return_value=[]))
         share = db_utils.create_share()
         share_id = share['id']
         share_instance_id = share.instance['id']
 
-        self.share_manager.unmanage_share(self.context, share_id)
+        smanager.unmanage_share(self.context, share_id)
 
-        self.share_manager.driver.unmanage.\
-            assert_called_once_with(mock.ANY)
-        self.share_manager._remove_share_access_rules.assert_called_once_with(
-            mock.ANY, mock.ANY, mock.ANY, mock.ANY
+        smanager.driver.unmanage.assert_called_once_with(mock.ANY)
+        smanager.access_helper.update_access_rules.assert_called_once_with(
+            mock.ANY, mock.ANY, delete_rules='all', share_server=None
         )
-        self.share_manager.db.share_instance_delete.assert_called_once_with(
+        smanager.db.share_instance_delete.assert_called_once_with(
             mock.ANY, share_instance_id)
 
-    def test_remove_share_access_rules(self):
-        self.mock_object(self.share_manager.db,
-                         'share_access_get_all_for_share',
-                         mock.Mock(return_value=['fake_ref', 'fake_ref2']))
-        self.mock_object(self.share_manager, '_deny_access')
-        share_ref = db_utils.create_share()
-        share_server = 'fake'
-
-        self.share_manager._remove_share_access_rules(
-            self.context, share_ref, share_ref.instance, share_server)
-
-        self.share_manager.db.share_access_get_all_for_share.\
-            assert_called_once_with(mock.ANY, share_ref['id'])
-        self.assertEqual(2, self.share_manager._deny_access.call_count)
-
     def test_delete_share_instance_share_server_not_found(self):
         share_net = db_utils.create_share_network()
         share = db_utils.create_share(share_network_id=share_net['id'],
@@ -1322,28 +1305,27 @@ class ShareManagerTestCase(test.TestCase):
 
     def test_allow_deny_access(self):
         """Test access rules to share can be created and deleted."""
-        self.mock_object(manager.LOG, 'info')
+        self.mock_object(share_access.LOG, 'info')
 
         share = db_utils.create_share()
         share_id = share['id']
         access = db_utils.create_access(share_id=share_id)
         access_id = access['id']
         self.share_manager.allow_access(self.context, share.instance['id'],
-                                        access_id)
-        self.assertEqual('active', db.share_access_get(self.context,
-                                                       access_id).state)
+                                        [access_id])
+        self.assertEqual('active', db.share_instance_get(
+            self.context, share.instance['id']).access_rules_status)
 
-        exp_args = {'access_level': access['access_level'],
-                    'share_instance_id': share.instance['id'],
-                    'access_to': access['access_to']}
-        manager.LOG.info.assert_called_with(mock.ANY, exp_args)
-        manager.LOG.info.reset_mock()
+        share_access.LOG.info.assert_called_with(mock.ANY,
+                                                 share.instance['id'])
+        share_access.LOG.info.reset_mock()
 
         self.share_manager.deny_access(self.context, share.instance['id'],
-                                       access_id)
-        exp_args = {'share_instance_id': share.instance['id'],
-                    'access_to': access['access_to']}
-        manager.LOG.info.assert_called_with(mock.ANY, exp_args)
+                                       [access_id])
+
+        share_access.LOG.info.assert_called_with(mock.ANY,
+                                                 share.instance['id'])
+        share_access.LOG.info.reset_mock()
 
     def test_allow_deny_access_error(self):
         """Test access rules to share can be created and deleted with error."""
@@ -1354,33 +1336,26 @@ class ShareManagerTestCase(test.TestCase):
         def _fake_deny_access(self, *args, **kwargs):
             raise exception.NotFound()
 
-        self.mock_object(self.share_manager.driver, "allow_access",
-                         _fake_allow_access)
-        self.mock_object(self.share_manager.driver, "deny_access",
-                         _fake_deny_access)
+        self.mock_object(self.share_manager.access_helper.driver,
+                         "allow_access", _fake_allow_access)
+        self.mock_object(self.share_manager.access_helper.driver,
+                         "deny_access", _fake_deny_access)
 
         share = db_utils.create_share()
         share_id = share['id']
         access = db_utils.create_access(share_id=share_id)
         access_id = access['id']
 
-        self.assertRaises(exception.NotFound,
-                          self.share_manager.allow_access,
-                          self.context,
-                          share.instance['id'],
-                          access_id)
+        def validate(method):
+            self.assertRaises(exception.ManilaException, method, self.context,
+                              share.instance['id'], [access_id])
 
-        acs = db.share_access_get(self.context, access_id)
-        self.assertEqual(constants.STATUS_ERROR, acs['state'])
+            inst = db.share_instance_get(self.context, share.instance['id'])
+            self.assertEqual(constants.STATUS_ERROR,
+                             inst['access_rules_status'])
 
-        self.assertRaises(exception.NotFound,
-                          self.share_manager.deny_access,
-                          self.context,
-                          share.instance['id'],
-                          access_id)
-
-        acs = db.share_access_get(self.context, access_id)
-        self.assertEqual(constants.STATUS_ERROR, acs['state'])
+        validate(self.share_manager.allow_access)
+        validate(self.share_manager.deny_access)
 
     def test_setup_server(self):
         # Setup required test data

manila/tests/share/test_migration.py

@@ -44,100 +44,44 @@ class ShareMigrationHelperTestCase(test.TestCase):
             driver.CONF.migration_wait_access_rules_timeout, self.share)
 
     def test_deny_rules_and_wait(self):
-        saved_rules = [db_utils.create_access(share_id=self.share['id'],
-                       state=constants.STATUS_ACTIVE)]
+        saved_rules = [db_utils.create_access(share_id=self.share['id'])]
+
+        fake_share_instances = [
+            {"id": "1", "access_rules_status": constants.STATUS_OUT_OF_SYNC},
+            {"id": "1", "access_rules_status": constants.STATUS_ACTIVE},
+        ]
 
         self.mock_object(share_api.API, 'deny_access_to_instance')
-        self.mock_object(db, 'share_access_get_all_for_share',
-                         mock.Mock(side_effect=[saved_rules, []]))
+        self.mock_object(db, 'share_instance_get',
+                         mock.Mock(side_effect=fake_share_instances))
         self.mock_object(time, 'sleep')
 
         self.helper.deny_rules_and_wait(
             self.context, self.share, saved_rules)
 
-        db.share_access_get_all_for_share.assert_any_call(
-            self.context, self.share['id'])
-
-    def test_deny_rules_and_wait_timeout(self):
-
-        saved_rules = [db_utils.create_access(share_id=self.share['id'],
-                       state=constants.STATUS_ACTIVE)]
-
-        self.mock_object(share_api.API, 'deny_access_to_instance')
-        self.mock_object(db, 'share_access_get_all_for_share',
-                         mock.Mock(return_value=saved_rules))
-        self.mock_object(time, 'sleep')
-
-        now = time.time()
-        timeout = now + 100
-
-        self.mock_object(time, 'time',
-                         mock.Mock(side_effect=[now, timeout]))
-
-        self.assertRaises(exception.ShareMigrationFailed,
-                          self.helper.deny_rules_and_wait,
-                          self.context, self.share, saved_rules)
-
-        db.share_access_get_all_for_share.assert_called_once_with(
-            self.context, self.share['id'])
+        db.share_instance_get.assert_any_call(
+            self.context, self.share.instance['id'])
 
     def test_add_rules_and_wait(self):
 
-        rules_active = [db_utils.create_access(share_id=self.share['id'],
-                        state=constants.STATUS_ACTIVE)]
-        rules_new = [db_utils.create_access(share_id=self.share['id'],
-                     state=constants.STATUS_NEW)]
+        fake_access_rules = [
+            {'access_type': 'fake', 'access_level': 'ro', 'access_to': 'fake'},
+            {'access_type': 'f0ke', 'access_level': 'rw', 'access_to': 'f0ke'},
+        ]
 
-        self.mock_object(share_api.API, 'allow_access')
-        self.mock_object(db, 'share_access_get_all_for_share',
-                         mock.Mock(side_effect=[rules_new,
-                                                rules_active]))
-        self.mock_object(time, 'sleep')
+        self.mock_object(share_api.API, 'allow_access_to_instance')
+        self.mock_object(self.helper, 'wait_for_access_update')
+        self.mock_object(db, 'share_access_create')
 
         self.helper.add_rules_and_wait(self.context, self.share,
-                                       rules_active)
+                                       fake_access_rules)
 
-        db.share_access_get_all_for_share.assert_any_call(
-            self.context, self.share['id'])
-
-    def test_add_rules_and_wait_access_level(self):
-
-        rules_active = [db_utils.create_access(share_id=self.share['id'],
-                        state=constants.STATUS_ACTIVE)]
-
-        self.mock_object(share_api.API, 'allow_access')
-        self.mock_object(db, 'share_access_get_all_for_share',
-                         mock.Mock(return_value=rules_active))
-        self.mock_object(time, 'sleep')
-
-        self.helper.add_rules_and_wait(self.context, self.share,
-                                       rules_active, 'access_level')
-
-        db.share_access_get_all_for_share.assert_any_call(
-            self.context, self.share['id'])
-
-    def test_add_rules_and_wait_timeout(self):
-
-        rules_new = [db_utils.create_access(share_id=self.share['id'],
-                     state=constants.STATUS_NEW)]
-
-        self.mock_object(share_api.API, 'allow_access')
-        self.mock_object(db, 'share_access_get_all_for_share',
-                         mock.Mock(return_value=rules_new))
-        self.mock_object(time, 'sleep')
-
-        now = time.time()
-        timeout = now + 100
-
-        self.mock_object(time, 'time',
-                         mock.Mock(side_effect=[now, timeout]))
-
-        self.assertRaises(exception.ShareMigrationFailed,
-                          self.helper.add_rules_and_wait, self.context,
-                          self.share, rules_new)
-
-        db.share_access_get_all_for_share.assert_called_once_with(
-            self.context, self.share['id'])
+        share_api.API.allow_access_to_instance.assert_called_once_with(
+            self.context, self.share.instance, mock.ANY
+        )
+        self.helper.wait_for_access_update.assert_called_once_with(
+            self.share.instance
+        )
 
     def test_delete_instance_and_wait(self):
 
@@ -258,29 +202,39 @@ class ShareMigrationHelperTestCase(test.TestCase):
         db.share_instance_get.assert_called_once_with(
             self.context, share_instance_creating['id'], with_share_data=True)
 
-    def test_wait_for_allow_access(self):
-
-        access_active = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                               share_id=self.share['id'])
-        access_new = db_utils.create_access(state=constants.STATUS_NEW,
-                                            share_id=self.share['id'])
+    def test_wait_for_access_update(self):
+        sid = 1
+        fake_share_instances = [
+            {'id': sid, 'access_rules_status': constants.STATUS_OUT_OF_SYNC},
+            {'id': sid, 'access_rules_status': constants.STATUS_ACTIVE},
+        ]
 
         self.mock_object(time, 'sleep')
+        self.mock_object(db, 'share_instance_get',
+                         mock.Mock(side_effect=fake_share_instances))
 
-        self.mock_object(self.helper.api, 'access_get',
-                         mock.Mock(side_effect=[access_new, access_active]))
+        self.helper.wait_for_access_update(fake_share_instances[0])
 
-        result = self.helper.wait_for_allow_access(access_new)
+        db.share_instance_get.assert_has_calls(
+            [mock.call(mock.ANY, sid), mock.call(mock.ANY, sid)]
+        )
+        time.sleep.assert_called_once_with(1)
 
-        self.assertEqual(access_active, result)
-
-    def test_wait_for_allow_access_timeout(self):
-
-        access_new = db_utils.create_access(state=constants.STATUS_NEW,
-                                            share_id=self.share['id'])
-
-        self.mock_object(self.helper.api, 'access_get',
-                         mock.Mock(return_value=access_new))
+    @ddt.data(
+        (
+            {'id': '1', 'access_rules_status': constants.STATUS_ERROR},
+            exception.ShareMigrationFailed
+        ),
+        (
+            {'id': '1', 'access_rules_status': constants.STATUS_OUT_OF_SYNC},
+            exception.ShareMigrationFailed
+        ),
+    )
+    @ddt.unpack
+    def test_wait_for_access_update_invalid(self, fake_instance, expected_exc):
+        self.mock_object(time, 'sleep')
+        self.mock_object(db, 'share_instance_get',
+                         mock.Mock(return_value=fake_instance))
 
         now = time.time()
         timeout = now + 100
@@ -288,59 +242,16 @@ class ShareMigrationHelperTestCase(test.TestCase):
         self.mock_object(time, 'time',
                          mock.Mock(side_effect=[now, timeout]))
 
-        self.assertRaises(exception.ShareMigrationFailed,
-                          self.helper.wait_for_allow_access, access_new)
-
-    def test_wait_for_allow_access_error(self):
-
-        access_new = db_utils.create_access(state=constants.STATUS_NEW,
-                                            share_id=self.share['id'])
-        access_error = db_utils.create_access(state=constants.STATUS_ERROR,
-                                              share_id=self.share['id'])
-
-        self.mock_object(self.helper.api, 'access_get',
-                         mock.Mock(return_value=access_error))
-
-        self.assertRaises(exception.ShareMigrationFailed,
-                          self.helper.wait_for_allow_access, access_new)
-
-    def test_wait_for_deny_access(self):
-
-        access_active = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                               share_id=self.share['id'])
-
-        self.mock_object(self.helper.api, 'access_get',
-                         mock.Mock(side_effect=[[access_active],
-                                                exception.NotFound]))
-
-        self.helper.wait_for_deny_access(access_active)
-
-    def test_wait_for_deny_access_timeout(self):
-
-        access_active = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                               share_id=self.share['id'])
-
-        self.mock_object(self.helper.api, 'access_get',
-                         mock.Mock(side_effect=[[access_active],
-                                                [access_active]]))
-
-        now = time.time()
-        timeout = now + 100
-
-        self.mock_object(time, 'time',
-                         mock.Mock(side_effect=[now, timeout]))
-
-        self.assertRaises(exception.ShareMigrationFailed,
-                          self.helper.wait_for_deny_access, access_active)
+        self.assertRaises(expected_exc,
+                          self.helper.wait_for_access_update, fake_instance)
 
     def test_allow_migration_access(self):
         access = {'access_to': 'fake_ip',
                   'access_type': 'fake_type'}
 
-        access_active = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                               share_id=self.share['id'])
+        access_active = db_utils.create_access(share_id=self.share['id'])
 
-        self.mock_object(self.helper, 'wait_for_allow_access',
+        self.mock_object(self.helper, 'wait_for_access_update',
                          mock.Mock(return_value=access_active))
 
         self.mock_object(self.helper.api, 'allow_access',
@@ -350,15 +261,14 @@ class ShareMigrationHelperTestCase(test.TestCase):
 
         self.assertEqual(access_active, result)
 
-        self.helper.wait_for_allow_access.assert_called_once_with(
-            access_active)
+        self.helper.wait_for_access_update.assert_called_once_with(
+            self.share.instance)
 
     def test_allow_migration_access_exists(self):
         access = {'access_to': 'fake_ip',
                   'access_type': 'fake_type'}
 
-        access_active = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                               share_id=self.share['id'],
+        access_active = db_utils.create_access(share_id=self.share['id'],
                                                access_to='fake_ip')
 
         self.mock_object(
@@ -377,8 +287,7 @@ class ShareMigrationHelperTestCase(test.TestCase):
         access = {'access_to': 'fake_ip',
                   'access_type': 'fake_type'}
 
-        access_active = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                               share_id=self.share['id'],
+        access_active = db_utils.create_access(share_id=self.share['id'],
                                                access_to='fake_ip')
 
         self.mock_object(self.helper.api, 'access_get',
@@ -386,19 +295,20 @@ class ShareMigrationHelperTestCase(test.TestCase):
 
         self.mock_object(self.helper.api, 'deny_access')
 
-        self.mock_object(self.helper, 'wait_for_deny_access')
+        self.mock_object(self.helper, 'wait_for_access_update')
 
         self.helper.deny_migration_access(access_active, access)
 
-        self.helper.wait_for_deny_access.assert_called_once_with(access_active)
+        self.helper.wait_for_access_update.assert_called_once_with(
+            self.share.instance
+        )
 
     def test_deny_migration_access_not_found(self):
 
         access = {'access_to': 'fake_ip',
                   'access_type': 'fake_type'}
 
-        access_active = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                               share_id=self.share['id'],
+        access_active = db_utils.create_access(share_id=self.share['id'],
                                                access_to='fake_ip')
 
         self.mock_object(self.helper.api, 'access_get',
@@ -411,8 +321,7 @@ class ShareMigrationHelperTestCase(test.TestCase):
         access = {'access_to': 'fake_ip',
                   'access_type': 'fake_type'}
 
-        access_active = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                               share_id=self.share['id'],
+        access_active = db_utils.create_access(share_id=self.share['id'],
                                                access_to='fake_ip')
 
         self.mock_object(self.helper.api, 'access_get_all',
@@ -420,19 +329,19 @@ class ShareMigrationHelperTestCase(test.TestCase):
 
         self.mock_object(self.helper.api, 'deny_access')
 
-        self.mock_object(self.helper, 'wait_for_deny_access')
+        self.mock_object(self.helper, 'wait_for_access_update')
 
         self.helper.deny_migration_access(None, access)
 
-        self.helper.wait_for_deny_access.assert_called_once_with(access_active)
+        self.helper.wait_for_access_update.assert_called_once_with(
+            self.share.instance)
 
     def test_deny_migration_access_exception(self):
 
         access = {'access_to': 'fake_ip',
                   'access_type': 'fake_type'}
 
-        access_active = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                               share_id=self.share['id'],
+        access_active = db_utils.create_access(share_id=self.share['id'],
                                                access_to='fake_ip')
 
         self.mock_object(self.helper.api, 'access_get',
@@ -468,8 +377,7 @@ class ShareMigrationHelperTestCase(test.TestCase):
 
     def test_change_to_read_only(self):
 
-        access_active = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                               share_id=self.share['id'],
+        access_active = db_utils.create_access(share_id=self.share['id'],
                                                access_to='fake_ip')
 
         self.mock_object(db, 'share_access_get_all_for_share',
@@ -492,8 +400,7 @@ class ShareMigrationHelperTestCase(test.TestCase):
 
     def test_revert_access_rules(self):
 
-        access_active = db_utils.create_access(state=constants.STATUS_ACTIVE,
-                                               share_id=self.share['id'],
+        access_active = db_utils.create_access(share_id=self.share['id'],
                                                access_to='fake_ip')
 
         self.mock_object(db, 'share_access_get_all_for_share',

manila/tests/share/test_rpcapi.py

@@ -83,7 +83,7 @@ class ShareRpcAPITestCase(test.TestCase):
         if 'access' in expected_msg:
             access = expected_msg['access']
             del expected_msg['access']
-            expected_msg['access_id'] = access['id']
+            expected_msg['access_rules'] = [access['id']]
         if 'host' in expected_msg:
             del expected_msg['host']
         if 'snapshot' in expected_msg:
@@ -153,14 +153,14 @@ class ShareRpcAPITestCase(test.TestCase):
     def test_allow_access(self):
         self._test_share_api('allow_access',
                              rpc_method='cast',
-                             version='1.4',
+                             version='1.7',
                              share_instance=self.fake_share,
                              access=self.fake_access)
 
     def test_deny_access(self):
         self._test_share_api('deny_access',
                              rpc_method='cast',
-                             version='1.4',
+                             version='1.7',
                              share_instance=self.fake_share,
                              access=self.fake_access)
 

manila_tempest_tests/config.py

@@ -36,7 +36,7 @@ ShareGroup = [
                help="The minimum api microversion is configured to be the "
                     "value of the minimum microversion supported by Manila."),
     cfg.StrOpt("max_api_microversion",
-               default="2.9",
+               default="2.10",
                help="The maximum api microversion is configured to be the "
                     "value of the latest microversion supported by Manila."),
     cfg.StrOpt("region",

manila_tempest_tests/services/share/v2/json/shares_client.py

@@ -323,6 +323,30 @@ class SharesV2Client(shares_client.SharesClient):
                            (instance_id, status, self.build_timeout))
                 raise exceptions.TimeoutException(message)
 
+    def wait_for_share_status(self, share_id, status, status_attr='status',
+                              version=LATEST_MICROVERSION):
+        """Waits for a share to reach a given status."""
+        body = self.get_share(share_id, version=version)
+        share_status = body[status_attr]
+        start = int(time.time())
+
+        while share_status != status:
+            time.sleep(self.build_interval)
+            body = self.get_share(share_id, version=version)
+            share_status = body[status_attr]
+            if share_status == status:
+                return
+            elif 'error' in share_status.lower():
+                raise share_exceptions.ShareBuildErrorException(
+                    share_id=share_id)
+
+            if int(time.time()) - start >= self.build_timeout:
+                message = ("Share's %(status_attr)s failed to transition to "
+                           "%(status)s within the required time %(seconds)s." %
+                           {"status_attr": status_attr, "status": status,
+                            "seconds": self.build_timeout})
+                raise exceptions.TimeoutException(message)
+
 ###############
 
     def extend_share(self, share_id, new_size, version=LATEST_MICROVERSION,

manila_tempest_tests/tests/api/admin/test_share_instances.py

@@ -64,6 +64,7 @@ class ShareInstancesTest(base.BaseSharesAdminTest):
         share_instances = self.shares_v2_client.get_instances_of_share(
             self.share['id'], version=version,
         )
+
         si = self.shares_v2_client.get_share_instance(
             share_instances[0]['id'], version=version)
 
@@ -73,6 +74,8 @@ class ShareInstancesTest(base.BaseSharesAdminTest):
         ]
         if utils.is_microversion_lt(version, '2.9'):
             expected_keys.extend(["export_location", "export_locations"])
+        if utils.is_microversion_ge(version, '2.10'):
+            expected_keys.append("access_rules_status")
         expected_keys = sorted(expected_keys)
         actual_keys = sorted(si.keys())
         self.assertEqual(expected_keys, actual_keys,
@@ -87,3 +90,7 @@ class ShareInstancesTest(base.BaseSharesAdminTest):
     @test.attr(type=["gate", ])
     def test_get_share_instance_v2_9(self):
         self._get_share_instance('2.9')
+
+    @test.attr(type=["gate", ])
+    def test_get_share_instance_v2_10(self):
+        self._get_share_instance('2.10')

manila_tempest_tests/tests/api/test_rules.py

@@ -14,32 +14,53 @@
 #    under the License.
 
 import ddt
-from tempest import config  # noqa
-from tempest import test  # noqa
-from tempest_lib import exceptions as lib_exc  # noqa
-import testtools  # noqa
+from tempest import config
+from tempest import test
+from tempest_lib import exceptions as lib_exc
+import testtools
 
 from manila_tempest_tests.tests.api import base
+from manila_tempest_tests import utils
 
 CONF = config.CONF
+LATEST_MICROVERSION = CONF.share.max_api_microversion
 
 
-def _create_delete_ro_access_rule(self, client_name):
+def _create_delete_ro_access_rule(self, version):
     """Common test case for usage in test suites with different decorators.
 
     :param self: instance of test class
     """
-    rule = getattr(self, client_name).create_access_rule(
-        self.share["id"], self.access_type, self.access_to, 'ro')
+
+    if utils.is_microversion_eq(version, '1.0'):
+        rule = self.shares_client.create_access_rule(
+            self.share["id"], self.access_type, self.access_to, 'ro')
+    else:
+        rule = self.shares_v2_client.create_access_rule(
+            self.share["id"], self.access_type, self.access_to, 'ro',
+            version=version)
 
     self.assertEqual('ro', rule['access_level'])
     for key in ('deleted', 'deleted_at', 'instance_mappings'):
         self.assertNotIn(key, rule.keys())
-    getattr(self, client_name).wait_for_access_rule_status(
-        self.share["id"], rule["id"], "active")
-    getattr(self, client_name).delete_access_rule(self.share["id"], rule["id"])
-    getattr(self, client_name).wait_for_resource_deletion(
-        rule_id=rule["id"], share_id=self.share['id'])
+
+    if utils.is_microversion_le(version, '2.9'):
+        self.shares_client.wait_for_access_rule_status(
+            self.share["id"], rule["id"], "active")
+    else:
+        self.shares_v2_client.wait_for_share_status(
+            self.share["id"], "active", status_attr='access_rules_status',
+            version=version)
+
+    if utils.is_microversion_eq(version, '1.0'):
+        self.shares_client.delete_access_rule(self.share["id"], rule["id"])
+        self.shares_client.wait_for_resource_deletion(
+            rule_id=rule["id"], share_id=self.share['id'])
+    else:
+        self.shares_v2_client.delete_access_rule(
+            self.share["id"], rule["id"], version=version)
+        self.shares_v2_client.wait_for_resource_deletion(
+            rule_id=rule["id"], share_id=self.share['id'], version=version)
 
 
 @ddt.ddt
@@ -58,56 +79,94 @@ class ShareIpRulesForNFSTest(base.BaseSharesTest):
         cls.access_to = "2.2.2.2"
 
     @test.attr(type=["gate", ])
-    @ddt.data('shares_client', 'shares_v2_client')
-    def test_create_delete_access_rules_with_one_ip(self, client_name):
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
+    def test_create_delete_access_rules_with_one_ip(self, version):
 
         # test data
         access_to = "1.1.1.1"
 
         # create rule
-        rule = getattr(self, client_name).create_access_rule(
-            self.share["id"], self.access_type, access_to)
+        if utils.is_microversion_eq(version, '1.0'):
+            rule = self.shares_client.create_access_rule(
+                self.share["id"], self.access_type, access_to)
+        else:
+            rule = self.shares_v2_client.create_access_rule(
+                self.share["id"], self.access_type, access_to,
+                version=version)
 
         self.assertEqual('rw', rule['access_level'])
         for key in ('deleted', 'deleted_at', 'instance_mappings'):
             self.assertNotIn(key, rule.keys())
-        getattr(self, client_name).wait_for_access_rule_status(
-            self.share["id"], rule["id"], "active")
+
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        elif utils.is_microversion_eq(version, '2.9'):
+            self.shares_v2_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        else:
+            self.shares_v2_client.wait_for_share_status(
+                self.share["id"], "active", status_attr='access_rules_status',
+                version=version)
 
         # delete rule and wait for deletion
-        getattr(self, client_name).delete_access_rule(self.share["id"],
-                                                      rule["id"])
-        getattr(self, client_name).wait_for_resource_deletion(
-            rule_id=rule["id"], share_id=self.share['id'])
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.delete_access_rule(self.share["id"], rule["id"])
+            self.shares_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'])
+        else:
+            self.shares_v2_client.delete_access_rule(
+                self.share["id"], rule["id"], version=version)
+            self.shares_v2_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'], version=version)
 
     @test.attr(type=["gate", ])
-    @ddt.data('shares_client', 'shares_v2_client')
-    def test_create_delete_access_rule_with_cidr(self, client_name):
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
+    def test_create_delete_access_rule_with_cidr(self, version):
 
         # test data
         access_to = "1.2.3.4/32"
 
         # create rule
-        rule = getattr(self, client_name).create_access_rule(
-            self.share["id"], self.access_type, access_to)
+        if utils.is_microversion_eq(version, '1.0'):
+            rule = self.shares_client.create_access_rule(
+                self.share["id"], self.access_type, access_to)
+        else:
+            rule = self.shares_v2_client.create_access_rule(
+                self.share["id"], self.access_type, access_to,
+                version=version)
 
         for key in ('deleted', 'deleted_at', 'instance_mappings'):
             self.assertNotIn(key, rule.keys())
         self.assertEqual('rw', rule['access_level'])
-        getattr(self, client_name).wait_for_access_rule_status(
-            self.share["id"], rule["id"], "active")
+
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        elif utils.is_microversion_eq(version, '2.9'):
+            self.shares_v2_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        else:
+            self.shares_v2_client.wait_for_share_status(
+                self.share["id"], "active", status_attr='access_rules_status',
+                version=version)
 
         # delete rule and wait for deletion
-        getattr(self, client_name).delete_access_rule(self.share["id"],
-                                                      rule["id"])
-        getattr(self, client_name).wait_for_resource_deletion(
-            rule_id=rule["id"], share_id=self.share['id'])
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.delete_access_rule(self.share["id"], rule["id"])
+            self.shares_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'])
+        else:
+            self.shares_v2_client.delete_access_rule(
+                self.share["id"], rule["id"], version=version)
+            self.shares_v2_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'], version=version)
 
     @test.attr(type=["gate", ])
     @testtools.skipIf(
         "nfs" not in CONF.share.enable_ro_access_level_for_protocols,
         "RO access rule tests are disabled for NFS protocol.")
-    @ddt.data('shares_client', 'shares_v2_client')
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
     def test_create_delete_ro_access_rule(self, client_name):
         _create_delete_ro_access_rule(self, client_name)
 
@@ -120,9 +179,9 @@ class ShareIpRulesForCIFSTest(ShareIpRulesForNFSTest):
     @testtools.skipIf(
         "cifs" not in CONF.share.enable_ro_access_level_for_protocols,
         "RO access rule tests are disabled for CIFS protocol.")
-    @ddt.data('shares_client', 'shares_v2_client')
-    def test_create_delete_ro_access_rule(self, client_name):
-        _create_delete_ro_access_rule(self, client_name)
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
+    def test_create_delete_ro_access_rule(self, version):
+        _create_delete_ro_access_rule(self, version)
 
 
 @ddt.ddt
@@ -142,32 +201,51 @@ class ShareUserRulesForNFSTest(base.BaseSharesTest):
         cls.access_to = CONF.share.username_for_user_rules
 
     @test.attr(type=["gate", ])
-    @ddt.data('shares_client', 'shares_v2_client')
-    def test_create_delete_user_rule(self, client_name):
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
+    def test_create_delete_user_rule(self, version):
 
         # create rule
-        rule = getattr(self, client_name).create_access_rule(
-            self.share["id"], self.access_type, self.access_to)
+        if utils.is_microversion_eq(version, '1.0'):
+            rule = self.shares_client.create_access_rule(
+                self.share["id"], self.access_type, self.access_to)
+        else:
+            rule = self.shares_v2_client.create_access_rule(
+                self.share["id"], self.access_type, self.access_to,
+                version=version)
 
         self.assertEqual('rw', rule['access_level'])
         for key in ('deleted', 'deleted_at', 'instance_mappings'):
             self.assertNotIn(key, rule.keys())
-        getattr(self, client_name).wait_for_access_rule_status(
-            self.share["id"], rule["id"], "active")
+
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        elif utils.is_microversion_eq(version, '2.9'):
+            self.shares_v2_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        else:
+            self.shares_v2_client.wait_for_share_status(
+                self.share["id"], "active", status_attr='access_rules_status',
+                version=version)
 
         # delete rule and wait for deletion
-        getattr(self, client_name).delete_access_rule(self.share["id"],
-                                                      rule["id"])
-        getattr(self, client_name).wait_for_resource_deletion(
-            rule_id=rule["id"], share_id=self.share['id'])
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.delete_access_rule(self.share["id"], rule["id"])
+            self.shares_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'])
+        else:
+            self.shares_v2_client.delete_access_rule(
+                self.share["id"], rule["id"], version=version)
+            self.shares_v2_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'], version=version)
 
     @test.attr(type=["gate", ])
     @testtools.skipIf(
         "nfs" not in CONF.share.enable_ro_access_level_for_protocols,
         "RO access rule tests are disabled for NFS protocol.")
-    @ddt.data('shares_client', 'shares_v2_client')
-    def test_create_delete_ro_access_rule(self, client_name):
-        _create_delete_ro_access_rule(self, client_name)
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
+    def test_create_delete_ro_access_rule(self, version):
+        _create_delete_ro_access_rule(self, version)
 
 
 @ddt.ddt
@@ -178,9 +256,9 @@ class ShareUserRulesForCIFSTest(ShareUserRulesForNFSTest):
     @testtools.skipIf(
         "cifs" not in CONF.share.enable_ro_access_level_for_protocols,
         "RO access rule tests are disabled for CIFS protocol.")
-    @ddt.data('shares_client', 'shares_v2_client')
-    def test_create_delete_ro_access_rule(self, client_name):
-        _create_delete_ro_access_rule(self, client_name)
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
+    def test_create_delete_ro_access_rule(self, version):
+        _create_delete_ro_access_rule(self, version)
 
 
 @ddt.ddt
@@ -202,41 +280,82 @@ class ShareCertRulesForGLUSTERFSTest(base.BaseSharesTest):
         cls.access_to = "client1.com"
 
     @test.attr(type=["gate", ])
-    @ddt.data('shares_client', 'shares_v2_client')
-    def test_create_delete_cert_rule(self, client_name):
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
+    def test_create_delete_cert_rule(self, version):
 
         # create rule
-        rule = getattr(self, client_name).create_access_rule(
-            self.share["id"], self.access_type, self.access_to)
+        if utils.is_microversion_eq(version, '1.0'):
+            rule = self.shares_client.create_access_rule(
+                self.share["id"], self.access_type, self.access_to)
+        else:
+            rule = self.shares_v2_client.create_access_rule(
+                self.share["id"], self.access_type, self.access_to,
+                version=version)
 
         self.assertEqual('rw', rule['access_level'])
         for key in ('deleted', 'deleted_at', 'instance_mappings'):
             self.assertNotIn(key, rule.keys())
-        getattr(self, client_name).wait_for_access_rule_status(
-            self.share["id"], rule["id"], "active")
 
-        # delete rule and wait for deletion
-        getattr(self, client_name).delete_access_rule(self.share["id"],
-                                                      rule["id"])
-        getattr(self, client_name).wait_for_resource_deletion(
-            rule_id=rule["id"], share_id=self.share['id'])
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        elif utils.is_microversion_eq(version, '2.9'):
+            self.shares_v2_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        else:
+            self.shares_v2_client.wait_for_share_status(
+                self.share["id"], "active", status_attr='access_rules_status',
+                version=version)
+
+        # delete rule
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.delete_access_rule(self.share["id"], rule["id"])
+            self.shares_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'])
+        else:
+            self.shares_v2_client.delete_access_rule(
+                self.share["id"], rule["id"], version=version)
+            self.shares_v2_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'], version=version)
 
     @test.attr(type=["gate", ])
     @testtools.skipIf(
         "glusterfs" not in CONF.share.enable_ro_access_level_for_protocols,
         "RO access rule tests are disabled for GLUSTERFS protocol.")
-    @ddt.data('shares_client', 'shares_v2_client')
-    def test_create_delete_cert_ro_access_rule(self, client_name):
-        rule = getattr(self, client_name).create_access_rule(
-            self.share["id"], 'cert', 'client2.com', 'ro')
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
+    def test_create_delete_cert_ro_access_rule(self, version):
+        if utils.is_microversion_eq(version, '1.0'):
+            rule = self.shares_client.create_access_rule(
+                self.share["id"], 'cert', 'client2.com', 'ro')
+        else:
+            rule = self.shares_v2_client.create_access_rule(
+                self.share["id"], 'cert', 'client2.com', 'ro',
+                version=version)
 
         self.assertEqual('ro', rule['access_level'])
         for key in ('deleted', 'deleted_at', 'instance_mappings'):
             self.assertNotIn(key, rule.keys())
-        getattr(self, client_name).wait_for_access_rule_status(
-            self.share["id"], rule["id"], "active")
-        getattr(self, client_name).delete_access_rule(self.share["id"],
-                                                      rule["id"])
+
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        elif utils.is_microversion_eq(version, '2.9'):
+            self.shares_v2_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        else:
+            self.shares_v2_client.wait_for_share_status(
+                self.share["id"], "active", status_attr='access_rules_status',
+                version=version)
+
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.delete_access_rule(self.share["id"], rule["id"])
+            self.shares_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'])
+        else:
+            self.shares_v2_client.delete_access_rule(
+                self.share["id"], rule["id"], version=version)
+            self.shares_v2_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'], version=version)
 
 
 @ddt.ddt
@@ -269,27 +388,43 @@ class ShareRulesTest(base.BaseSharesTest):
         cls.share = cls.create_share()
 
     @test.attr(type=["gate", ])
-    @ddt.data('shares_client', 'shares_v2_client')
-    def test_list_access_rules(self, client_name):
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
+    def test_list_access_rules(self, version):
 
         # create rule
-        rule = getattr(self, client_name).create_access_rule(
-            self.share["id"], self.access_type, self.access_to)
+        if utils.is_microversion_eq(version, '1.0'):
+            rule = self.shares_client.create_access_rule(
+                self.share["id"], self.access_type, self.access_to)
+        else:
+            rule = self.shares_v2_client.create_access_rule(
+                self.share["id"], self.access_type, self.access_to,
+                version=version)
 
-        getattr(self, client_name).wait_for_access_rule_status(
-            self.share["id"], rule["id"], "active")
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        elif utils.is_microversion_eq(version, '2.9'):
+            self.shares_v2_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        else:
+            self.shares_v2_client.wait_for_share_status(
+                self.share["id"], "active", status_attr='access_rules_status',
+                version=version)
 
         # list rules
-        rules = getattr(self, client_name).list_access_rules(self.share["id"])
+        if utils.is_microversion_eq(version, '1.0'):
+            rules = self.shares_client.list_access_rules(self.share["id"])
+        else:
+            rules = self.shares_v2_client.list_access_rules(self.share["id"],
+                                                            version=version)
 
         # verify keys
-        for key in ("state", "id", "access_type", "access_to", "access_level"):
+        for key in ("id", "access_type", "access_to", "access_level"):
             [self.assertIn(key, r.keys()) for r in rules]
         for key in ('deleted', 'deleted_at', 'instance_mappings'):
             [self.assertNotIn(key, r.keys()) for r in rules]
 
         # verify values
-        self.assertEqual("active", rules[0]["state"])
         self.assertEqual(self.access_type, rules[0]["access_type"])
         self.assertEqual(self.access_to, rules[0]["access_to"])
         self.assertEqual('rw', rules[0]["access_level"])
@@ -299,31 +434,58 @@ class ShareRulesTest(base.BaseSharesTest):
         msg = "expected id lists %s times in rule list" % (len(gen))
         self.assertEqual(len(gen), 1, msg)
 
-        getattr(self, client_name).delete_access_rule(
-            self.share['id'], rule['id'])
-
-        getattr(self, client_name).wait_for_resource_deletion(
-            rule_id=rule["id"], share_id=self.share['id'])
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.delete_access_rule(self.share["id"], rule["id"])
+            self.shares_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'])
+        else:
+            self.shares_v2_client.delete_access_rule(
+                self.share["id"], rule["id"], version=version)
+            self.shares_v2_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share['id'], version=version)
 
     @test.attr(type=["gate", ])
-    @ddt.data('shares_client', 'shares_v2_client')
-    def test_access_rules_deleted_if_share_deleted(self, client_name):
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
+    def test_access_rules_deleted_if_share_deleted(self, version):
 
         # create share
         share = self.create_share()
 
         # create rule
-        rule = getattr(self, client_name).create_access_rule(
-            share["id"], self.access_type, self.access_to)
-        getattr(self, client_name).wait_for_access_rule_status(
-            share["id"], rule["id"], "active")
+        if utils.is_microversion_eq(version, '1.0'):
+            rule = self.shares_client.create_access_rule(
+                share["id"], self.access_type, self.access_to)
+        else:
+            rule = self.shares_v2_client.create_access_rule(
+                share["id"], self.access_type, self.access_to,
+                version=version)
+
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.wait_for_access_rule_status(
+                share["id"], rule["id"], "active")
+        elif utils.is_microversion_eq(version, '2.9'):
+            self.shares_v2_client.wait_for_access_rule_status(
+                share["id"], rule["id"], "active")
+        else:
+            self.shares_v2_client.wait_for_share_status(
+                share["id"], "active", status_attr='access_rules_status',
+                version=version)
 
         # delete share
-        getattr(self, client_name).delete_share(share['id'])
-        getattr(self, client_name).wait_for_resource_deletion(
-            share_id=share['id'])
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.delete_share(share['id'])
+            self.shares_client.wait_for_resource_deletion(share_id=share['id'])
+        else:
+            self.shares_v2_client.delete_share(share['id'], version=version)
+            self.shares_v2_client.wait_for_resource_deletion(
+                share_id=share['id'], version=version)
 
         # verify absence of rules for nonexistent share id
-        self.assertRaises(lib_exc.NotFound,
-                          getattr(self, client_name).list_access_rules,
-                          share['id'])
+        if utils.is_microversion_eq(version, '1.0'):
+            self.assertRaises(lib_exc.NotFound,
+                              self.shares_client.list_access_rules,
+                              share['id'])
+        else:
+            self.assertRaises(lib_exc.NotFound,
+                              self.shares_v2_client.list_access_rules,
+                              share['id'], version)

manila_tempest_tests/tests/api/test_rules_negative.py

@@ -14,14 +14,16 @@
 #    under the License.
 
 import ddt
-from tempest import config  # noqa
-from tempest import test  # noqa
-from tempest_lib import exceptions as lib_exc  # noqa
-import testtools  # noqa
+from tempest import config
+from tempest import test
+from tempest_lib import exceptions as lib_exc
+import testtools
 
 from manila_tempest_tests.tests.api import base
+from manila_tempest_tests import utils
 
 CONF = config.CONF
+LATEST_MICROVERSION = CONF.share.max_api_microversion
 
 
 @ddt.ddt
@@ -108,28 +110,53 @@ class ShareIpRulesForNFSNegativeTest(base.BaseSharesTest):
                           'su')
 
     @test.attr(type=["negative", "gate", ])
-    @ddt.data('shares_client', 'shares_v2_client')
-    def test_create_duplicate_of_ip_rule(self, client_name):
+    @ddt.data('1.0', '2.9', LATEST_MICROVERSION)
+    def test_create_duplicate_of_ip_rule(self, version):
         # test data
         access_type = "ip"
         access_to = "1.2.3.4"
 
         # create rule
-        rule = getattr(self, client_name).create_access_rule(
-            self.share["id"], access_type, access_to)
-        getattr(self, client_name).wait_for_access_rule_status(
-            self.share["id"], rule["id"], "active")
+        if utils.is_microversion_eq(version, '1.0'):
+            rule = self.shares_client.create_access_rule(
+                self.share["id"], access_type, access_to)
+        else:
+            rule = self.shares_v2_client.create_access_rule(
+                self.share["id"], access_type, access_to, version=version)
+
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        elif utils.is_microversion_eq(version, '2.9'):
+            self.shares_v2_client.wait_for_access_rule_status(
+                self.share["id"], rule["id"], "active")
+        else:
+            self.shares_v2_client.wait_for_share_status(
+                self.share["id"], "active", status_attr='access_rules_status',
+                version=version)
 
         # try create duplicate of rule
-        self.assertRaises(lib_exc.BadRequest,
-                          getattr(self, client_name).create_access_rule,
-                          self.share["id"], access_type, access_to)
+        if utils.is_microversion_eq(version, '1.0'):
+            self.assertRaises(lib_exc.BadRequest,
+                              self.shares_client.create_access_rule,
+                              self.share["id"], access_type, access_to)
+        else:
+            self.assertRaises(lib_exc.BadRequest,
+                              self.shares_v2_client.create_access_rule,
+                              self.share["id"], access_type, access_to,
+                              version=version)
 
         # delete rule and wait for deletion
-        getattr(self, client_name).delete_access_rule(self.share["id"],
-                                                      rule["id"])
-        getattr(self, client_name).wait_for_resource_deletion(
-            rule_id=rule["id"], share_id=self.share['id'])
+        if utils.is_microversion_eq(version, '1.0'):
+            self.shares_client.delete_access_rule(self.share["id"],
+                                                  rule["id"])
+            self.shares_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share["id"])
+        else:
+            self.shares_v2_client.delete_access_rule(self.share["id"],
+                                                     rule["id"])
+            self.shares_v2_client.wait_for_resource_deletion(
+                rule_id=rule["id"], share_id=self.share["id"], version=version)
 
 
 @ddt.ddt

manila_tempest_tests/tests/api/test_shares_actions.py

@@ -95,6 +95,8 @@ class SharesActionsTest(base.BaseSharesTest):
                                   "source_cgsnapshot_member_id"])
         if utils.is_microversion_ge(version, '2.5'):
             expected_keys.append("share_type_name")
+        if utils.is_microversion_ge(version, '2.10'):
+            expected_keys.append("access_rules_status")
         actual_keys = list(share.keys())
         [self.assertIn(key, actual_keys) for key in expected_keys]
 
@@ -136,6 +138,11 @@ class SharesActionsTest(base.BaseSharesTest):
     def test_get_share_export_locations_removed(self):
         self._get_share('2.9')
 
+    @test.attr(type=["gate", ])
+    @utils.skip_if_microversion_not_supported('2.10')
+    def test_get_share_with_access_rules_status(self):
+        self._get_share('2.10')
+
     @test.attr(type=["gate", ])
     def test_list_shares(self):
 
@@ -174,6 +181,8 @@ class SharesActionsTest(base.BaseSharesTest):
                          "source_cgsnapshot_member_id"])
         if utils.is_microversion_ge(version, '2.6'):
             keys.append("share_type_name")
+        if utils.is_microversion_ge(version, '2.10'):
+            keys.append("access_rules_status")
 
         [self.assertIn(key, sh.keys()) for sh in shares for key in keys]
 
@@ -206,6 +215,11 @@ class SharesActionsTest(base.BaseSharesTest):
     def test_list_shares_with_detail_export_locations_removed(self):
         self._list_shares_with_detail('2.9')
 
+    @test.attr(type=["gate", ])
+    @utils.skip_if_microversion_not_supported('2.10')
+    def test_list_shares_with_detail_with_access_rules_status(self):
+        self._list_shares_with_detail('2.10')
+
     @test.attr(type=["gate", ])
     def test_list_shares_with_detail_filter_by_metadata(self):
         filters = {'metadata': self.metadata}

manila_tempest_tests/tests/scenario/manager_share.py

@@ -133,7 +133,12 @@ class ShareScenarioTest(manager.NetworkScenarioTest):
         """
         client = client or self.shares_client
         access = client.create_access_rule(share_id, access_type, access_to)
-        client.wait_for_access_rule_status(share_id, access['id'], "active")
+
+        # NOTE(u_glide): Ignore provided client, because we always need v2
+        # client to make this call
+        self.shares_v2_client.wait_for_share_status(
+            share_id, "active", status_attr='access_rules_status')
+
         if cleanup:
             self.addCleanup(client.delete_access_rule, share_id, access['id'])
         return access