Issue 501: break retry loop on timeout or illegal state

compute/src/main/java/org/jclouds/compute/strategy/CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap.java

@@ -22,6 +22,7 @@ package org.jclouds.compute.strategy;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Throwables.getRootCause;
+import static java.lang.String.format;
 import static org.jclouds.compute.util.ComputeServiceUtils.findReachableSocketOnNode;
 
 import java.util.Map;
@@ -126,6 +127,7 @@ public class CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap implements Cal
    public Void call() {
       checkState(!tainted, "this object is not designed to be reused: %s", toString());
       tainted = true;
+      String originalId = node.getId();
       try {
          if (options.shouldBlockUntilRunning()) {
             if (nodeRunning.apply(node)) {
@@ -133,10 +135,12 @@ public class CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap implements Cal
             } else {
                NodeMetadata nodeForState = getNode.getNode(node.getId());
                NodeState state = nodeForState == null ? NodeState.TERMINATED : nodeForState.getState();
-               throw new IllegalStateException(String.format(
+               throw new IllegalStateException(format(
-                        "node didn't achieve the state running on node %s within %d seconds, final state: %s", node
+                        "node %s didn't achieve the state running within %d seconds, final state: %s", originalId,
-                                 .getId(), timeouts.nodeRunning / 1000, state));
+                        timeouts.nodeRunning / 1000, state));
             }
+            if (node == null)
+               throw new IllegalStateException(format("node %s terminated before applying options", originalId));
             if (statement != null) {
                RunScriptOnNode runner = initScriptRunnerFactory.create(node, statement, options, badNodes).call();
                if (runner != null) {
@@ -148,10 +152,10 @@ public class CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap implements Cal
                findReachableSocketOnNode(socketTester.seconds(options.getSeconds()), node, options.getPort());
             }
          }
-         logger.debug("<< options applied node(%s)", node.getId());
+         logger.debug("<< options applied node(%s)", originalId);
          goodNodes.add(node);
       } catch (Exception e) {
-         logger.error(e, "<< problem applying options to node(%s): ", node.getId(), getRootCause(e).getMessage());
+         logger.error(e, "<< problem applying options to node(%s): ", originalId, getRootCause(e).getMessage());
          badNodes.put(node, e);
       }
       return null;

core/src/main/java/org/jclouds/predicates/RetryablePredicate.java

@@ -19,14 +19,16 @@
 
 package org.jclouds.predicates;
 
+import static org.jclouds.util.Throwables2.getFirstThrowableOfType;
+
 import java.util.Date;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 import javax.annotation.Resource;
 
 import org.jclouds.logging.Logger;
-import org.jclouds.util.Throwables2;
 
 import com.google.common.base.Predicate;
 
@@ -75,10 +77,16 @@ public class RetryablePredicate<T> implements Predicate<T> {
       } catch (InterruptedException e) {
          logger.warn(e, "predicate %s on %s interrupted, returning false", input, predicate);
       } catch (RuntimeException e) {
-         ExecutionException exec = Throwables2.getFirstThrowableOfType(e, ExecutionException.class);
+         if (getFirstThrowableOfType(e, ExecutionException.class) != null) {
-         if (exec != null)
+            logger.warn(e, "predicate %s on %s errored [%s], returning false", input, predicate, e.getMessage());
-            logger.warn(exec, "predicate %s on %s error, returning false", input, predicate);
+            return false;
-         else
+         } else if (getFirstThrowableOfType(e, IllegalStateException.class) != null) {
+            logger.warn(e, "predicate %s on %s illegal state [%s], returning false", input, predicate, e.getMessage());
+            return false;
+         } else if (getFirstThrowableOfType(e, TimeoutException.class) != null) {
+            logger.warn(e, "predicate %s on %s timed out [%s], returning false", input, predicate, e.getMessage());
+            return false;
+         } else
             throw e;
       }
       return false;

core/src/test/java/org/jclouds/predicates/RetryablePredicateTest.java

@@ -20,12 +20,15 @@
 package org.jclouds.predicates;
 
 import java.util.Date;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 import org.testng.annotations.Test;
 
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
+import com.google.common.base.Supplier;
 
 /**
  * 
@@ -36,6 +39,56 @@ import com.google.common.base.Predicates;
 public class RetryablePredicateTest {
    public static int SLOW_BUILD_SERVER_GRACE = 50;
 
+   @Test
+   void testFalseOnIllegalStateExeception() {
+      ensureImmediateReturnFor(new IllegalStateException());
+   }
+
+   @SuppressWarnings("serial")
+   @Test
+   void testFalseOnExecutionException() {
+      ensureImmediateReturnFor(new ExecutionException() {
+      });
+   }
+
+   @SuppressWarnings("serial")
+   @Test
+   void testFalseOnTimeoutException() {
+      ensureImmediateReturnFor(new TimeoutException() {
+      });
+   }
+
+   @SuppressWarnings("serial")
+   @Test(expectedExceptions = RuntimeException.class)
+   void testPropagateOnException() {
+      ensureImmediateReturnFor(new Exception() {
+      });
+   }
+
+   private void ensureImmediateReturnFor(final Exception ex) {
+      RetryablePredicate<Supplier<String>> predicate = new RetryablePredicate<Supplier<String>>(
+               new Predicate<Supplier<String>>() {
+
+                  @Override
+                  public boolean apply(Supplier<String> input) {
+                     return "goo".equals(input.get());
+                  }
+
+               }, 3, 1, TimeUnit.SECONDS);
+      Date startPlusThird = new Date(System.currentTimeMillis() + 1000);
+      assert !predicate.apply(new Supplier<String>() {
+
+         @Override
+         public String get() {
+            throw new RuntimeException(ex);
+         }
+
+      });
+      Date now = new Date();
+      assert now.compareTo(startPlusThird) < 0 : String.format("%s should be less than %s", now.getTime(),
+               startPlusThird.getTime());
+   }
+
    @Test
    void testAlwaysTrue() {
       RetryablePredicate<String> predicate = new RetryablePredicate<String>(Predicates.<String> alwaysTrue(), 3, 1,

providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/predicates/SpotInstanceRequestActive.java

@@ -20,7 +20,6 @@
 package org.jclouds.aws.ec2.predicates;
 
 import java.util.NoSuchElementException;
-import java.util.concurrent.ExecutionException;
 
 import javax.annotation.Resource;
 import javax.inject.Singleton;
@@ -31,7 +30,6 @@ import org.jclouds.logging.Logger;
 import org.jclouds.rest.ResourceNotFoundException;
 
 import com.google.common.base.Predicate;
-import com.google.common.base.Throwables;
 import com.google.common.collect.Iterables;
 import com.google.inject.Inject;
 
@@ -60,14 +58,10 @@ public class SpotInstanceRequestActive implements Predicate<SpotInstanceRequest>
          logger.trace("%s: looking for spot state %s: currently: %s", spot.getId(), SpotInstanceRequest.State.ACTIVE,
                   spot.getState());
          if (spot.getState() == SpotInstanceRequest.State.CANCELLED)
-            Throwables.propagate(new ExecutionException(String.format("spot request %s cancelled", spot.getId())) {
+            throw new IllegalStateException(String.format("spot request %s cancelled", spot.getId()));
-               private static final long serialVersionUID = 1L;
-            });
          if (spot.getFaultCode() != null)
-            Throwables.propagate(new ExecutionException(String.format("spot request %s fault code(%s) message(%s)",
+            throw new IllegalStateException(String.format("spot request %s fault code(%s) message(%s)", spot.getId(),
-                  spot.getId(), spot.getFaultCode(), spot.getFaultMessage())) {
+                     spot.getFaultCode(), spot.getFaultMessage()));
-               private static final long serialVersionUID = 1L;
-            });
          return spot.getState() == SpotInstanceRequest.State.ACTIVE;
       } catch (ResourceNotFoundException e) {
          return false;

sandbox-apis/cloudstack/src/main/java/org/jclouds/cloudstack/predicates/JobComplete.java

@@ -21,8 +21,6 @@ package org.jclouds.cloudstack.predicates;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
-import java.util.concurrent.ExecutionException;
-
 import javax.annotation.Resource;
 import javax.inject.Singleton;
 
@@ -31,7 +29,6 @@ import org.jclouds.cloudstack.domain.AsyncJob;
 import org.jclouds.logging.Logger;
 
 import com.google.common.base.Predicate;
-import com.google.common.base.Throwables;
 import com.google.inject.Inject;
 
 /**
@@ -60,10 +57,8 @@ public class JobComplete implements Predicate<Long> {
          return false;
       logger.trace("%s: looking for job status %s: currently: %s", job.getId(), 1, job.getStatus());
       if (job.getError() != null)
-         Throwables.propagate(new ExecutionException(String.format("job %s failed with exception %s", job.getId(), job
+         throw new IllegalStateException(String.format("job %s failed with exception %s", job.getId(), job.getError()
-                  .getError().toString())) {
+                  .toString()));
-                     private static final long serialVersionUID = 4371112085613620239L;
-         });
       return job.getStatus() > 0;
    }