openstack/cinder
Code Issues Proposed changes

Dell PowerMax: Added timeout into rest API call.

Browse Source 
Added connect and read timeout into rest API call
of PowerMax to avoid cinder hang issue.
Deafult value of connect and read timeout
is 30 seconds.

Closes-Bug: #2051830
Change-Id: I2d419b4257bae75c69577a34758910c4889e2507
This commit is contained in:
Nilesh Thathagar 2024-02-09 09:39:49 +00:00
parent b8cd101fe4
commit 5082cb3e07
7 changed files with 139 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_fake_objects.py
View File

@ -54,7 +54,7 @@ class FakeRequestsSession(object):
def __init__(self, *args, **kwargs):
self.data = tpd.PowerMaxData()
def request(self, method, url, params=None, data=None):
def request(self, method, url, params=None, data=None, timeout=None):
return_object = ''
status_code = 200
if method == 'GET':
@ -69,6 +69,12 @@ class FakeRequestsSession(object):
elif method == 'TIMEOUT':
raise requests.Timeout
elif method == 'READTIMEOUT' and timeout is not None:
raise requests.ReadTimeout
elif method == 'CONNECTTIMEOUT' and timeout is not None:
raise requests.ConnectTimeout
elif method == 'EXCEPTION':
raise Exception
@ -326,6 +332,8 @@ class FakeConfiguration(object):
self.initiator_check = False
self.powermax_service_level = None
self.vmax_workload = None
self.rest_api_connect_timeout = 30
self.rest_api_read_timeout = 30
if replication_device:
self.replication_device = replication_device
for key, value in kwargs.items():
@ -422,3 +430,9 @@ class FakeConfiguration(object):
def append_config_values(self, values):
pass
def set_rest_api_connect_timeout(self, value):
self.rest_api_connect_timeout = value
def set_rest_api_read_timeout(self, value):
self.rest_api_read_timeout = value

71
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py
View File

