JCLOUDS-1294: Attempt to retry RetryableErrors in Azure ARM

core/src/main/java/org/jclouds/http/handlers/RateLimitRetryHandler.java

@@ -87,7 +87,7 @@ public abstract class RateLimitRetryHandler implements HttpRetryHandler {
       }
    }
 
-   private boolean delayRequestUntilAllowed(final HttpCommand command, final HttpResponse response) {
+   protected boolean delayRequestUntilAllowed(final HttpCommand command, final HttpResponse response) {
       Optional<Long> millisToNextAvailableRequest = millisToNextAvailableRequest(command, response);
       if (!millisToNextAvailableRequest.isPresent()) {
          logger.error("Cannot retry after rate limit error, no retry information provided in the response");

providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/domain/Error.java

@@ -0,0 +1,58 @@
+/*
+ * 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 org.jclouds.azurecompute.arm.domain;
+
+import java.util.List;
+
+import org.jclouds.javax.annotation.Nullable;
+import org.jclouds.json.SerializedNames;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
+
+@AutoValue
+public abstract class Error {
+
+   public abstract Details details();
+
+   @SerializedNames({ "error" })
+   public static Error create(Details details) {
+      return new AutoValue_Error(details);
+   }
+
+   Error() {
+
+   }
+
+   @AutoValue
+   public abstract static class Details {
+      public abstract String code();
+      public abstract String message();
+      public abstract List<Details> details();
+
+      @SerializedNames({ "code", "message", "details" })
+      public static Details create(String code, String message, @Nullable List<Details> details) {
+         return new AutoValue_Error_Details(code, message, details == null ? ImmutableList.<Details> of()
+               : ImmutableList.copyOf(details));
+      }
+
+      Details() {
+
+      }
+   }
+
+}

providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureComputeErrorHandler.java

@@ -16,6 +16,8 @@
  */
 package org.jclouds.azurecompute.arm.handlers;
 
+import static org.jclouds.azurecompute.arm.handlers.AzureRateLimitRetryHandler.isRateLimitError;
+
 import java.io.IOException;
 
 import javax.inject.Singleton;
@@ -38,7 +40,10 @@ public class AzureComputeErrorHandler implements HttpErrorHandler {
 
    @Override
    public void handleError(final HttpCommand command, final HttpResponse response) {
-      // it is important to always read fully and close streams
+      // It is important to always read fully and close streams
+      // For 429 errors the response body might have already been consumed as
+      // some errors report information in the response body that needs to be
+      // handled by the retry handlers.
       String message = parseMessage(response);
       Exception exception = message == null
               ? new HttpResponseException(command, response)
@@ -70,7 +75,11 @@ public class AzureComputeErrorHandler implements HttpErrorHandler {
                exception = new IllegalStateException(message, exception);
                break;
             case 429:
+               if (isRateLimitError(response)) {
                   exception = new AzureComputeRateLimitExceededException(response, exception);
+               } else {
+                  exception = new IllegalStateException(message, exception);
+               }
                break;
             default:
          }

providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRateLimitRetryHandler.java

@@ -16,6 +16,7 @@
  */
 package org.jclouds.azurecompute.arm.handlers;
 
+import javax.inject.Inject;
 import javax.inject.Singleton;
 
 import org.jclouds.http.HttpCommand;
@@ -26,14 +27,41 @@ import com.google.common.annotations.Beta;
 import com.google.common.base.Optional;
 import com.google.common.net.HttpHeaders;
 
+/**
+ * Handles 429 Too Many Requests responses.
+ * <p>
+ * The Azure ARM provider also returns this 429 HTTP status code for some errors
+ * when resources are busy or in a state where they cannot be modified. In this
+ * case this handler delegates to the {@link AzureRetryableErrorHandler} to
+ * determine if the requests can be retried.
+ */
 @Beta
 @Singleton
 public class AzureRateLimitRetryHandler extends RateLimitRetryHandler {
 
+   private final AzureRetryableErrorHandler retryableErrorHandler;
+
+   @Inject
+   AzureRateLimitRetryHandler(AzureRetryableErrorHandler retryableErrorHandler) {
+      this.retryableErrorHandler = retryableErrorHandler;
+   }
+
+   @Override
+   protected boolean delayRequestUntilAllowed(HttpCommand command, HttpResponse response) {
+      if (!isRateLimitError(response)) {
+         return retryableErrorHandler.shouldRetryRequest(command, response);
+      }
+      return super.delayRequestUntilAllowed(command, response);
+   }
+
    @Override
    protected Optional<Long> millisToNextAvailableRequest(HttpCommand command, HttpResponse response) {
       String secondsToNextAvailableRequest = response.getFirstHeaderOrNull(HttpHeaders.RETRY_AFTER);
       return secondsToNextAvailableRequest != null ? Optional.of(Long.valueOf(secondsToNextAvailableRequest) * 1000)
             : Optional.<Long> absent();
    }
+
+   public static boolean isRateLimitError(HttpResponse response) {
+      return response.getFirstHeaderOrNull(HttpHeaders.RETRY_AFTER) != null;
+   }
 }

providers/azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandler.java

@@ -0,0 +1,81 @@
+/*
+ * 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 org.jclouds.azurecompute.arm.handlers;
+
+import static org.jclouds.azurecompute.arm.handlers.AzureRateLimitRetryHandler.isRateLimitError;
+
+import javax.annotation.Resource;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+
+import org.jclouds.azurecompute.arm.domain.Error;
+import org.jclouds.http.HttpCommand;
+import org.jclouds.http.HttpResponse;
+import org.jclouds.http.functions.ParseJson;
+import org.jclouds.http.handlers.BackoffLimitedRetryHandler;
+import org.jclouds.logging.Logger;
+
+import com.google.common.annotations.Beta;
+
+/**
+ * This handles failed responses that return a <code>RetryableError</code>.
+ * <p>
+ * In order to determine if the error is retryable, the response body must be
+ * read, so this handler will have to buffer the response payload in memory so
+ * the response body can be read again in subsequent steps of the response
+ * processing flow.
+ */
+@Singleton
+@Beta
+public class AzureRetryableErrorHandler extends BackoffLimitedRetryHandler {
+
+   private static final String RETRYABLE_ERROR_CODE = "RetryableError";
+
+   @Resource
+   protected Logger logger = Logger.NULL;
+   private final ParseJson<Error> parseError;
+
+   @Inject
+   AzureRetryableErrorHandler(ParseJson<Error> parseError) {
+      this.parseError = parseError;
+   }
+
+   @Override
+   public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) {
+      // Only consider retryable errors and discard rate limit ones
+      if (response.getStatusCode() != 429 || isRateLimitError(response)) {
+         return false;
+      }
+
+      try {
+         // Note that this will consume the response body. At this point,
+         // subsequent retry handlers or error handlers will not be able to read
+         // again the payload, but that should only be attempted when the
+         // command is not retryable and an exception should be thrown.
+         Error error = parseError.apply(response);
+         logger.debug("processing error: %s", error);
+
+         boolean isRetryable = RETRYABLE_ERROR_CODE.equals(error.details().code());
+         return isRetryable ? super.shouldRetryRequest(command, response) : false;
+      } catch (Exception ex) {
+         // If we can't parse the error, just assume it is not a retryable error
+         logger.warn("could not parse error. Request won't be retried: %s", ex.getMessage());
+         return false;
+      }
+   }
+
+}

providers/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/handlers/AzureRetryableErrorHandlerTest.java

@@ -0,0 +1,88 @@
+/*
+ * 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 org.jclouds.azurecompute.arm.handlers;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+import org.jclouds.http.HttpCommand;
+import org.jclouds.http.HttpRequest;
+import org.jclouds.http.HttpResponse;
+import org.jclouds.http.HttpRetryHandler;
+import org.jclouds.json.config.GsonModule;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.net.HttpHeaders;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+
+@Test(groups = "unit", testName = "AzureRetryableErrorHandlerTest")
+public class AzureRetryableErrorHandlerTest {
+
+   private HttpRetryHandler handler;
+
+   @BeforeClass
+   public void setup() {
+      // Initialize an injector with just the Json features to get all
+      // serialization stuff
+      Injector injector = Guice.createInjector(new GsonModule());
+      handler = injector.getInstance(AzureRetryableErrorHandler.class);
+   }
+
+   @Test
+   public void testDoesNotRetryWhenNot429() {
+      HttpCommand command = new HttpCommand(HttpRequest.builder().method("GET").endpoint("http://localhost").build());
+      HttpResponse response = HttpResponse.builder().statusCode(400).build();
+
+      assertFalse(handler.shouldRetryRequest(command, response));
+   }
+
+   @Test
+   public void testDoesNotRetryWhenRateLimitError() {
+      HttpCommand command = new HttpCommand(HttpRequest.builder().method("GET").endpoint("http://localhost").build());
+      HttpResponse response = HttpResponse.builder().statusCode(429).addHeader(HttpHeaders.RETRY_AFTER, "15").build();
+
+      assertFalse(handler.shouldRetryRequest(command, response));
+   }
+
+   @Test
+   public void testDoesNotRetryWhenCannotParseError() {
+      HttpCommand command = new HttpCommand(HttpRequest.builder().method("GET").endpoint("http://localhost").build());
+      HttpResponse response = HttpResponse.builder().statusCode(429).payload("foo").build();
+
+      assertFalse(handler.shouldRetryRequest(command, response));
+   }
+
+   @Test
+   public void testDoesNotRetryWhenErrorNotRetryable() {
+      String nonRetryable = "{\"error\":{\"code\":\"ReferencedResourceNotProvisioned\",\"message\":\"Not provisioned\"}}";
+      HttpCommand command = new HttpCommand(HttpRequest.builder().method("GET").endpoint("http://localhost").build());
+      HttpResponse response = HttpResponse.builder().statusCode(429).payload(nonRetryable).build();
+
+      assertFalse(handler.shouldRetryRequest(command, response));
+   }
+
+   @Test
+   public void testRetriesWhenRetryableError() {
+      String retryable = "{\"error\":{\"code\":\"RetryableError\",\"message\":\"Resource busy\"}}";
+      HttpCommand command = new HttpCommand(HttpRequest.builder().method("GET").endpoint("http://localhost").build());
+      HttpResponse response = HttpResponse.builder().statusCode(429).payload(retryable).build();
+
+      assertTrue(handler.shouldRetryRequest(command, response));
+   }
+}