diff --git a/keystoneauth1/session.py b/keystoneauth1/session.py index aa49f724..935e022f 100644 --- a/keystoneauth1/session.py +++ b/keystoneauth1/session.py @@ -330,6 +330,9 @@ class Session(object): :param rate_semaphore: Semaphore to be used to control concurrency and rate limiting of requests. (optional, defaults to no concurrency or rate control) + :param int connect_retries: the maximum number of retries that should + be attempted for connection errors. + (optional, defaults to 0 - never retry). """ user_agent = None @@ -343,7 +346,8 @@ class Session(object): redirect=_DEFAULT_REDIRECT_LIMIT, additional_headers=None, app_name=None, app_version=None, additional_user_agent=None, discovery_cache=None, split_loggers=None, - collect_timing=False, rate_semaphore=None): + collect_timing=False, rate_semaphore=None, + connect_retries=0): self.auth = auth self.session = _construct_session(session) @@ -371,6 +375,7 @@ class Session(object): # so we can distinguish between the value being set or not. self._split_loggers = split_loggers self._collect_timing = collect_timing + self._connect_retries = connect_retries self._api_times = [] self._rate_semaphore = rate_semaphore or NoOpSemaphore() @@ -621,7 +626,7 @@ class Session(object): user_agent=None, redirect=None, authenticated=None, endpoint_filter=None, auth=None, requests_auth=None, raise_exc=True, allow_reauth=True, log=True, - endpoint_override=None, connect_retries=0, logger=None, + endpoint_override=None, connect_retries=None, logger=None, allow=None, client_name=None, client_version=None, microversion=None, microversion_service_type=None, status_code_retries=0, retriable_status_codes=None, @@ -654,7 +659,7 @@ class Session(object): for forever/never. (optional) :param int connect_retries: the maximum number of retries that should be attempted for connection errors. - (optional, defaults to 0 - never retry). + (optional, defaults to None - never retry). :param bool authenticated: True if a token should be attached to this request, False if not or None for attach if an auth_plugin is available. @@ -748,6 +753,8 @@ class Session(object): else: split_loggers = None logger = logger or utils.get_logger(__name__) + if connect_retries is None: + connect_retries = self._connect_retries # HTTP 503 - Service Unavailable retriable_status_codes = retriable_status_codes or [503] rate_semaphore = rate_semaphore or self._rate_semaphore diff --git a/keystoneauth1/tests/unit/test_session.py b/keystoneauth1/tests/unit/test_session.py index 4a6de3c9..8817e4e7 100644 --- a/keystoneauth1/tests/unit/test_session.py +++ b/keystoneauth1/tests/unit/test_session.py @@ -421,24 +421,66 @@ class SessionTests(utils.TestCase): self.assertIn('--cacert', self.logger.output) self.assertIn(path_to_certs, self.logger.output) - def test_connect_retries(self): + def _connect_retries_check(self, session, expected_retries=0, + call_args=None): + call_args = call_args or {} + self.stub_url('GET', exc=requests.exceptions.Timeout()) - session = client_session.Session() - retries = 3 + call_args['url'] = self.TEST_URL with mock.patch('time.sleep') as m: self.assertRaises(exceptions.ConnectTimeout, session.get, - self.TEST_URL, connect_retries=retries) + **call_args) - self.assertEqual(retries, m.call_count) + self.assertEqual(expected_retries, m.call_count) # 3 retries finishing with 2.0 means 0.5, 1.0 and 2.0 m.assert_called_with(2.0) # we count retries so there will be one initial request + 3 retries self.assertThat(self.requests_mock.request_history, - matchers.HasLength(retries + 1)) + matchers.HasLength(expected_retries + 1)) + + def test_session_connect_retries(self): + retries = 3 + session = client_session.Session(connect_retries=retries) + self._connect_retries_check(session=session, expected_retries=retries) + + def test_call_args_connect_retries_session_init(self): + session = client_session.Session() + retries = 3 + call_args = {'connect_retries': retries} + self._connect_retries_check(session=session, + expected_retries=retries, + call_args=call_args) + + def test_call_args_connect_retries_overrides_session_retries(self): + session_retries = 6 + call_arg_retries = 3 + call_args = {'connect_retries': call_arg_retries} + session = client_session.Session(connect_retries=session_retries) + self._connect_retries_check(session=session, + expected_retries=call_arg_retries, + call_args=call_args) + + def test_override_session_connect_retries_for_request(self): + session_retries = 1 + session = client_session.Session(connect_retries=session_retries) + + self.stub_url('GET', exc=requests.exceptions.Timeout()) + call_args = {'connect_retries': 0} + + with mock.patch('time.sleep') as m: + self.assertRaises( + exceptions.ConnectTimeout, + session.request, + self.TEST_URL, + 'GET', + **call_args + ) + + self.assertEqual(0, m.call_count) def test_connect_retries_interval_limit(self): self.stub_url('GET', exc=requests.exceptions.Timeout()) diff --git a/releasenotes/notes/bug-1840235-ef2946d149ac329c.yaml b/releasenotes/notes/bug-1840235-ef2946d149ac329c.yaml new file mode 100644 index 00000000..e45d00ce --- /dev/null +++ b/releasenotes/notes/bug-1840235-ef2946d149ac329c.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + [`feature bug 1840235 `_] + Adds ``connect_retries`` to Session.__init__(), that can then be used + by projects when creating session objects, to set the required number of + retries for new connection requests. This would specifically help avoid + a scalability issue that results in number of ConnectTimeout errors when + doing endpoint discovery and fetching roles using an auth plugin under + heavy load. This still allows for it to be overridden per service with + the adapter interface.