openstack/cinder
Code Issues Proposed changes

Dell SC: Incorrect values in REST API Login call

Browse Source 
The Dell SC REST API driver (dell_storagecenter_api.py) could
specify the wrong api version header. On connection failure
the driver needs to parse the failure text to see if it is
the wrong value and try a second time.

Added the apiversion variable to the StorageCenterApiHelper
class so that this value only needs to be discovered once.

Updated open_connection appropriately.

Change-Id: Ibab6cf227f298b0fbf990aabcc674350a56c5439
Closes-Bug: 1548496
This commit is contained in:
Tom Swanson 2016-02-22 14:56:39 -06:00
parent 8fbc112a99
commit 3274e4c49d
4 changed files with 86 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cinder/tests/unit/test_dellfc.py
View File

@ -23,7 +23,7 @@ from cinder.volume.drivers.dell import dell_storagecenter_fc
# We patch these here as they are used by every test to keep # We patch these here as they are used by every test to keep
# from trying to contact a Dell Storage Center. # from trying to contact a Dell Storage Center.
@mock.patch.object(dell_storagecenter_api.StorageCenterApi, @mock.patch.object(dell_storagecenter_api.HttpClient,
'__init__', '__init__',
return_value=None) return_value=None)
@mock.patch.object(dell_storagecenter_api.StorageCenterApi, @mock.patch.object(dell_storagecenter_api.StorageCenterApi,

5
cinder/tests/unit/test_dellsc.py
View File

@ -26,11 +26,12 @@ from cinder.volume import volume_types
# We patch these here as they are used by every test to keep # We patch these here as they are used by every test to keep
# from trying to contact a Dell Storage Center. # from trying to contact a Dell Storage Center.
@mock.patch.object(dell_storagecenter_api.StorageCenterApi, @mock.patch.object(dell_storagecenter_api.HttpClient,
'__init__', '__init__',
return_value=None) return_value=None)
@mock.patch.object(dell_storagecenter_api.StorageCenterApi, @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
'open_connection') 'open_connection',
return_value=mock.MagicMock())
@mock.patch.object(dell_storagecenter_api.StorageCenterApi, @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
'close_connection') 'close_connection')
class DellSCSanISCSIDriverTestCase(test.TestCase): class DellSCSanISCSIDriverTestCase(test.TestCase):

30
cinder/tests/unit/test_dellscapi.py
View File

