Fix issue when registering storage

dolphin/api/v1/access_info.py

@@ -11,17 +11,15 @@
 # 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 dolphin.api import validation
 from webob import exc
 
-from dolphin import db, cryptor
+from dolphin import db
 from dolphin import exception
-from dolphin.api.common import wsgi, LOG
+from dolphin.api import validation
+from dolphin.api.common import wsgi
+from dolphin.api.schemas import access_info as schema_access_info
 from dolphin.api.views import access_info as access_info_viewer
 from dolphin.drivers import api as driverapi
-from dolphin.api.views import storages as storage_view
-from dolphin.i18n import _
-from dolphin.api.schemas import access_info as schema_access_info
 
 
 class AccessInfoController(wsgi.Controller):
@@ -46,30 +44,18 @@ def show(self, req, id):
     def update(self, req, id, body):
         """Update storage access information."""
         ctxt = req.environ.get('dolphin.context')
-        # Get existing access_info and storage from DB
         try:
             access_info = db.access_info_get(ctxt, id)
             access_info.update(body)
-            storage = self.driver_api.discover_storage(ctxt, access_info)
-            storage_present = db.storage_get(ctxt, id)
-            if storage['serial_number'] != storage_present.serial_number:
-                reason = (_("Existing storage serial Number is not matching \
-                with th new storage serial number: '%s'  ") % storage['serial_number'])
-                raise exception.StorageSerialNumberMismatch(reason=reason)
-            db.storage_update(ctxt, id, storage)
-            access_info['password'] = cryptor.encode(
-                access_info['password'])
-            db.access_info_update(ctxt, id, access_info)
+            access_info = self.driver_api.update_access_info(ctxt, access_info)
         except (exception.InvalidCredential,
+                exception.InvalidResults,
                 exception.StorageDriverNotFound,
                 exception.AccessInfoNotFound,
                 exception.StorageNotFound,
                 exception.StorageSerialNumberMismatch) as e:
-            raise exc.HTTPBadRequest(explanation=e.message)
-        except Exception as e:
-            msg = _('Failed to to update  access info: {0}'.format(e))
-            LOG.error(msg)
-            raise exc.HTTPBadRequest(explanation=msg)
+            raise exc.HTTPBadRequest(explanation=e.msg)
+
         return self._view_builder.show(access_info)
 
 

dolphin/api/v1/storages.py

@@ -20,7 +20,6 @@
 
 from dolphin import context
 from dolphin import coordination
-from dolphin import cryptor
 from dolphin import db
 from dolphin import exception
 from dolphin.api import api_utils
@@ -96,27 +95,20 @@ def create(self, req, body):
         ctxt = req.environ['dolphin.context']
         access_info_dict = body
 
-        if self._is_registered(ctxt, access_info_dict):
-            msg = _("Storage has been registered.")
+        if self._storage_exist(ctxt, access_info_dict):
+            msg = _("Storage already exists.")
             raise exc.HTTPBadRequest(explanation=msg)
 
         try:
-            storage = self.driver_api.discover_storage(ctxt, access_info_dict)
-            storage = db.storage_create(context, storage)
-
-            # Need to encode the password before saving.
-            access_info_dict['storage_id'] = storage['id']
-            access_info_dict['password'] = cryptor.encode(access_info_dict['password'])
-            db.access_info_create(context, access_info_dict)
+            storage = self.driver_api.discover_storage(ctxt,
+                                                       access_info_dict)
         except (exception.InvalidCredential,
-                exception.StorageDriverNotFound,
+                exception.InvalidRequest,
+                exception.InvalidResults,
                 exception.AccessInfoNotFound,
+                exception.StorageDriverNotFound,
                 exception.StorageNotFound) as e:
-            raise exc.HTTPBadRequest(explanation=e.message)
-        except Exception as e:
-            msg = _('Failed to register storage: {0}'.format(e))
-            LOG.error(msg)
-            raise exc.HTTPBadRequest(explanation=msg)
+            raise exc.HTTPBadRequest(explanation=e.msg)
 
         return storage_view.build_storage(storage)
 
@@ -184,19 +176,30 @@ def sync(self, req, id):
 
         return
 
-    def _is_registered(self, context, access_info):
+    def _storage_exist(self, context, access_info):
         access_info_dict = copy.deepcopy(access_info)
 
         # Remove unrelated query fields
-        access_info_dict.pop('username', None)
-        access_info_dict.pop('password', None)
-        access_info_dict.pop('vendor', None)
-        access_info_dict.pop('model', None)
+        unrelated_fields = ['username', 'password']
+        for key in unrelated_fields:
+            access_info_dict.pop(key)
 
         # Check if storage is registered
