Cinder client should retry with Retry-After value

If a request fails but the response contains a "Retry-After",
the cinder client should wait the amount of time and then retry.
Cinder client should report a warning to user and continue with
retry, so that user can cancel the operation if not interested
in retry. The value in "Retry-After" header will be in seconds
or GMT value, client should handle both the cases.

How many times client should retry will be controlled by user
through "--retries" argument to cinder api example,
$ cinder --retries 3 availability-zone-list

If request was not sucessful within the retries, client should
raise the exception.

Change-Id: I99af957bfbbe3a202b148dc2fcafdd20b5d7cda0
Partial-Bug: #1263069

cinderclient/client.py

@@ -100,6 +100,8 @@ class SessionClient(adapter.LegacyJsonAdapter):
     def __init__(self, *args, **kwargs):
         self.api_version = kwargs.pop('api_version', None)
         self.api_version = self.api_version or api_versions.APIVersion()
+        self.retries = kwargs.pop('retries', 0)
+        self._logger = logging.getLogger(__name__)
         super(SessionClient, self).__init__(*args, **kwargs)
 
     def request(self, *args, **kwargs):
@@ -125,7 +127,17 @@ class SessionClient(adapter.LegacyJsonAdapter):
     def _cs_request(self, url, method, **kwargs):
         # this function is mostly redundant but makes compatibility easier
         kwargs.setdefault('authenticated', True)
-        return self.request(url, method, **kwargs)
+        attempts = 0
+        while True:
+            attempts += 1
+            try:
+                return self.request(url, method, **kwargs)
+            except exceptions.OverLimit as overlim:
+                if attempts > self.retries or overlim.retry_after < 1:
+                    raise
+                msg = "Retrying after %s seconds." % overlim.retry_after
+                self._logger.debug(msg)
+                sleep(overlim.retry_after)
 
     def get(self, url, **kwargs):
         return self._cs_request(url, 'GET', **kwargs)
@@ -334,6 +346,13 @@ class HTTPClient(object):
                 attempts -= 1
                 auth_attempts += 1
                 continue
+            except exceptions.OverLimit as overlim:
+                if attempts > self.retries or overlim.retry_after < 1:
+                    raise
+                msg = "Retrying after %s seconds." % overlim.retry_after
+                self._logger.debug(msg)
+                sleep(overlim.retry_after)
+                continue
             except exceptions.ClientException as e:
                 if attempts > self.retries:
                     raise
@@ -576,6 +595,7 @@ def _construct_http_client(username=None, password=None, project_id=None,
                              service_type=service_type,
                              service_name=service_name,
                              region_name=region_name,
+                             retries=retries,
                              api_version=api_version,
                              **kwargs)
     else:

cinderclient/exceptions.py

@@ -16,6 +16,9 @@
 """
 Exception definitions.
 """
+from datetime import datetime
+
+from oslo_utils import timeutils
 
 
 class UnsupportedVersion(Exception):
@@ -80,7 +83,8 @@ class ClientException(Exception):
     """
     The base exception class for all exceptions this library raises.
     """
-    def __init__(self, code, message=None, details=None, request_id=None):
+    def __init__(self, code, message=None, details=None,
+                 request_id=None, response=None):
         self.code = code
         # NOTE(mriedem): Use getattr on self.__class__.message since
         # BaseException.message was dropped in python 3, see PEP 0352.
@@ -147,6 +151,27 @@ class OverLimit(ClientException):
     http_status = 413
     message = "Over limit"
 
+    def __init__(self, code, message=None, details=None,
+                 request_id=None, response=None):
+        super(OverLimit, self).__init__(code, message=message,
+                                        details=details, request_id=request_id,
+                                        response=response)
+        self.retry_after = 0
+        self._get_rate_limit(response)
+
+    def _get_rate_limit(self, resp):
+        if resp.headers:
+            utc_now = timeutils.utcnow()
+            value = resp.headers.get('Retry-After', '0')
+            try:
+                value = datetime.strptime(value, '%a, %d %b %Y %H:%M:%S %Z')
+                if value > utc_now:
+                    self.retry_after = ((value - utc_now).seconds)
+                else:
+                    self.retry_after = 0
+            except ValueError:
+                self.retry_after = int(value)
+
 
 # NotImplemented is a python keyword.
 class HTTPNotImplemented(ClientException):
@@ -193,10 +218,10 @@ def from_response(response, body):
             message = error.get('message', message)
             details = error.get('details', details)
         return cls(code=response.status_code, message=message, details=details,
-                   request_id=request_id)
+                   request_id=request_id, response=response)
     else:
         return cls(code=response.status_code, request_id=request_id,
-                   message=response.reason)
+                   message=response.reason, response=response)
 
 
 class VersionNotFoundForAPIMethod(Exception):

cinderclient/tests/unit/test_client.py

@@ -162,6 +162,31 @@ class ClientTest(utils.TestCase):
                           mock.sentinel.url, 'POST', **kwargs)
         self.assertEqual(1, mock_log.call_count)
 
