openstack/cinder
Code Issues Proposed changes

Merge "Dell PowerStore: Added timeout into rest API call."

Browse Source
This commit is contained in:
Zuul 2024-09-04 23:39:56 +00:00 committed by Gerrit Code Review
commit 25699617a1
7 changed files with 144 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
cinder/tests/unit/volume/drivers/dell_emc/powerstore/test_client.py
View File

@ -17,6 +17,7 @@ from unittest import mock
import uuid
import ddt
import requests.exceptions
from cinder import exception
from cinder.tests.unit import test
@ -31,7 +32,9 @@ CLIENT_OPTIONS = {
"rest_username": "fake_user",
"rest_password": "fake_password",
"verify_certificate": False,
"certificate_path": None
"certificate_path": None,
"rest_api_connect_timeout": 60,
"rest_api_read_timeout": 60
}
ISCSI_IP_POOL_RESP = [
@ -277,3 +280,52 @@ class TestClient(test.TestCase):
self.client.update_qos_io_rule,
"io-rule-6b6e5489-4b5b-4468-a1f7-32cec2ffa3bf",
QOS_UPDATE_IO_RULE_PARAMS)
@mock.patch("requests.request")
def test_get_request_timeout_exception(self, mock_request):
mock_request.return_value = MockResponse(
rc=501
)
error = self.assertRaises(
exception.VolumeBackendAPIException,
self.client.get_array_version)
self.assertEqual('Bad or unexpected response from the '
'storage volume backend API: Failed to '
'query PowerStore array version.',
error.msg)
@mock.patch("requests.request")
def test_send_get_request_connect_timeout_exception(self,
mock_request):
mock_request.side_effect = requests.exceptions.ConnectTimeout()
r, resp = self.client._send_request("GET",
"/api/version")
self.assertEqual(500, r.status_code)
@mock.patch("requests.request")
def test_send_get_request_read_timeout_exception(self,
mock_request):
mock_request.side_effect = requests.exceptions.ReadTimeout()
r, resp = self.client._send_request("GET",
"/api/version")
self.assertEqual(500, r.status_code)
@mock.patch("requests.request")
def test_send_post_request_connect_timeout_exception(self,
mock_request):
params = {}
mock_request.side_effect = requests.exceptions.ConnectTimeout()
r, resp = self.client._send_request("POST",
"/metrics/generate",
params)
self.assertEqual(500, r.status_code)
@mock.patch("requests.request")
def test_send_post_request_read_timeout_exception(self,
mock_request):
params = {}
mock_request.side_effect = requests.exceptions.ReadTimeout()
r, resp = self.client._send_request("POST",
"/metrics/generate",
params)
self.assertEqual(500, r.status_code)

14
cinder/tests/unit/volume/drivers/dell_emc/powerstore/test_volume_create_delete_extend.py
View File

@ -157,3 +157,17 @@ class TestVolumeCreateDeleteExtend(powerstore.TestPowerStoreDriver):
self.volume,
16)
self.assertIn("Failed to extend PowerStore volume", error.msg)
@mock.patch("requests.request")
def test_post_request_timeout_exception(self, mock_request):
mock_request.return_value = powerstore.MockResponse(
rc=501
)
error = self.assertRaises(exception.VolumeBackendAPIException,
self.driver.extend_volume,
self.volume,
16)
self.assertEqual('Bad or unexpected response from the '
'storage volume backend API: Failed to '
'extend PowerStore volume with id fake_id.',
error.msg)

75
cinder/volume/drivers/dell_emc/powerstore/client.py
View File

