Add retries to cinderclient.

HTTPClient now supports a retries argument. It will reissue requests
for any 5xx or socket (400 with n/a) errors. This retry loop was
"inspired" by swiftclient's loop. It reauths one extra time if
necessary.  It uses backoff times of 1, 2, 4... seconds.

The default is 0 retries. It is also exposed to the shell as well with
a --retries arg.

Change-Id: I67bed02d65155f4a4d5d879bb233f56cc78849fa

cinderclient/client.py

@@ -11,6 +11,10 @@ import httplib2
 import logging
 import os
 import urlparse
+try:
+    from eventlet import sleep
+except ImportError:
+    from time import sleep
 
 try:
     import json
@@ -42,7 +46,7 @@ class HTTPClient(httplib2.Http):
                  timeout=None, tenant_id=None, proxy_tenant_id=None,
                  proxy_token=None, region_name=None,
                  endpoint_type='publicURL', service_type=None,
-                 service_name=None, volume_service_name=None):
+                 service_name=None, volume_service_name=None, retries=None):
         super(HTTPClient, self).__init__(timeout=timeout)
         self.user = user
         self.password = password
@@ -55,6 +59,7 @@ class HTTPClient(httplib2.Http):
         self.service_type = service_type
         self.service_name = service_name
         self.volume_service_name = volume_service_name
+        self.retries = int(retries or 0)
 
         self.management_url = None
         self.auth_token = None
@@ -111,28 +116,47 @@ class HTTPClient(httplib2.Http):
         return resp, body
 
     def _cs_request(self, url, method, **kwargs):
-        if not self.management_url:
+        auth_attempts = 0
-            self.authenticate()
+        attempts = 0
-
+        backoff = 1
-        # Perform the request once. If we get a 401 back then it
+        while True:
-        # might be because the auth token expired, so try to
+            attempts += 1
-        # re-authenticate and try again. If it still fails, bail.
+            if not self.management_url or not self.auth_token:
-        try:
+                self.authenticate()
             kwargs.setdefault('headers', {})['X-Auth-Token'] = self.auth_token
             if self.projectid:
                 kwargs['headers']['X-Auth-Project-Id'] = self.projectid
-
-            resp, body = self.request(self.management_url + url, method,
-                                      **kwargs)
-            return resp, body
-        except exceptions.Unauthorized, ex:
             try:
-                self.authenticate()
                 resp, body = self.request(self.management_url + url, method,
                                           **kwargs)
                 return resp, body
+            except exceptions.BadRequest as e:
+                if attempts > self.retries:
+                    raise
+                # Socket errors show up here (400) when
+                # force_exception_to_status_code = True
+                if e.message != 'n/a':
+                    raise
             except exceptions.Unauthorized:
-                raise ex
+                if auth_attempts > 0:
+                    raise
+                _logger.debug("Unauthorized, reauthenticating.")
+                self.management_url = self.auth_token = None
+                # First reauth. Discount this attempt.
+                attempts -= 1
+                auth_attempts += 1
+                continue
+            except exceptions.ClientException as e:
+                if attempts > self.retries:
+                    raise
+                if 500 <= e.code <= 599:
+                    pass
+                else:
+                    raise
+            _logger.debug("Failed attempt(%s of %s), retrying in %s seconds" %
+                          (attempts, self.retries, backoff))
+            sleep(backoff)
+            backoff *= 2
 
     def get(self, url, **kwargs):
         return self._cs_request(url, 'GET', **kwargs)

cinderclient/shell.py

@@ -169,6 +169,12 @@ class OpenStackCinderShell(object):
                             action='store_true',
                             help=argparse.SUPPRESS)
 
+        parser.add_argument('--retries',
+                            metavar='<retries>',
+                            type=int,
+                            default=0,
+                            help='Number of retries.')
+
         # FIXME(dtroyer): The args below are here for diablo compatibility,
         #                 remove them in folsum cycle
 
@@ -408,7 +414,8 @@ class OpenStackCinderShell(object):
                                 extensions=self.extensions,
                                 service_type=service_type,
                                 service_name=service_name,
-                                volume_service_name=volume_service_name)
+                                volume_service_name=volume_service_name,
+                                retries=options.retries)
 
         try:
             if not utils.isunauthenticated(args.func):

cinderclient/v1/client.py

@@ -27,7 +27,7 @@ class Client(object):
                  proxy_tenant_id=None, proxy_token=None, region_name=None,
                  endpoint_type='publicURL', extensions=None,
                  service_type='volume', service_name=None,
-                 volume_service_name=None):
+                 volume_service_name=None, retries=None):
         # FIXME(comstud): Rename the api_key argument above when we
         # know it's not being used as keyword argument
         password = api_key
@@ -61,7 +61,8 @@ class Client(object):
             endpoint_type=endpoint_type,
             service_type=service_type,
             service_name=service_name,
