Allow initializing session with connection retries

Though we can now set ``connect_retires`` while creating an adapter object,
that  would allow retries in case of connection timeout (ex. with session
clients derived from Adapater/LegacyJsonAdapater), it can't be used in
certain scenarios like endpoint discovery with auth plugin get_discovery()
or getting AccessInfo with get_access()/get_auth_ref().

Having ``connect_retries`` in Session constructor would allow users
with option of setting it when creating session objects (if they want)
and can be overridden per service with the adapter interface.

This commit also changes the default value of ``connect_retries`` from
0 to None to allow for adapter's to override retries on the session
object.

Depends-On: https://review.opendev.org/#/c/680497/

Change-Id: Iffb671fefae23926b1f09017d9db438341eae238
Partial-Bug: #1840235

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)
@@ -364,6 +368,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()
 
@@ -603,7 +608,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,
@@ -636,7 +641,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.
@@ -730,6 +735,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()
-        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())

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.