@ -1662,6 +1662,7 @@ class DellSCSanAPITestCase(test.TestCase):
self.configuration.iscsi_ip_address = '192.168.1.1' self.configuration.iscsi_ip_address = '192.168.1.1'
self.configuration.iscsi_port = 3260 self.configuration.iscsi_port = 3260
self._context = context.get_admin_context() self._context = context.get_admin_context()
self.apiversion = '2.0'
# Set up the StorageCenterApi # Set up the StorageCenterApi
self.scapi = dell_storagecenter_api.StorageCenterApi( self.scapi = dell_storagecenter_api.StorageCenterApi(
@ -1669,7 +1670,8 @@ class DellSCSanAPITestCase(test.TestCase):
self.configuration.dell_sc_api_port, self.configuration.dell_sc_api_port,
self.configuration.san_login, self.configuration.san_login,
self.configuration.san_password, self.configuration.san_password,
self.configuration.dell_sc_verify_cert) self.configuration.dell_sc_verify_cert,
self.apiversion)
# Set up the scapi configuration vars # Set up the scapi configuration vars
self.scapi.ssn = self.configuration.dell_sc_ssn self.scapi.ssn = self.configuration.dell_sc_ssn
@ -6453,6 +6455,7 @@ class DellSCSanAPIConnectionTestCase(test.TestCase):
self.configuration.iscsi_ip_address = '192.168.1.1' self.configuration.iscsi_ip_address = '192.168.1.1'
self.configuration.iscsi_port = 3260 self.configuration.iscsi_port = 3260
self._context = context.get_admin_context() self._context = context.get_admin_context()
self.apiversion = '2.0'
# Set up the StorageCenterApi # Set up the StorageCenterApi
self.scapi = dell_storagecenter_api.StorageCenterApi( self.scapi = dell_storagecenter_api.StorageCenterApi(
@ -6460,7 +6463,8 @@ class DellSCSanAPIConnectionTestCase(test.TestCase):
self.configuration.dell_sc_api_port, self.configuration.dell_sc_api_port,
self.configuration.san_login, self.configuration.san_login,
self.configuration.san_password, self.configuration.san_password,
self.configuration.dell_sc_verify_cert) self.configuration.dell_sc_verify_cert,
self.apiversion)
# Set up the scapi configuration vars # Set up the scapi configuration vars
self.scapi.ssn = self.configuration.dell_sc_ssn self.scapi.ssn = self.configuration.dell_sc_ssn
@ -6482,10 +6486,32 @@ class DellSCSanAPIConnectionTestCase(test.TestCase):
@mock.patch.object(dell_storagecenter_api.HttpClient, @mock.patch.object(dell_storagecenter_api.HttpClient,
'post', 'post',
return_value=RESPONSE_400) return_value=RESPONSE_400)
@mock.patch.object(dell_storagecenter_api.StorageCenterApi,
'_check_version_fail',
return_value=RESPONSE_400)
def test_open_connection_failure(self, def test_open_connection_failure(self,
mock_check_version_fail,
mock_post): mock_post):
self.assertRaises(exception.VolumeBackendAPIException, self.assertRaises(exception.VolumeBackendAPIException,
self.scapi.open_connection) self.scapi.open_connection)
self.assertTrue(mock_check_version_fail.called)
@mock.patch.object(dell_storagecenter_api.StorageCenterApi,
'_check_version_fail',
return_value=RESPONSE_200)
@mock.patch.object(dell_storagecenter_api.StorageCenterApi,
'_get_json',
return_value=APIDICT)
@mock.patch.object(dell_storagecenter_api.HttpClient,
'post',
return_value=RESPONSE_400)
def test_open_connection_sc(self,
mock_post,
mock_get_json,
mock_check_version_fail):
self.scapi.open_connection()
self.assertTrue(mock_check_version_fail.called)
@mock.patch.object(dell_storagecenter_api.HttpClient, @mock.patch.object(dell_storagecenter_api.HttpClient,
'post', 'post',

68
cinder/volume/drivers/dell/dell_storagecenter_api.py
View File

@ -75,7 +75,7 @@ class HttpClient(object):
Helper for making the REST calls. Helper for making the REST calls.
""" """
def __init__(self, host, port, user, password, verify): def __init__(self, host, port, user, password, verify, apiversion):
"""HttpClient handles the REST requests. """HttpClient handles the REST requests.
:param host: IP address of the Dell Data Collector. :param host: IP address of the Dell Data Collector.
@ -84,6 +84,7 @@ class HttpClient(object):
:param password: Password. :param password: Password.
:param verify: Boolean indicating whether certificate verification :param verify: Boolean indicating whether certificate verification
should be turned on or not. should be turned on or not.
:param apiversion: Dell API version.
""" """
self.baseUrl = 'https://%s:%s/api/rest/' % (host, port) self.baseUrl = 'https://%s:%s/api/rest/' % (host, port)
@ -93,7 +94,7 @@ class HttpClient(object):
self.header = {} self.header = {}
self.header['Content-Type'] = 'application/json; charset=utf-8' self.header['Content-Type'] = 'application/json; charset=utf-8'
self.header['Accept'] = 'application/json' self.header['Accept'] = 'application/json'
self.header['x-dell-api-version'] = '2.0' self.header['x-dell-api-version'] = apiversion
self.verify = verify self.verify = verify
# Verify is a configurable option. So if this is false do not # Verify is a configurable option. So if this is false do not
@ -164,6 +165,7 @@ class StorageCenterApiHelper(object):
# Use that if set. Mark the backend as failed over. # Use that if set. Mark the backend as failed over.
self.active_backend_id = active_backend_id self.active_backend_id = active_backend_id
self.ssn = self.config.dell_sc_ssn self.ssn = self.config.dell_sc_ssn
self.apiversion = '2.0'
def open_connection(self): def open_connection(self):
"""Creates the StorageCenterApi object. """Creates the StorageCenterApi object.
@ -181,7 +183,8 @@ class StorageCenterApiHelper(object):
self.config.dell_sc_api_port, self.config.dell_sc_api_port,
self.config.san_login, self.config.san_login,
self.config.san_password, self.config.san_password,
self.config.dell_sc_verify_cert) self.config.dell_sc_verify_cert,
self.apiversion)
# This instance is for a single backend. That backend has a # This instance is for a single backend. That backend has a
# few items of information we should save rather than passing them # few items of information we should save rather than passing them
# about. # about.
@ -197,6 +200,12 @@ class StorageCenterApiHelper(object):
connection.failed_over = False connection.failed_over = False
# Open connection. # Open connection.
connection.open_connection() connection.open_connection()
# Save our api version for next time.
if self.apiversion != connection.apiversion:
LOG.info(_LI('open_connection: Updating API version to %s'),
connection.apiversion)
self.apiversion = connection.apiversion
else: else:
raise exception.VolumeBackendAPIException( raise exception.VolumeBackendAPIException(
data=_('Configuration error: dell_sc_ssn not set.')) data=_('Configuration error: dell_sc_ssn not set.'))
@ -222,9 +231,10 @@ class StorageCenterApi(object):
2.4.1 - Updated Replication support to V2.1. 2.4.1 - Updated Replication support to V2.1.
2.5.0 - ManageableSnapshotsVD implemented. 2.5.0 - ManageableSnapshotsVD implemented.
""" """
APIVERSION = '2.5.0'
def __init__(self, host, port, user, password, verify): APIDRIVERVERSION = '2.5.0'
def __init__(self, host, port, user, password, verify, apiversion):
"""This creates a connection to Dell SC or EM. """This creates a connection to Dell SC or EM.
:param host: IP address of the REST interface.. :param host: IP address of the REST interface..
@ -233,6 +243,7 @@ class StorageCenterApi(object):
:param password: Password. :param password: Password.
:param verify: Boolean indicating whether certificate verification :param verify: Boolean indicating whether certificate verification
should be turned on or not. should be turned on or not.
:param apiversion: Version used on login.
""" """
self.notes = 'Created by Dell Cinder Driver' self.notes = 'Created by Dell Cinder Driver'
self.repl_prefix = 'Cinder repl of ' self.repl_prefix = 'Cinder repl of '
@ -242,14 +253,12 @@ class StorageCenterApi(object):
self.sfname = 'openstack' self.sfname = 'openstack'
self.legacypayloadfilters = False self.legacypayloadfilters = False
self.consisgroups = True self.consisgroups = True
self.apiversion = apiversion
# Nothing other than Replication should care if we are direct connect # Nothing other than Replication should care if we are direct connect
# or not. # or not.
self.is_direct_connect = False self.is_direct_connect = False
self.client = HttpClient(host, self.client = HttpClient(host, port, user, password,
port, verify, apiversion)
user,
password,
verify)
def __enter__(self): def __enter__(self):
return self return self
@ -262,7 +271,6 @@ class StorageCenterApi(object):
"""Checks and logs API responses. """Checks and logs API responses.
:param rest_response: The result from a REST API call. :param rest_response: The result from a REST API call.
:param expected_response: The expected result.
:returns: ``True`` if success, ``False`` otherwise. :returns: ``True`` if success, ``False`` otherwise.
""" """
if 200 <= rest_response.status_code < 300: if 200 <= rest_response.status_code < 300:
@ -402,18 +410,45 @@ class StorageCenterApi(object):
return LegacyPayloadFilter(filterType) return LegacyPayloadFilter(filterType)
return PayloadFilter(filterType) return PayloadFilter(filterType)
def _check_version_fail(self, payload, response):
try:
# Is it even our error?
if response.text.startswith('Invalid API version specified, '
'the version must be in the range ['):
# We're looking for something very specific. The except
# will catch any errors.
# Update our version and update our header.
self.apiversion = response.text.split('[')[1].split(',')[0]
self.client.header['x-dell-api-version'] = self.apiversion
LOG.debug('API version updated to %s', self.apiversion)
# Give login another go.
r = self.client.post('ApiConnection/Login', payload)
return r
except Exception:
# We don't care what failed. The clues are already in the logs.
# Just log a parsing error and move on.
LOG.error(_LE('_check_version_fail: Parsing error.'))
# Just eat this if it isn't a version error.
return response
def open_connection(self): def open_connection(self):
"""Authenticate with Dell REST interface. """Authenticate with Dell REST interface.
:raises: VolumeBackendAPIException. :raises: VolumeBackendAPIException.
""" """
# Login
payload = {} payload = {}
payload['Application'] = 'Cinder REST Driver' payload['Application'] = 'Cinder REST Driver'
payload['ApplicationVersion'] = self.APIVERSION payload['ApplicationVersion'] = self.APIDRIVERVERSION
r = self.client.post('ApiConnection/Login', LOG.debug('open_connection %s',
payload) self.client.header['x-dell-api-version'])
r = self.client.post('ApiConnection/Login', payload)
if not self._check_result(r):
# SC requires a specific version. See if we can get it.
r = self._check_version_fail(payload, r)
# Either we tried to login and have a new result or we are
# just checking the same result. Either way raise on fail.
if not self._check_result(r): if not self._check_result(r):
raise exception.VolumeBackendAPIException( raise exception.VolumeBackendAPIException(
data=_('Failed to connect to Dell REST API')) data=_('Failed to connect to Dell REST API'))
@ -787,7 +822,8 @@ class StorageCenterApi(object):
This is the cinder volume ID. This is the cinder volume ID.
:param size: The size of the volume to be created in GB. :param size: The size of the volume to be created in GB.
:param storage_profile: Optional storage profile to set for the volume. :param storage_profile: Optional storage profile to set for the volume.
:param replay_profile: Optional replay profile to set for the volume. :param replay_profile_string: Optional replay profile to set for
the volume.
:returns: Dell Volume object or None. :returns: Dell Volume object or None.
""" """
LOG.debug('Create Volume %(name)s %(ssn)s %(folder)s %(profile)s', LOG.debug('Create Volume %(name)s %(ssn)s %(folder)s %(profile)s',