openstack/cinder
Code Issues Proposed changes

Dell PowerFlex: Added timeout into rest API call.

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

Closes-Bug: #2052995
Change-Id: I032d76627466f74121e3dc4fb2c8e175d830fa14
This commit is contained in:
Nilesh Thathagar 2024-02-22 11:20:34 +00:00
parent 4c4b51bb2f
commit cf83449bbd
7 changed files with 278 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
cinder/tests/unit/volume/drivers/dell_emc/powerflex/test_create_volume.py
View File

@ -16,6 +16,7 @@
from unittest import mock
import ddt
import requests.exceptions
from cinder import context
from cinder import exception
@ -104,3 +105,15 @@ class TestCreateVolume(powerflex.TestPowerFlexDriver):
self.driver._get_volumetype_extraspecs.return_value = extraspecs
self.assertRaises(exception.VolumeBackendAPIException,
self.test_create_volume)
@mock.patch("requests.post")
def test_volume_post_connect_timeout_request(self, mock_request):
mock_request.side_effect = requests.exceptions.ConnectTimeout()
self.assertRaises(exception.VolumeBackendAPIException,
self.test_create_volume)
@mock.patch("requests.post")
def test_volume_post_read_timeout_request(self, mock_request):
mock_request.side_effect = requests.exceptions.ReadTimeout()
self.assertRaises(exception.VolumeBackendAPIException,
self.test_create_volume)

167
cinder/tests/unit/volume/drivers/dell_emc/powerflex/test_power_flex_client.py Normal file
View File

