From 68d51b348822bcd5b02056c1a04eeee9d099d091 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Feb 15 2022 13:23:49 +0000
Subject: [PATCH 1/3] Retry gssapi_login if it makes sense


Related: https://pagure.io/koji/issue/3170

---

diff --git a/koji/__init__.py b/koji/__init__.py
index fa89f3b..50d1c34 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -2548,7 +2548,18 @@ class ClientSession(object):
                 kwargs = {'proxyuser': proxyuser}
                 if proxyauthtype is not None:
                     kwargs['proxyauthtype'] = proxyauthtype
-                sinfo = self._callMethod('sslLogin', [], kwargs, retry=False)
+                for i in range(self.opts.get('max_retries', 30)):
+                    try:
+                        sinfo = self._callMethod('sslLogin', [], kwargs, retry=False)
+                    except Exception as ex:
+                        # retry on connection error requests.exceptions.ConnectionError
+                        # die on:
+                        #   - any non-connection related error (http code, etc.)
+                        #   - requests.exceptions.SSLError - CA-mismatch, etc. - die on all SSL
+                        #                                    related errors
+                        if (not is_conn_error(ex) or isinstance(ex, requests.exceptions.SSLError)):
+                            raise
+                        time.sleep(self.opts.get('retry_interval', 20))
             except Exception as e:
                 e_str = ''.join(traceback.format_exception_only(type(e), e)).strip('\n')
                 e_str = '(gssapi auth failed: %s)\n' % e_str

From fa4ec5a3526f195415edcabf592c91c88bbdf026 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Feb 15 2022 13:23:49 +0000
Subject: [PATCH 2/3] fix test


---

diff --git a/tests/test_lib/test_gssapi.py b/tests/test_lib/test_gssapi.py
index c5f71be..6249221 100644
--- a/tests/test_lib/test_gssapi.py
+++ b/tests/test_lib/test_gssapi.py
@@ -26,8 +26,8 @@ class TestGSSAPI(unittest.TestCase):
     def test_gssapi_login(self):
         old_environ = dict(**os.environ)
         self.session.gssapi_login()
-        self.session._callMethod.assert_called_once_with(
-                'sslLogin', [], {'proxyuser': None}, retry=False)
+        self.session._callMethod.assert_called_with(
+            'sslLogin', [], {'proxyuser': None}, retry=False)
         self.assertEqual(old_environ, dict(**os.environ))
 
     @mock.patch('koji.reqgssapi.HTTPKerberosAuth')
@@ -46,8 +46,8 @@ class TestGSSAPI(unittest.TestCase):
         for accepted_version in accepted_versions:
             koji.reqgssapi.__version__ = accepted_version
             rv = self.session.gssapi_login(principal, keytab, ccache)
-            self.session._callMethod.assert_called_once_with(
-                    'sslLogin', [], {'proxyuser': None}, retry=False)
+            self.session._callMethod.assert_called_with(
+                'sslLogin', [], {'proxyuser': None}, retry=False)
             self.assertEqual(old_environ, dict(**os.environ))
             self.assertTrue(rv)
             self.session._callMethod.reset_mock()
@@ -83,8 +83,8 @@ class TestGSSAPI(unittest.TestCase):
         self.session._callMethod.side_effect = Exception('login failed')
         with self.assertRaises(koji.GSSAPIAuthError):
             self.session.gssapi_login()
-        self.session._callMethod.assert_called_once_with(
-                'sslLogin', [], {'proxyuser': None}, retry=False)
+        self.session._callMethod.assert_called_with(
+            'sslLogin', [], {'proxyuser': None}, retry=False)
         self.assertEqual(old_environ, dict(**os.environ))
 
     def test_gssapi_login_http(self):

From 8f3ef04b619f6e1983d9445530c161912ba3e3d5 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Feb 15 2022 13:32:54 +0000
Subject: [PATCH 3/3] logging and comment update


---

diff --git a/koji/__init__.py b/koji/__init__.py
index 50d1c34..f9a79f4 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -2545,20 +2545,29 @@ class ClientSession(object):
                 # Depending on the server configuration, we might not be able to
                 # connect without client certificate, which means that the conn
                 # will fail with a handshake failure, which is retried by default.
+                # For this case we're now using retry=False and test errors for
+                # this exact usecase.
                 kwargs = {'proxyuser': proxyuser}
                 if proxyauthtype is not None:
                     kwargs['proxyauthtype'] = proxyauthtype
-                for i in range(self.opts.get('max_retries', 30)):
+                for tries in range(self.opts.get('max_retries', 30)):
                     try:
                         sinfo = self._callMethod('sslLogin', [], kwargs, retry=False)
+                        break
                     except Exception as ex:
-                        # retry on connection error requests.exceptions.ConnectionError
+                        # retry on requests.exceptions.ConnectionError
                         # die on:
                         #   - any non-connection related error (http code, etc.)
                         #   - requests.exceptions.SSLError - CA-mismatch, etc. - die on all SSL
                         #                                    related errors
                         if (not is_conn_error(ex) or isinstance(ex, requests.exceptions.SSLError)):
                             raise
+                        # logging similar to normal retry
+                        if self.logger.isEnabledFor(logging.DEBUG):
+                            tb_str = ''.join(traceback.format_exception(*sys.exc_info()))
+                            self.logger.debug(tb_str)
+                        self.logger.info("Try #%s for call %s (sslLogin) failed: %s",
+                                         tries, self.callnum, ex)
                         time.sleep(self.opts.get('retry_interval', 20))
             except Exception as e:
                 e_str = ''.join(traceback.format_exception_only(type(e), e)).strip('\n')