client: renew token on 401 even if retries is 0

Gnocchi uses a client with retries=0 to maximize throughtput and not retry N
times on e.g. 404 when checking existence of an object. However, this as the
side effect of never renewing the token since there' no retry on 401 either.

This patches change the behavior so that 401 errors are always retried,
whatever the retries value is.

Closes-Bug: #1589926
Change-Id: Ie06adf4cf17ea4592b5bbd7bbde9828e5e134e3e

swiftclient/client.py

@@ -1544,7 +1544,7 @@ class Connection(object):
         backoff = self.starting_backoff
         caller_response_dict = kwargs.pop('response_dict', None)
         self.attempts = kwargs.pop('attempts', 0)
-        while self.attempts <= self.retries:
+        while self.attempts <= self.retries or retried_auth:
             self.attempts += 1
             try:
                 if not self.url or not self.token:
@@ -1573,9 +1573,6 @@ class Connection(object):
                 self.http_conn = None
             except ClientException as err:
                 self._add_response_dict(caller_response_dict, kwargs)
-                if self.attempts > self.retries or err.http_status is None:
-                    logger.exception(err)
-                    raise
                 if err.http_status == 401:
                     self.url = self.token = self.service_token = None
                     if retried_auth or not all((self.authurl,
@@ -1584,6 +1581,9 @@ class Connection(object):
                         logger.exception(err)
                         raise
                     retried_auth = True
+                elif self.attempts > self.retries or err.http_status is None:
+                    logger.exception(err)
+                    raise
                 elif err.http_status == 408:
                     self.http_conn = None
                 elif 500 <= err.http_status <= 599:

tests/unit/test_swiftclient.py

@@ -2556,6 +2556,84 @@ class TestServiceToken(MockHttpTest):
         self.assertEqual('service_project_name',
                          auth_kwargs['os_options']['tenant_name'])
 
+    def test_service_token_reauth_retries_0(self):
+        get_auth_call_list = []
+
+        def get_auth(url, user, key, **kwargs):
+            # The real get_auth function will always return the os_option
+            # dict's object_storage_url which will be overridden by the
+            # preauthurl parameter to Connection if it is provided.
+            args = {'url': url, 'user': user, 'key': key, 'kwargs': kwargs}
+            get_auth_call_list.append(args)
+            return_dict = {'asdf': 'new', 'service_username': 'newserv'}
+            storage_url = kwargs['os_options'].get('object_storage_url')
+            return storage_url, return_dict[user]
+
+        def swap_sleep(*args):
+            self.swap_sleep_called = True
+            c.get_auth = get_auth
+
+        with mock.patch('swiftclient.client.http_connection',
+                        self.fake_http_connection(401, 200)):
+            with mock.patch('swiftclient.client.sleep', swap_sleep):
+                self.swap_sleep_called = False
+
+                conn = c.Connection('http://www.test.com', 'asdf', 'asdf',
+                                    preauthurl='http://www.old.com',
+                                    preauthtoken='old',
+                                    os_options=self.os_options,
+                                    retries=0)
+
+                self.assertEqual(conn.attempts, 0)
+                self.assertEqual(conn.url, 'http://www.old.com')
+                self.assertEqual(conn.token, 'old')
+
+                conn.head_account()
+
+        self.assertTrue(self.swap_sleep_called)
+        self.assertEqual(conn.attempts, 2)
+        # The original 'preauth' storage URL *must* be preserved
+        self.assertEqual(conn.url, 'http://www.old.com')
+        self.assertEqual(conn.token, 'new')
+        self.assertEqual(conn.service_token, 'newserv')
+
+        # Check get_auth was called with expected args
+        auth_args = get_auth_call_list[0]
+        auth_kwargs = get_auth_call_list[0]['kwargs']
+        self.assertEqual('asdf', auth_args['user'])
+        self.assertEqual('asdf', auth_args['key'])
+        self.assertEqual('service_key',
+                         auth_kwargs['os_options']['service_key'])
+        self.assertEqual('service_username',
+                         auth_kwargs['os_options']['service_username'])
+        self.assertEqual('service_project_name',
+                         auth_kwargs['os_options']['service_project_name'])
+
+        auth_args = get_auth_call_list[1]
+        auth_kwargs = get_auth_call_list[1]['kwargs']
+        self.assertEqual('service_username', auth_args['user'])
+        self.assertEqual('service_key', auth_args['key'])
+        self.assertEqual('service_project_name',
+                         auth_kwargs['os_options']['tenant_name'])
+
+        # Ensure this is not an endless loop - it fails after the second 401
+        with mock.patch('swiftclient.client.http_connection',
+                        self.fake_http_connection(401, 401, 401, 401)):
+            with mock.patch('swiftclient.client.sleep', swap_sleep):
+                self.swap_sleep_called = False
+
+                conn = c.Connection('http://www.test.com', 'asdf', 'asdf',
+                                    preauthurl='http://www.old.com',
+                                    preauthtoken='old',
+                                    os_options=self.os_options,
+                                    retries=0)
+
+                self.assertEqual(conn.attempts, 0)
+                self.assertRaises(c.ClientException, conn.head_account)
+                self.assertEqual(conn.attempts, 2)
+                unused_responses = list(self.fake_connect.code_iter)
+                self.assertEqual(unused_responses, [401, 401])
+
     def test_service_token_get_account(self):
         with mock.patch('swiftclient.client.http_connection',
                         self.fake_http_connection(200)):