You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jclouds.apache.org by ga...@apache.org on 2013/10/16 19:34:32 UTC
git commit: Reauthenticate on Keystone HTTP 401 (JCLOUDS-178)
Updated Branches:
refs/heads/master 952d8444d -> 578a77d63
Reauthenticate on Keystone HTTP 401 (JCLOUDS-178)
The number of retries here is not the same as for 500 errors; expected
behavior is a quick fail while retaining some robustness. This fix
should not reintroduce JCLOUDS-231.
Project: http://git-wip-us.apache.org/repos/asf/incubator-jclouds/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-jclouds/commit/578a77d6
Tree: http://git-wip-us.apache.org/repos/asf/incubator-jclouds/tree/578a77d6
Diff: http://git-wip-us.apache.org/repos/asf/incubator-jclouds/diff/578a77d6
Branch: refs/heads/master
Commit: 578a77d6313ce0945f8d29e82103e09787622c58
Parents: 952d844
Author: Zack Shoylev <za...@rackspace.com>
Authored: Wed Aug 7 16:20:41 2013 -0500
Committer: Andrew Gaul <ga...@apache.org>
Committed: Wed Oct 16 10:34:27 2013 -0700
----------------------------------------------------------------------
.../keystone/v2_0/handlers/RetryOnRenew.java | 62 +++++++++++++++-----
.../v2_0/handlers/RetryOnRenewTest.java | 33 ++++++++++-
2 files changed, 80 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-jclouds/blob/578a77d6/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenew.java
----------------------------------------------------------------------
diff --git a/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenew.java b/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenew.java
index d358484..0739521 100644
--- a/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenew.java
+++ b/apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenew.java
@@ -19,6 +19,8 @@ package org.jclouds.openstack.keystone.v2_0.handlers;
import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream;
import static org.jclouds.http.HttpUtils.releasePayload;
+import java.util.concurrent.TimeUnit;
+
import javax.annotation.Resource;
import org.jclouds.domain.Credentials;
@@ -27,9 +29,14 @@ import org.jclouds.http.HttpResponse;
import org.jclouds.http.HttpRetryHandler;
import org.jclouds.logging.Logger;
import org.jclouds.openstack.keystone.v2_0.domain.Access;
+import org.jclouds.openstack.v2_0.reference.AuthHeaders;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.Multimap;
+import com.google.common.util.concurrent.Uninterruptibles;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -37,13 +44,16 @@ import com.google.inject.Singleton;
* This will parse and set an appropriate exception on the command object.
*
* @author Adrian Cole
- *
+ * @author Zack Shoylev
*/
@Singleton
public class RetryOnRenew implements HttpRetryHandler {
@Resource
protected Logger logger = Logger.NULL;
+ @VisibleForTesting
+ final static int NUM_RETRIES = 5;
+
private final LoadingCache<Credentials, Access> authenticationResponseCache;
@Inject
@@ -51,32 +61,56 @@ public class RetryOnRenew implements HttpRetryHandler {
this.authenticationResponseCache = authenticationResponseCache;
}
+ /*
+ * The reason retries need to be tracked is that it is possible that a token
+ * can be expired at any time. The reason we track by request is that only
+ * some requests might return a 401 (such as temporary URLs). However
+ * consistent failures of the magnitude this code tracks should indicate a
+ * problem.
+ */
+ private static final Cache<HttpCommand, Integer> retryCountMap = CacheBuilder.newBuilder()
+ .expireAfterWrite(5, TimeUnit.MINUTES).build();
+
@Override
public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) {
- boolean retry = false; // default
try {
switch (response.getStatusCode()) {
case 401:
// Do not retry on 401 from authentication request
Multimap<String, String> headers = command.getCurrentRequest().getHeaders();
- if (headers != null && headers.containsKey("X-Auth-User")
- && headers.containsKey("X-Auth-Key") && !headers.containsKey("X-Auth-Token")) {
- retry = false;
+ if (headers != null && headers.containsKey(AuthHeaders.AUTH_USER)
+ && headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) {
+ return false;
} else {
- byte[] content = closeClientButKeepContentStream(response);
- //TODO: what is the error when the session token expires??
- if (content != null && new String(content).contains("lease renew")) {
- logger.debug("invalidating authentication token");
+ closeClientButKeepContentStream(response);
+ // This is not an authentication request returning 401
+ // Check if we already had seen this request
+ Integer count = retryCountMap.getIfPresent(command);
+
+ if (count == null) {
+ // First time this non-authentication request failed
+ logger.debug("invalidating authentication token - first time for %s", command);
+ retryCountMap.put(command, 1);
authenticationResponseCache.invalidateAll();
- retry = true;
+ return true;
} else {
- retry = false;
+ // This request has failed before
+ if (count + 1 >= NUM_RETRIES) {
+ logger.debug("too many 401s - giving up after: %s for %s", count, command);
+ return false;
+ } else {
+ // Retry just in case
+ logger.debug("invalidating authentication token - retry %s for %s", count, command);
+ retryCountMap.put(command, count + 1);
+ // Wait between retries
+ authenticationResponseCache.invalidateAll();
+ Uninterruptibles.sleepUninterruptibly(5, TimeUnit.SECONDS);
+ return true;
+ }
}
}
- break;
}
- return retry;
-
+ return false;
} finally {
releasePayload(response);
}
http://git-wip-us.apache.org/repos/asf/incubator-jclouds/blob/578a77d6/apis/openstack-keystone/src/test/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenewTest.java
----------------------------------------------------------------------
diff --git a/apis/openstack-keystone/src/test/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenewTest.java b/apis/openstack-keystone/src/test/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenewTest.java
index f152701..17e687f 100644
--- a/apis/openstack-keystone/src/test/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenewTest.java
+++ b/apis/openstack-keystone/src/test/java/org/jclouds/openstack/keystone/v2_0/handlers/RetryOnRenewTest.java
@@ -21,6 +21,7 @@ import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.expectLastCall;
import static org.easymock.EasyMock.replay;
import static org.easymock.EasyMock.verify;
+import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;
import org.jclouds.domain.Credentials;
@@ -53,7 +54,7 @@ public class RetryOnRenewTest {
cache.invalidateAll();
expectLastCall();
- expect(response.getPayload()).andReturn(Payloads.newStringPayload("token expired, please renew")).anyTimes();
+ expect(response.getPayload()).andReturn(Payloads.newStringPayload("")).anyTimes();
expect(response.getStatusCode()).andReturn(401).atLeastOnce();
replay(command);
@@ -68,4 +69,34 @@ public class RetryOnRenewTest {
verify(response);
verify(cache);
}
+ @Test
+ public void test401ShouldRetry4Times() {
+ HttpCommand command = createMock(HttpCommand.class);
+ HttpRequest request = createMock(HttpRequest.class);
+ HttpResponse response = createMock(HttpResponse.class);
+
+ @SuppressWarnings("unchecked")
+ LoadingCache<Credentials, Access> cache = createMock(LoadingCache.class);
+
+ expect(command.getCurrentRequest()).andReturn(request).anyTimes();
+ expect(request.getHeaders()).andStubReturn(null);
+
+ cache.invalidateAll();
+ expectLastCall().anyTimes();
+
+ expect(response.getPayload()).andReturn(Payloads.newStringPayload("")).anyTimes();
+ expect(response.getStatusCode()).andReturn(401).anyTimes();
+
+ replay(command, request, response, cache);
+
+ RetryOnRenew retry = new RetryOnRenew(cache);
+
+ for (int i = 0; i < RetryOnRenew.NUM_RETRIES - 1; ++i) {
+ assertTrue(retry.shouldRetryRequest(command, response), "Expected retry to succeed");
+ }
+
+ assertFalse(retry.shouldRetryRequest(command, response), "Expected retry to fail on attempt " + RetryOnRenew.NUM_RETRIES);
+
+ verify(command, response, cache);
+ }
}