openstack
/
sushy
Code Issues Proposed changes

Requests must always have a read/connect timeout 

Before this change, a request to a nonresponsive BMC may have hung
forever waiting for requests to connect and read. Now, we ensure all
calls have a timeout unless explicitly disabled by the API user.

Because timeout was already an argument for _op, anyone attempting
to previously override the requests timeout (send one at all) wouldn't
have been able to for pythonic reasons.

Closes-Bug: 2021995
Change-Id: I76c543c6e39bab81f46a7aa00f1d38328c30cf7b
This commit is contained in:
Jay Faulkner 2023-07-11 13:07:17 +01:00
parent f03d77ebcb
commit 7c2a2511fc
3 changed files with 21 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
releasenotes/notes/add-read-and-connect-timeout-9f7dc3ed63c192c8.yaml Normal file
View File

@ -0,0 +1,5 @@
fixes:
- Fixes bug where sushy would not pass a read/connect timeout through to
requests when making requests to a redfish service. This means that an
ill-timed failure could cause python processes calling sushy to freeze
indefinately.

9
sushy/connector.py
View File

@ -117,7 +117,11 @@ class Connector(object):
:param data: Optional JSON data.
:param headers: Optional dictionary of headers.
:param blocking: Whether to block for asynchronous operations.
:param timeout: Max time in seconds to wait for blocking async call.
:param timeout: Max time in seconds to wait for blocking async call or
for requests library to connect and read. If a custom
timeout for requests is provided in
extra_session_req_kwargs, it will be used instead for
those calls.
:param server_side_retries_left: Remaining retries. If not provided
will use limit provided by instance's server_side_retries
:param extra_session_req_kwargs: Optional keyword argument to pass
@ -127,7 +131,6 @@ class Connector(object):
:raises: ConnectionError
:raises: HTTPError
"""
if server_side_retries_left is None:
server_side_retries_left = self._server_side_retries
@ -153,6 +156,7 @@ class Connector(object):
response = self._session.request(method, url, json=data,
headers=headers,
verify=self._verify,
timeout=timeout,
**extra_session_req_kwargs)
except requests.exceptions.RequestException as e:
# Capture any general exception by looking for the parent
@ -211,6 +215,7 @@ class Connector(object):
method, url, json=data,
headers=headers,
verify=self._verify,
timeout=timeout,
**extra_session_req_kwargs)
except exceptions.HTTPError as retry_exc:
LOG.error("Failure occured while attempting to retry "

18
sushy/tests/unit/test_connector.py
View File

@ -178,7 +178,7 @@ class ConnectorOpTestCase(base.TestCase):
self.conn._op('GET', path='fake/path', headers=self.headers)
self.request.assert_called_once_with(
'GET', 'http://foo.bar:1234/fake/path',
headers=self.headers, json=None, verify=True)
headers=self.headers, json=None, verify=True, timeout=60)
def test_response_callback(self):
mock_response_callback = mock.MagicMock()
@ -193,21 +193,21 @@ class ConnectorOpTestCase(base.TestCase):
self.request.assert_called_once_with(
'GET', 'http://foo.bar:1234/fake/path',
headers=self.headers, json=None, allow_redirects=False,
verify=True)
verify=True, timeout=60)
def test_ok_post(self):
self.conn._op('POST', path='fake/path', data=self.data.copy(),
headers=self.headers)
self.request.assert_called_once_with(
'POST', 'http://foo.bar:1234/fake/path',
json=self.data, headers=self.headers, verify=True)
json=self.data, headers=self.headers, verify=True, timeout=60)
def test_ok_put(self):
self.conn._op('PUT', path='fake/path', data=self.data.copy(),
headers=self.headers)
self.request.assert_called_once_with(
'PUT', 'http://foo.bar:1234/fake/path',
json=self.data, headers=self.headers, verify=True)
json=self.data, headers=self.headers, verify=True, timeout=60)
def test_ok_delete(self):
expected_headers = self.headers.copy()
@ -215,7 +215,7 @@ class ConnectorOpTestCase(base.TestCase):
self.conn._op('DELETE', path='fake/path', headers=self.headers.copy())
self.request.assert_called_once_with(
'DELETE', 'http://foo.bar:1234/fake/path',
headers=expected_headers, json=None, verify=True)
headers=expected_headers, json=None, verify=True, timeout=60)
def test_ok_post_with_session(self):
self.conn._session.headers = {}
@ -227,7 +227,7 @@ class ConnectorOpTestCase(base.TestCase):
data=self.data)
self.request.assert_called_once_with(
'POST', 'http://foo.bar:1234/fake/path',
json=self.data, headers=expected_headers, verify=True)
json=self.data, headers=expected_headers, verify=True, timeout=60)
self.assertEqual(self.conn._session.headers,
{'X-Auth-Token': 'asdf1234'})
@ -240,7 +240,7 @@ class ConnectorOpTestCase(base.TestCase):
self.conn._op('GET', path=path, headers=headers)
self.request.assert_called_once_with(
'GET', 'http://foo.bar:1234' + path,
headers=expected_headers, json=None, verify=True)
headers=expected_headers, json=None, verify=True, timeout=60)
def test_odata_version_header_redfish_no_headers(self):
path = '/redfish/v1/bar'
@ -248,7 +248,7 @@ class ConnectorOpTestCase(base.TestCase):
self.conn._op('GET', path=path)
self.request.assert_called_once_with(
'GET', 'http://foo.bar:1234' + path,
headers=expected_headers, json=None, verify=True)
headers=expected_headers, json=None, verify=True, timeout=60)
def test_odata_version_header_redfish_existing_header(self):
path = '/redfish/v1/foo'
@ -257,7 +257,7 @@ class ConnectorOpTestCase(base.TestCase):
self.conn._op('GET', path=path, headers=headers)
self.request.assert_called_once_with(
'GET', 'http://foo.bar:1234' + path,
headers=expected_headers, json=None, verify=True)
headers=expected_headers, json=None, verify=True, timeout=60)
def test_timed_out_session_unable_to_create_session(self):
self.conn._auth.can_refresh_session.return_value = False