@ -0,0 +1,167 @@
# Copyright (c) 2021 Dell Inc. or its subsidiaries.
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import http.client as http_client
import json
from unittest import mock
import ddt
import requests.exceptions
from requests.models import Response
from cinder.tests.unit.volume.drivers.dell_emc import powerflex
from cinder.volume import configuration as conf
from cinder.volume.drivers.dell_emc.powerflex import driver
from cinder.volume.drivers.dell_emc.powerflex import rest_client
@ddt.ddt
class TestPowerFlexClient(powerflex.TestPowerFlexDriver):
params = {'protectionDomainId': '1',
'storagePoolId': '1',
'name': 'HlF355XlSg+xcORfS0afag==',
'volumeType': 'ThinProvisioned',
'volumeSizeInKb': '1048576',
'compressionMethod': 'None'}
expected_status_code = 500
def setUp(self):
super(TestPowerFlexClient, self).setUp()
self.configuration = conf.Configuration(driver.powerflex_opts,
conf.SHARED_CONF_GROUP)
self._set_overrides()
self.client = rest_client.RestClient(self.configuration)
self.client.do_setup()
def _set_overrides(self):
# Override the defaults to fake values
self.override_config('san_ip', override='127.0.0.1',
group=conf.SHARED_CONF_GROUP)
self.override_config('powerflex_rest_server_port', override='8888',
group=conf.SHARED_CONF_GROUP)
self.override_config('san_login', override='test',
group=conf.SHARED_CONF_GROUP)
self.override_config('san_password', override='pass',
group=conf.SHARED_CONF_GROUP)
self.override_config('powerflex_storage_pools',
override='PD1:SP1',
group=conf.SHARED_CONF_GROUP)
self.override_config('max_over_subscription_ratio',
override=5.0, group=conf.SHARED_CONF_GROUP)
self.override_config('powerflex_server_api_version',
override='2.0.0', group=conf.SHARED_CONF_GROUP)
self.override_config('rest_api_connect_timeout',
override=120, group=conf.SHARED_CONF_GROUP)
self.override_config('rest_api_read_timeout',
override=120, group=conf.SHARED_CONF_GROUP)
@mock.patch("requests.get")
def test_rest_get_request_connect_timeout_exception(self, mock_request):
mock_request.side_effect = (requests.
exceptions.ConnectTimeout
('Fake Connect Timeout Exception'))
r, res = (self.client.
execute_powerflex_get_request(url="/version", **{}))
self.assertEqual(self.expected_status_code, r.status_code)
self.assertEqual(self.expected_status_code, res['errorCode'])
(self.assertEqual
('The request to URL /version failed with timeout exception '
'Fake Connect Timeout Exception', res['message']))
@mock.patch("requests.get")
def test_rest_get_request_read_timeout_exception(self, mock_request):
mock_request.side_effect = (requests.exceptions.ReadTimeout
('Fake Read Timeout Exception'))
r, res = (self.client.
execute_powerflex_get_request(url="/version", **{}))
self.assertEqual(self.expected_status_code, r.status_code)
self.assertEqual(self.expected_status_code, res['errorCode'])
(self.assertEqual
('The request to URL /version failed with timeout exception '
'Fake Read Timeout Exception', res['message']))
@mock.patch("requests.post")
def test_rest_post_request_connect_timeout_exception(self, mock_request):
mock_request.side_effect = (requests.exceptions.ConnectTimeout
('Fake Connect Timeout Exception'))
r, res = (self.client.execute_powerflex_post_request
(url="/types/Volume/instances", params=self.params, **{}))
self.assertEqual(self.expected_status_code, r.status_code)
self.assertEqual(self.expected_status_code, res['errorCode'])
(self.assertEqual
('The request to URL /types/Volume/instances failed with '
'timeout exception Fake Connect Timeout Exception', res['message']))
@mock.patch("requests.post")
def test_rest_post_request_read_timeout_exception(self, mock_request):
mock_request.side_effect = (requests.exceptions.ReadTimeout
('Fake Read Timeout Exception'))
r, res = (self.client.execute_powerflex_post_request
(url="/types/Volume/instances", params=self.params, **{}))
self.assertEqual(self.expected_status_code, r.status_code)
self.assertEqual(self.expected_status_code, res['errorCode'])
(self.assertEqual
('The request to URL /types/Volume/instances failed with '
'timeout exception Fake Read Timeout Exception', res['message']))
@mock.patch("requests.get")
def test_response_check_read_timeout_exception_1(self, mock_request):
r = requests.Response
r.status_code = http_client.UNAUTHORIZED
mock_request.side_effect = [r, (requests.exceptions.ReadTimeout
('Fake Read Timeout Exception'))]
r, res = (self.client.
execute_powerflex_get_request(url="/version", **{}))
self.assertEqual(self.expected_status_code, r.status_code)
self.assertEqual(self.expected_status_code, res['errorCode'])
(self.assertEqual
('The request to URL /version failed with '
'timeout exception Fake Read Timeout Exception', res['message']))
@mock.patch("requests.get")
def test_response_check_read_timeout_exception_2(self, mock_request):
res1 = requests.Response
res1.status_code = http_client.UNAUTHORIZED
res2 = Response()
res2.status_code = 200
res2._content = str.encode(json.dumps('faketoken'))
mock_request.side_effect = [res1, res2,
(requests.exceptions.ReadTimeout
('Fake Read Timeout Exception'))]
r, res = (self.client.
execute_powerflex_get_request(url="/version", **{}))
self.assertEqual(self.expected_status_code, r.status_code)
self.assertEqual(self.expected_status_code, res['errorCode'])
(self.assertEqual
('The request to URL /version failed with '
'timeout exception Fake Read Timeout Exception', res['message']))
@mock.patch("requests.post")
@mock.patch("requests.get")
def test_response_check_read_timeout_exception_3(self, mock_post_request,
mock_get_request):
r = requests.Response
r.status_code = http_client.UNAUTHORIZED
mock_post_request.side_effect = r
mock_get_request.side_effect = (requests.exceptions.ReadTimeout
('Fake Read Timeout Exception'))
r, res = (self.client.execute_powerflex_post_request
(url="/types/Volume/instances", params=self.params, **{}))
self.assertEqual(self.expected_status_code, r.status_code)
self.assertEqual(self.expected_status_code, res['errorCode'])
(self.assertEqual
('The request to URL /types/Volume/instances failed with '
'timeout exception Fake Read Timeout Exception', res['message']))

