Repository: incubator-brooklyn Updated Branches: refs/heads/master e4b5f2428 -> 52a6c708f
Undo #19, fixed upstream RetryOnRenew fixed in jclouds upstream: * https://issues.apache.org/jira/browse/JCLOUDS-178 * https://issues.apache.org/jira/browse/JCLOUDS-589 Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/0141dc91 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/0141dc91 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/0141dc91 Branch: refs/heads/master Commit: 0141dc91f697c749040e7863ac6ae45fd06d3e67 Parents: e4b5f24 Author: Svetoslav Neykov <[email protected]> Authored: Tue Feb 3 22:21:17 2015 +0200 Committer: Svetoslav Neykov <[email protected]> Committed: Tue Mar 17 22:02:44 2015 +0200 ---------------------------------------------------------------------- .../JcloudsBlobStoreBasedObjectStore.java | 2 +- .../brooklyn/location/jclouds/JcloudsUtil.java | 14 +- .../jclouds/config/AlwaysRetryOnRenew.java | 139 ------------------- .../persister/jclouds/BlobStoreExpiryTest.java | 28 +--- .../rebind/persister/jclouds/BlobStoreTest.java | 2 +- .../jclouds/JcloudsExpect100ContinueTest.java | 3 +- .../main/java/brooklyn/cli/CloudExplorer.java | 2 +- 7 files changed, 11 insertions(+), 179 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/0141dc91/locations/jclouds/src/main/java/brooklyn/entity/rebind/persister/jclouds/JcloudsBlobStoreBasedObjectStore.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/brooklyn/entity/rebind/persister/jclouds/JcloudsBlobStoreBasedObjectStore.java b/locations/jclouds/src/main/java/brooklyn/entity/rebind/persister/jclouds/JcloudsBlobStoreBasedObjectStore.java index 6acaa97..60d7f1b 100644 --- a/locations/jclouds/src/main/java/brooklyn/entity/rebind/persister/jclouds/JcloudsBlobStoreBasedObjectStore.java +++ b/locations/jclouds/src/main/java/brooklyn/entity/rebind/persister/jclouds/JcloudsBlobStoreBasedObjectStore.java @@ -102,7 +102,7 @@ public class JcloudsBlobStoreBasedObjectStore implements PersistenceObjectStore String provider = checkNotNull(location.getConfig(LocationConfigKeys.CLOUD_PROVIDER), "provider must not be null"); String endpoint = location.getConfig(CloudLocationConfig.CLOUD_ENDPOINT); - context = JcloudsUtil.newBlobstoreContext(provider, endpoint, identity, credential, true); + context = JcloudsUtil.newBlobstoreContext(provider, endpoint, identity, credential); // TODO do we need to get location from region? can't see the jclouds API. // doesn't matter in some places because it's already in the endpoint http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/0141dc91/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsUtil.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsUtil.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsUtil.java index 4a304cb..eb0e605 100644 --- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsUtil.java +++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsUtil.java @@ -65,9 +65,7 @@ import org.jclouds.util.Predicates2; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import brooklyn.entity.basic.Entities; import brooklyn.entity.basic.Sanitizer; -import brooklyn.location.jclouds.config.AlwaysRetryOnRenew; import brooklyn.util.collections.MutableList; import brooklyn.util.config.ConfigBag; import brooklyn.util.exceptions.Exceptions; @@ -266,10 +264,7 @@ public class JcloudsUtil implements JcloudsLocationConfig { * * @since 1.7.0 */ @Beta - public static BlobStoreContext newBlobstoreContext(String provider, @Nullable String endpoint, String identity, String credential, boolean useSoftlayerFix) { - AlwaysRetryOnRenew.InterceptRetryOnRenewModule fix = - useSoftlayerFix ? new AlwaysRetryOnRenew.InterceptRetryOnRenewModule() : null; - + public static BlobStoreContext newBlobstoreContext(String provider, @Nullable String endpoint, String identity, String credential) { Properties overrides = new Properties(); // * Java 7,8 bug workaround - sockets closed by GC break the internal bookkeeping // of HttpUrlConnection, leading to invalid handling of the "HTTP/1.1 100 Continue" @@ -282,17 +277,12 @@ public class JcloudsUtil implements JcloudsLocationConfig { overrides.setProperty(Constants.PROPERTY_STRIP_EXPECT_HEADER, "true"); ContextBuilder contextBuilder = ContextBuilder.newBuilder(provider).credentials(identity, credential); - contextBuilder.modules(MutableList.copyOf(JcloudsUtil.getCommonModules()) - .appendIfNotNull(fix)); + contextBuilder.modules(MutableList.copyOf(JcloudsUtil.getCommonModules())); if (!brooklyn.util.text.Strings.isBlank(endpoint)) { contextBuilder.endpoint(endpoint); } contextBuilder.overrides(overrides); BlobStoreContext context = contextBuilder.buildView(BlobStoreContext.class); - - if (useSoftlayerFix) - fix.inject(context.utils().injector()); - return context; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/0141dc91/locations/jclouds/src/main/java/brooklyn/location/jclouds/config/AlwaysRetryOnRenew.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/config/AlwaysRetryOnRenew.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/config/AlwaysRetryOnRenew.java deleted file mode 100644 index 25580df..0000000 --- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/config/AlwaysRetryOnRenew.java +++ /dev/null @@ -1,139 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package brooklyn.location.jclouds.config; - -import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream; -import static org.jclouds.http.HttpUtils.releasePayload; - -import java.lang.reflect.Method; - -import javax.annotation.Resource; - -import org.aopalliance.intercept.MethodInterceptor; -import org.aopalliance.intercept.MethodInvocation; -import org.jclouds.domain.Credentials; -import org.jclouds.http.HttpCommand; -import org.jclouds.http.HttpResponse; -import org.jclouds.http.HttpRetryHandler; -import org.jclouds.logging.Logger; -import org.jclouds.openstack.domain.AuthenticationResponse; -import org.jclouds.openstack.handlers.RetryOnRenew; -import org.jclouds.openstack.reference.AuthHeaders; - -import com.google.common.annotations.Beta; -import com.google.common.cache.LoadingCache; -import com.google.common.collect.Multimap; -import com.google.inject.AbstractModule; -import com.google.inject.Inject; -import com.google.inject.Injector; -import com.google.inject.Singleton; -import com.google.inject.matcher.AbstractMatcher; -import com.google.inject.matcher.Matcher; -import com.google.inject.matcher.Matchers; - -/** Fix for RetryOnRenew so that it always retries on 401 when using a token. - * The "lease renew" text is not necessarily returned from swift servers. - * <p> - * See https://issues.apache.org/jira/browse/BROOKLYN-6 . - * When https://issues.apache.org/jira/browse/JCLOUDS-615 is fixed in the jclouds we use, - * we can remove this. - * <p> - * (Marked Beta as this will likely be removed.) - * - * @since 1.7.0 */ -@Beta -@Singleton -public class AlwaysRetryOnRenew implements HttpRetryHandler { - @Resource - protected Logger logger = Logger.NULL; - - private final LoadingCache<Credentials, AuthenticationResponse> authenticationResponseCache; - - @Inject - protected AlwaysRetryOnRenew(LoadingCache<Credentials, AuthenticationResponse> authenticationResponseCache) { - this.authenticationResponseCache = authenticationResponseCache; - } - - @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(AuthHeaders.AUTH_USER) - && headers.containsKey(AuthHeaders.AUTH_KEY) && !headers.containsKey(AuthHeaders.AUTH_TOKEN)) { - retry = false; - } else { - closeClientButKeepContentStream(response); - authenticationResponseCache.invalidateAll(); - retry = true; - - // always retry. not all swift servers say 'lease renew', e.g. softlayer - -// byte[] content = closeClientButKeepContentStream(response); -// if (content != null && new String(content).contains("lease renew")) { -// logger.debug("invalidating authentication token"); -// authenticationResponseCache.invalidateAll(); -// retry = true; -// } else { -// retry = false; -// } - } - break; - } - return retry; - - } finally { - releasePayload(response); - } - } - - /** - * Intercepts calls to the *other* RetryOnRenew instance, and uses the one above. - * It's messy, but the only way I could find in the maze of guice. */ - public static class InterceptRetryOnRenewModule extends AbstractModule { - AlwaysRetryOnRenew intereceptingRetryOnRenew; - - public void inject(Injector injector) { - intereceptingRetryOnRenew = injector.getInstance(AlwaysRetryOnRenew.class); - } - - @Override - protected void configure() { - Matcher<? super Class<?>> classMatcher = Matchers.subclassesOf(RetryOnRenew.class); - Matcher<? super Method> methodMatcher = new AbstractMatcher<Method>() { - @Override - public boolean matches(Method t) { - return "shouldRetryRequest".matches(t.getName()); - } - }; - MethodInterceptor interceptors = new MethodInterceptor() { - @Override - public Object invoke(MethodInvocation invocation) throws Throwable { - if (intereceptingRetryOnRenew==null) - throw new IllegalStateException("inject() must be called to set up before use"); - return intereceptingRetryOnRenew.shouldRetryRequest((HttpCommand)invocation.getArguments()[0], (HttpResponse)invocation.getArguments()[1]); - } - }; - bindInterceptor(classMatcher, methodMatcher, interceptors); - } - } -} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/0141dc91/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreExpiryTest.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreExpiryTest.java b/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreExpiryTest.java index 58eb2c3..76618a5 100644 --- a/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreExpiryTest.java +++ b/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreExpiryTest.java @@ -91,7 +91,7 @@ public class BlobStoreExpiryTest { private String provider; private String endpoint; - public synchronized BlobStoreContext getBlobStoreContext(boolean applyFix) { + public synchronized BlobStoreContext getBlobStoreContext() { if (context==null) { if (location==null) { Preconditions.checkNotNull(locationSpec, "locationSpec required for remote object store when location is null"); @@ -104,7 +104,7 @@ public class BlobStoreExpiryTest { provider = checkNotNull(location.getConfig(LocationConfigKeys.CLOUD_PROVIDER), "provider must not be null"); endpoint = location.getConfig(CloudLocationConfig.CLOUD_ENDPOINT); - context = JcloudsUtil.newBlobstoreContext(provider, endpoint, identity, credential, applyFix); + context = JcloudsUtil.newBlobstoreContext(provider, endpoint, identity, credential); } return context; } @@ -122,19 +122,12 @@ public class BlobStoreExpiryTest { context = null; } - // test disabled as https://issues.apache.org/jira/browse/JCLOUDS-589 fixed the issue for Keystone greater than 1.1 - // the test would be applicable if pointed at a Swift endpoint that was using Keystone v1.1. - @Test(enabled = false) - public void testRenewAuthFailsInKeystoneV1_1() throws IOException { - doTestRenewAuth(false); - } - public void testRenewAuthSucceedsWithOurOverride() throws IOException { - doTestRenewAuth(true); + doTestRenewAuth(); } - protected void doTestRenewAuth(boolean applyFix) throws IOException { - getBlobStoreContext(applyFix); + protected void doTestRenewAuth() throws IOException { + getBlobStoreContext(); injectShortLivedTokenForKeystoneV1_1(); @@ -146,17 +139,6 @@ public class BlobStoreExpiryTest { Time.sleep(Duration.TEN_SECONDS); - if (!applyFix) { - // with the fix not applied, we have to invalidate the cache manually - try { - assertContainerFound(); - } catch (Exception e) { - log.info("failed, as expected: "+e); - } - getAuthCache().invalidateAll(); - log.info("invalidated, should now succeed"); - } - assertContainerFound(); context.getBlobStore().deleteContainer(testContainerName); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/0141dc91/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreTest.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreTest.java b/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreTest.java index bad73e6..c01fa35 100644 --- a/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreTest.java +++ b/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreTest.java @@ -82,7 +82,7 @@ public class BlobStoreTest { String provider = checkNotNull(location.getConfig(LocationConfigKeys.CLOUD_PROVIDER), "provider must not be null"); String endpoint = location.getConfig(CloudLocationConfig.CLOUD_ENDPOINT); - context = JcloudsUtil.newBlobstoreContext(provider, endpoint, identity, credential, true); + context = JcloudsUtil.newBlobstoreContext(provider, endpoint, identity, credential); } return context; } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/0141dc91/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/JcloudsExpect100ContinueTest.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/JcloudsExpect100ContinueTest.java b/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/JcloudsExpect100ContinueTest.java index 1d5fbde..30cbfed 100644 --- a/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/JcloudsExpect100ContinueTest.java +++ b/locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/JcloudsExpect100ContinueTest.java @@ -64,8 +64,7 @@ public class JcloudsExpect100ContinueTest { jcloudsLocation.getProvider(), jcloudsLocation.getEndpoint(), jcloudsLocation.getIdentity(), - jcloudsLocation.getCredential(), - false); + jcloudsLocation.getCredential()); containerName = BlobStoreTest.CONTAINER_PREFIX+"-"+Identifiers.makeRandomId(8); context.getBlobStore().createContainerInLocation(null, containerName); } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/0141dc91/usage/cli/src/main/java/brooklyn/cli/CloudExplorer.java ---------------------------------------------------------------------- diff --git a/usage/cli/src/main/java/brooklyn/cli/CloudExplorer.java b/usage/cli/src/main/java/brooklyn/cli/CloudExplorer.java index 3dd8462..17db6de 100644 --- a/usage/cli/src/main/java/brooklyn/cli/CloudExplorer.java +++ b/usage/cli/src/main/java/brooklyn/cli/CloudExplorer.java @@ -297,7 +297,7 @@ public class CloudExplorer { String provider = checkNotNull(loc.getConfig(LocationConfigKeys.CLOUD_PROVIDER), "provider must not be null"); String endpoint = loc.getConfig(CloudLocationConfig.CLOUD_ENDPOINT); - BlobStoreContext context = JcloudsUtil.newBlobstoreContext(provider, endpoint, identity, credential, true); + BlobStoreContext context = JcloudsUtil.newBlobstoreContext(provider, endpoint, identity, credential); try { BlobStore blobStore = context.getBlobStore(); doCall(blobStore, indent);
