From fd8aa394580c281f55af31874abb9183aeb469a9 Mon Sep 17 00:00:00 2001 From: Xu Chen Date: Wed, 23 Jul 2014 14:10:19 -0400 Subject: [PATCH] Retry when connection to cinder is refused Currently, cinder client does not retry when connections to cinder service is refused. There are many legitimate scenarios under which retry should be attempted: 1) cinder service being restarted; 2) cinder service is running on multiple API nodes behind a LB, which might be temporarily overwhelmed or being maintained. In any scenario, retry with a backoff timer does not seem to hurt. Change-Id: I3c290c59fa67262c4a3473815b4380ee39e24332 Closes-Bug: 1347843 --- cinderclient/client.py | 5 +++-- cinderclient/tests/test_http.py | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/cinderclient/client.py b/cinderclient/client.py index 71d2a42b0..314532423 100644 --- a/cinderclient/client.py +++ b/cinderclient/client.py @@ -317,8 +317,9 @@ class HTTPClient(CinderClientMixin): except requests.exceptions.ConnectionError as e: # Catch a connection refused from requests.request self._logger.debug("Connection refused: %s" % e) - msg = 'Unable to establish connection: %s' % e - raise exceptions.ConnectionError(msg) + if attempts > self.retries: + msg = 'Unable to establish connection: %s' % e + raise exceptions.ConnectionError(msg) self._logger.debug( "Failed attempt(%s of %s), retrying in %s seconds" % (attempts, self.retries, backoff)) diff --git a/cinderclient/tests/test_http.py b/cinderclient/tests/test_http.py index 18eafebea..8d162bed5 100644 --- a/cinderclient/tests/test_http.py +++ b/cinderclient/tests/test_http.py @@ -51,6 +51,9 @@ bad_500_response = utils.TestResponse({ }) bad_500_request = mock.Mock(return_value=(bad_500_response)) +connection_error_request = mock.Mock( + side_effect=requests.exceptions.ConnectionError) + def get_client(retries=0): cl = client.HTTPClient("username", "password", @@ -127,6 +130,23 @@ class ClientTest(utils.TestCase): test_get_call() self.assertEqual(self.requests, []) + def test_get_retry_connection_error(self): + cl = get_authed_client(retries=1) + + self.requests = [connection_error_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") + + test_get_call() + self.assertEqual(self.requests, []) + def test_retry_limit(self): cl = get_authed_client(retries=1)