14
cinder/tests/unit/volume/drivers/dell_emc/powerflex/test_versions.py
View File

@ -12,8 +12,10 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from unittest import mock
import ddt
import requests.exceptions
from cinder import exception
from cinder.tests.unit.volume.drivers.dell_emc import powerflex
@ -91,3 +93,15 @@ class TestMultipleVersions(powerflex.TestPowerFlexDriver):
self.driver.primary_client.query_rest_api_version(False),
vers
)
@mock.patch("requests.get")
def test_get_version_connect_timeout_request(self, mock_request):
mock_request.side_effect = requests.exceptions.ConnectTimeout()
self.assertRaises(exception.VolumeBackendAPIException,
self.test_version)
@mock.patch("requests.get")
def test_get_version_read_timeout_request(self, mock_request):
mock_request.side_effect = requests.exceptions.ReadTimeout()
self.assertRaises(exception.VolumeBackendAPIException,
self.test_version)

10
cinder/volume/drivers/dell_emc/powerflex/options.py
View File

@ -19,6 +19,8 @@ named Dell EMC VxFlex OS).
from oslo_config import cfg
from cinder.volume.drivers.dell_emc.powerflex import utils as flex_utils
# deprecated options
VXFLEXOS_REST_SERVER_PORT = "vxflexos_rest_server_port"
VXFLEXOS_ROUND_VOLUME_CAPACITY = "vxflexos_round_volume_capacity"
@ -147,4 +149,12 @@ actual_opts = [
default=False,
help='Allow volume migration during rebuild.',
deprecated_name=VXFLEXOS_ALLOW_MIGRATION_DURING_REBUILD),
cfg.IntOpt(flex_utils.POWERFLEX_REST_CONNECT_TIMEOUT,
default=30, min=1,
help='Use this value to specify connect '
'timeout value (in seconds) for rest call.'),
cfg.IntOpt(flex_utils.POWERFLEX_REST_READ_TIMEOUT,
default=30, min=1,
help='Use this value to specify read '
'timeout value (in seconds) for rest call.')
]

84
cinder/volume/drivers/dell_emc/powerflex/rest_client.py
View File

