Merge "Allow initializing session with connection retries"

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

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()
+        call_args['url'] = self.TEST_URL
-        retries = 3
 
         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())

releasenotes/notes/bug-1840235-ef2946d149ac329c.yaml

@@ -0,0 +1,11 @@
+---
+features:
+  - |
+    [`feature bug 1840235 <https://bugs.launchpad.net/keystoneauth/+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.