-        if db.access_info_get_all(context,
-                                  filters=access_info_dict):
-            return True
+        access_info_list = db.access_info_get_all(context,
+                                                  filters=access_info_dict)
+        for _access_info in access_info_list:
+            try:
+                storage = db.storage_get(context, _access_info['storage_id'])
+                if storage:
+                    return True
+            except exception.StorageNotFound:
+                # Suppose storage was not saved successfully after access
+                # information was saved in database when registering storage.
+                # Therefore, removing access info if storage doesn't exist to
+                # ensure the database has no residual data.
+                LOG.debug("Remove residual access information.")
+                db.access_info_delete(context, _access_info['storage_id'])
+
         return False
 
 

dolphin/db/api.py

@@ -259,16 +259,21 @@ def access_info_create(context, values):
     return IMPL.access_info_create(context, values)
 
 
-def access_info_update(context, access_info_id, values):
+def access_info_update(context, storage_id, values):
     """Update a storage access information with the values dictionary."""
-    return IMPL.access_info_update(context, access_info_id, values)
+    return IMPL.access_info_update(context, storage_id, values)
 
 
 def access_info_get(context, storage_id):
     """Get a storage access information."""
     return IMPL.access_info_get(context, storage_id)
 
 
+def access_info_delete(context, storage_id):
+    """Delete a storage access information."""
+    return IMPL.access_info_delete(context, storage_id)
+
+
 def access_info_get_all(context, marker=None, limit=None, sort_keys=None,
                         sort_dirs=None, filters=None, offset=None):
     """Retrieves all storage access information.
@@ -294,11 +299,6 @@ def access_info_get_all(context, marker=None, limit=None, sort_keys=None,
                                     filters, offset)
 
 
-def access_info_delete(context, storage_id):
-    """Delete a storage access information."""
-    return IMPL.access_info_delete(context, storage_id)
-
-
 def is_orm_value(obj):
     """Check if object is an ORM field."""
     return IMPL.is_orm_value(obj)

dolphin/db/sqlalchemy/api.py

@@ -171,12 +171,18 @@ def access_info_create(context, values):
                             session=session)
 
 
-def access_info_update(context, access_info_id, values):
+def access_info_update(context, storage_id, values):
     """Update a storage access information with the values dictionary."""
     session = get_session()
     with session.begin():
-        result = _access_info_get(context, access_info_id, session).update(values)
-        return result
+        _access_info_get(context, storage_id, session).update(values)
+        return _access_info_get(context, storage_id, session)
+
+
+def access_info_delete(context, storage_id):
+    """Delete a storage access information."""
+    _access_info_get_query(context).\
+        filter_by(storage_id=storage_id).delete()
 
 
 def access_info_get(context, storage_id):
@@ -213,11 +219,6 @@ def access_info_get_all(context, marker=None, limit=None, sort_keys=None,
         return query.all()
 
 
-def access_info_delete(context, storage_id):
-    """Delete a storage access information."""
-    _access_info_get_query(context).filter_by(storage_id=storage_id).delete()
-
-
 @apply_like_filters(model=models.AccessInfo)
 def _process_access_info_filters(query, filters):
     """Common filter processing for AccessInfo queries."""

dolphin/db/sqlalchemy/models.py

@@ -48,6 +48,8 @@ class AccessInfo(BASE, DolphinBase):
     """Represent access info required for storage accessing."""
     __tablename__ = "access_info"
     storage_id = Column(String(36), primary_key=True)
+    vendor = Column(String(255))
+    model = Column(String(255))
     host = Column(String(255))
     port = Column(String(255))
     username = Column(String(255))

dolphin/drivers/api.py

@@ -17,6 +17,7 @@
 from oslo_log import log
 from oslo_utils import uuidutils
 
+from dolphin.drivers import helper
 from dolphin.drivers import manager
 
 LOG = log.getLogger(__name__)
@@ -26,35 +27,46 @@ class API(object):
     def __init__(self):
         self.driver_manager = manager.DriverManager()
 
-    @staticmethod
-    def get_storage_registry():
-        """Show register parameters which the driver needs."""
-        pass
-
     def discover_storage(self, context, access_info):
-        """Discover a storage system with access information and create driver instance ."""
-
+        """Discover a storage system with access information."""
         if 'storage_id' not in access_info:
             access_info['storage_id'] = six.text_type(uuidutils.generate_uuid())
-        else:
-            # Already registered storage , remove driver and create driver instance \
-            # again with new access_info
-            self.driver_manager.remove_driver(context,access_info['storage_id'])
 
-        driver = self.driver_manager.create_driver(context, **access_info)
+        driver = self.driver_manager.get_driver(context,
+                                                cache_on_load=False,
+                                                **access_info)
         storage = driver.get_storage(context)
-        if storage:
-            storage['id'] = access_info['storage_id']
-        else:
-            self.driver_manager.remove_driver(context, access_info['storage_id'])
 
-        LOG.info("Storage was found successfully.")
+        # Need to validate storage response from driver
+        helper.check_storage_repetition(context, storage)
+        access_info = helper.create_access_info(context, access_info)
+        storage['id'] = access_info['storage_id']
+        storage = helper.create_storage(context, storage)
+        self.driver_manager.update_driver(storage['id'], driver)
+
+        LOG.info("Storage found successfully.")
         return storage
 
+    def update_access_info(self, context, access_info):
+        """Validate and update access information."""
+        driver = self.driver_manager.get_driver(context,
+                                                cache_on_load=False,
+                                                **access_info)
+        storage_new = driver.get_storage(context)
+
+        # Need to validate storage response from driver
+        storage_id = access_info['storage_id']
+        helper.check_storage_consistency(context, storage_id, storage_new)
+        access_info = helper.update_access_info(context, storage_id, access_info)
+        helper.update_storage(context, storage_id, storage_new)
+        self.driver_manager.update_driver(storage_id, driver)
+
+        LOG.info("Access information updated successfully.")
+        return access_info
+
     def remove_storage(self, context, storage_id):
         """Clear driver instance from driver factory."""
-        driver = self.driver_manager.get_driver()
-        driver.remove_driver(context, storage_id)
+        self.driver_manager.remove_driver(storage_id)
 
     def get_storage(self, context, storage_id):
         """Get storage device information from storage system"""
@@ -86,3 +98,5 @@ def parse_alert(self, context, storage_id, alert):
     def clear_alert(self, context, storage_id, alert):
         """Clear alert from storage system."""
         pass
+
+

dolphin/drivers/driver.py

@@ -28,11 +28,6 @@ def __init__(self, **kwargs):
         """
         self.storage_id = kwargs.get('storage_id', None)
 