+    @mock.patch.object(cinderclient.client, '_log_request_id')
+    @mock.patch.object(adapter.Adapter, 'request')
+    def test_sessionclient_request_method_raises_overlimit(
+            self, mock_request, mock_log):
+        resp = {
+            "overLimitFault": {
+                "message": "This request was rate-limited.",
+                "code": 413
+            }
+        }
+
+        mock_response = utils.TestResponse({
+            "status_code": 413,
+            "text": json.dumps(resp),
+        })
+
+        # 'request' method of Adaptor will return 413 response
+        mock_request.return_value = mock_response
+        session_client = cinderclient.client.SessionClient(
+            session=mock.Mock())
+
+        self.assertRaises(exceptions.OverLimit, session_client.request,
+                          mock.sentinel.url, 'GET')
+        self.assertEqual(1, mock_log.call_count)
+
     @mock.patch.object(exceptions, 'from_response')
     def test_keystone_request_raises_auth_failure_exception(
             self, mock_from_resp):

cinderclient/tests/unit/test_exceptions.py

@@ -14,6 +14,8 @@
 
 """Tests the cinderclient.exceptions module."""
 
+import datetime
+import mock
 import requests
 
 from cinderclient import exceptions
@@ -30,3 +32,33 @@ class ExceptionsTest(utils.TestCase):
         ex = exceptions.from_response(response, body)
         self.assertIs(exceptions.ClientException, type(ex))
         self.assertEqual('n/a', ex.message)
+
+    def test_from_response_overlimit(self):
+        response = requests.Response()
+        response.status_code = 413
+        response.headers = {"Retry-After": '10'}
+        body = {'keys': ({})}
+        ex = exceptions.from_response(response, body)
+        self.assertEqual(10, ex.retry_after)
+        self.assertIs(exceptions.OverLimit, type(ex))
+
+    @mock.patch('oslo_utils.timeutils.utcnow',
+                return_value=datetime.datetime(2016, 6, 30, 12, 41, 55))
+    def test_from_response_overlimit_gmt(self, mock_utcnow):
+        response = requests.Response()
+        response.status_code = 413
+        response.headers = {"Retry-After": "Thu, 30 Jun 2016 12:43:20 GMT"}
+        body = {'keys': ({})}
+        ex = exceptions.from_response(response, body)
+        self.assertEqual(85, ex.retry_after)
+        self.assertIs(exceptions.OverLimit, type(ex))
+        self.assertTrue(mock_utcnow.called)
+
+    def test_from_response_overlimit_without_header(self):
+        response = requests.Response()
+        response.status_code = 413
+        response.headers = {}
+        body = {'keys': ({})}
+        ex = exceptions.from_response(response, body)
+        self.assertEqual(0, ex.retry_after)
+        self.assertIs(exceptions.OverLimit, type(ex))

cinderclient/tests/unit/test_http.py

@@ -45,6 +45,12 @@ bad_401_response = utils.TestResponse({
 })
 bad_401_request = mock.Mock(return_value=(bad_401_response))
 
+bad_413_response = utils.TestResponse({
+    "status_code": 413,
+    "headers": {"Retry-After": "1", "x-compute-request-id": "1234"},
+})
+bad_413_request = mock.Mock(return_value=(bad_413_response))
+
 bad_500_response = utils.TestResponse({
     "status_code": 500,
     "text": '{"error": {"message": "FAILED!", "details": "DETAILS!"}}',
@@ -158,6 +164,43 @@ class ClientTest(utils.TestCase):
         test_get_call()
         self.assertEqual([], self.requests)
 
+    def test_rate_limit_overlimit_exception(self):
+        cl = get_authed_client(retries=1)
+
+        self.requests = [bad_413_request,
+                         bad_413_request,
+                         mock_request]
+
+        def request(*args, **kwargs):
+            next_request = self.requests.pop(0)
+            return next_request(*args, **kwargs)
+
+        @mock.patch.object(requests, "request", request)
+        @mock.patch('time.time', mock.Mock(return_value=1234))
+        def test_get_call():
+            resp, body = cl.get("/hi")
+        self.assertRaises(exceptions.OverLimit, test_get_call)
+        self.assertEqual([mock_request], self.requests)
+
+    def test_rate_limit(self):
+        cl = get_authed_client(retries=1)
+
+        self.requests = [bad_413_request, mock_request]
+
+        def request(*args, **kwargs):
+            next_request = self.requests.pop(0)
+            return next_request(*args, **kwargs)
+
+        @mock.patch.object(requests, "request", request)
+        @mock.patch('time.time', mock.Mock(return_value=1234))
+        def test_get_call():
+            resp, body = cl.get("/hi")
+            return resp, body
+
+        resp, body = test_get_call()
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual([], self.requests)
+
     def test_retry_limit(self):
         cl = get_authed_client(retries=1)