@ -21,6 +21,7 @@ import json
from oslo_log import log as logging
from oslo_utils import strutils
import requests
import requests.exceptions
from cinder import exception
from cinder.i18n import _
@ -45,7 +46,9 @@ class PowerStoreClient(object):
rest_username,
rest_password,
verify_certificate,
certificate_path):
certificate_path,
rest_api_connect_timeout,
rest_api_read_timeout):
self.rest_ip = rest_ip
self.rest_username = rest_username
self.rest_password = rest_password
@ -59,6 +62,8 @@ class PowerStoreClient(object):
requests.codes.no_content,
requests.codes.partial_content
]
self.rest_api_connect_timeout = rest_api_connect_timeout
self.rest_api_read_timeout = rest_api_read_timeout
@property
def _verify_cert(self):
@ -92,36 +97,46 @@ class PowerStoreClient(object):
payload=None,
params=None,
log_response_data=True):
if not params:
params = {}
request_params = {
"auth": (self.rest_username, self.rest_password),
"verify": self._verify_cert,
"params": params
}
if payload and method != "GET":
request_params["data"] = json.dumps(payload)
request_url = self.base_url + url
r = requests.request(method, request_url, **request_params)
log_level = logging.DEBUG
if r.status_code not in self.ok_codes:
log_level = logging.ERROR
LOG.log(log_level,
"REST Request: %s %s with body %s",
r.request.method,
r.request.url,
strutils.mask_password(r.request.body))
if log_response_data or log_level == logging.ERROR:
msg = "REST Response: %s with data %s" % (r.status_code, r.text)
else:
msg = "REST Response: %s" % r.status_code
LOG.log(log_level, msg)
response = None
r = requests.Response
try:
response = r.json()
except ValueError:
response = None
if not params:
params = {}
request_params = {
"auth": (self.rest_username, self.rest_password),
"verify": self._verify_cert,
"params": params
}
if payload and method != "GET":
request_params["data"] = json.dumps(payload)
request_url = self.base_url + url
timeout = (self.rest_api_connect_timeout,
self.rest_api_read_timeout)
r = requests.request(method, request_url, **request_params,
timeout=timeout)
log_level = logging.DEBUG
if r.status_code not in self.ok_codes:
log_level = logging.ERROR
LOG.log(log_level,
"REST Request: %s %s with body %s",
r.request.method,
r.request.url,
strutils.mask_password(r.request.body))
if (log_response_data or
log_level == logging.ERROR):
msg = ("REST Response: %s with data %s" %
(r.status_code, r.text))
else:
msg = "REST Response: %s" % r.status_code
LOG.log(log_level, msg)
try:
response = r.json()
except ValueError:
response = None
except requests.exceptions.Timeout as e:
r.status_code = requests.codes.internal_server_error
LOG.error("The request to URL %(url)s failed with timeout "
"exception %(exc)s", {"url": url, "exc": e})
return r, response
_send_get_request = functools.partialmethod(_send_request, "GET")

4
cinder/volume/drivers/dell_emc/powerstore/driver.py
View File

@ -306,4 +306,8 @@ class PowerStoreDriver(driver.VolumeDriver):
conf["rest_password"] = get_value("san_password")
conf["verify_certificate"] = get_value("driver_ssl_cert_verify")
conf["certificate_path"] = get_value("driver_ssl_cert_path")
conf["rest_api_connect_timeout"] = (
self.configuration.safe_get(utils.POWERSTORE_REST_CONNECT_TIMEOUT))
conf["rest_api_read_timeout"] = (
self.configuration.safe_get(utils.POWERSTORE_REST_READ_TIMEOUT))
return conf

15
cinder/volume/drivers/dell_emc/powerstore/options.py
View File

@ -17,6 +17,8 @@
from oslo_config import cfg
from cinder.volume.drivers.dell_emc.powerstore import utils as store_utils
POWERSTORE_APPLIANCES = "powerstore_appliances"
POWERSTORE_PORTS = "powerstore_ports"
POWERSTORE_NVME = "powerstore_nvme"
@ -39,5 +41,16 @@ POWERSTORE_OPTS = [
),
cfg.BoolOpt(POWERSTORE_NVME,
default=False,
help="Connect PowerStore volumes using NVMe-OF.")
help="Connect PowerStore volumes using NVMe-OF."),
cfg.IntOpt(store_utils.POWERSTORE_REST_CONNECT_TIMEOUT,
default=30, min=1,
help='Use this value to specify the connect '
'timeout value (in seconds) for REST API calls '
'to the PowerStore backend.'),
cfg.IntOpt(store_utils.POWERSTORE_REST_READ_TIMEOUT,
default=30, min=1,
help='Use this value to specify the read '
'timeout value (in seconds) for REST API calls '
'to the PowerStore backend.')
]

2
cinder/volume/drivers/dell_emc/powerstore/utils.py
View File

@ -38,6 +38,8 @@ PROTOCOL_NVME = "NVMe"
POWERSTORE_PP_KEY = "powerstore:protection_policy"
VOLUME_ATTACH_OPERATION = 1
VOLUME_DETACH_OPERATION = 2
POWERSTORE_REST_CONNECT_TIMEOUT = "rest_api_call_connect_timeout"
POWERSTORE_REST_READ_TIMEOUT = "rest_api_call_read_timeout"
def bytes_to_gib(size_in_bytes):

12
releasenotes/notes/bug-2055022-dell-powerstore-rest-api-timeout-51b3ae19266757f9.yaml Normal file
View File

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