You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2014/06/25 16:10:26 UTC

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

GitHub user ahgittin opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/19

    fixes the "RetryOnRenew" lease-renewal needed bug in jclouds, locally, with some guice magic

    https://issues.apache.org/jira/browse/JCLOUDS-615 / https://issues.apache.org/jira/browse/BROOKLYN-6
    
    when the former above is fixed properly upstream we can undo this.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ahgittin/incubator-brooklyn issue-6-jclouds-blobstore-renewal

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/19.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #19
    
----
commit a5a9de81873d2f7cbf3cf4a9f4ee2b673dd15899
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date:   2014-06-25T14:03:41Z

    fixes the "RetryOnRenew" lease-renewal needed bug in jclouds, locally, with some guice magic.
    
    https://issues.apache.org/jira/browse/JCLOUDS-615 / https://issues.apache.org/jira/browse/BROOKLYN-6
    
    when the former above is fixed properly upstream we can undo this.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/19#discussion_r14232802
  
    --- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/config/AlwaysRetryOnRenew.java ---
    @@ -0,0 +1,121 @@
    +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>() {
    --- End diff --
    
    Wow, nice workaround!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/19#discussion_r14233115
  
    --- Diff: locations/jclouds/src/main/java/brooklyn/entity/rebind/persister/jclouds/JcloudsBlobStoreBasedObjectStore.java ---
    @@ -72,11 +71,7 @@ public synchronized BlobStoreContext getBlobStoreContext() {
                 String provider = checkNotNull(location.getConfig(LocationConfigKeys.CLOUD_PROVIDER), "provider must not be null");
                 String endpoint = location.getConfig(CloudLocationConfig.CLOUD_ENDPOINT);
     
    -            ContextBuilder contextBuilder = ContextBuilder.newBuilder(provider).credentials(identity, credential);
    -            if (!Strings.isBlank(endpoint)) {
    -                contextBuilder.endpoint(endpoint);
    -            }
    -            context = contextBuilder.buildView(BlobStoreContext.class);
    +            context = JcloudsUtil.newBlobstoreContext(provider, endpoint, identity, credential, true);
    --- End diff --
    
    What are implication of always passing in `useSoftlayerFix == true`? Could that affect/break other providers? Should we check if `provider.equals("softlayer")` (or whatever softlayer blobstore provider is called)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/19#discussion_r14233432
  
    --- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/BlobStoreCapturingError.java ---
    @@ -0,0 +1,119 @@
    +package brooklyn.location.jclouds;
    +
    +import java.util.Set;
    +
    +import org.jclouds.blobstore.BlobStore;
    +import org.jclouds.blobstore.BlobStoreContext;
    +import org.jclouds.blobstore.domain.Blob;
    +import org.jclouds.blobstore.domain.BlobBuilder;
    +import org.jclouds.blobstore.domain.BlobMetadata;
    +import org.jclouds.blobstore.domain.PageSet;
    +import org.jclouds.blobstore.domain.StorageMetadata;
    +import org.jclouds.blobstore.options.CreateContainerOptions;
    +import org.jclouds.blobstore.options.GetOptions;
    +import org.jclouds.blobstore.options.ListContainerOptions;
    +import org.jclouds.blobstore.options.PutOptions;
    +import org.jclouds.domain.Location;
    +
    +public class BlobStoreCapturingError implements BlobStore {
    --- End diff --
    
    This just delegates, rather than captures errors - did I miss something?
    Should this be called `DelegatingBlobStore`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/19#discussion_r14233615
  
    --- Diff: locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreExpiryTest.java ---
    @@ -0,0 +1,195 @@
    +package brooklyn.entity.rebind.persister.jclouds;
    +
    +import static com.google.common.base.Preconditions.checkNotNull;
    +import static org.jclouds.openstack.reference.AuthHeaders.URL_SUFFIX;
    +
    +import java.io.IOException;
    +import java.net.URI;
    +import java.util.List;
    +import java.util.Map.Entry;
    +
    +import org.apache.http.client.HttpClient;
    +import org.jclouds.blobstore.BlobStoreContext;
    +import org.jclouds.blobstore.domain.PageSet;
    +import org.jclouds.blobstore.domain.StorageMetadata;
    +import org.jclouds.domain.Credentials;
    +import org.jclouds.openstack.domain.AuthenticationResponse;
    +import org.jclouds.openstack.handlers.RetryOnRenew;
    +import org.jclouds.openstack.reference.AuthHeaders;
    +import org.junit.Assert;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +import org.testng.annotations.AfterMethod;
    +import org.testng.annotations.BeforeMethod;
    +import org.testng.annotations.Test;
    +
    +import brooklyn.config.BrooklynProperties;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.location.basic.LocationConfigKeys;
    +import brooklyn.location.cloud.CloudLocationConfig;
    +import brooklyn.location.jclouds.JcloudsLocation;
    +import brooklyn.location.jclouds.JcloudsUtil;
    +import brooklyn.management.ManagementContext;
    +import brooklyn.test.entity.LocalManagementContextForTests;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.http.HttpTool;
    +import brooklyn.util.http.HttpToolResponse;
    +import brooklyn.util.text.Identifiers;
    +import brooklyn.util.time.Duration;
    +import brooklyn.util.time.Time;
    +
    +import com.google.common.base.Preconditions;
    +import com.google.common.cache.LoadingCache;
    +import com.google.common.collect.ImmutableMap;
    +import com.google.common.collect.ImmutableMap.Builder;
    +import com.google.inject.Inject;
    +import com.google.inject.Module;
    +
    +@Test(groups="Integration")
    +public class BlobStoreExpiryTest {
    +
    +    private static final Logger log = LoggerFactory.getLogger(BlobStoreExpiryTest.class);
    +    
    +    /**
    +     * Integration tests as written require a location defined as follows:
    +     * 
    +     * <code>
    +     * brooklyn.location.named.brooklyn-jclouds-objstore-test-1==jclouds:swift:https://ams01.objectstorage.softlayer.net/auth/v1.0
    +     * brooklyn.location.named.brooklyn-jclouds-objstore-test-1.identity=IBMOS1234-5:yourname
    +     * brooklyn.location.named.brooklyn-jclouds-objstore-test-1.credential=0123abcd.......
    +     * </code>
    +     */
    +    
    +    public static final String PERSIST_TO_OBJECT_STORE_FOR_TEST_SPEC = BlobStoreTest.PERSIST_TO_OBJECT_STORE_FOR_TEST_SPEC;
    +    public static final String CONTAINER_PREFIX = "brooklyn-persistence-test";
    +    private String locationSpec = PERSIST_TO_OBJECT_STORE_FOR_TEST_SPEC;
    +    
    +    private JcloudsLocation location;
    +    private BlobStoreContext context;
    +
    +    private ManagementContext mgmt;
    +    private String testContainerName;
    +
    +    Module myAuth;
    +    private String identity;
    +    private String credential;
    +    private String provider;
    +    private String endpoint;
    +
    +    public synchronized BlobStoreContext getBlobStoreContext(boolean applyFix) {
    +        if (context==null) {
    +            if (location==null) {
    +                Preconditions.checkNotNull(locationSpec, "locationSpec required for remote object store when location is null");
    +                Preconditions.checkNotNull(mgmt, "mgmt required for remote object store when location is null");
    +                location = (JcloudsLocation) mgmt.getLocationRegistry().resolve(locationSpec);
    +            }
    +            
    +            identity = checkNotNull(location.getConfig(LocationConfigKeys.ACCESS_IDENTITY), "identity must not be null");
    +            credential = checkNotNull(location.getConfig(LocationConfigKeys.ACCESS_CREDENTIAL), "credential must not be null");
    +            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);
    +        }
    +        return context;
    +    }
    +    
    +    @BeforeMethod(alwaysRun=true)
    +    public void setup() {
    +        testContainerName = CONTAINER_PREFIX+"-"+Identifiers.makeRandomId(8);
    +        mgmt = new LocalManagementContextForTests(BrooklynProperties.Factory.newDefault());
    +    }
    +    
    +    @AfterMethod(alwaysRun=true)
    +    public void teardown() {
    +        Entities.destroyAll(mgmt);
    +        if (context!=null) context.close();
    +        context = null;
    +    }
    +    
    +    public void testRenewAuthFailsInSoftlayer() throws IOException {
    +        doTestRenewAuth(false);
    +    }
    +
    +    public void testRenewAuthSucceedsWithOurOverride() throws IOException {
    +        doTestRenewAuth(true);
    +    }
    +    
    +    protected void doTestRenewAuth(boolean applyFix) throws IOException {
    +        getBlobStoreContext(applyFix);
    +        
    +        injectShortLivedTokenForSoftlayerAmsterdam();
    +        
    +        context.getBlobStore().createContainerInLocation(null, testContainerName);
    +        
    +        assertContainerFound();
    +        
    +        log.info("created container, now sleeping for expiration");
    +        
    +        Time.sleep(Duration.TEN_SECONDS);
    +        
    +        if (!applyFix) {
    +            // with the fix not applied, we have to invalidate the cache manually
    +            try {
    +                assertContainerFound();
    +                Assert.fail("should fail as long as "+RetryOnRenew.class+" is not working");
    +            } catch (Exception e) {
    +                log.info("failed, as expected: "+e);
    +            }
    +            getAuthCache().invalidateAll();
    +            log.info("invalidated, should now succeed");
    +        }
    +
    +        assertContainerFound();
    +
    +        context.getBlobStore().deleteContainer(testContainerName);
    +    }
    +
    +    private void assertContainerFound() {
    +        PageSet<? extends StorageMetadata> ps = context.getBlobStore().list();
    +        BlobStoreTest.assertHasItemNamed(ps, testContainerName);
    +    }
    +
    +    private void injectShortLivedTokenForSoftlayerAmsterdam() {
    +        HttpToolResponse tokenHttpResponse1 = requestTokenWithExplicitLifetime("https://ams01.objectstorage.softlayer.net/auth/v1.0/v1.0", "ams01.objectstorage.softlayer.net", 
    +            identity, credential, Duration.FIVE_SECONDS);
    +        
    +        Builder<String, URI> servicesMapBuilder = ImmutableMap.builder();
    +        for (Entry<String, List<String>> entry : tokenHttpResponse1.getHeaderLists().entrySet()) {
    +           if (entry.getKey().toLowerCase().endsWith(URL_SUFFIX.toLowerCase()))
    +              servicesMapBuilder.put(entry.getKey(), URI.create(entry.getValue().iterator().next()));
    +        }
    +        AuthenticationResponse authResponse = new AuthenticationResponse(tokenHttpResponse1.getHeaderLists().get(AuthHeaders.AUTH_TOKEN).get(0), servicesMapBuilder.build());
    +        
    +        getAuthCache().put(new Credentials(identity, credential), authResponse);
    +    }
    +
    +    private LoadingCache<Credentials, AuthenticationResponse> getAuthCache() {
    +        return context.utils().injector().getInstance(CachePeeker.class).authenticationResponseCache;
    +    }
    +    
    +    public static class CachePeeker {
    +        private final LoadingCache<Credentials, AuthenticationResponse> authenticationResponseCache;
    +
    +        @Inject
    +        protected CachePeeker(LoadingCache<Credentials, AuthenticationResponse> authenticationResponseCache) {
    +           this.authenticationResponseCache = authenticationResponseCache;
    +        }
    +    }
    +    
    +    public static HttpToolResponse requestTokenWithExplicitLifetime(String url, String host, String user, String key, Duration expiration) {
    +        HttpClient client = HttpTool.httpClientBuilder().build();
    +        HttpToolResponse response = HttpTool.httpGet(client, URI.create(url), MutableMap.<String,String>of()
    +            .add(AuthHeaders.AUTH_USER, user)
    +            .add(AuthHeaders.AUTH_KEY, key)
    +            .add("Host", host)
    +            .add("X-Auth-New-Token", ""+true)
    +            .add("X-Auth-Token-Lifetime", ""+expiration.toSeconds())
    --- End diff --
    
    Nice testing!
    Should this be a `Live` test rather than an `Integration` test though?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/19#issuecomment-47109909
  
    lgtm +1
    
    hopefully jclouds will react fast so that we can remove soon those guice magics
    
    nice tests as well


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/19#issuecomment-47209834
  
    Looks good; a few minor comments only.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/19#issuecomment-47212295
  
    i have a separate commit to fix the test cases, but i will push to #12 rebased on this to prevent conflicts
    
    ready for merge


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/19


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/19#discussion_r14233833
  
    --- Diff: locations/jclouds/src/main/java/brooklyn/location/jclouds/BlobStoreCapturingError.java ---
    @@ -0,0 +1,119 @@
    +package brooklyn.location.jclouds;
    +
    +import java.util.Set;
    +
    +import org.jclouds.blobstore.BlobStore;
    +import org.jclouds.blobstore.BlobStoreContext;
    +import org.jclouds.blobstore.domain.Blob;
    +import org.jclouds.blobstore.domain.BlobBuilder;
    +import org.jclouds.blobstore.domain.BlobMetadata;
    +import org.jclouds.blobstore.domain.PageSet;
    +import org.jclouds.blobstore.domain.StorageMetadata;
    +import org.jclouds.blobstore.options.CreateContainerOptions;
    +import org.jclouds.blobstore.options.GetOptions;
    +import org.jclouds.blobstore.options.ListContainerOptions;
    +import org.jclouds.blobstore.options.PutOptions;
    +import org.jclouds.domain.Location;
    +
    +public class BlobStoreCapturingError implements BlobStore {
    --- End diff --
    
    whoops this should be erased, it was my first thought at capturing timeouts and retrying in our code rather than at jclouds level


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/19#discussion_r14233877
  
    --- Diff: locations/jclouds/src/test/java/brooklyn/entity/rebind/persister/jclouds/BlobStoreExpiryTest.java ---
    @@ -0,0 +1,195 @@
    +package brooklyn.entity.rebind.persister.jclouds;
    +
    +import static com.google.common.base.Preconditions.checkNotNull;
    +import static org.jclouds.openstack.reference.AuthHeaders.URL_SUFFIX;
    +
    +import java.io.IOException;
    +import java.net.URI;
    +import java.util.List;
    +import java.util.Map.Entry;
    +
    +import org.apache.http.client.HttpClient;
    +import org.jclouds.blobstore.BlobStoreContext;
    +import org.jclouds.blobstore.domain.PageSet;
    +import org.jclouds.blobstore.domain.StorageMetadata;
    +import org.jclouds.domain.Credentials;
    +import org.jclouds.openstack.domain.AuthenticationResponse;
    +import org.jclouds.openstack.handlers.RetryOnRenew;
    +import org.jclouds.openstack.reference.AuthHeaders;
    +import org.junit.Assert;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +import org.testng.annotations.AfterMethod;
    +import org.testng.annotations.BeforeMethod;
    +import org.testng.annotations.Test;
    +
    +import brooklyn.config.BrooklynProperties;
    +import brooklyn.entity.basic.Entities;
    +import brooklyn.location.basic.LocationConfigKeys;
    +import brooklyn.location.cloud.CloudLocationConfig;
    +import brooklyn.location.jclouds.JcloudsLocation;
    +import brooklyn.location.jclouds.JcloudsUtil;
    +import brooklyn.management.ManagementContext;
    +import brooklyn.test.entity.LocalManagementContextForTests;
    +import brooklyn.util.collections.MutableMap;
    +import brooklyn.util.http.HttpTool;
    +import brooklyn.util.http.HttpToolResponse;
    +import brooklyn.util.text.Identifiers;
    +import brooklyn.util.time.Duration;
    +import brooklyn.util.time.Time;
    +
    +import com.google.common.base.Preconditions;
    +import com.google.common.cache.LoadingCache;
    +import com.google.common.collect.ImmutableMap;
    +import com.google.common.collect.ImmutableMap.Builder;
    +import com.google.inject.Inject;
    +import com.google.inject.Module;
    +
    +@Test(groups="Integration")
    +public class BlobStoreExpiryTest {
    +
    +    private static final Logger log = LoggerFactory.getLogger(BlobStoreExpiryTest.class);
    +    
    +    /**
    +     * Integration tests as written require a location defined as follows:
    +     * 
    +     * <code>
    +     * brooklyn.location.named.brooklyn-jclouds-objstore-test-1==jclouds:swift:https://ams01.objectstorage.softlayer.net/auth/v1.0
    +     * brooklyn.location.named.brooklyn-jclouds-objstore-test-1.identity=IBMOS1234-5:yourname
    +     * brooklyn.location.named.brooklyn-jclouds-objstore-test-1.credential=0123abcd.......
    +     * </code>
    +     */
    +    
    +    public static final String PERSIST_TO_OBJECT_STORE_FOR_TEST_SPEC = BlobStoreTest.PERSIST_TO_OBJECT_STORE_FOR_TEST_SPEC;
    +    public static final String CONTAINER_PREFIX = "brooklyn-persistence-test";
    +    private String locationSpec = PERSIST_TO_OBJECT_STORE_FOR_TEST_SPEC;
    +    
    +    private JcloudsLocation location;
    +    private BlobStoreContext context;
    +
    +    private ManagementContext mgmt;
    +    private String testContainerName;
    +
    +    Module myAuth;
    +    private String identity;
    +    private String credential;
    +    private String provider;
    +    private String endpoint;
    +
    +    public synchronized BlobStoreContext getBlobStoreContext(boolean applyFix) {
    +        if (context==null) {
    +            if (location==null) {
    +                Preconditions.checkNotNull(locationSpec, "locationSpec required for remote object store when location is null");
    +                Preconditions.checkNotNull(mgmt, "mgmt required for remote object store when location is null");
    +                location = (JcloudsLocation) mgmt.getLocationRegistry().resolve(locationSpec);
    +            }
    +            
    +            identity = checkNotNull(location.getConfig(LocationConfigKeys.ACCESS_IDENTITY), "identity must not be null");
    +            credential = checkNotNull(location.getConfig(LocationConfigKeys.ACCESS_CREDENTIAL), "credential must not be null");
    +            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);
    +        }
    +        return context;
    +    }
    +    
    +    @BeforeMethod(alwaysRun=true)
    +    public void setup() {
    +        testContainerName = CONTAINER_PREFIX+"-"+Identifiers.makeRandomId(8);
    +        mgmt = new LocalManagementContextForTests(BrooklynProperties.Factory.newDefault());
    +    }
    +    
    +    @AfterMethod(alwaysRun=true)
    +    public void teardown() {
    +        Entities.destroyAll(mgmt);
    +        if (context!=null) context.close();
    +        context = null;
    +    }
    +    
    +    public void testRenewAuthFailsInSoftlayer() throws IOException {
    +        doTestRenewAuth(false);
    +    }
    +
    +    public void testRenewAuthSucceedsWithOurOverride() throws IOException {
    +        doTestRenewAuth(true);
    +    }
    +    
    +    protected void doTestRenewAuth(boolean applyFix) throws IOException {
    +        getBlobStoreContext(applyFix);
    +        
    +        injectShortLivedTokenForSoftlayerAmsterdam();
    +        
    +        context.getBlobStore().createContainerInLocation(null, testContainerName);
    +        
    +        assertContainerFound();
    +        
    +        log.info("created container, now sleeping for expiration");
    +        
    +        Time.sleep(Duration.TEN_SECONDS);
    +        
    +        if (!applyFix) {
    +            // with the fix not applied, we have to invalidate the cache manually
    +            try {
    +                assertContainerFound();
    +                Assert.fail("should fail as long as "+RetryOnRenew.class+" is not working");
    +            } catch (Exception e) {
    +                log.info("failed, as expected: "+e);
    +            }
    +            getAuthCache().invalidateAll();
    +            log.info("invalidated, should now succeed");
    +        }
    +
    +        assertContainerFound();
    +
    +        context.getBlobStore().deleteContainer(testContainerName);
    +    }
    +
    +    private void assertContainerFound() {
    +        PageSet<? extends StorageMetadata> ps = context.getBlobStore().list();
    +        BlobStoreTest.assertHasItemNamed(ps, testContainerName);
    +    }
    +
    +    private void injectShortLivedTokenForSoftlayerAmsterdam() {
    +        HttpToolResponse tokenHttpResponse1 = requestTokenWithExplicitLifetime("https://ams01.objectstorage.softlayer.net/auth/v1.0/v1.0", "ams01.objectstorage.softlayer.net", 
    +            identity, credential, Duration.FIVE_SECONDS);
    +        
    +        Builder<String, URI> servicesMapBuilder = ImmutableMap.builder();
    +        for (Entry<String, List<String>> entry : tokenHttpResponse1.getHeaderLists().entrySet()) {
    +           if (entry.getKey().toLowerCase().endsWith(URL_SUFFIX.toLowerCase()))
    +              servicesMapBuilder.put(entry.getKey(), URI.create(entry.getValue().iterator().next()));
    +        }
    +        AuthenticationResponse authResponse = new AuthenticationResponse(tokenHttpResponse1.getHeaderLists().get(AuthHeaders.AUTH_TOKEN).get(0), servicesMapBuilder.build());
    +        
    +        getAuthCache().put(new Credentials(identity, credential), authResponse);
    +    }
    +
    +    private LoadingCache<Credentials, AuthenticationResponse> getAuthCache() {
    +        return context.utils().injector().getInstance(CachePeeker.class).authenticationResponseCache;
    +    }
    +    
    +    public static class CachePeeker {
    +        private final LoadingCache<Credentials, AuthenticationResponse> authenticationResponseCache;
    +
    +        @Inject
    +        protected CachePeeker(LoadingCache<Credentials, AuthenticationResponse> authenticationResponseCache) {
    +           this.authenticationResponseCache = authenticationResponseCache;
    +        }
    +    }
    +    
    +    public static HttpToolResponse requestTokenWithExplicitLifetime(String url, String host, String user, String key, Duration expiration) {
    +        HttpClient client = HttpTool.httpClientBuilder().build();
    +        HttpToolResponse response = HttpTool.httpGet(client, URI.create(url), MutableMap.<String,String>of()
    +            .add(AuthHeaders.AUTH_USER, user)
    +            .add(AuthHeaders.AUTH_KEY, key)
    +            .add("Host", host)
    +            .add("X-Auth-New-Token", ""+true)
    +            .add("X-Auth-Token-Lifetime", ""+expiration.toSeconds())
    --- End diff --
    
    oops yeah probably.  i've been marking a lot of jclouds-based tests as `Integration` but that's probably wrong, eh?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: fixes the "RetryOnRenew" lease-re...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/19#discussion_r14233806
  
    --- Diff: locations/jclouds/src/main/java/brooklyn/entity/rebind/persister/jclouds/JcloudsBlobStoreBasedObjectStore.java ---
    @@ -72,11 +71,7 @@ public synchronized BlobStoreContext getBlobStoreContext() {
                 String provider = checkNotNull(location.getConfig(LocationConfigKeys.CLOUD_PROVIDER), "provider must not be null");
                 String endpoint = location.getConfig(CloudLocationConfig.CLOUD_ENDPOINT);
     
    -            ContextBuilder contextBuilder = ContextBuilder.newBuilder(provider).credentials(identity, credential);
    -            if (!Strings.isBlank(endpoint)) {
    -                contextBuilder.endpoint(endpoint);
    -            }
    -            context = contextBuilder.buildView(BlobStoreContext.class);
    +            context = JcloudsUtil.newBlobstoreContext(provider, endpoint, identity, credential, true);
    --- End diff --
    
    `useSoftlayerFix` should only affect openstack.  the guice `bindInterceptor` is targetted against an openstack class.  i think there is a good chance this bug will affect some other openstacks (report of same problem in hp cloud, for instance).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---