Raise correct exception when validate_connector failed

Cinder volume manager uses validate_connector() method to verify if required
information is in connector when handling initialize_connection() request.
validate_connector() is actually a pure input validation method, basically
checking if 'initiator' or 'wwpns' is in connector if storage protocol is
iSCSI or FC.  However, when required information is missing, currently drivers
raises either VolumeBackendAPIException or VolumeDriverException, which would
then bubble up to API and then to user (Nova) as InternalServerError.

This change adds a new exception - InvalidConnectorException, that drivers
should raise when connector is found not valid.  With that, Cinder API would
raise BadRequest instead to user, suggesting things are missing in request.

Change-Id: I4f04f5d0c558404836a2bd270f7f22f3f2d4f314
Closes-bug: #1409580

diff --git a/cinder/api/contrib/volume_actions.py b/cinder/api/contrib/volume_actions.py
index fa018923c86dd642cc3cb58f3526d85d2ccc2d29..93fd046611a70a058f8e97f35c3b47c55406fa92 100644 (file)
--- a/cinder/api/contrib/volume_actions.py
+++ b/cinder/api/contrib/volume_actions.py
@@ -195,6 +195,9 @@ class VolumeActionsController(wsgi.Controller):
             info = self.volume_api.initialize_connection(context,
                                                          volume,
                                                          connector)
+        except exception.InvalidInput as err:
+            raise webob.exc.HTTPBadRequest(
+                explanation=err)
         except exception.VolumeBackendAPIException as error:
             msg = _("Unable to fetch connection information from backend.")
             raise webob.exc.HTTPInternalServerError(explanation=msg)

diff --git a/cinder/exception.py b/cinder/exception.py
index 2e2f852ab76f28c5c0efce2bd1bf675303597b23..b5458de25c67c16705c43458dde7ec4771194429 100644 (file)
--- a/cinder/exception.py
+++ b/cinder/exception.py
@@ -492,6 +492,10 @@ class FailedCmdWithDump(VolumeDriverException):
     message = _("Operation failed with status=%(status)s. Full dump: %(data)s")
 
 
+class InvalidConnectorException(VolumeDriverException):
+    message = _("Connector doesn't have required information: %(missing)s")
+
+
 class GlanceMetadataExists(Invalid):
     message = _("Glance metadata cannot be updated, key %(key)s"
                 " exists for volume id %(volume_id)s")

diff --git a/cinder/tests/api/contrib/test_admin_actions.py b/cinder/tests/api/contrib/test_admin_actions.py
index 82cbb723171cabfa2ea975d394e6e7e12b51b943..249d678740ca3cc189524410613b88dccda9f29a 100644 (file)
--- a/cinder/tests/api/contrib/test_admin_actions.py
+++ b/cinder/tests/api/contrib/test_admin_actions.py
@@ -563,7 +563,7 @@ class AdminActionsTest(test.TestCase):
         connector = {}
         # start service to handle rpc messages for attach requests
         svc = self.start_service('volume', host='test')
