added runScriptOnNodesMatching; removed runScriptOnNodesWithTag as it proved unflexible and the new method is a broader case; use runScriptOnNodesMatching instead

compute/src/main/java/org/jclouds/compute/ComputeService.java

@@ -21,6 +21,7 @@ package org.jclouds.compute;
 import java.util.Map;
 import java.util.Set;
 
+import com.google.common.base.Predicate;
 import org.jclouds.compute.domain.ComputeMetadata;
 import org.jclouds.compute.domain.Image;
 import org.jclouds.compute.domain.NodeMetadata;
@@ -170,27 +171,26 @@ public interface ComputeService {
    /**
     * Runs the script without any additional options
     * 
-    * @see #runScriptOnNodesWithTag(String, byte[], org.jclouds.compute.options.RunScriptOptions)
+    * @see #runScriptOnNodesMatching(Predicate, byte[], org.jclouds.compute.options.RunScriptOptions)
     */
-   Map<NodeMetadata, ExecResponse> runScriptOnNodesWithTag(String tag, byte[] runScript)
+   Map<NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter, byte[] runScript)
             throws RunScriptOnNodesException;
 
    /**
     * Run the script on all nodes with the specific tag.
     *
-    * @param tag
+    * @param filter
-    *           tag to look up the nodes
+    *           Predicate-based filter to define on which nodes the script is to be
+    *           executed
     * @param runScript
     *           script to run in byte format. If the script is a string, use
     *           {@link String#getBytes()} to retrieve the bytes
     * @param options
-    *           nullable options to how to run the script
+    *           nullable options to how to run the script, whether to override credentials
     * @return map with node identifiers and corresponding responses
-    * @throws RunScriptOnNodesException
+    * @throws RunScriptOnNodesException if anything goes wrong during script execution
-    *            when there's a problem running the script on the nodes. Note that successful and
-    *            failed nodes are a part of this exception, so be sure to inspect this carefully.
     */
-   Map<NodeMetadata, ExecResponse> runScriptOnNodesWithTag(String tag, byte[] runScript,
+   Map<NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter, byte[] runScript,
             RunScriptOptions options) throws RunScriptOnNodesException;
 
 }

compute/src/main/java/org/jclouds/compute/RunScriptOnNodesException.java

@@ -22,6 +22,7 @@ import java.util.Map;
 
 import javax.annotation.Nullable;
 
+import com.google.common.base.Predicate;
 import org.jclouds.compute.domain.NodeMetadata;
 import org.jclouds.compute.options.RunScriptOptions;
 import org.jclouds.compute.util.ComputeUtils;