-            volume_service_name=volume_service_name)
+            volume_service_name=volume_service_name,
+            retries=retries)
 
     def authenticate(self):
         """

tests/test_http.py

@@ -11,14 +11,14 @@ fake_body = '{"hi": "there"}'
 mock_request = mock.Mock(return_value=(fake_response, fake_body))
 
 
-def get_client():
+def get_client(retries=0):
     cl = client.HTTPClient("username", "password",
-                           "project_id", "auth_test")
+                           "project_id", "auth_test", retries=retries)
     return cl
 
 
-def get_authed_client():
+def get_authed_client(retries=0):
-    cl = get_client()
+    cl = get_client(retries=retries)
     cl.management_url = "http://example.com"
     cl.auth_token = "token"
     return cl
@@ -44,6 +44,111 @@ class ClientTest(utils.TestCase):
 
         test_get_call()
 
+    def test_get_reauth_0_retries(self):
+        cl = get_authed_client(retries=0)
+
+        bad_response = httplib2.Response({"status": 401})
+        bad_body = '{"error": {"message": "FAILED!", "details": "DETAILS!"}}'
+        bad_request = mock.Mock(return_value=(bad_response, bad_body))
+        self.requests = [bad_request, mock_request]
+
+        def request(*args, **kwargs):
+            next_request = self.requests.pop(0)
+            return next_request(*args, **kwargs)
+
+        def reauth():
+            cl.management_url = "http://example.com"
+            cl.auth_token = "token"
+
+        @mock.patch.object(cl, 'authenticate', reauth)
+        @mock.patch.object(httplib2.Http, "request", request)
+        @mock.patch('time.time', mock.Mock(return_value=1234))
+        def test_get_call():
+            resp, body = cl.get("/hi")
+
+        test_get_call()
+        self.assertEqual(self.requests, [])
+
+    def test_get_retry_500(self):
+        cl = get_authed_client(retries=1)
+
+        bad_response = httplib2.Response({"status": 500})
+        bad_body = '{"error": {"message": "FAILED!", "details": "DETAILS!"}}'
+        bad_request = mock.Mock(return_value=(bad_response, bad_body))
+        self.requests = [bad_request, mock_request]
+
+        def request(*args, **kwargs):
+            next_request = self.requests.pop(0)
+            return next_request(*args, **kwargs)
+
+        @mock.patch.object(httplib2.Http, "request", request)
+        @mock.patch('time.time', mock.Mock(return_value=1234))
+        def test_get_call():
+            resp, body = cl.get("/hi")
+
+        test_get_call()
+        self.assertEqual(self.requests, [])
+
+    def test_retry_limit(self):
+        cl = get_authed_client(retries=1)
+
+        bad_response = httplib2.Response({"status": 500})
+        bad_body = '{"error": {"message": "FAILED!", "details": "DETAILS!"}}'
+        bad_request = mock.Mock(return_value=(bad_response, bad_body))
+        self.requests = [bad_request, bad_request, mock_request]
+
+        def request(*args, **kwargs):
+            next_request = self.requests.pop(0)
+            return next_request(*args, **kwargs)
+
+        @mock.patch.object(httplib2.Http, "request", request)
+        @mock.patch('time.time', mock.Mock(return_value=1234))
+        def test_get_call():
+            resp, body = cl.get("/hi")
+
+        self.assertRaises(exceptions.ClientException, test_get_call)
+        self.assertEqual(self.requests, [mock_request])
+
+    def test_get_no_retry_400(self):
+        cl = get_authed_client(retries=1)
+
+        bad_response = httplib2.Response({"status": 400})
+        bad_body = '{"error": {"message": "Bad!", "details": "Terrible!"}}'
+        bad_request = mock.Mock(return_value=(bad_response, bad_body))
+        self.requests = [bad_request, mock_request]
+
+        def request(*args, **kwargs):
+            next_request = self.requests.pop(0)
+            return next_request(*args, **kwargs)
+
+        @mock.patch.object(httplib2.Http, "request", request)
+        @mock.patch('time.time', mock.Mock(return_value=1234))
+        def test_get_call():
+            resp, body = cl.get("/hi")
+
+        self.assertRaises(exceptions.BadRequest, test_get_call)
+        self.assertEqual(self.requests, [mock_request])
+
+    def test_get_retry_400_socket(self):
+        cl = get_authed_client(retries=1)
+
+        bad_response = httplib2.Response({"status": 400})
+        bad_body = '{"error": {"message": "n/a", "details": "n/a"}}'
+        bad_request = mock.Mock(return_value=(bad_response, bad_body))
+        self.requests = [bad_request, mock_request]
+
+        def request(*args, **kwargs):
+            next_request = self.requests.pop(0)
+            return next_request(*args, **kwargs)
+
+        @mock.patch.object(httplib2.Http, "request", request)
+        @mock.patch('time.time', mock.Mock(return_value=1234))
+        def test_get_call():
+            resp, body = cl.get("/hi")
+
+        test_get_call()
+        self.assertEqual(self.requests, [])
+
     def test_post(self):
         cl = get_authed_client()