From c7d97c4ad70cc49dd529a10c593fbd9b20f89d90 Mon Sep 17 00:00:00 2001 From: Zhiteng Huang Date: Mon, 12 Jan 2015 13:27:15 +0800 Subject: [PATCH] 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 --- cinder/api/contrib/volume_actions.py | 3 +++ cinder/exception.py | 4 ++++ .../tests/api/contrib/test_admin_actions.py | 2 +- .../tests/targets/test_base_iscsi_driver.py | 2 +- cinder/tests/test_huawei_t_dorado.py | 2 +- cinder/tests/test_ibm_flashsystem.py | 4 ++-- cinder/tests/test_storwize_svc.py | 10 +++++----- cinder/tests/test_volume.py | 10 +++++----- cinder/volume/driver.py | 20 ++++++++++--------- cinder/volume/drivers/huawei/huawei_t.py | 8 ++++---- cinder/volume/drivers/ibm/flashsystem.py | 8 ++++---- .../drivers/ibm/storwize_svc/__init__.py | 7 ++++--- cinder/volume/manager.py | 4 +++- cinder/volume/targets/iscsi.py | 6 +++--- 14 files changed, 51 insertions(+), 39 deletions(-) diff --git a/cinder/api/contrib/volume_actions.py b/cinder/api/contrib/volume_actions.py index fa018923c..93fd04661 100644 --- 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 2e2f852ab..b5458de25 100644 --- 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 82cbb7231..249d67874 100644 --- 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 14c80db93..8b2f5a980 100644 --- 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 36d84648b..57858dfbf 100644 --- 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 3060f451f..70791cb1a 100644 --- 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 b8bcfe50e..543452422 100644 --- 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 f5ea37e51..a04817ffe 100644 --- 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 dfdf93f2e..9036aed63 100644 --- 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 2964d30b8..6be3787d3 100644 --- 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 16c11f0ce..2269dd5f9 100644 --- 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 e2b838d76..e514daa16 100644 --- 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 4c3c31b93..dbc4d8def 100644 --- 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 1197add95..ca05933ec 100644 --- 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 -- 2.45.2