@ -3217,7 +3217,8 @@ class PowerMaxCommonTest(test.TestCase):
{'RestServerIp': '1.1.1.1', 'RestServerPort': 8443,
'RestUserName': 'smc', 'RestPassword': 'smc', 'SSLVerify': False,
'SerialNumber': self.data.array, 'srpName': 'SRP_1',
'PortGroup': [self.data.port_group_name_i]})
'PortGroup': [self.data.port_group_name_i],
'RestAPIConnectTimeout': 30, 'RestAPIReadTimeout': 30})
old_conf = tpfo.FakeConfiguration(None, 'CommonTests', 1, 1)
configuration = tpfo.FakeConfiguration(
None, 'CommonTests', 1, 1, san_ip='1.1.1.1', san_login='smc',
@ -3236,7 +3237,8 @@ class PowerMaxCommonTest(test.TestCase):
{'RestServerIp': '1.1.1.1', 'RestServerPort': 3448,
'RestUserName': 'smc', 'RestPassword': 'smc', 'SSLVerify': False,
'SerialNumber': self.data.array, 'srpName': 'SRP_1',
'PortGroup': [self.data.port_group_name_i]})
'PortGroup': [self.data.port_group_name_i],
'RestAPIConnectTimeout': 30, 'RestAPIReadTimeout': 30})
configuration = tpfo.FakeConfiguration(
None, 'CommonTests', 1, 1, san_ip='1.1.1.1', san_login='smc',
powermax_array=self.data.array, powermax_srp='SRP_1',
@ -3251,7 +3253,8 @@ class PowerMaxCommonTest(test.TestCase):
{'RestServerIp': '1.1.1.1', 'RestServerPort': 8443,
'RestUserName': 'smc', 'RestPassword': 'smc', 'SSLVerify': False,
'SerialNumber': self.data.array, 'srpName': 'SRP_1',
'PortGroup': [self.data.port_group_name_i]})
'PortGroup': [self.data.port_group_name_i],
'RestAPIConnectTimeout': 30, 'RestAPIReadTimeout': 30})
configuration = tpfo.FakeConfiguration(
None, 'CommonTests', 1, 1, san_ip='1.1.1.1', san_login='smc',
powermax_array=self.data.array, powermax_srp='SRP_1',
@ -3606,7 +3609,8 @@ class PowerMaxCommonTest(test.TestCase):
'RestServerIp': '1.1.1.1', 'RestServerPort': 8443,
'RestUserName': 'smc', 'RestPassword': 'smc', 'SSLVerify': False,
'SerialNumber': '000197800123', 'srpName': 'SRP_1',
'PortGroup': ['OS-fibre-PG']}
'PortGroup': ['OS-fibre-PG'],
'RestAPIConnectTimeout': 30, 'RestAPIReadTimeout': 30}
self.mock_object(self.common.configuration, 'powermax_service_level',
None)
@ -3662,7 +3666,9 @@ class PowerMaxCommonTest(test.TestCase):
'RestServerIp': '1.1.1.1', 'RestServerPort': 8443,
'RestUserName': 'test', 'RestPassword': 'test',
'SerialNumber': '000197800123', 'srpName': 'SRP_1',
'PortGroup': None, 'SSLVerify': True}}
'PortGroup': None,
'RestAPIConnectTimeout': 30, 'RestAPIReadTimeout': 30,
'SSLVerify': True}}
self.mock_object(self.common, 'configuration', configuration)
self.common._get_u4p_failover_info()
self.assertIsNotNone(self.rest.u4p_failover_targets)
@ -4839,3 +4845,58 @@ class PowerMaxCommonTest(test.TestCase):
self.data.remote_array, self.data.srp, 'Diamond', 'DSS',
self.data.rep_extra_specs_rep_config, False, is_re=True,
rep_mode='Synchronous')
def test_get_connect_timeout_from_cinder_config(self):
kwargs_expected = (
{'RestServerIp': '1.1.1.1', 'RestServerPort': 3448,
'RestUserName': 'smc', 'RestPassword': 'smc', 'SSLVerify': False,
'SerialNumber': self.data.array, 'srpName': 'SRP_1',
'PortGroup': [self.data.port_group_name_i],
'RestAPIConnectTimeout': 120, 'RestAPIReadTimeout': 30})
configuration = tpfo.FakeConfiguration(
None, 'CommonTests',
1, 1, san_ip='1.1.1.1', san_login='smc',
powermax_array=self.data.array, powermax_srp='SRP_1',
san_password='smc', san_api_port=3448,
powermax_port_groups=[self.data.port_group_name_i])
configuration.set_rest_api_connect_timeout(120)
self.mock_object(self.common, 'configuration', configuration)
kwargs_returned = self.common.get_attributes_from_cinder_config()
self.assertEqual(kwargs_expected, kwargs_returned)
def test_get_read_timeout_from_cinder_config(self):
kwargs_expected = (
{'RestServerIp': '1.1.1.1', 'RestServerPort': 3448,
'RestUserName': 'smc', 'RestPassword': 'smc', 'SSLVerify': False,
'SerialNumber': self.data.array, 'srpName': 'SRP_1',
'PortGroup': [self.data.port_group_name_i],
'RestAPIConnectTimeout': 30, 'RestAPIReadTimeout': 120})
configuration = tpfo.FakeConfiguration(
None, 'CommonTests',
1, 1, san_ip='1.1.1.1', san_login='smc',
powermax_array=self.data.array, powermax_srp='SRP_1',
san_password='smc', san_api_port=3448,
powermax_port_groups=[self.data.port_group_name_i])
configuration.set_rest_api_read_timeout(120)
self.mock_object(self.common, 'configuration', configuration)
kwargs_returned = self.common.get_attributes_from_cinder_config()
self.assertEqual(kwargs_expected, kwargs_returned)
def test_get_connect_and_read_timeout_from_cinder_config(self):
kwargs_expected = (
{'RestServerIp': '1.1.1.1', 'RestServerPort': 3448,
'RestUserName': 'smc', 'RestPassword': 'smc', 'SSLVerify': False,
'SerialNumber': self.data.array, 'srpName': 'SRP_1',
'PortGroup': [self.data.port_group_name_i],
'RestAPIConnectTimeout': 90, 'RestAPIReadTimeout': 90})
configuration = tpfo.FakeConfiguration(
None, 'CommonTests',
1, 1, san_ip='1.1.1.1', san_login='smc',
powermax_array=self.data.array, powermax_srp='SRP_1',
san_password='smc', san_api_port=3448,
powermax_port_groups=[self.data.port_group_name_i])
configuration.set_rest_api_connect_timeout(90)
configuration.set_rest_api_read_timeout(90)
self.mock_object(self.common, 'configuration', configuration)
kwargs_returned = self.common.get_attributes_from_cinder_config()
self.assertEqual(kwargs_expected, kwargs_returned)

8
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py
View File

@ -63,6 +63,14 @@ class PowerMaxRestTest(test.TestCase):
self.assertRaises(requests.exceptions.Timeout,
self.rest.request, '', 'TIMEOUT')
def test_rest_request_read_timeout_exception(self):
self.assertRaises(requests.exceptions.ReadTimeout,
self.rest.request, '', 'READTIMEOUT', (60, 60))
def test_rest_request_connect_timeout_exception(self):
self.assertRaises(requests.exceptions.ConnectTimeout,
self.rest.request, '', 'CONNECTTIMEOUT', (60, 60))
def test_rest_request_connection_exception(self):
self.assertRaises(requests.exceptions.ConnectionError,
self.rest.request, '', 'CONNECTION')

18
cinder/volume/drivers/dell_emc/powermax/common.py
View File

