summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKota Tsuyuzaki <tsuyuzaki.kota@lab.ntt.co.jp>2018-03-12 17:54:17 +0900
committerKota Tsuyuzaki <tsuyuzaki.kota@lab.ntt.co.jp>2018-03-13 12:29:48 +0900
commite65070964c7b1e04119c87e5f344d39358780d18 (patch)
treea4821ebdebd3e9402042e78693be8a2e6e598c05
parenta08d9b109f7ac3c5b844429addbb7e8507268c8e (diff)
downloadpython-swiftclient-e65070964c7b1e04119c87e5f344d39358780d18.tar.gz
Add force auth retry mode in swiftclient
This patch attemps to add an option to force get_auth call while retrying an operation even if it gets errors other than 401 Unauthorized. Why we need this: The main reason why we need this is current python-swiftclient requests could never get succeeded under certion situation using third party proxies/load balancers between the client and swift-proxy server. I think, it would be general situation of the use case. Specifically describing nginx case, the nginx can close the socket from the client when the response code from swift is not 2xx series. In default, nginx can wait the buffers from the client for a while (default 30s)[1] but after the time past, nginx will close the socket immediately. Unfortunately, if python-swiftclient has still been sending the data into the socket, python-swiftclient will get socket error (EPIPE, BrokenPipe). From the swiftclient perspective, this is absolutely not an auth error, so current python-swiftclient will continue to retry without re-auth. However, if the root cause is sort of 401 (i.e. nginx got 401 unauthorized from the swift-proxy because of token expiration), swiftclient will loop 401 -> EPIPE -> 401... until it consume the max retry times. In particlar, less time to live of the token and multipart object upload with large segments could not get succeeded as below: Connection Model: python-swiftclient -> nginx -> swift-proxy -> swift-backend Case: Try to create slo with large segments and the auth token expired with 1 hour 1. client create a connection to nginx with successful response from swift-proxy and its auth 2. client continue to put large segment objects (e.g. 1~5GB for each and the total would 20~30GB, i.e. 20~30 segments) 3. after some of segments uploaded, 1 hour past but client is still trying to send remaining segment objects. 4. nginx got 401 from swift-proxy for a request and wait that the connection is closed from the client but timeout past because the python-swiftclient is still sending much data into the socket before reading the 401 response. 5. client got socket error because nginx closed the connection during sending the buffer. 6. client retries a new connection to nginx without re-auth... <loop 4-6> 7. finally python-swiftclient failed with socket error (Broken Pipe) In operational perspective, setting longer timeout for lingering close would be an option but it's not complete solution because any other proxy/LB may not support the options. If we actually do THE RIGHT THING in python-swiftclient, we should send expects: 100-continue header and handle the first response to re-auth correctly. HOWEVER, the current python's httplib and requests module used by python-swiftclient doesn't support expects: 100-continue header [2] and the thread proposed a fix [3] is not super active. And we know the reason we depends on the library is to fix a security issue that existed in older python-swiftclient [4] so that we should touch around it super carefully. In the reality, as the hot fix, this patch try to mitigate the unfortunate situation described above WITHOUT 100-continue fix, just users can force to re-auth when any errors occurred during the retries that can be accepted in the upstream. 1: http://nginx.org/en/docs/http/ngx_http_core_module.html#lingering_close 2: https://github.com/requests/requests/issues/713 3: https://bugs.python.org/issue1346874 4: https://review.openstack.org/#/c/69187/ Change-Id: I3470b56e3f9cf9cdb8c2fc2a94b2c551927a3440
-rw-r--r--swiftclient/client.py9
-rw-r--r--swiftclient/service.py4
-rwxr-xr-xswiftclient/shell.py6
-rw-r--r--tests/unit/test_swiftclient.py67
4 files changed, 84 insertions, 2 deletions
diff --git a/swiftclient/client.py b/swiftclient/client.py
index 60abbd8..ab0fde8 100644
--- a/swiftclient/client.py
+++ b/swiftclient/client.py
@@ -1542,7 +1542,7 @@ class Connection(object):
os_options=None, auth_version="1", cacert=None,
insecure=False, cert=None, cert_key=None,
ssl_compression=True, retry_on_ratelimit=False,
- timeout=None, session=None):
+ timeout=None, session=None, force_auth_retry=False):
"""
:param authurl: authentication URL
:param user: user name to authenticate as
@@ -1578,6 +1578,8 @@ class Connection(object):
after a backoff.
:param timeout: The connect timeout for the HTTP connection.
:param session: A keystoneauth session object.
+ :param force_auth_retry: reset auth info even if client got unexpected
+ error except 401 Unauthorized.
"""
self.session = session
self.authurl = authurl
@@ -1610,6 +1612,7 @@ class Connection(object):
self.auth_end_time = 0
self.retry_on_ratelimit = retry_on_ratelimit
self.timeout = timeout
+ self.force_auth_retry = force_auth_retry
def close(self):
if (self.http_conn and isinstance(self.http_conn, tuple)
@@ -1724,6 +1727,10 @@ class Connection(object):
pass
else:
raise
+
+ if self.force_auth_retry:
+ self.url = self.token = self.service_token = None
+
sleep(backoff)
backoff = min(backoff * 2, self.max_backoff)
if reset_func:
diff --git a/swiftclient/service.py b/swiftclient/service.py
index ed5e9e9..0679fec 100644
--- a/swiftclient/service.py
+++ b/swiftclient/service.py
@@ -144,6 +144,7 @@ def _build_default_global_options():
"user": environ.get('ST_USER'),
"key": environ.get('ST_KEY'),
"retries": 5,
+ "force_auth_retry": False,
"os_username": environ.get('OS_USERNAME'),
"os_user_id": environ.get('OS_USER_ID'),
"os_user_domain_name": environ.get('OS_USER_DOMAIN_NAME'),
@@ -261,7 +262,8 @@ def get_conn(options):
insecure=options['insecure'],
cert=options['os_cert'],
cert_key=options['os_key'],
- ssl_compression=options['ssl_compression'])
+ ssl_compression=options['ssl_compression'],
+ force_auth_retry=options['force_auth_retry'])
def mkdirs(path):
diff --git a/swiftclient/shell.py b/swiftclient/shell.py
index d02c709..6010b5d 100755
--- a/swiftclient/shell.py
+++ b/swiftclient/shell.py
@@ -1501,6 +1501,7 @@ def main(arguments=None):
[--os-cert <client-certificate-file>]
[--os-key <client-certificate-key-file>]
[--no-ssl-compression]
+ [--force-auth-retry]
<subcommand> [--help] [<subcommand options>]
Command-line interface to the OpenStack Swift API.
@@ -1610,6 +1611,11 @@ Examples:
help='This option is deprecated and not used anymore. '
'SSL compression should be disabled by default '
'by the system SSL library.')
+ parser.add_argument('--force-auth-retry',
+ action='store_true', dest='force_auth_retry',
+ default=False,
+ help='Force a re-auth attempt on '
+ 'any error other than 401 unauthorized')
os_grp = parser.add_argument_group("OpenStack authentication options")
os_grp.add_argument('--os-username',
diff --git a/tests/unit/test_swiftclient.py b/tests/unit/test_swiftclient.py
index 7b8628b..52153dc 100644
--- a/tests/unit/test_swiftclient.py
+++ b/tests/unit/test_swiftclient.py
@@ -26,11 +26,13 @@ import tempfile
from hashlib import md5
from six import binary_type
from six.moves.urllib.parse import urlparse
+from requests.exceptions import RequestException
from .utils import (MockHttpTest, fake_get_auth_keystone, StubResponse,
FakeKeystone, _make_fake_import_keystone_client)
from swiftclient.utils import EMPTY_ETAG
+from swiftclient.exceptions import ClientException
from swiftclient import client as c
import swiftclient.utils
import swiftclient
@@ -1978,6 +1980,71 @@ class TestConnection(MockHttpTest):
self.assertIn('Account HEAD failed', str(exc_context.exception))
self.assertEqual(conn.attempts, 1)
+ def test_retry_with_socket_error(self):
+ def quick_sleep(*args):
+ pass
+ c.sleep = quick_sleep
+ conn = c.Connection('http://www.test.com', 'asdf', 'asdf')
+ with mock.patch('swiftclient.client.http_connection') as \
+ fake_http_connection, \
+ mock.patch('swiftclient.client.get_auth_1_0') as mock_auth:
+ mock_auth.return_value = ('http://mock.com', 'mock_token')
+ fake_http_connection.side_effect = socket.error
+ self.assertRaises(socket.error, conn.head_account)
+ self.assertEqual(mock_auth.call_count, 1)
+ self.assertEqual(conn.attempts, conn.retries + 1)
+
+ def test_retry_with_force_auth_retry_exceptions(self):
+ def quick_sleep(*args):
+ pass
+
+ def do_test(exception):
+ c.sleep = quick_sleep
+ conn = c.Connection(
+ 'http://www.test.com', 'asdf', 'asdf',
+ force_auth_retry=True)
+ with mock.patch('swiftclient.client.http_connection') as \
+ fake_http_connection, \
+ mock.patch('swiftclient.client.get_auth_1_0') as mock_auth:
+ mock_auth.return_value = ('http://mock.com', 'mock_token')
+ fake_http_connection.side_effect = exception
+ self.assertRaises(exception, conn.head_account)
+ self.assertEqual(mock_auth.call_count, conn.retries + 1)
+ self.assertEqual(conn.attempts, conn.retries + 1)
+
+ do_test(socket.error)
+ do_test(RequestException)
+
+ def test_retry_with_force_auth_retry_client_exceptions(self):
+ def quick_sleep(*args):
+ pass
+
+ def do_test(http_status, count):
+
+ def mock_http_connection(*args, **kwargs):
+ raise ClientException('fake', http_status=http_status)
+
+ c.sleep = quick_sleep
+ conn = c.Connection(
+ 'http://www.test.com', 'asdf', 'asdf',
+ force_auth_retry=True)
+ with mock.patch('swiftclient.client.http_connection') as \
+ fake_http_connection, \
+ mock.patch('swiftclient.client.get_auth_1_0') as mock_auth:
+ mock_auth.return_value = ('http://mock.com', 'mock_token')
+ fake_http_connection.side_effect = mock_http_connection
+ self.assertRaises(ClientException, conn.head_account)
+ self.assertEqual(mock_auth.call_count, count)
+ self.assertEqual(conn.attempts, count)
+
+ # sanity, in case of 401, the auth will be called only twice because of
+ # retried_auth mechanism
+ do_test(401, 2)
+ # others will be tried until retry limits
+ do_test(408, 6)
+ do_test(500, 6)
+ do_test(503, 6)
+
def test_resp_read_on_server_error(self):
conn = c.Connection('http://www.test.com', 'asdf', 'asdf', retries=0)