Dell PowerStore: Added timeout into rest API call.
Added connect and read timeout into rest API call of PowerStore to avoid cinder hang issue. Deafult value of connect and read timeout is 30 seconds. Closes-Bug: #2055022 Change-Id: Id44a4b01c9ca238470e7d984d39b5e4c4883fd6a
This commit is contained in:
parent
a72c4894e9
commit
897980b252
@ -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)
|
||||
|
@ -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)
|
||||
|
@ -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,6 +97,9 @@ class PowerStoreClient(object):
|
||||
payload=None,
|
||||
params=None,
|
||||
log_response_data=True):
|
||||
response = None
|
||||
r = requests.Response
|
||||
try:
|
||||
if not params:
|
||||
params = {}
|
||||
request_params = {
|
||||
@ -102,8 +110,10 @@ class PowerStoreClient(object):
|
||||
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)
|
||||
|
||||
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
|
||||
@ -112,16 +122,21 @@ class PowerStoreClient(object):
|
||||
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)
|
||||
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")
|
||||
|
@ -266,4 +266,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
|
||||
|
@ -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.')
|
||||
|
||||
]
|
||||
|
@ -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):
|
||||
|
@ -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.
|
Loading…
Reference in New Issue
Block a user