From a62b7ee06c1a300c73d9a01b2c9bc05028f1fabd Mon Sep 17 00:00:00 2001
From: Julien Danjou <julien@danjou.info>
Date: Tue, 7 Jun 2016 15:49:32 +0200
Subject: [PATCH] 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 | 8 ++--
tests/unit/test_swiftclient.py | 78 ++++++++++++++++++++++++++++++++++
2 files changed, 82 insertions(+), 4 deletions(-)
diff --git a/swiftclient/client.py b/swiftclient/client.py
index 744a876b..f556afd9 100644
--- a/swiftclient/client.py
+++ b/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:
diff --git a/tests/unit/test_swiftclient.py b/tests/unit/test_swiftclient.py
index cbb95dbe..4a39458b 100644
--- a/tests/unit/test_swiftclient.py
+++ b/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)):