-        self.assertRaises(exception.VolumeBackendAPIException,
+        self.assertRaises(exception.InvalidInput,
                           self.volume_api.initialize_connection,
                           ctx, volume, connector)
         # cleanup

diff --git a/cinder/tests/targets/test_base_iscsi_driver.py b/cinder/tests/targets/test_base_iscsi_driver.py
index 14c80db93b1e81de87a25f75f7fb050f861e29d2..8b2f5a9809185f9ade0cdb40957c9a77112efd47 100644 (file)
--- a/cinder/tests/targets/test_base_iscsi_driver.py
+++ b/cinder/tests/targets/test_base_iscsi_driver.py
@@ -129,7 +129,7 @@ class TestBaseISCSITargetDriver(test.TestCase):
 
     def test_validate_connector(self):
         bad_connector = {'no_initiator': 'nada'}
-        self.assertRaises(exception.VolumeBackendAPIException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.target.validate_connector,
                           bad_connector)
 

diff --git a/cinder/tests/test_huawei_t_dorado.py b/cinder/tests/test_huawei_t_dorado.py
index 36d84648bc5a3e592ff6d984c9c7dc4eae3b84bb..57858dfbf0ac1ed91a99a6700a418cd9a2c6ad23 100644 (file)
--- a/cinder/tests/test_huawei_t_dorado.py
+++ b/cinder/tests/test_huawei_t_dorado.py
@@ -1485,7 +1485,7 @@ class HuaweiTFCDriverTestCase(test.TestCase):
 
     def test_validate_connector_failed(self):
         invalid_connector = {'host': 'testhost'}
-        self.assertRaises(exception.VolumeBackendAPIException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.driver.validate_connector,
                           invalid_connector)
 

diff --git a/cinder/tests/test_ibm_flashsystem.py b/cinder/tests/test_ibm_flashsystem.py
index 3060f451f45fc9bfe2ab4ef7c90e55a7da37f717..70791cb1a643dec1f3d76b9612b93a8022b53d03 100644 (file)
--- a/cinder/tests/test_ibm_flashsystem.py
+++ b/cinder/tests/test_ibm_flashsystem.py
@@ -843,9 +843,9 @@ class FlashSystemDriverTestCase(test.TestCase):
         self.driver._protocol = 'FC'
         self.driver.validate_connector(conn_fc)
         self.driver.validate_connector(conn_both)
-        self.assertRaises(exception.VolumeDriverException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.driver.validate_connector, conn_iscsi)
-        self.assertRaises(exception.VolumeDriverException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.driver.validate_connector, conn_neither)
 
         # clear environment

diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py
index b8bcfe50e89e64647b61d80a1e533a3b0c05d727..543452422750110a6df4727a5f2ba6d015fb3653 100644 (file)
--- a/cinder/tests/test_storwize_svc.py
+++ b/cinder/tests/test_storwize_svc.py
@@ -2042,24 +2042,24 @@ class StorwizeSVCDriverTestCase(test.TestCase):
         self.driver._state['enabled_protocols'] = set(['iSCSI'])
         self.driver.validate_connector(conn_iscsi)
         self.driver.validate_connector(conn_both)
-        self.assertRaises(exception.VolumeDriverException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.driver.validate_connector, conn_fc)
-        self.assertRaises(exception.VolumeDriverException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.driver.validate_connector, conn_neither)
 
         self.driver._state['enabled_protocols'] = set(['FC'])
         self.driver.validate_connector(conn_fc)
         self.driver.validate_connector(conn_both)
-        self.assertRaises(exception.VolumeDriverException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.driver.validate_connector, conn_iscsi)
-        self.assertRaises(exception.VolumeDriverException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.driver.validate_connector, conn_neither)
 
         self.driver._state['enabled_protocols'] = set(['iSCSI', 'FC'])
         self.driver.validate_connector(conn_iscsi)
         self.driver.validate_connector(conn_fc)
         self.driver.validate_connector(conn_both)
-        self.assertRaises(exception.VolumeDriverException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.driver.validate_connector, conn_neither)
 
     def test_storwize_svc_host_maps(self):

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index f5ea37e512fac8c298f0f347d4fc6f50d7be2992..a04817ffe73dac4dd164f52c91ee368df6a0cc67 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -4127,7 +4127,7 @@ class ISCSITestCase(DriverTestCase):
 
         # Validate a connector without the initiator
         connector = {'ip': '10.0.0.2', 'host': 'fakehost'}
-        self.assertRaises(exception.VolumeBackendAPIException,
+        self.assertRaises(exception.InvalidConnectorException,
                           iscsi_driver.validate_connector, connector)
 
 
@@ -4213,27 +4213,27 @@ class FibreChannelTestCase(DriverTestCase):
     def test_validate_connector_no_wwpns(self):
         """validate_connector() throws exception when it has no wwpns."""
         connector = {'wwnns': ["not empty"]}
-        self.assertRaises(exception.VolumeDriverException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.volume.driver.validate_connector, connector)
 
     def test_validate_connector_empty_wwpns(self):
         """validate_connector() throws exception when it has empty wwpns."""
         connector = {'wwpns': [],
                      'wwnns': ["not empty"]}
-        self.assertRaises(exception.VolumeDriverException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.volume.driver.validate_connector, connector)
 
     def test_validate_connector_no_wwnns(self):
         """validate_connector() throws exception when it has no wwnns."""
         connector = {'wwpns': ["not empty"]}
-        self.assertRaises(exception.VolumeDriverException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.volume.driver.validate_connector, connector)
 
     def test_validate_connector_empty_wwnns(self):
         """validate_connector() throws exception when it has empty wwnns."""
         connector = {'wwnns': [],
                      'wwpns': ["not empty"]}
-        self.assertRaises(exception.VolumeDriverException,
+        self.assertRaises(exception.InvalidConnectorException,
                           self.volume.driver.validate_connector, connector)
 
 

diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py
index dfdf93f2ed4b478942d2f197c1db56d6ab32008f..9036aed631c45be98185379ab53a47b6ca92c99b 100644 (file)
--- a/cinder/volume/driver.py
+++ b/cinder/volume/driver.py
@@ -1079,11 +1079,12 @@ class ISCSIDriver(VolumeDriver):
 
     def validate_connector(self, connector):
         # iSCSI drivers require the initiator information
-        if 'initiator' not in connector:
-            err_msg = (_('The volume driver requires the iSCSI initiator '
-                         'name in the connector.'))
-            LOG.error(err_msg)
-            raise exception.VolumeBackendAPIException(data=err_msg)
+        required = 'initiator'
+        if required not in connector:
+            err_msg = (_LE('The volume driver requires %(data)s '
+                           'in the connector.'), {'data': required})
+            LOG.error(*err_msg)
+            raise exception.InvalidConnectorException(missing=required)
 
     def terminate_connection(self, volume, connector, **kwargs):
         pass
@@ -1361,8 +1362,9 @@ class FibreChannelDriver(VolumeDriver):
     def validate_connector_has_setting(connector, setting):
         """Test for non-empty setting in connector."""
         if setting not in connector or not connector[setting]:
-            msg = (_(
+            msg = (_LE(
                 "FibreChannelDriver validate_connector failed. "
-                "No '%s'. Make sure HBA state is Online.") % setting)
-            LOG.error(msg)
-            raise exception.VolumeDriverException(message=msg)
+                "No '%(setting)s'. Make sure HBA state is Online."),
+                {'setting': setting})
+            LOG.error(*msg)
+            raise exception.InvalidConnectorException(missing=setting)

diff --git a/cinder/volume/drivers/huawei/huawei_t.py b/cinder/volume/drivers/huawei/huawei_t.py
index 2964d30b83c278b8e892c170c2d5d02a79b4eca6..6be3787d3938cdf39cb5ccd426289d80aae5c21a 100644 (file)
--- a/cinder/volume/drivers/huawei/huawei_t.py
+++ b/cinder/volume/drivers/huawei/huawei_t.py
@@ -21,7 +21,7 @@ import re
 import time
 
 from cinder import exception
-from cinder.i18n import _, _LW
+from cinder.i18n import _, _LE, _LW
 from cinder.openstack.common import log as logging
 from cinder.volume import driver
 from cinder.volume.drivers.huawei import huawei_utils
@@ -435,10 +435,10 @@ class HuaweiTFCDriver(driver.FibreChannelDriver):
     def validate_connector(self, connector):
         """Check for wwpns in connector."""
         if 'wwpns' not in connector:
-            err_msg = (_('validate_connector: The FC driver requires the'
-                         ' wwpns in the connector.'))
+            err_msg = (_LE('validate_connector: The FC driver requires the'
+                           ' wwpns in the connector.'))
             LOG.error(err_msg)
-            raise exception.VolumeBackendAPIException(data=err_msg)
+            raise exception.InvalidConnectorException(missing='wwpns')
 
     @fczm_utils.AddFCZone
     def initialize_connection(self, volume, connector):

diff --git a/cinder/volume/drivers/ibm/flashsystem.py b/cinder/volume/drivers/ibm/flashsystem.py
index 16c11f0ce911d9c83a46105359c167a4fc1df324..2269dd5f9c8613446a0d601a397eed8d36b10326 100644 (file)
--- a/cinder/volume/drivers/ibm/flashsystem.py
+++ b/cinder/volume/drivers/ibm/flashsystem.py
@@ -1137,11 +1137,11 @@ class FlashSystemDriver(san.SanDriver):
 
     def validate_connector(self, connector):
         """Check connector."""
-        if 'FC' != self._protocol or 'wwpns' not in connector:
-            msg = (_('The connector does not contain the '
-                     'required information.'))
+        if 'FC' == self._protocol and 'wwpns' not in connector:
+            msg = (_LE('The connector does not contain the '
+                       'required information: wwpns is missing'))
             LOG.error(msg)
-            raise exception.VolumeDriverException(data=msg)
+            raise exception.InvalidConnectorException(missing='wwpns')
 
     def create_volume(self, volume):
         """Create volume."""

diff --git a/cinder/volume/drivers/ibm/storwize_svc/__init__.py b/cinder/volume/drivers/ibm/storwize_svc/__init__.py
index e2b838d761c69980f16416654385453550736a3d..e514daa16340b75282707a0d2a79a87529e1474c 100644 (file)
--- a/cinder/volume/drivers/ibm/storwize_svc/__init__.py
+++ b/cinder/volume/drivers/ibm/storwize_svc/__init__.py
@@ -308,10 +308,11 @@ class StorwizeSVCDriver(san.SanDriver):
         if 'FC' in self._state['enabled_protocols'] and 'wwpns' in connector:
             valid = True
         if not valid:
-            msg = (_('The connector does not contain the required '
-                     'information.'))
+            msg = (_LE('The connector does not contain the required '
+                       'information.'))
             LOG.error(msg)
-            raise exception.VolumeDriverException(message=msg)
+            raise exception.InvalidConnectorException(
+                missing='initiator or wwpns')
 
     def _get_vdisk_params(self, type_id, volume_type=None,
                           volume_metadata=None):

diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 4c3c31b93fb3361b4a5d240e93d1f4ae3c08cf41..dbc4d8defa64fa4e7462de0aff6f784c2c96d401 100644 (file)
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -886,8 +886,10 @@ class VolumeManager(manager.SchedulerDependentManager):
         utils.require_driver_initialized(self.driver)
         try:
             self.driver.validate_connector(connector)
+        except exception.InvalidConnectorException as err:
+            raise exception.InvalidInput(reason=err)
         except Exception as err:
-            err_msg = (_('Unable to fetch connection information from '
+            err_msg = (_('Unable to validate connector information in '
                          'backend: %(err)s') % {'err': err})
             LOG.error(err_msg)
             raise exception.VolumeBackendAPIException(data=err_msg)

diff --git a/cinder/volume/targets/iscsi.py b/cinder/volume/targets/iscsi.py
index 1197add95a768b2090a3c21a9d859c7524fa1ef5..ca05933ec69afdc0ba69f1867beb93099ea038ae 100644 (file)
--- a/cinder/volume/targets/iscsi.py
+++ b/cinder/volume/targets/iscsi.py
@@ -184,8 +184,8 @@ class ISCSITarget(driver.Target):
     def validate_connector(self, connector):
         # NOTE(jdg): api passes in connector which is initiator info
         if 'initiator' not in connector:
-            err_msg = (_('The volume driver requires the iSCSI initiator '
-                         'name in the connector.'))
+            err_msg = (_LE('The volume driver requires the iSCSI initiator '
+                           'name in the connector.'))
             LOG.error(err_msg)
-            raise exception.VolumeBackendAPIException(data=err_msg)
+            raise exception.InvalidConnectorException(missing='initiator')
         return True