@ -21,6 +21,7 @@ import urllib.parse
from oslo_log import log as logging
from oslo_utils import units
import requests
from requests.exceptions import Timeout
from cinder import exception
from cinder.i18n import _
@ -60,6 +61,8 @@ class RestClient(object):
self.certificate_path = None
self.base_url = None
self.is_configured = False
self.rest_api_connect_timeout = 30
self.rest_api_read_timeout = 30
@staticmethod
def _get_headers():
@ -103,6 +106,12 @@ class RestClient(object):
)
self.rest_username = get_config_value("san_login")
self.rest_password = get_config_value("san_password")
self.rest_api_connect_timeout = (
get_config_value(flex_utils.POWERFLEX_REST_CONNECT_TIMEOUT)
or self.rest_api_connect_timeout)
self.rest_api_read_timeout = (
get_config_value(flex_utils.POWERFLEX_REST_READ_TIMEOUT)
or self.rest_api_read_timeout)
if self.verify_certificate:
self.certificate_path = (
get_config_value("sio_server_certificate_path") or
@ -125,13 +134,17 @@ class RestClient(object):
"server_port": self.rest_port
})
LOG.info("REST server IP: %(ip)s, port: %(port)s, "
"username: %(user)s. Verify server's certificate: "
"username: %(user)s, rest connect timeout: "
"%(rest_api_connect_timeout)s, rest read timeout: "
"%(rest_api_read_timeout)s. Verify server's certificate: "
"%(verify_cert)s.",
{
"ip": self.rest_ip,
"port": self.rest_port,
"user": self.rest_username,
"verify_cert": self.verify_certificate,
"rest_api_connect_timeout": self.rest_api_connect_timeout,
"rest_api_read_timeout": self.rest_api_read_timeout
})
self.is_configured = True
@ -454,29 +467,50 @@ class RestClient(object):
return verify_cert
def execute_powerflex_get_request(self, url, **url_params):
request = self.base_url + url % url_params
r = requests.get(request,
auth=(self.rest_username, self.rest_token),
verify=self._get_verify_cert())
r = self._check_response(r, request)
response = r.json()
r = requests.Response
try:
request = self.base_url + url % url_params
timeout = (self.rest_api_connect_timeout,
self.rest_api_read_timeout)
r = requests.get(request,
auth=(self.rest_username, self.rest_token),
verify=self._get_verify_cert(), timeout=timeout)
r = self._check_response(r, request)
response = r.json()
except Timeout as e:
r.status_code = http_client.INTERNAL_SERVER_ERROR
msg = _("The request to URL %s failed with timeout "
"exception %s" % (url, str(e)))
LOG.error(msg)
response = {'errorCode': http_client.INTERNAL_SERVER_ERROR,
'message': msg}
return r, response
def execute_powerflex_post_request(self, url, params=None, **url_params):
if not params:
params = {}
request = self.base_url + url % url_params
r = requests.post(request,
data=json.dumps(params),
headers=self._get_headers(),
auth=(self.rest_username, self.rest_token),
verify=self._get_verify_cert())
r = self._check_response(r, request, False, params)
response = None
r = requests.Response
try:
response = r.json()
except ValueError:
response = None
if not params:
params = {}
request = self.base_url + url % url_params
timeout = (self.rest_api_connect_timeout,
self.rest_api_read_timeout)
r = requests.post(request,
data=json.dumps(params),
headers=self._get_headers(),
auth=(self.rest_username, self.rest_token),
verify=self._get_verify_cert(), timeout=timeout)
r = self._check_response(r, request, False, params)
try:
response = r.json()
except ValueError:
response = None
except Timeout as e:
r.status_code = http_client.INTERNAL_SERVER_ERROR
msg = _("The request to URL %s failed with timeout "
"exception %s" % (url, str(e)))
LOG.error(msg)
response = {'errorCode': http_client.INTERNAL_SERVER_ERROR,
'message': msg}
return r, response
def _check_response(self,
@ -492,9 +526,11 @@ class RestClient(object):
"a new one.")
login_request = self.base_url + login_url
verify_cert = self._get_verify_cert()
timeout = (self.rest_api_connect_timeout,
self.rest_api_read_timeout)
r = requests.get(login_request,
auth=(self.rest_username, self.rest_password),
verify=verify_cert)
verify=verify_cert, timeout=timeout)
token = r.json()
self.rest_token = token
# Repeat request with valid token.
@ -506,7 +542,8 @@ class RestClient(object):
self.rest_username,
self.rest_token
),
verify=verify_cert)
verify=verify_cert,
timeout=timeout)
else:
response = requests.post(request,
data=json.dumps(params),
@ -515,7 +552,8 @@ class RestClient(object):
self.rest_username,
self.rest_token
),
verify=verify_cert)
verify=verify_cert,
timeout=timeout)
level = logging.DEBUG
# for anything other than an OK from the REST API, log an error
if response.status_code != http_client.OK:

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

@ -21,6 +21,8 @@ from oslo_utils import units
from packaging import version
LOG = logging.getLogger(__name__)
POWERFLEX_REST_CONNECT_TIMEOUT = "rest_api_connect_timeout"
POWERFLEX_REST_READ_TIMEOUT = "rest_api_read_timeout"
def version_gte(ver1, ver2):

11
releasenotes/notes/bug-2052995-dell-powerflex-rest-api-timeout-3a05b6b5d5460176.yaml Normal file
View File

@ -0,0 +1,11 @@
---
fixes:
- |
PowerFlex Driver `bug #2052995
<https://bugs.launchpad.net/cinder/+bug/2052995>`_: REST
API calls to the PowerFlex 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.