@@ -35,21 +36,19 @@ public class RunScriptOnNodesException extends Exception {
 
    /** The serialVersionUID */
    private static final long serialVersionUID = -2272965726680821281L;
-   private final String tag;
    private final byte[] runScript;
    private final RunScriptOptions options;
    private final Map<NodeMetadata, ExecResponse> successfulNodes;
    private final Map<? extends NodeMetadata, ? extends Throwable> failedNodes;
    private final Map<?, Exception> executionExceptions;
 
-   public RunScriptOnNodesException(String tag, final byte[] runScript,
+   public RunScriptOnNodesException(final byte[] runScript,
             @Nullable final RunScriptOptions options,
             Map<NodeMetadata, ExecResponse> successfulNodes, Map<?, Exception> executionExceptions,
             Map<? extends NodeMetadata, ? extends Throwable> failedNodes) {
-      super(String.format("error runScript on node tag(%s) options(%s)%n%s%n%s", tag,
+      super(String.format("error runScript on filtered nodes options(%s)%n%s%n%s",
                options, ComputeUtils.createExecutionErrorMessage(executionExceptions), ComputeUtils
                         .createNodeErrorMessage(failedNodes)));
-      this.tag = tag;
       this.runScript = runScript;
       this.options = options;
       this.successfulNodes = successfulNodes;
@@ -58,7 +57,6 @@ public class RunScriptOnNodesException extends Exception {
    }
 
    /**
-    * 
     * @return Nodes that performed ssh without error
     */
    public Map<NodeMetadata, ExecResponse> getSuccessfulNodes() {
@@ -81,10 +79,6 @@ public class RunScriptOnNodesException extends Exception {
       return failedNodes;
    }
 
-   public String getTag() {
-      return tag;
-   }
-
    public byte[] getRunScript() {
       return runScript;
    }

compute/src/main/java/org/jclouds/compute/internal/BaseComputeService.java

@@ -27,7 +27,6 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.jclouds.concurrent.ConcurrentUtils.awaitCompletion;
 import static org.jclouds.concurrent.ConcurrentUtils.makeListenable;
-import static org.jclouds.util.Utils.checkNotEmpty;
 
 import java.util.Map;
 import java.util.Set;
@@ -99,6 +98,7 @@ public class BaseComputeService implements ComputeService {
    protected final Provider<TemplateBuilder> templateBuilderProvider;
    protected final ComputeUtils utils;
    protected final ExecutorService executor;
+   protected final ComputeMetadataToNodeMetadata computeMetadataToNodeMetadata;
 
    private static class NodeMatchesTag implements Predicate<NodeMetadata> {
       private final String tag;
@@ -116,28 +116,29 @@ public class BaseComputeService implements ComputeService {
 
    @Inject
    protected BaseComputeService(ComputeServiceContext context,
-            Provider<Set<? extends Image>> images, Provider<Set<? extends Size>> sizes,
+                                Provider<Set<? extends Image>> images, Provider<Set<? extends Size>> sizes,
-            Provider<Set<? extends Location>> locations, ListNodesStrategy listNodesStrategy,
+                                Provider<Set<? extends Location>> locations, ListNodesStrategy listNodesStrategy,
-            GetNodeMetadataStrategy getNodeMetadataStrategy,
+                                GetNodeMetadataStrategy getNodeMetadataStrategy,
-            RunNodesAndAddToSetStrategy runNodesAndAddToSetStrategy,
+                                RunNodesAndAddToSetStrategy runNodesAndAddToSetStrategy,
-            RebootNodeStrategy rebootNodeStrategy, DestroyNodeStrategy destroyNodeStrategy,
+                                RebootNodeStrategy rebootNodeStrategy, DestroyNodeStrategy destroyNodeStrategy,
-            Provider<TemplateBuilder> templateBuilderProvider, ComputeUtils utils,
+                                Provider<TemplateBuilder> templateBuilderProvider, ComputeUtils utils,
-            @Named(Constants.PROPERTY_USER_THREADS) ExecutorService executor) {
+                                @Named(Constants.PROPERTY_USER_THREADS) ExecutorService executor) {
       this.context = checkNotNull(context, "context");
       this.images = checkNotNull(images, "images");
       this.sizes = checkNotNull(sizes, "sizes");
       this.locations = checkNotNull(locations, "locations");
       this.listNodesStrategy = checkNotNull(listNodesStrategy, "listNodesStrategy");
       this.getNodeMetadataStrategy = checkNotNull(getNodeMetadataStrategy,
-               "getNodeMetadataStrategy");
+              "getNodeMetadataStrategy");
       this.runNodesAndAddToSetStrategy = checkNotNull(runNodesAndAddToSetStrategy,
-               "runNodesAndAddToSetStrategy");
+              "runNodesAndAddToSetStrategy");
       this.rebootNodeStrategy = checkNotNull(rebootNodeStrategy, "rebootNodeStrategy");
       this.destroyNodeStrategy = checkNotNull(destroyNodeStrategy, "destroyNodeStrategy");
       this.templateBuilderProvider = checkNotNull(templateBuilderProvider,
-               "templateBuilderProvider");
+              "templateBuilderProvider");
       this.utils = checkNotNull(utils, "utils");
       this.executor = checkNotNull(executor, "executor");
+      this.computeMetadataToNodeMetadata = new ComputeMetadataToNodeMetadata();
    }
 
    @Override
@@ -147,18 +148,18 @@ public class BaseComputeService implements ComputeService {
 
    @Override
    public Set<? extends NodeMetadata> runNodesWithTag(final String tag, int count,
-            final Template template) throws RunNodesException {
+                                                      final Template template) throws RunNodesException {
       checkArgument(tag.indexOf('-') == -1, "tag cannot contain hyphens");
       checkNotNull(template.getLocation(), "location");
       logger.debug(">> running %d node%s tag(%s) location(%s) image(%s) size(%s) options(%s)",
-               count, count > 1 ? "s" : "", tag, template.getLocation().getId(), template
+              count, count > 1 ? "s" : "", tag, template.getLocation().getId(), template
-                        .getImage().getId(), template.getSize().getId(), template.getOptions());
+                      .getImage().getId(), template.getSize().getId(), template.getOptions());
       final Set<NodeMetadata> nodes = Sets.newHashSet();
       final Map<NodeMetadata, Exception> badNodes = Maps.newLinkedHashMap();
       Map<?, ListenableFuture<Void>> responses = runNodesAndAddToSetStrategy.execute(tag, count,
-               template, nodes, badNodes);
+              template, nodes, badNodes);
       Map<?, Exception> executionExceptions = awaitCompletion(responses, executor, null, logger,
-               "starting nodes");
+              "starting nodes");
       if (executionExceptions.size() > 0 || badNodes.size() > 0) {
          throw new RunNodesException(tag, count, template, nodes, executionExceptions, badNodes);
       }
@@ -168,7 +169,7 @@ public class BaseComputeService implements ComputeService {
    @Override
    public void destroyNode(ComputeMetadata node) {
       checkArgument(node.getType() == ComputeType.NODE, "this is only valid for nodes, not "
-               + node.getType());
+              + node.getType());
       checkNotNull(node.getId(), "node.id");
       logger.debug(">> destroying node(%s)", node.getId());
       boolean successful = destroyNodeStrategy.execute(node);
@@ -179,13 +180,13 @@ public class BaseComputeService implements ComputeService {
    public void destroyNodesWithTag(String tag) { // TODO parallel
       logger.debug(">> destroying nodes by tag(%s)", tag);
       Iterable<? extends NodeMetadata> nodesToDestroy = Iterables.filter(doListNodesWithTag(tag),
-               new Predicate<NodeMetadata>() {
+              new Predicate<NodeMetadata>() {
-                  @Override
+                 @Override
-                  public boolean apply(NodeMetadata input) {
+                 public boolean apply(NodeMetadata input) {
-                     return input.getState() != NodeState.TERMINATED;
+                    return input.getState() != NodeState.TERMINATED;
 
-                  }
+                 }
-               });
+              });
       Map<NodeMetadata, ListenableFuture<Void>> responses = Maps.newHashMap();
       for (final NodeMetadata node : nodesToDestroy) {
          responses.put(node, makeListenable(executor.submit(new Callable<Void>() {
@@ -212,7 +213,7 @@ public class BaseComputeService implements ComputeService {
          options = GetNodesOptions.NONE;
       }
       Set<? extends ComputeMetadata> set = Sets
-               .newLinkedHashSet(listNodesStrategy.execute(options));
+              .newLinkedHashSet(listNodesStrategy.execute(options));
       logger.debug("<< list(%d)", set.size());
       return set;
    }
@@ -223,17 +224,20 @@ public class BaseComputeService implements ComputeService {
     */
    protected Set<? extends NodeMetadata> doListNodesWithTag(final String tag) {
       return Sets.newHashSet(Iterables.filter(Iterables.transform(listNodesStrategy
-               .execute(GetNodesOptions.NONE), new Function<ComputeMetadata, NodeMetadata>() {
+              .execute(GetNodesOptions.NONE), computeMetadataToNodeMetadata), new NodeMatchesTag(tag)));
-
-         @Override
-         public NodeMetadata apply(ComputeMetadata from) {
-            return from instanceof NodeMetadata ? NodeMetadata.class.cast(from)
-                     : getNodeMetadata(from);
-         }
-
-      }), new NodeMatchesTag(tag)));
    }
 
+   class ComputeMetadataToNodeMetadata
+           implements Function<ComputeMetadata, NodeMetadata> {
+
+      @Override
+      public NodeMetadata apply(ComputeMetadata from) {
+         return from instanceof NodeMetadata ? NodeMetadata.class.cast(from)
+                 : getNodeMetadata(from);
+      }
+   }
+
+
    @Override
    public Set<? extends NodeMetadata> listNodesWithTag(String tag) {
       logger.debug(">> listing nodes by tag(%s)", tag);
@@ -265,14 +269,14 @@ public class BaseComputeService implements ComputeService {
    @Override
    public NodeMetadata getNodeMetadata(ComputeMetadata node) {
       checkArgument(node.getType() == ComputeType.NODE, "this is only valid for nodes, not "
-               + node.getType());
+              + node.getType());
       return getNodeMetadataStrategy.execute(node);
    }
 
    @Override
    public void rebootNode(ComputeMetadata node) {
       checkArgument(node.getType() == ComputeType.NODE, "this is only valid for nodes, not "
-               + node.getType());
+              + node.getType());
       checkNotNull(node.getId(), "node.id");
       logger.debug(">> rebooting node(%s)", node.getId());
       boolean successful = rebootNodeStrategy.execute(node);
@@ -283,13 +287,13 @@ public class BaseComputeService implements ComputeService {
    public void rebootNodesWithTag(String tag) { // TODO parallel
       logger.debug(">> rebooting nodes by tag(%s)", tag);
       Iterable<? extends NodeMetadata> nodesToReboot = Iterables.filter(doListNodesWithTag(tag),
-               new Predicate<NodeMetadata>() {
+              new Predicate<NodeMetadata>() {
-                  @Override
+                 @Override
-                  public boolean apply(NodeMetadata input) {
+                 public boolean apply(NodeMetadata input) {
-                     return input.getState() != NodeState.TERMINATED;
+                    return input.getState() != NodeState.TERMINATED;
 
-                  }
+                 }
-               });
+              });
       Map<NodeMetadata, ListenableFuture<Void>> responses = Maps.newHashMap();
       for (final NodeMetadata node : nodesToReboot) {
          responses.put(node, makeListenable(executor.submit(new Callable<Void>() {
@@ -306,31 +310,33 @@ public class BaseComputeService implements ComputeService {
 
    /**
     * @throws RunScriptOnNodesException
-    * @see #runScriptOnNodesWithTag(String, byte[], org.jclouds.compute.options.RunScriptOptions)
+    * @see #runScriptOnNodesMatching(Predicate, byte[], org.jclouds.compute.options.RunScriptOptions)
     */
-   public Map<NodeMetadata, ExecResponse> runScriptOnNodesWithTag(String tag, byte[] runScript)
+   public Map<NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter, byte[] runScript)
-            throws RunScriptOnNodesException {
+           throws RunScriptOnNodesException {
-      return runScriptOnNodesWithTag(tag, runScript, RunScriptOptions.NONE);
+      return runScriptOnNodesMatching(filter, runScript, RunScriptOptions.NONE);
    }
 
    /**
     * Run the script on all nodes with the specific tag.
     *
-    * @param tag
+    * @param filter
-    *           tag to look up the nodes
+    *           Predicate-based filter to define on which nodes the script is to be
+    *           executed
     * @param runScript
     *           script to run in byte format. If the script is a string, use
     *           {@link String#getBytes()} to retrieve the bytes
     * @param options
     *           nullable options to how to run the script, whether to override credentials
     * @return map with node identifiers and corresponding responses
-    * @throws RunScriptOnNodesException
+    * @throws RunScriptOnNodesException if anything goes wrong during script execution
     */
-   public Map<NodeMetadata, ExecResponse> runScriptOnNodesWithTag(String tag,
+   public Map<NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter,
-            final byte[] runScript, @Nullable final RunScriptOptions options)
+                                                                   final byte[] runScript, @Nullable final RunScriptOptions options)
-            throws RunScriptOnNodesException {
+           throws RunScriptOnNodesException {
-      Iterable<? extends NodeMetadata> nodes = verifyParametersAndGetNodes(tag, runScript,
+      Iterable<? extends NodeMetadata> nodes = verifyParametersAndGetNodes(filter, runScript,
-               (options != null) ? options : RunScriptOptions.NONE);
+              (options != null) ? options : RunScriptOptions.NONE);
+
       final Map<NodeMetadata, ExecResponse> execs = Maps.newHashMap();
 
       final Map<NodeMetadata, Exception> badNodes = Maps.newLinkedHashMap();
@@ -367,48 +373,44 @@ public class BaseComputeService implements ComputeService {
 
       }
       Map<?, Exception> exceptions = awaitCompletion(responses, executor, null, logger,
-               "starting nodes");
+              "starting nodes");
       if (exceptions.size() > 0 || badNodes.size() > 0) {
-         throw new RunScriptOnNodesException(tag, runScript, options, execs, exceptions, badNodes);
+         throw new RunScriptOnNodesException(runScript, options, execs, exceptions, badNodes);
       }
       return execs;
+
    }
 
-   private Iterable<? extends NodeMetadata> verifyParametersAndGetNodes(String tag,
+   private Iterable<? extends NodeMetadata> verifyParametersAndGetNodes(Predicate<NodeMetadata> filter,
-            byte[] runScript, final RunScriptOptions options) {
+                                                                        byte[] runScript, final RunScriptOptions options) {
-      checkNotEmpty(tag, "Tag must be provided");
+      checkNotNull(filter, "Filter must be provided");
       checkNotNull(runScript,
-               "The script (represented by bytes array - use \"script\".getBytes() must be provided");
+              "The script (represented by bytes array - use \"script\".getBytes() must be provided");
       checkNotNull(options, "options");
-      Iterable<? extends NodeMetadata> nodes = Iterables.filter(listNodesWithTag(tag),
+      Iterable<? extends NodeMetadata> nodes = Iterables.filter(
-               new Predicate<NodeMetadata>() {
+                           Iterables.transform(listNodes(), computeMetadataToNodeMetadata),
+                           filter);
       
-                  @Override
-                  public boolean apply(NodeMetadata input) {
-                     return input.getState() == NodeState.RUNNING;
-                  }
-
-               });
       return Iterables.transform(nodes, new Function<NodeMetadata, NodeMetadata>() {
 
          @Override
          public NodeMetadata apply(NodeMetadata node) {
 
             checkArgument(node.getPublicAddresses().size() > 0, "no public ip addresses on node: "
-                     + node);
+                    + node);
             if (options.getOverrideCredentials() != null) {
                // override the credentials with provided to this method
                node = ComputeUtils.installNewCredentials(node, options.getOverrideCredentials());
             } else {
                // don't override
                checkNotNull(node.getCredentials(),
-                        "If the default credentials need to be used, they can't be null");
+                       "If the default credentials need to be used, they can't be null");
                checkNotNull(node.getCredentials().account,
-                        "Account name for ssh authentication must be "
+                       "Account name for ssh authentication must be "
-                                 + "specified. Try passing RunScriptOptions with new credentials");
+                               + "specified. Try passing RunScriptOptions with new credentials");
                checkNotNull(node.getCredentials().key,
-                        "Key or password for ssh authentication must be "
+                       "Key or password for ssh authentication must be "
-                                 + "specified. Try passing RunScriptOptions with new credentials");
+                               + "specified. Try passing RunScriptOptions with new credentials");
             }
             return node;
          }

compute/src/main/java/org/jclouds/compute/util/ComputeUtils.java

@@ -43,6 +43,7 @@ import javax.annotation.Nullable;
 import javax.annotation.Resource;
 import javax.inject.Named;
 
+import com.google.common.base.Function;
 import org.jclouds.Constants;
 import org.jclouds.compute.domain.ComputeMetadata;
 import org.jclouds.compute.domain.NodeMetadata;
@@ -464,5 +465,4 @@ public class ComputeUtils {
       }
       return providers;
    }
-
 }

compute/src/test/java/org/jclouds/compute/BaseComputeServiceLiveTest.java

@@ -77,6 +77,8 @@ import com.google.inject.Guice;
 import com.google.inject.Injector;
 import com.google.inject.Module;
 
+import javax.annotation.Nullable;
+
 /**
  * 
  * @author Adrian Cole
@@ -247,8 +249,15 @@ public abstract class BaseComputeServiceLiveTest {
 
    private Map<NodeMetadata, ExecResponse> runScriptWithCreds(String tag, OsFamily osFamily,
             Credentials creds) throws RunScriptOnNodesException {
+      Predicate<NodeMetadata> filter = new Predicate<NodeMetadata>() {
+         @Override
+         public boolean apply(@Nullable NodeMetadata nodeMetadata) {
+            return true; /*accept all*/
+         }
+      };
+      
       try {
-         return client.runScriptOnNodesWithTag(tag, buildScript(osFamily).getBytes(),
+         return client.runScriptOnNodesMatching(filter, buildScript(osFamily).getBytes(),
                   RunScriptOptions.Builder.overrideCredentialsWith(creds));
       } catch (SshException e) {
          if (Throwables.getRootCause(e).getMessage().contains("Auth fail")) {