@ -146,7 +146,15 @@ powermax_opts = [
help='Metric used for port group load calculation.'),
cfg.StrOpt(utils.PORT_LOAD_METRIC,
default='PercentBusy',
help='Metric used for port load calculation.')]
help='Metric used for port load calculation.'),
cfg.IntOpt(utils.REST_API_CONNECT_TIMEOUT,
default=30, min=1,
help='Use this value to specify connect '
'timeout value (in seconds) for rest call.'),
cfg.IntOpt(utils.REST_API_READ_TIMEOUT,
default=30, min=1,
help='Use this value to specify read '
'timeout value (in seconds) for rest call.')]
CONF.register_opts(powermax_opts, group=configuration.SHARED_CONF_GROUP)
@ -7122,6 +7130,10 @@ class PowerMaxCommon(object):
workload = self.configuration.safe_get(utils.VMAX_WORKLOAD)
port_groups = self.configuration.safe_get(
utils.POWERMAX_PORT_GROUPS)
rest_api_connect_timeout = (
self.configuration.safe_get(utils.REST_API_CONNECT_TIMEOUT))
rest_api_read_timeout = (
self.configuration.safe_get(utils.REST_API_READ_TIMEOUT))
kwargs = (
{'RestServerIp': self.configuration.safe_get(
@ -7131,7 +7143,9 @@ class PowerMaxCommon(object):
'RestPassword': password,
'SerialNumber': serial_number,
'srpName': srp_name,
'PortGroup': port_groups})
'PortGroup': port_groups,
utils.REST_API_CONNECT_TIMEOUT_KEY: rest_api_connect_timeout,
utils.REST_API_READ_TIMEOUT_KEY: rest_api_read_timeout})
if self.configuration.safe_get('driver_ssl_cert_verify'):
if self.configuration.safe_get('driver_ssl_cert_path'):

23
cinder/volume/drivers/dell_emc/powermax/rest.py
View File

@ -92,6 +92,8 @@ class PowerMaxRest(object):
self.ucode_minor_level = None
self.is_snap_id = False
self.u4p_version = None
self.rest_api_connect_timeout = 30
self.rest_api_read_timeout = 30
def set_rest_credentials(self, array_info):
"""Given the array record set the rest server credentials.
@ -107,6 +109,14 @@ class PowerMaxRest(object):
self.base_uri = ("https://%(ip_port)s/univmax/restapi" % {
'ip_port': ip_port})
self.session = self._establish_rest_session()
new_connect_timeout = (
int(array_info.get(utils.REST_API_CONNECT_TIMEOUT_KEY, 0)))
if new_connect_timeout > 0:
self.rest_api_connect_timeout = new_connect_timeout
new_read_timeout = (
int(array_info.get(utils.REST_API_READ_TIMEOUT_KEY, 0)))
if new_read_timeout > 0:
self.rest_api_read_timeout = new_read_timeout
def set_residuals(self, serial_number):
"""Set ucode and snapid information.
@ -242,18 +252,19 @@ class PowerMaxRest(object):
url = ("%(self.base_uri)s%(target_uri)s" % {
'self.base_uri': self.base_uri,
'target_uri': target_uri})
timeout = (self.rest_api_connect_timeout,
self.rest_api_read_timeout)
if request_object:
response = self.session.request(
method=method, url=url,
data=json.dumps(request_object, sort_keys=True,
indent=4))
indent=4), timeout=timeout)
elif params:
response = self.session.request(
method=method, url=url, params=params)
method=method, url=url, params=params, timeout=timeout)
else:
response = self.session.request(
method=method, url=url)
method=method, url=url, timeout=timeout)
status_code = response.status_code
if retry and status_code and status_code in [STATUS_200,
@ -288,6 +299,10 @@ class PowerMaxRest(object):
except (r_exc.Timeout, r_exc.ConnectionError,
r_exc.HTTPError) as e:
if isinstance(e, r_exc.Timeout):
msg = _("The %s request to URL %s failed with timeout "
"exception %s" % (method, url, str(e)))
LOG.error(msg)
if self.u4p_failover_enabled or u4p_check:
if not u4p_check:
# Failover process

4
cinder/volume/drivers/dell_emc/powermax/utils.py
View File

@ -136,6 +136,10 @@ POWERMAX_ARRAY_TAG_LIST = 'powermax_array_tag_list'
POWERMAX_SHORT_HOST_NAME_TEMPLATE = 'powermax_short_host_name_template'
POWERMAX_PORT_GROUP_NAME_TEMPLATE = 'powermax_port_group_name_template'
PORT_GROUP_LABEL = 'port_group_label'
REST_API_CONNECT_TIMEOUT = 'rest_api_connect_timeout'
REST_API_READ_TIMEOUT = 'rest_api_read_timeout'
REST_API_CONNECT_TIMEOUT_KEY = 'RestAPIConnectTimeout'
REST_API_READ_TIMEOUT_KEY = 'RestAPIReadTimeout'
# Array Models, Service Levels & Workloads
VMAX_HYBRID_MODELS = ['VMAX100K', 'VMAX200K', 'VMAX400K']

11
releasenotes/notes/bug-2051830-dell-powermax-rest-api-timeout-b70bd2754debf16a.yaml Normal file
View File

@ -0,0 +1,11 @@
---
fixes:
- |
PowerMax Driver `bug #2051830
<https://bugs.launchpad.net/cinder/+bug/2051830>`_: REST
API calls to the PowerMax backend did not have a timeout
set, which could result in cinder waiting forever.
This fix introduces two configuration options,
``rest_api_connect_timeout`` and ``rest_api_read_timeout``,
to control timeouts when connecting to the backend.
The default value of each is 30 seconds.