openstack/cinder
Code Issues Proposed changes

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

Browse Source
This commit is contained in:
Zuul 2024-02-29 10:09:30 +00:00 committed by Gerrit Code Review
commit ba424aae92
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.