-    @staticmethod
-    def get_storage_registry():
-        """Show register parameters which the driver needs."""
-        pass
-
     @abc.abstractmethod
     def get_storage(self, context):
         """Get storage device information from storage system"""

dolphin/drivers/fake_storage/__init__.py

@@ -23,10 +23,6 @@ class FakeStorageDriver(driver.StorageDriver):
     def __init__(self, **kwargs):
         super().__init__(**kwargs)
 
-    @staticmethod
-    def get_storage_registry():
-        pass
-
     def get_storage(self, context):
         # Do something here
         return {

dolphin/drivers/helper.py

@@ -16,6 +16,8 @@
 
 from dolphin import cryptor
 from dolphin import db
+from dolphin import exception
+from dolphin.i18n import _
 
 LOG = log.getLogger(__name__)
 
@@ -25,3 +27,67 @@ def get_access_info(context, storage_id):
     access_info_dict = access_info.to_dict()
     access_info_dict['password'] = cryptor.decode(access_info_dict['password'])
     return access_info_dict
+
+
+def create_access_info(context, access_info):
+    access_info['password'] = cryptor.encode(access_info['password'])
+    return db.access_info_create(context, access_info)
+
+
+def update_access_info(context, storage_id, access_info):
+    return db.access_info_update(context,
+                                 storage_id,
+                                 access_info)
+
+
+def create_storage(context, storage):
+    return db.storage_create(context, storage)
+
+
+def update_storage(context, storage_id, storage):
+    return db.storage_update(context, storage_id, storage)
+
+
+def check_storage_repetition(context, storage):
+    if not storage:
+        msg = _("Storage could not be found.")
+        raise exception.StorageNotFound(message=msg)
+
+    if not storage.get('serial_number'):
+        msg = _("Serial number should be provided by storage.")
+        raise exception.InvalidResults(message=msg)
+
+    filters = dict(serial_number=storage['serial_number'])
+    storage_list = db.storage_get_all(context, filters=filters)
+    if storage_list:
+        msg = (_("Failed to register storage. Reason: same serial number: "
+                 "%s detected.") % storage['serial_number'])
+        LOG.error(msg)
+        raise exception.InvalidRequest(msg)
+
+
+def check_storage_consistency(context, storage_id, storage_new):
+    """Check storage response returned by driver whether it matches the
+    storage stored in database.
+
+    :param context: The context of dolphin.
+    :type context: dolphin.context.RequestContext
+    :param storage_id: The uuid of storage in database.
+    :type storage_id: string
+    :param storage_new: The storage response returned by driver.
+    :type storage_new: dict
+    """
+    if not storage_new:
+        msg = _("Storage could not be found.")
+        raise exception.StorageNotFound(message=msg)
+
+    if not storage_new.get('serial_number'):
+        msg = _("Serial number should be provided by storage.")
+        raise exception.InvalidResults(message=msg)
+
+    storage_present = db.storage_get(context, storage_id)
+    if storage_new['serial_number'] != storage_present['serial_number']:
+        msg = (_("New storage serial number is not matching "
+                 "with the existing storage serial number: %s") %
+               storage_present['serial_number'])
+        raise exception.StorageSerialNumberMismatch(message=msg)