runscript bulletproofing

apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java

@@ -45,11 +45,11 @@ import org.jclouds.compute.options.TemplateOptions;
 import org.jclouds.compute.reference.ComputeServiceConstants.Timeouts;
 import org.jclouds.compute.strategy.DestroyNodeStrategy;
 import org.jclouds.compute.strategy.GetNodeMetadataStrategy;
+import org.jclouds.compute.strategy.InitializeRunScriptOnNodeOrPlaceInBadMap;
 import org.jclouds.compute.strategy.ListNodesStrategy;
 import org.jclouds.compute.strategy.RebootNodeStrategy;
 import org.jclouds.compute.strategy.ResumeNodeStrategy;
 import org.jclouds.compute.strategy.RunNodesAndAddToSetStrategy;
-import org.jclouds.compute.strategy.RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap;
 import org.jclouds.compute.strategy.SuspendNodeStrategy;
 import org.jclouds.domain.Credentials;
 import org.jclouds.domain.Location;
@@ -90,7 +90,7 @@ public class EC2ComputeService extends BaseComputeService {
             @Named("NODE_RUNNING") Predicate<NodeMetadata> nodeRunning,
             @Named("NODE_TERMINATED") Predicate<NodeMetadata> nodeTerminated,
             @Named("NODE_SUSPENDED") Predicate<NodeMetadata> nodeSuspended,
-            RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap.Factory statementRunner, Timeouts timeouts,
+            InitializeRunScriptOnNodeOrPlaceInBadMap.Factory initScriptRunnerFactory, Timeouts timeouts,
             @Named(Constants.PROPERTY_USER_THREADS) ExecutorService executor, EC2Client ec2Client,
             Map<RegionAndName, KeyPair> credentialsMap, @Named("SECURITY") Map<RegionAndName, String> securityGroupMap,
             @Named("PLACEMENT") Map<RegionAndName, String> placementGroupMap,
@@ -98,7 +98,7 @@ public class EC2ComputeService extends BaseComputeService {
       super(context, credentialStore, images, sizes, locations, listNodesStrategy, getNodeMetadataStrategy,
                runNodesAndAddToSetStrategy, rebootNodeStrategy, destroyNodeStrategy, startNodeStrategy,
                stopNodeStrategy, templateBuilderProvider, templateOptionsProvider, nodeRunning, nodeTerminated,
-               nodeSuspended, statementRunner, timeouts, executor);
+               nodeSuspended, initScriptRunnerFactory, timeouts, executor);
       this.ec2Client = ec2Client;
       this.credentialsMap = credentialsMap;
       this.securityGroupMap = securityGroupMap;

apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2ComputeServiceTest.java

@@ -33,11 +33,11 @@ import org.jclouds.compute.ComputeServiceContext;
 import org.jclouds.compute.reference.ComputeServiceConstants.Timeouts;
 import org.jclouds.compute.strategy.DestroyNodeStrategy;
 import org.jclouds.compute.strategy.GetNodeMetadataStrategy;
+import org.jclouds.compute.strategy.InitializeRunScriptOnNodeOrPlaceInBadMap;
 import org.jclouds.compute.strategy.ListNodesStrategy;
 import org.jclouds.compute.strategy.RebootNodeStrategy;
 import org.jclouds.compute.strategy.ResumeNodeStrategy;
 import org.jclouds.compute.strategy.RunNodesAndAddToSetStrategy;
-import org.jclouds.compute.strategy.RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap;
 import org.jclouds.compute.strategy.SuspendNodeStrategy;
 import org.jclouds.ec2.EC2Client;
 import org.jclouds.ec2.services.PlacementGroupClient;
@@ -62,7 +62,7 @@ public class EC2ComputeServiceTest {
                createMock(DestroyNodeStrategy.class), createMock(ResumeNodeStrategy.class),
                createMock(SuspendNodeStrategy.class), createMock(Provider.class), createMock(Provider.class),
                createMock(Predicate.class), createMock(Predicate.class), createMock(Predicate.class),
-               createMock(RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap.Factory.class),
+               createMock(InitializeRunScriptOnNodeOrPlaceInBadMap.Factory.class),
                createMock(Timeouts.class), createMock(ExecutorService.class), client, createMock(Map.class),
                createMock(Map.class), createMock(Map.class), createMock(Predicate.class));
 

apis/vcloud/src/main/java/org/jclouds/vcloud/compute/functions/VAppToNodeMetadata.java

@@ -28,6 +28,7 @@ import static org.jclouds.vcloud.compute.util.VCloudComputeUtils.toComputeOs;
 
 import java.util.Map;
 
+import javax.annotation.Resource;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 
@@ -36,6 +37,7 @@ import org.jclouds.compute.domain.NodeMetadata;
 import org.jclouds.compute.domain.NodeMetadataBuilder;
 import org.jclouds.compute.domain.NodeState;
 import org.jclouds.domain.Credentials;
+import org.jclouds.logging.Logger;
 import org.jclouds.vcloud.domain.Status;
 import org.jclouds.vcloud.domain.VApp;
 
@@ -46,6 +48,9 @@ import com.google.common.base.Function;
  */
 @Singleton
 public class VAppToNodeMetadata implements Function<VApp, NodeMetadata> {
+   @Resource
+   protected static Logger logger = Logger.NULL;
+
    protected final FindLocationForResource findLocationForResourceInVDC;
    protected final Function<VApp, Hardware> hardwareForVApp;
    protected final Map<Status, NodeState> vAppStatusToNodeState;
@@ -66,7 +71,12 @@ public class VAppToNodeMetadata implements Function<VApp, NodeMetadata> {
       builder.uri(from.getHref());
       builder.name(from.getName());
       builder.location(findLocationForResourceInVDC.apply(from.getVDC()));
-      builder.tag(parseTagFromName(from.getName()));
+      String tag = parseTagFromName(from.getName());
+      builder.tag(tag);
+      if (logger.isTraceEnabled()) {
+         if (tag.startsWith("NOTAG#"))
+            logger.warn("failed to parse tag from name %s", from);
+      }
       builder.operatingSystem(toComputeOs(from, null));
       builder.hardware(hardwareForVApp.apply(from));
       builder.state(vAppStatusToNodeState.get(from.getStatus()));

common/trmk/src/main/java/org/jclouds/vcloud/terremark/compute/TerremarkVCloudComputeService.java

@@ -41,11 +41,11 @@ import org.jclouds.compute.options.TemplateOptions;
 import org.jclouds.compute.reference.ComputeServiceConstants.Timeouts;
 import org.jclouds.compute.strategy.DestroyNodeStrategy;
 import org.jclouds.compute.strategy.GetNodeMetadataStrategy;
+import org.jclouds.compute.strategy.InitializeRunScriptOnNodeOrPlaceInBadMap;
 import org.jclouds.compute.strategy.ListNodesStrategy;
 import org.jclouds.compute.strategy.RebootNodeStrategy;
 import org.jclouds.compute.strategy.ResumeNodeStrategy;
 import org.jclouds.compute.strategy.RunNodesAndAddToSetStrategy;
-import org.jclouds.compute.strategy.RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap;
 import org.jclouds.compute.strategy.SuspendNodeStrategy;
 import org.jclouds.domain.Credentials;
 import org.jclouds.domain.Location;
@@ -76,13 +76,13 @@ public class TerremarkVCloudComputeService extends BaseComputeService {
             @Named("NODE_RUNNING") Predicate<NodeMetadata> nodeRunning,
             @Named("NODE_TERMINATED") Predicate<NodeMetadata> nodeTerminated,
             @Named("NODE_SUSPENDED") Predicate<NodeMetadata> nodeSuspended,
-            RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap.Factory statementRunner, Timeouts timeouts,
+            InitializeRunScriptOnNodeOrPlaceInBadMap.Factory initScriptRunnerFactory, Timeouts timeouts,
             @Named(Constants.PROPERTY_USER_THREADS) ExecutorService executor, CleanupOrphanKeys cleanupOrphanKeys,
             ConcurrentMap<OrgAndName, KeyPairCredentials> credentialsMap, NodeMetadataToOrgAndName nodeToOrgAndName) {
       super(context, credentialStore, images, sizes, locations, listNodesStrategy, getNodeMetadataStrategy,
                runNodesAndAddToSetStrategy, rebootNodeStrategy, destroyNodeStrategy, resumeNodeStrategy,
                suspendNodeStrategy, templateBuilderProvider, templateOptionsProvider, nodeRunning, nodeTerminated,
-               nodeSuspended, statementRunner, timeouts, executor);
+               nodeSuspended, initScriptRunnerFactory, timeouts, executor);
       this.cleanupOrphanKeys = cleanupOrphanKeys;
    }
 

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

@@ -166,6 +166,8 @@ public interface ComputeService {
     * 
     * @throws UnsupportedOperationException
     *            if the underlying provider doesn't support suspend/resume
+    * @throws NoSuchElementException
+    *            if no nodes matched the predicate specified
     */
    void resumeNodesMatching(Predicate<NodeMetadata> filter);
 
@@ -189,6 +191,11 @@ public interface ComputeService {
     * <h4>note</h4>
     * 
     * affected nodes may not resume with the same IP address(es)
+    * 
+    * @throws UnsupportedOperationException
+    *            if the underlying provider doesn't support suspend/resume
+    * @throws NoSuchElementException
+    *            if no nodes matched the predicate specified
     */
    void suspendNodesMatching(Predicate<NodeMetadata> filter);
 
@@ -215,6 +222,9 @@ public interface ComputeService {
    /**
     * nodes matching the filter are treated as a logical set. Using this command, you can save time
     * by rebooting the nodes in parallel.
+    * 
+    * @throws NoSuchElementException
+    *            if no nodes matched the predicate specified
     */
    void rebootNodesMatching(Predicate<NodeMetadata> filter);
 
@@ -233,54 +243,34 @@ public interface ComputeService {
    Set<? extends NodeMetadata> listNodesDetailsMatching(Predicate<ComputeMetadata> filter);
 
    /**
-    * Runs the script without any additional options
+    * @see org.jclouds.io.Payloads
-    * 
+    * @see ComputeService#runScriptOnNodesMatching(Predicate, Statement, RunScriptOptions)
-    * @see #runScriptOnNodesMatching(Predicate, Payload,
-    *      org.jclouds.compute.options.RunScriptOptions)
-    * @see org.jclouds.compute.predicates.NodePredicates#runningWithTag(String)
     */
    @Deprecated
    Map<? extends NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter, Payload runScript)
             throws RunScriptOnNodesException;
 
    /**
-    * Run the script on all nodes with the specific predicate.
-    * 
-    * @param filter
-    *           Predicate-based filter to define on which nodes the script is to be executed
-    * @param runScript
-    *           payload containing the script to run
-    * @param options
-    *           nullable options to how to run the script, whether to override credentials
-    * @return map with node identifiers and corresponding responses
-    * @throws RunScriptOnNodesException
-    *            if anything goes wrong during script execution
-    * 
-    * @see org.jclouds.compute.predicates.NodePredicates#runningWithTag(String)
     * @see org.jclouds.io.Payloads
+    * @see ComputeService#runScriptOnNodesMatching(Predicate, Statement, RunScriptOptions)
     */
    @Deprecated
    Map<? extends NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter,
             Payload runScript, RunScriptOptions options) throws RunScriptOnNodesException;
 
    /**
-    * Run the script on all nodes with the specific predicate.
     * 
-    * @param filter
+    * @see ComputeService#runScriptOnNodesMatching(Predicate, Statement, RunScriptOptions)
-    *           Predicate-based filter to define on which nodes the script is to be executed
-    * @param runScript
-    *           string containing the script to run
-    * @param options
-    *           nullable options to how to run the script, whether to override credentials
-    * @return map with node identifiers and corresponding responses
-    * @throws RunScriptOnNodesException
-    *            if anything goes wrong during script execution
-    * 
-    * @see org.jclouds.compute.predicates.NodePredicates#runningWithTag(String)
-    * @see org.jclouds.scriptbuilder.domain.Statements
     */
-   Map<? extends NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter, String runScript,
+   Map<? extends NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter, String runScript)
-            RunScriptOptions options) throws RunScriptOnNodesException;
+            throws RunScriptOnNodesException;
+
+   /**
+    * 
+    * @see ComputeService#runScriptOnNodesMatching(Predicate, Statement, RunScriptOptions)
+    */
+   Map<? extends NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter,
+            Statement runScript) throws RunScriptOnNodesException;
 
    /**
     * Run the script on all nodes with the specific predicate.
@@ -292,6 +282,8 @@ public interface ComputeService {
     * @param options
     *           nullable options to how to run the script, whether to override credentials
     * @return map with node identifiers and corresponding responses
+    * @throws NoSuchElementException
+    *            if no nodes matched the predicate specified
     * @throws RunScriptOnNodesException
     *            if anything goes wrong during script execution
     * 

compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNode.java

@@ -0,0 +1,65 @@
+/**
+ *
+ * Copyright (C) 2010 Cloud Conscious, LLC. <info@cloudconscious.com>
+ *
+ * ====================================================================
+ * Licensed 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.compute.callables;
+
+import java.util.concurrent.Callable;
+
+import org.jclouds.compute.domain.NodeMetadata;
+import org.jclouds.compute.options.RunScriptOptions;
+import org.jclouds.scriptbuilder.domain.Statement;
+import org.jclouds.ssh.ExecResponse;
+
+import com.google.common.annotations.Beta;
+
+/**
+ * Separates out how one implements the ability to run a script on a node.
+ * 
+ * @author Adrian Cole
+ */
+@Beta
+public interface RunScriptOnNode extends Callable<ExecResponse> {
+
+   public interface Factory {
+      RunScriptOnNode create(NodeMetadata node, String script);
+
+      RunScriptOnNode create(NodeMetadata node, Statement script);
+
+      RunScriptOnNode create(NodeMetadata node, Statement script, RunScriptOptions options);
+   }
+
+   /**
+    * Note that {@link #init} must be called first.
+    */
+   @Override
+   ExecResponse call();
+
+   /**
+    * verifies that the command can execute on the node. For example, if this is ssh, it may attempt
+    * to find a reachable socket. If this is using an API, it may attempt to validate that
+    * connection.
+    */
+   RunScriptOnNode init();
+
+   /**
+    * the node this command is being executed on.
+    */
+   NodeMetadata getNode();
+
+}

compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNodeAsInitScriptUsingSsh.java

@@ -20,16 +20,15 @@
 package org.jclouds.compute.callables;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 
 import java.util.Collections;
-import java.util.concurrent.Callable;
 
-import javax.annotation.Nullable;
 import javax.annotation.Resource;
-import javax.inject.Inject;
 import javax.inject.Named;
 
 import org.jclouds.compute.domain.NodeMetadata;
+import org.jclouds.compute.options.RunScriptOptions;
 import org.jclouds.compute.reference.ComputeServiceConstants;
 import org.jclouds.logging.Logger;
 import org.jclouds.scriptbuilder.InitBuilder;
@@ -40,28 +39,19 @@ import org.jclouds.ssh.SshClient;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
-import com.google.common.collect.Iterables;
+import com.google.common.base.Objects;
 import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
 
 /**
  * 
  * @author Adrian Cole
  */
-public class StartInitScriptOnNode implements Callable<ExecResponse> {
+public class RunScriptOnNodeAsInitScriptUsingSsh implements RunScriptOnNode {
    @Resource
    @Named(ComputeServiceConstants.COMPUTE_LOGGER)
    protected Logger logger = Logger.NULL;
 
-   public interface Factory {
-      @Named("blocking")
-      StartInitScriptOnNode blockOnComplete(NodeMetadata node, @Nullable String name, Statement script,
-               boolean runAsRoot);
-
-      @Named("nonblocking")
-      StartInitScriptOnNode dontBlockOnComplete(NodeMetadata node, @Nullable String name, Statement script,
-               boolean runAsRoot);
-   }
-
    protected final Function<NodeMetadata, SshClient> sshFactory;
    protected final NodeMetadata node;
    protected final Statement init;
@@ -70,21 +60,22 @@ public class StartInitScriptOnNode implements Callable<ExecResponse> {
 
    protected SshClient ssh;
 
-   @Inject
+   @AssistedInject
-   public StartInitScriptOnNode(Function<NodeMetadata, SshClient> sshFactory, @Assisted NodeMetadata node,
+   public RunScriptOnNodeAsInitScriptUsingSsh(Function<NodeMetadata, SshClient> sshFactory,
-            @Assisted @Nullable String name, @Assisted Statement script, @Assisted boolean runAsRoot) {
+            @Assisted NodeMetadata node, @Assisted Statement script, @Assisted RunScriptOptions options) {
       this.sshFactory = checkNotNull(sshFactory, "sshFactory");
       this.node = checkNotNull(node, "node");
+      String name = options.getTaskName();
       if (name == null) {
          if (checkNotNull(script, "script") instanceof InitBuilder)
             name = InitBuilder.class.cast(script).getInstanceName();
          else
             name = "jclouds-script-" + System.currentTimeMillis();
       }
-      this.init = checkNotNull(script, "script") instanceof InitBuilder ? InitBuilder.class.cast(script)
-               : createInitScript(checkNotNull(name, "name"), script);
       this.name = checkNotNull(name, "name");
-      this.runAsRoot = runAsRoot;
+      this.init = checkNotNull(script, "script") instanceof InitBuilder ? InitBuilder.class.cast(script)
+               : createInitScript(name, script);
+      this.runAsRoot = options.shouldRunAsRoot();
    }
 
    public static InitBuilder createInitScript(String name, Statement script) {
@@ -94,7 +85,7 @@ public class StartInitScriptOnNode implements Callable<ExecResponse> {
 
    @Override
    public ExecResponse call() {
-      ssh = sshFactory.apply(node);
+      checkState(ssh != null, "please call init() before invoking call");
       try {
          ssh.connect();
          return doCall();
@@ -102,7 +93,12 @@ public class StartInitScriptOnNode implements Callable<ExecResponse> {
          if (ssh != null)
             ssh.disconnect();
       }
+   }
 
+   @Override
+   public RunScriptOnNode init() {
+      ssh = sshFactory.apply(node);
+      return this;
    }
 
    /**
@@ -127,8 +123,7 @@ public class StartInitScriptOnNode implements Callable<ExecResponse> {
    protected ExecResponse runCommand(String command) {
       ExecResponse returnVal;
       logger.debug(">> running [%s] as %s@%s", command.replace(node.getAdminPassword() != null ? node
-               .getAdminPassword() : "XXXXX", "XXXXX"), node.getCredentials().identity, Iterables.get(node
+               .getAdminPassword() : "XXXXX", "XXXXX"), ssh.getUsername(), ssh.getHostAddress());
-               .getPublicAddresses(), 0));
       returnVal = ssh.exec(command);
       return returnVal;
    }
@@ -153,4 +148,10 @@ public class StartInitScriptOnNode implements Callable<ExecResponse> {
    public NodeMetadata getNode() {
       return node;
    }
+
+   @Override
+   public String toString() {
+      return Objects.toStringHelper(this).add("node", node).add("name", name).add("runAsRoot", runAsRoot).toString();
+   }
+
 }

compute/src/main/java/org/jclouds/compute/callables/RunScriptOnNodeAsInitScriptUsingSshAndBlockUntilComplete.java

@@ -25,6 +25,7 @@ import javax.inject.Inject;
 import javax.inject.Named;
 
 import org.jclouds.compute.domain.NodeMetadata;
+import org.jclouds.compute.options.RunScriptOptions;
 import org.jclouds.compute.predicates.ScriptStatusReturnsZero.CommandUsingClient;
 import org.jclouds.scriptbuilder.domain.Statement;
 import org.jclouds.ssh.ExecResponse;
@@ -39,15 +40,15 @@ import com.google.inject.assistedinject.Assisted;
  * 
  * @author Adrian Cole
  */
-public class StartInitScriptOnNodeAndBlockUntilComplete extends StartInitScriptOnNode {
+public class RunScriptOnNodeAsInitScriptUsingSshAndBlockUntilComplete extends RunScriptOnNodeAsInitScriptUsingSsh {
    protected final Predicate<CommandUsingClient> runScriptNotRunning;
 
    @Inject
-   public StartInitScriptOnNodeAndBlockUntilComplete(
+   public RunScriptOnNodeAsInitScriptUsingSshAndBlockUntilComplete(
             @Named("SCRIPT_COMPLETE") Predicate<CommandUsingClient> runScriptNotRunning,
-            Function<NodeMetadata, SshClient> sshFactory, @Assisted NodeMetadata node, @Assisted String scriptName,
+            Function<NodeMetadata, SshClient> sshFactory, @Assisted NodeMetadata node, @Assisted Statement script,
-            @Assisted Statement script, @Assisted boolean runAsRoot) {
+            @Assisted RunScriptOptions options) {
-      super(sshFactory, node, scriptName, script, runAsRoot);
+      super(sshFactory, node, script, options);
       this.runScriptNotRunning = checkNotNull(runScriptNotRunning, "runScriptNotRunning");
    }
 

compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java

@@ -19,6 +19,7 @@
 
 package org.jclouds.compute.config;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static org.jclouds.Constants.PROPERTY_SESSION_INTERVAL;
 import static org.jclouds.compute.domain.OsFamily.UBUNTU;
 
@@ -31,8 +32,9 @@ import javax.inject.Named;
 import javax.inject.Singleton;
 
 import org.jclouds.collect.Memoized;
-import org.jclouds.compute.callables.StartInitScriptOnNode;
+import org.jclouds.compute.callables.RunScriptOnNode;
-import org.jclouds.compute.callables.StartInitScriptOnNodeAndBlockUntilComplete;
+import org.jclouds.compute.callables.RunScriptOnNodeAsInitScriptUsingSsh;
+import org.jclouds.compute.callables.RunScriptOnNodeAsInitScriptUsingSshAndBlockUntilComplete;
 import org.jclouds.compute.domain.ComputeMetadata;
 import org.jclouds.compute.domain.Hardware;
 import org.jclouds.compute.domain.Image;
@@ -41,16 +43,17 @@ import org.jclouds.compute.domain.OsFamily;
 import org.jclouds.compute.domain.TemplateBuilder;
 import org.jclouds.compute.functions.CreateSshClientOncePortIsListeningOnNode;
 import org.jclouds.compute.functions.TemplateOptionsToStatement;
+import org.jclouds.compute.options.RunScriptOptions;
 import org.jclouds.compute.options.TemplateOptions;
 import org.jclouds.compute.reference.ComputeServiceConstants;
 import org.jclouds.compute.strategy.CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap;
-import org.jclouds.compute.strategy.RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap;
+import org.jclouds.compute.strategy.InitializeRunScriptOnNodeOrPlaceInBadMap;
-import org.jclouds.compute.util.ComputeServiceUtils;
 import org.jclouds.domain.Location;
 import org.jclouds.json.Json;
 import org.jclouds.rest.AuthorizationException;
 import org.jclouds.rest.suppliers.RetryOnTimeOutButNotOnAuthorizationExceptionSupplier;
 import org.jclouds.scriptbuilder.domain.Statement;
+import org.jclouds.scriptbuilder.domain.Statements;
 import org.jclouds.ssh.SshClient;
 
 import com.google.common.base.Function;
@@ -58,6 +61,7 @@ import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import com.google.common.collect.Maps;
 import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
 import com.google.inject.Injector;
 import com.google.inject.Provides;
 import com.google.inject.TypeLiteral;
@@ -77,9 +81,12 @@ public abstract class BaseComputeServiceContextModule extends AbstractModule {
       bind(new TypeLiteral<Function<TemplateOptions, Statement>>() {
       }).to(TemplateOptionsToStatement.class);
 
-      install(new FactoryModuleBuilder().implement(StartInitScriptOnNode.class, Names.named("blocking"),
+      install(new FactoryModuleBuilder().implement(RunScriptOnNode.class, Names.named("blocking"),
-               StartInitScriptOnNodeAndBlockUntilComplete.class).implement(StartInitScriptOnNode.class,
+               RunScriptOnNodeAsInitScriptUsingSshAndBlockUntilComplete.class).implement(RunScriptOnNode.class,
-               Names.named("nonblocking"), StartInitScriptOnNode.class).build(StartInitScriptOnNode.Factory.class));
+               Names.named("nonblocking"), RunScriptOnNodeAsInitScriptUsingSsh.class).build(
+               RunScriptOnNodeFactoryImpl.Factory.class));
+      
+      bind(RunScriptOnNode.Factory.class).to(RunScriptOnNodeFactoryImpl.class);
 
       install(new FactoryModuleBuilder().implement(new TypeLiteral<Callable<Void>>() {
       }, CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap.class).implement(
@@ -87,10 +94,47 @@ public abstract class BaseComputeServiceContextModule extends AbstractModule {
                }, CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap.class).build(
                CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap.Factory.class));
 
-      install(new FactoryModuleBuilder().implement(new TypeLiteral<Callable<Void>>() {
+      install(new FactoryModuleBuilder().implement(new TypeLiteral<Callable<RunScriptOnNode>>() {
-      }, RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap.class).build(
+      }, InitializeRunScriptOnNodeOrPlaceInBadMap.class).build(InitializeRunScriptOnNodeOrPlaceInBadMap.Factory.class));
-               RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap.Factory.class));
+   }
-      requestStaticInjection(ComputeServiceUtils.class);
+
+   @Singleton
+   static class RunScriptOnNodeFactoryImpl implements RunScriptOnNode.Factory {
+
+      static interface Factory {
+
+         @Named("blocking")
+         RunScriptOnNode blockOnComplete(NodeMetadata node, Statement script, RunScriptOptions options);
+
+         @Named("nonblocking")
+         RunScriptOnNode dontBlockOnComplete(NodeMetadata node, Statement script, RunScriptOptions options);
+      }
+
+      private final Factory factory;
+
+      @Inject
+      RunScriptOnNodeFactoryImpl(Factory factory) {
+         this.factory = checkNotNull(factory, "factory");
+      }
+
+      @Override
+      public RunScriptOnNode create(NodeMetadata node, Statement runScript, RunScriptOptions options) {
+         checkNotNull(node, "node");
+         checkNotNull(runScript, "runScript");
+         checkNotNull(options, "options");
+         return options.shouldBlockOnComplete() ? factory.blockOnComplete(node, runScript, options) : factory
+                  .dontBlockOnComplete(node, runScript, options);
+      }
+
+      @Override
+      public RunScriptOnNode create(NodeMetadata node, String script) {
+         return create(node, Statements.exec(checkNotNull(script, "script")));
+      }
+
+      @Override
+      public RunScriptOnNode create(NodeMetadata node, Statement script) {
+         return create(node, script, RunScriptOptions.NONE);
+      }
    }
 
    @Provides

compute/src/main/java/org/jclouds/compute/functions/CreateSshClientOncePortIsListeningOnNode.java

@@ -52,8 +52,10 @@ public class CreateSshClientOncePortIsListeningOnNode implements Function<NodeMe
    @Override
    public SshClient apply(NodeMetadata node) {
       checkState(sshFactory != null, "ssh requested, but no SshModule configured");
-      checkNotNull(node.getCredentials(), "credentials for node " + node.getName());
+      checkNotNull(node.getCredentials(), "no credentials found for node %s", node.getId());
-      checkNotNull(node.getCredentials().credential, "credentials.credential for node " + node.getName());
+      checkNotNull(node.getCredentials().identity, "no login identity found for node %s", node.getId());
+      checkNotNull(node.getCredentials().credential, "no credential found for $s on node %s", node
+               .getCredentials().identity, node.getId());
       IPSocket socket = ComputeServiceUtils.findReachableSocketOnNode(socketTester, node, node.getLoginPort());
       return sshFactory.create(socket, node.getCredentials());
    }

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

@@ -19,14 +19,12 @@
 
 package org.jclouds.compute.internal;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Predicates.and;
 import static com.google.common.base.Predicates.not;
 import static com.google.common.base.Predicates.notNull;
 import static com.google.common.collect.Iterables.concat;
 import static com.google.common.collect.Iterables.filter;
-import static com.google.common.collect.Iterables.transform;
 import static com.google.common.collect.Maps.newLinkedHashMap;
 import static com.google.common.collect.Sets.filter;
 import static com.google.common.collect.Sets.newLinkedHashSet;
@@ -38,6 +36,7 @@ import static org.jclouds.concurrent.FutureIterables.transformParallel;
 
 import java.io.IOException;
 import java.util.Map;
+import java.util.NoSuchElementException;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
@@ -45,7 +44,6 @@ import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 
-import javax.annotation.Nullable;
 import javax.annotation.Resource;
 import javax.inject.Inject;
 import javax.inject.Named;
@@ -58,6 +56,7 @@ import org.jclouds.compute.ComputeService;
 import org.jclouds.compute.ComputeServiceContext;
 import org.jclouds.compute.RunNodesException;
 import org.jclouds.compute.RunScriptOnNodesException;
+import org.jclouds.compute.callables.RunScriptOnNode;
 import org.jclouds.compute.config.CustomizationResponse;
 import org.jclouds.compute.domain.ComputeMetadata;
 import org.jclouds.compute.domain.Hardware;
@@ -72,11 +71,12 @@ import org.jclouds.compute.reference.ComputeServiceConstants;
 import org.jclouds.compute.reference.ComputeServiceConstants.Timeouts;
 import org.jclouds.compute.strategy.DestroyNodeStrategy;
 import org.jclouds.compute.strategy.GetNodeMetadataStrategy;
+import org.jclouds.compute.strategy.InitializeRunScriptOnNodeOrPlaceInBadMap;
 import org.jclouds.compute.strategy.ListNodesStrategy;
 import org.jclouds.compute.strategy.RebootNodeStrategy;
 import org.jclouds.compute.strategy.ResumeNodeStrategy;
 import org.jclouds.compute.strategy.RunNodesAndAddToSetStrategy;
-import org.jclouds.compute.strategy.RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap;
+import org.jclouds.compute.strategy.RunScriptOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap;
 import org.jclouds.compute.strategy.SuspendNodeStrategy;
 import org.jclouds.domain.Credentials;
 import org.jclouds.domain.Location;
@@ -92,6 +92,8 @@ import com.google.common.base.Function;
 import com.google.common.base.Predicate;
 import com.google.common.base.Supplier;
 import com.google.common.base.Throwables;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Multimap;
 
@@ -124,7 +126,7 @@ public class BaseComputeService implements ComputeService {
    private final Predicate<NodeMetadata> nodeRunning;
    private final Predicate<NodeMetadata> nodeTerminated;
    private final Predicate<NodeMetadata> nodeSuspended;
-   private final RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap.Factory statementRunner;
+   private final InitializeRunScriptOnNodeOrPlaceInBadMap.Factory initScriptRunnerFactory;
    private final Timeouts timeouts;
    private final ExecutorService executor;
 
@@ -140,7 +142,7 @@ public class BaseComputeService implements ComputeService {
             @Named("NODE_RUNNING") Predicate<NodeMetadata> nodeRunning,
             @Named("NODE_TERMINATED") Predicate<NodeMetadata> nodeTerminated,
             @Named("NODE_SUSPENDED") Predicate<NodeMetadata> nodeSuspended,
-            RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap.Factory statementRunner, Timeouts timeouts,
+            InitializeRunScriptOnNodeOrPlaceInBadMap.Factory initScriptRunnerFactory, Timeouts timeouts,
             @Named(Constants.PROPERTY_USER_THREADS) ExecutorService executor) {
       this.context = checkNotNull(context, "context");
       this.credentialStore = checkNotNull(credentialStore, "credentialStore");
@@ -159,7 +161,7 @@ public class BaseComputeService implements ComputeService {
       this.nodeRunning = checkNotNull(nodeRunning, "nodeRunning");
       this.nodeTerminated = checkNotNull(nodeTerminated, "nodeTerminated");
       this.nodeSuspended = checkNotNull(nodeSuspended, "nodeSuspended");
-      this.statementRunner = checkNotNull(statementRunner, "statementRunner");
+      this.initScriptRunnerFactory = checkNotNull(initScriptRunnerFactory, "initScriptRunnerFactory");
       this.timeouts = checkNotNull(timeouts, "timeouts");
       this.executor = checkNotNull(executor, "executor");
    }
@@ -189,7 +191,8 @@ public class BaseComputeService implements ComputeService {
 
       Map<?, Future<Void>> responses = runNodesAndAddToSetStrategy.execute(tag, count, template, goodNodes, badNodes,
                customizationResponses);
-      Map<?, Exception> executionExceptions = awaitCompletion(responses, executor, null, logger, "resuming nodes");
+      Map<?, Exception> executionExceptions = awaitCompletion(responses, executor, null, logger, "runNodesWithTag("
+               + tag + ")");
       for (NodeMetadata node : concat(goodNodes, badNodes.keySet()))
          if (node.getCredentials() != null)
             credentialStore.put("node#" + node.getId(), node.getCredentials());
@@ -269,13 +272,25 @@ public class BaseComputeService implements ComputeService {
                      });
                   }
 
-               }, executor, null, logger, "destroying nodes"));
+               }, executor, null, logger, "destroyNodesMatching(" + filter + ")"));
       logger.debug("<< destroyed(%d)", set.size());
       return set;
    }
 
-   private Iterable<? extends NodeMetadata> nodesMatchingFilterAndNotTerminated(Predicate<NodeMetadata> filter) {
+   Iterable<? extends NodeMetadata> nodesMatchingFilterAndNotTerminated(Predicate<NodeMetadata> filter) {
-      return filter(detailsOnAllNodes(), and(filter, not(TERMINATED)));
+      return filter(detailsOnAllNodes(), and(checkNotNull(filter, "filter"), not(TERMINATED)));
+   }
+
+   /**
+    * @throws NoSuchElementException
+    *            if none found
+    */
+   Iterable<? extends NodeMetadata> nodesMatchingFilterAndNotTerminatedExceptionIfNotFound(
+            Predicate<NodeMetadata> filter) {
+      Iterable<? extends NodeMetadata> nodes = nodesMatchingFilterAndNotTerminated(filter);
+      if (Iterables.size(nodes) == 0)
+         throw new NoSuchElementException("no nodes matched filter: " + filter);
+      return nodes;
    }
 
    /**
@@ -360,7 +375,8 @@ public class BaseComputeService implements ComputeService {
    @Override
    public void rebootNodesMatching(Predicate<NodeMetadata> filter) {
       logger.debug(">> rebooting nodes matching(%s)", filter);
-      transformParallel(nodesMatchingFilterAndNotTerminated(filter), new Function<NodeMetadata, Future<Void>>() {
+      transformParallel(nodesMatchingFilterAndNotTerminatedExceptionIfNotFound(filter),
+               new Function<NodeMetadata, Future<Void>>() {
                   // TODO use native async
                   @Override
                   public Future<Void> apply(NodeMetadata from) {
@@ -368,7 +384,7 @@ public class BaseComputeService implements ComputeService {
                      return immediateFuture(null);
                   }
 
-      }, executor, null, logger, "rebooting nodes");
+               }, executor, null, logger, "rebootNodesMatching(" + filter + ")");
       logger.debug("<< rebooted");
    }
 
@@ -378,10 +394,10 @@ public class BaseComputeService implements ComputeService {
    @Override
    public void resumeNode(String id) {
       checkNotNull(id, "id");
-      logger.debug(">> resumeing node(%s)", id);
+      logger.debug(">> resuming node(%s)", id);
       NodeMetadata node = resumeNodeStrategy.resumeNode(id);
       boolean successful = nodeRunning.apply(node);
-      logger.debug("<< resumeed node(%s) success(%s)", id, successful);
+      logger.debug("<< resumed node(%s) success(%s)", id, successful);
    }
 
    /**
@@ -389,8 +405,9 @@ public class BaseComputeService implements ComputeService {
     */
    @Override
    public void resumeNodesMatching(Predicate<NodeMetadata> filter) {
-      logger.debug(">> resumeing nodes matching(%s)", filter);
+      logger.debug(">> resuming nodes matching(%s)", filter);
-      transformParallel(nodesMatchingFilterAndNotTerminated(filter), new Function<NodeMetadata, Future<Void>>() {
+      transformParallel(nodesMatchingFilterAndNotTerminatedExceptionIfNotFound(filter),
+               new Function<NodeMetadata, Future<Void>>() {
                   // TODO use native async
                   @Override
                   public Future<Void> apply(NodeMetadata from) {
@@ -398,8 +415,8 @@ public class BaseComputeService implements ComputeService {
                      return immediateFuture(null);
                   }
 
-      }, executor, null, logger, "resumeing nodes");
+               }, executor, null, logger, "resumeNodesMatching(" + filter + ")");
-      logger.debug("<< resumeed");
+      logger.debug("<< resumed");
    }
 
    /**
@@ -420,7 +437,8 @@ public class BaseComputeService implements ComputeService {
    @Override
    public void suspendNodesMatching(Predicate<NodeMetadata> filter) {
       logger.debug(">> suspending nodes matching(%s)", filter);
-      transformParallel(nodesMatchingFilterAndNotTerminated(filter), new Function<NodeMetadata, Future<Void>>() {
+      transformParallel(nodesMatchingFilterAndNotTerminatedExceptionIfNotFound(filter),
+               new Function<NodeMetadata, Future<Void>>() {
                   // TODO use native async
                   @Override
                   public Future<Void> apply(NodeMetadata from) {
@@ -428,7 +446,7 @@ public class BaseComputeService implements ComputeService {
                      return immediateFuture(null);
                   }
 
-      }, executor, null, logger, "suspending nodes");
+               }, executor, null, logger, "suspendNodesMatching(" + filter + ")");
       logger.debug("<< suspended");
    }
 
@@ -446,7 +464,7 @@ public class BaseComputeService implements ComputeService {
     */
    @Override
    public Map<NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter, Payload runScript,
-            @Nullable RunScriptOptions options) throws RunScriptOnNodesException {
+            RunScriptOptions options) throws RunScriptOnNodesException {
       try {
          return runScriptOnNodesMatching(filter, Statements.exec(Strings2.toStringAndClose(checkNotNull(runScript,
                   "runScript").getInput())), options);
@@ -460,9 +478,18 @@ public class BaseComputeService implements ComputeService {
     * {@inheritDoc}
     */
    @Override
-   public Map<NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter, String runScript,
+   public Map<NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter, String runScript)
-            @Nullable RunScriptOptions options) throws RunScriptOnNodesException {
+            throws RunScriptOnNodesException {
-      return runScriptOnNodesMatching(filter, Statements.exec(checkNotNull(runScript, "runScript")), options);
+      return runScriptOnNodesMatching(filter, Statements.exec(checkNotNull(runScript, "runScript")));
+   }
+
+   /**
+    * {@inheritDoc}
+    */
+   @Override
+   public Map<NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter, Statement runScript)
+            throws RunScriptOnNodesException {
+      return runScriptOnNodesMatching(filter, runScript, RunScriptOptions.NONE);
    }
 
    /**
@@ -470,57 +497,38 @@ public class BaseComputeService implements ComputeService {
     */
    @Override
    public Map<NodeMetadata, ExecResponse> runScriptOnNodesMatching(Predicate<NodeMetadata> filter, Statement runScript,
-            @Nullable RunScriptOptions options) throws RunScriptOnNodesException {
+            RunScriptOptions options) throws RunScriptOnNodesException {
 
-      checkNotNull(filter, "Filter must be provided");
+      checkNotNull(filter, "filter");
       checkNotNull(runScript, "runScript");
       checkNotNull(options, "options");
 
-      Iterable<? extends NodeMetadata> nodes = filter(detailsOnAllNodes(), filter);
-
       Map<NodeMetadata, ExecResponse> goodNodes = newLinkedHashMap();
       Map<NodeMetadata, Exception> badNodes = newLinkedHashMap();
+      Map<NodeMetadata, Future<ExecResponse>> responses = newLinkedHashMap();
+      Map<?, Exception> exceptions = ImmutableMap.<Object, Exception> of();
 
-      Map<NodeMetadata, Future<Void>> responses = newLinkedHashMap();
+      Iterable<? extends RunScriptOnNode> scriptRunners = transformNodesIntoInitializedScriptRunners(
-      nodes = filterNodesWhoCanRunScripts(nodes, badNodes, options.getOverrideCredentials());
+               nodesMatchingFilterAndNotTerminatedExceptionIfNotFound(filter), runScript, options, badNodes);
-
+      if (Iterables.size(scriptRunners) > 0) {
-      for (NodeMetadata node : nodes) {
+         for (RunScriptOnNode runner : scriptRunners) {
-         responses.put(node, executor.submit(statementRunner.create(node, runScript, options, goodNodes, badNodes)));
+            responses.put(runner.getNode(), executor.submit(new RunScriptOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap(
+                     runner, goodNodes, badNodes)));
+         }
+         exceptions = awaitCompletion(responses, executor, null, logger, "runScriptOnNodesMatching(" + filter + ")");
       }
 
-      Map<?, Exception> exceptions = awaitCompletion(responses, executor, null, logger, "running script on nodes");
       if (exceptions.size() > 0 || badNodes.size() > 0) {
          throw new RunScriptOnNodesException(runScript, options, goodNodes, exceptions, badNodes);
       }
       return goodNodes;
-
    }
 
-   private Iterable<? extends NodeMetadata> filterNodesWhoCanRunScripts(Iterable<? extends NodeMetadata> nodes,
+   private Iterable<? extends RunScriptOnNode> transformNodesIntoInitializedScriptRunners(
-            final Map<NodeMetadata, Exception> badNodes, final @Nullable Credentials overridingCredentials) {
+            Iterable<? extends NodeMetadata> nodes, Statement script, RunScriptOptions options,
-      nodes = filter(transform(nodes, new Function<NodeMetadata, NodeMetadata>() {
+            Map<NodeMetadata, Exception> badNodes) {
-
+      return filter(transformParallel(nodes, new TransformNodesIntoInitializedScriptRunners(script, options, badNodes),
-         @Override
+               executor, null, logger, "transformNodesIntoInitializedScriptRunners(" + nodes + ")"), notNull());
-         public NodeMetadata apply(NodeMetadata node) {
-            try {
-               checkArgument(node.getPublicAddresses().size() > 0, "no public ip addresses on node: " + node);
-               if (overridingCredentials != null) {
-                  node = NodeMetadataBuilder.fromNodeMetadata(node).credentials(overridingCredentials).build();
-               } else {
-                  checkNotNull(node.getCredentials(), "If the default credentials need to be used, they can't be null");
-                  checkNotNull(node.getCredentials().identity, "Account name for ssh authentication must be "
-                           + "specified. Try passing RunScriptOptions with new credentials");
-                  checkNotNull(node.getCredentials().credential, "Key or password for ssh authentication must be "
-                           + "specified. Try passing RunScriptOptions with new credentials");
-               }
-               return node;
-            } catch (Exception e) {
-               badNodes.put(node, e);
-               return null;
-            }
-         }
-      }), notNull());
-      return nodes;
    }
 
    private Set<? extends NodeMetadata> detailsOnAllNodes() {
@@ -531,4 +539,27 @@ public class BaseComputeService implements ComputeService {
    public TemplateOptions templateOptions() {
       return templateOptionsProvider.get();
    }
+
+   private final class TransformNodesIntoInitializedScriptRunners implements
+            Function<NodeMetadata, Future<RunScriptOnNode>> {
+      private final Map<NodeMetadata, Exception> badNodes;
+      private final Statement script;
+      private final RunScriptOptions options;
+
+      private TransformNodesIntoInitializedScriptRunners(Statement script, RunScriptOptions options,
+               Map<NodeMetadata, Exception> badNodes) {
+         this.badNodes = checkNotNull(badNodes, "badNodes");
+         this.script = checkNotNull(script, "script");
+         this.options = checkNotNull(options, "options");
+      }
+
+      @Override
+      public Future<RunScriptOnNode> apply(NodeMetadata node) {
+         checkNotNull(node, "node");
+         if (options.getOverrideCredentials() != null) {
+            node = NodeMetadataBuilder.fromNodeMetadata(node).credentials(options.getOverrideCredentials()).build();
+         }
+         return executor.submit(initScriptRunnerFactory.create(node, script, options, badNodes));
+      }
+   }
 }

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

@@ -32,7 +32,7 @@ import javax.annotation.Nullable;
 import javax.annotation.Resource;
 import javax.inject.Named;
 
-import org.jclouds.compute.callables.StartInitScriptOnNode;
+import org.jclouds.compute.callables.RunScriptOnNode;
 import org.jclouds.compute.config.CustomizationResponse;
 import org.jclouds.compute.domain.NodeMetadata;
 import org.jclouds.compute.options.TemplateOptions;
@@ -70,7 +70,7 @@ public class CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap implements Cal
    protected Logger logger = Logger.NULL;
 
    private final Predicate<NodeMetadata> nodeRunning;
-   private final ScriptInvokerForNodeAndStatement scriptInvokerForNodeAndStatement;
+   private final InitializeRunScriptOnNodeOrPlaceInBadMap.Factory initScriptRunnerFactory;
    private final GetNodeMetadataStrategy getNode;
    private final RetryIfSocketNotYetOpen socketTester;
    private final Timeouts timeouts;
@@ -90,15 +90,14 @@ public class CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap implements Cal
             @Named("NODE_RUNNING") Predicate<NodeMetadata> nodeRunning, GetNodeMetadataStrategy getNode,
             RetryIfSocketNotYetOpen socketTester, Timeouts timeouts,
             Function<TemplateOptions, Statement> templateOptionsToStatement,
-            ScriptInvokerForNodeAndStatement scriptInvokerForNodeAndStatement, @Assisted TemplateOptions options,
+            InitializeRunScriptOnNodeOrPlaceInBadMap.Factory initScriptRunnerFactory,
-            @Assisted @Nullable NodeMetadata node, @Assisted Set<NodeMetadata> goodNodes,
+            @Assisted TemplateOptions options, @Assisted @Nullable NodeMetadata node,
-            @Assisted Map<NodeMetadata, Exception> badNodes,
+            @Assisted Set<NodeMetadata> goodNodes, @Assisted Map<NodeMetadata, Exception> badNodes,
             @Assisted Multimap<NodeMetadata, CustomizationResponse> customizationResponses) {
       this.statement = checkNotNull(templateOptionsToStatement, "templateOptionsToStatement").apply(
                checkNotNull(options, "options"));
       this.nodeRunning = checkNotNull(nodeRunning, "nodeRunning");
-      this.scriptInvokerForNodeAndStatement = checkNotNull(scriptInvokerForNodeAndStatement,
+      this.initScriptRunnerFactory = checkNotNull(initScriptRunnerFactory, "initScriptRunnerFactory");
-               "scriptInvokerForNodeAndStatement");
       this.getNode = checkNotNull(getNode, "getNode");
       this.socketTester = checkNotNull(socketTester, "socketTester");
       this.timeouts = checkNotNull(timeouts, "timeouts");
@@ -114,11 +113,12 @@ public class CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap implements Cal
             @Named("NODE_RUNNING") Predicate<NodeMetadata> nodeRunning, GetNodeMetadataStrategy getNode,
             RetryIfSocketNotYetOpen socketTester, Timeouts timeouts,
             Function<TemplateOptions, Statement> templateOptionsToStatement,
-            ScriptInvokerForNodeAndStatement scriptInvokerForNodeAndStatement, @Assisted TemplateOptions options,
+            InitializeRunScriptOnNodeOrPlaceInBadMap.Factory initScriptRunnerFactory,
-            @Assisted Set<NodeMetadata> goodNodes, @Assisted Map<NodeMetadata, Exception> badNodes,
+            @Assisted TemplateOptions options, @Assisted Set<NodeMetadata> goodNodes,
+            @Assisted Map<NodeMetadata, Exception> badNodes,
             @Assisted Multimap<NodeMetadata, CustomizationResponse> customizationResponses) {
-      this(nodeRunning, getNode, socketTester, timeouts, templateOptionsToStatement, scriptInvokerForNodeAndStatement,
+      this(nodeRunning, getNode, socketTester, timeouts, templateOptionsToStatement, initScriptRunnerFactory, options,
-               options, null, goodNodes, badNodes, customizationResponses);
+               null, goodNodes, badNodes, customizationResponses);
    }
 
    @Override
@@ -135,11 +135,12 @@ public class CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap implements Cal
                                  .getId(), timeouts.nodeRunning / 1000, node.getState()));
             }
             if (statement != null) {
-               StartInitScriptOnNode scriptInstructions = scriptInvokerForNodeAndStatement.create(node, statement,
+               RunScriptOnNode runner = initScriptRunnerFactory.create(node, statement, options, badNodes).call();
-                        options);
+               if (runner != null) {
-               ExecResponse exec = scriptInstructions.call();
+                  ExecResponse exec = runner.call();
                   customizationResponses.put(node, exec);
                }
+            }
             if (options.getPort() > 0) {
                findReachableSocketOnNode(socketTester.seconds(options.getSeconds()), node, options.getPort());
             }

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

@@ -0,0 +1,79 @@
+/**
+ *
+ * Copyright (C) 2010 Cloud Conscious, LLC. <info@cloudconscious.com>
+ *
+ * ====================================================================
+ * Licensed 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.compute.strategy;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.util.Map;
+import java.util.concurrent.Callable;
+
+import javax.inject.Inject;
+
+import org.jclouds.compute.callables.RunScriptOnNode;
+import org.jclouds.compute.domain.NodeMetadata;
+import org.jclouds.compute.options.RunScriptOptions;
+import org.jclouds.scriptbuilder.domain.Statement;
+
+import com.google.common.base.Objects;
+import com.google.inject.assistedinject.Assisted;
+
+/**
+ * 
+ * @author Adrian Cole
+ */
+public class InitializeRunScriptOnNodeOrPlaceInBadMap implements Callable<RunScriptOnNode> {
+
+   public interface Factory {
+      Callable<RunScriptOnNode> create(NodeMetadata node, Statement script, RunScriptOptions options,
+               Map<NodeMetadata, Exception> badNodes);
+   }
+
+   private final RunScriptOnNode.Factory runScriptOnNodeFactory;
+   private final Statement script;
+   private final Map<NodeMetadata, Exception> badNodes;
+   private final NodeMetadata node;
+   private final RunScriptOptions options;
+
+   @Inject
+   InitializeRunScriptOnNodeOrPlaceInBadMap(RunScriptOnNode.Factory runScriptOnNodeFactory,
+            @Assisted NodeMetadata node, @Assisted Statement script, @Assisted RunScriptOptions options,
+            @Assisted Map<NodeMetadata, Exception> badNodes) {
+      this.runScriptOnNodeFactory = checkNotNull(runScriptOnNodeFactory, "runScriptOnNodeFactory");
+      this.script = checkNotNull(script, "script");
+      this.badNodes = checkNotNull(badNodes, "badNodes");
+      this.node = checkNotNull(node, "node");
+      this.options = checkNotNull(options, "options");
+   }
+
+   @Override
+   public RunScriptOnNode call() throws Exception {
+      try {
+         return runScriptOnNodeFactory.create(node, script, options).init();
+      } catch (Exception e) {
+         badNodes.put(node, e);
+         return null;
+      }
+   }
+
+   @Override
+   public String toString() {
+      return Objects.toStringHelper(this).add("node", node).add("options", options).toString();
+   }
+}

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

@@ -0,0 +1,89 @@
+/**
+ *
+ * Copyright (C) 2010 Cloud Conscious, LLC. <info@cloudconscious.com>
+ *
+ * ====================================================================
+ * Licensed 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.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 java.util.Map;
+import java.util.concurrent.Callable;
+
+import javax.annotation.Resource;
+import javax.inject.Named;
+
+import org.jclouds.compute.callables.RunScriptOnNode;
+import org.jclouds.compute.domain.NodeMetadata;
+import org.jclouds.compute.reference.ComputeServiceConstants;
+import org.jclouds.logging.Logger;
+import org.jclouds.ssh.ExecResponse;
+
+import com.google.common.base.Objects;
+import com.google.inject.assistedinject.AssistedInject;
+
+/**
+ * 
+ * @author Adrian Cole
+ */
+public class RunScriptOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap implements Callable<ExecResponse> {
+
+   @Resource
+   @Named(ComputeServiceConstants.COMPUTE_LOGGER)
+   protected Logger logger = Logger.NULL;
+   private final RunScriptOnNode runScriptOnNode;
+   private final Map<NodeMetadata, Exception> badNodes;
+   private final Map<NodeMetadata, ExecResponse> goodNodes;
+
+   private transient boolean tainted;
+
+   @AssistedInject
+   public RunScriptOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap(RunScriptOnNode runScriptOnNode,
+            Map<NodeMetadata, ExecResponse> goodNodes, Map<NodeMetadata, Exception> badNodes) {
+      this.runScriptOnNode = checkNotNull(runScriptOnNode, "runScriptOnNode");
+      this.badNodes = checkNotNull(badNodes, "badNodes");
+      this.goodNodes = checkNotNull(goodNodes, "goodNodes");
+   }
+
+   @Override
+   public ExecResponse call() {
+      checkState(runScriptOnNode != null, "runScriptOnNode must be set");
+      checkState(!tainted, "this object is not designed to be reused: %s", toString());
+      tainted = true;
+      try {
+         ExecResponse exec = runScriptOnNode.call();
+         logger.trace("<< script output for node(%s): %s", runScriptOnNode.getNode().getId(), exec);
+         logger.debug("<< options applied node(%s)", runScriptOnNode.getNode().getId());
+         goodNodes.put(runScriptOnNode.getNode(), exec);
+         return exec;
+      } catch (Exception e) {
+         logger.error(e, "<< problem applying options to node(%s): ", runScriptOnNode.getNode().getId(),
+                  getRootCause(e).getMessage());
+         badNodes.put(runScriptOnNode.getNode(), e);
+      }
+      return null;
+   }
+
+   @Override
+   public String toString() {
+      return Objects.toStringHelper(this).add("runScriptOnNode", runScriptOnNode).add("goodNodes", goodNodes).add(
+               "badNodes", badNodes).toString();
+   }
+
+}

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

@@ -1,108 +0,0 @@
-/**
- *
- * Copyright (C) 2010 Cloud Conscious, LLC. <info@cloudconscious.com>
- *
- * ====================================================================
- * Licensed 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.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 java.util.Map;
-import java.util.concurrent.Callable;
-
-import javax.annotation.Nullable;
-import javax.annotation.Resource;
-import javax.inject.Named;
-
-import org.jclouds.compute.callables.StartInitScriptOnNode;
-import org.jclouds.compute.domain.NodeMetadata;
-import org.jclouds.compute.options.RunScriptOptions;
-import org.jclouds.compute.reference.ComputeServiceConstants;
-import org.jclouds.logging.Logger;
-import org.jclouds.scriptbuilder.domain.Statement;
-import org.jclouds.ssh.ExecResponse;
-
-import com.google.inject.assistedinject.Assisted;
-import com.google.inject.assistedinject.AssistedInject;
-
-/**
- * 
- * @author Adrian Cole
- */
-public class RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap implements Callable<Void> {
-   public static interface Factory {
-      Callable<Void> create(NodeMetadata node, Statement statement, RunScriptOptions options,
-               Map<NodeMetadata, ExecResponse> goodNodes, Map<NodeMetadata, Exception> badNodes);
-
-   }
-
-   @Resource
-   @Named(ComputeServiceConstants.COMPUTE_LOGGER)
-   protected Logger logger = Logger.NULL;
-   // NOTE this is mutable
-   protected NodeMetadata node;
-   private final Map<NodeMetadata, Exception> badNodes;
-   private final Map<NodeMetadata, ExecResponse> goodNodes;
-   private final RunScriptOptions options;
-
-   protected final Statement statement;
-   private final ScriptInvokerForNodeAndStatement scriptInvokerForNodeAndStatement;
-   private transient boolean tainted;
-
-   @AssistedInject
-   public RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap(
-            ScriptInvokerForNodeAndStatement scriptInvokerForNodeAndStatement, @Assisted NodeMetadata node,
-            @Assisted @Nullable Statement statement, @Assisted RunScriptOptions options,
-            @Assisted Map<NodeMetadata, ExecResponse> goodNodes, @Assisted Map<NodeMetadata, Exception> badNodes) {
-      this.statement = checkNotNull(statement, "statement");
-      this.scriptInvokerForNodeAndStatement = checkNotNull(scriptInvokerForNodeAndStatement,
-               "scriptInvokerForNodeAndStatement");
-      this.node = node;
-      this.badNodes = checkNotNull(badNodes, "badNodes");
-      this.goodNodes = checkNotNull(goodNodes, "goodNodes");
-      this.options = checkNotNull(options, "options");
-   }
-
-   @AssistedInject
-   public RunStatementOnNodeAndAddToGoodMapOrPutExceptionIntoBadMap(
-            ScriptInvokerForNodeAndStatement scriptInvokerForNodeAndStatement, @Assisted @Nullable Statement statement,
-            @Assisted RunScriptOptions options, @Assisted Map<NodeMetadata, ExecResponse> goodNodes,
-            @Assisted Map<NodeMetadata, Exception> badNodes) {
-      this(scriptInvokerForNodeAndStatement, null, statement, options, goodNodes, badNodes);
-   }
-
-   @Override
-   public Void call() {
-      checkState(node != null, "node must be set");
-      checkState(!tainted, "this object is not designed to be reused: %s", toString());
-      tainted = true;
-      try {
-         StartInitScriptOnNode scriptInstructions = scriptInvokerForNodeAndStatement.create(node, statement, options);
-         ExecResponse exec = scriptInstructions.call();
-         logger.trace("<< script output for node(%s): %s", node.getId(), exec);
-         logger.debug("<< options applied node(%s)", node.getId());
-         goodNodes.put(node, exec);
-      } catch (Exception e) {
-         logger.error(e, "<< problem applying options to node(%s): ", node.getId(), getRootCause(e).getMessage());
-         badNodes.put(node, e);
-      }
-      return null;
-   }
-
-}

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

@@ -1,56 +0,0 @@
-/**
- *
- * Copyright (C) 2010 Cloud Conscious, LLC. <info@cloudconscious.com>
- *
- * ====================================================================
- * Licensed 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.compute.strategy;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-
-import javax.inject.Singleton;
-
-import org.jclouds.compute.callables.StartInitScriptOnNode;
-import org.jclouds.compute.domain.NodeMetadata;
-import org.jclouds.compute.options.RunScriptOptions;
-import org.jclouds.scriptbuilder.domain.Statement;
-
-import com.google.inject.Inject;
-
-/**
- * 
- * @author Adrian Cole
- */
-@Singleton
-public class ScriptInvokerForNodeAndStatement {
-
-   private final StartInitScriptOnNode.Factory initAndStartScriptOnNodeFactory;
-
-   @Inject
-   public ScriptInvokerForNodeAndStatement(StartInitScriptOnNode.Factory initAndStartScriptOnNodeFactory) {
-      this.initAndStartScriptOnNodeFactory = checkNotNull(initAndStartScriptOnNodeFactory,
-               "initAndStartScriptOnNodeFactory");
-   }
-
-   public StartInitScriptOnNode create(NodeMetadata node, Statement runScript, RunScriptOptions options) {
-      checkNotNull(node, "node");
-      checkNotNull(runScript, "runScript");
-      checkNotNull(options, "options");
-      return options.shouldBlockOnComplete() ? initAndStartScriptOnNodeFactory.blockOnComplete(node, options
-               .getTaskName(), runScript, options.shouldRunAsRoot()) : initAndStartScriptOnNodeFactory
-               .dontBlockOnComplete(node, options.getTaskName(), runScript, options.shouldRunAsRoot());
-   }
-}

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

@@ -36,9 +36,6 @@ import java.util.Map.Entry;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import javax.annotation.Resource;
-import javax.inject.Named;
-
 import org.jclouds.compute.ComputeServiceContextBuilder;
 import org.jclouds.compute.domain.ComputeMetadata;
 import org.jclouds.compute.domain.Hardware;
@@ -47,9 +44,7 @@ import org.jclouds.compute.domain.OsFamily;
 import org.jclouds.compute.domain.Processor;
 import org.jclouds.compute.domain.Volume;
 import org.jclouds.compute.predicates.RetryIfSocketNotYetOpen;
-import org.jclouds.compute.reference.ComputeServiceConstants;
 import org.jclouds.http.HttpRequest;
-import org.jclouds.logging.Logger;
 import org.jclouds.net.IPSocket;
 import org.jclouds.rest.Providers;
 import org.jclouds.scriptbuilder.domain.Statement;
@@ -106,10 +101,6 @@ public class ComputeServiceUtils {
       return extractZipIntoDirectory(new HttpRequest("GET", zip), directory);
    }
 
-   @Resource
-   @Named(ComputeServiceConstants.COMPUTE_LOGGER)
-   protected static Logger logger = Logger.NULL;
-
    /**
     * 
     * 
@@ -117,12 +108,7 @@ public class ComputeServiceUtils {
     */
    public static String parseTagFromName(String from) {
       Matcher matcher = DELIMETED_BY_HYPHEN_ENDING_IN_HYPHEN_HEX.matcher(from);
-      String returnVal = matcher.find() ? matcher.group(1) : "NOTAG#" + from;
+      return matcher.find() ? matcher.group(1) : "NOTAG#" + from;
-      if (logger.isTraceEnabled()) {
-         if (returnVal.startsWith("NOTAG#"))
-            logger.trace("failed to parse tag from name %s", from);
-      }
-      return returnVal;
    }
 
    public static double getCores(Hardware input) {
@@ -207,7 +193,8 @@ public class ComputeServiceUtils {
                      }
                   }), socketTester);
       } catch (NoSuchElementException e) {
-         throw new RuntimeException(String.format("could not connect to any ip address port %d on node %s", port, node));
+         throw new NoSuchElementException(String.format("could not connect to any ip address port %d on node %s", port,
+                  node));
       }
       return socket;
    }

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

@@ -62,6 +62,8 @@ import org.jclouds.compute.domain.Image;
 import org.jclouds.compute.domain.NodeMetadata;
 import org.jclouds.compute.domain.NodeState;
 import org.jclouds.compute.domain.OperatingSystem;
+import org.jclouds.compute.domain.OperatingSystemBuilder;
+import org.jclouds.compute.domain.OsFamily;
 import org.jclouds.compute.domain.Template;
 import org.jclouds.compute.domain.TemplateBuilder;
 import org.jclouds.compute.options.TemplateOptions;
@@ -201,6 +203,12 @@ public abstract class BaseComputeServiceLiveTest {
       assert duration < 1000 : String.format("%dms to get images", duration);
    }
 
+   @Test(enabled = true, expectedExceptions = NoSuchElementException.class)
+   public void testCorrectExceptionRunningNodesNotFound() throws Exception {
+      client.runScriptOnNodesMatching(runningWithTag("zebras-are-awesome"), buildScript(new OperatingSystemBuilder()
+               .family(OsFamily.UBUNTU).description("ffoo").build()));
+   }
+
    // since surefire and eclipse don't otherwise guarantee the order, we are
    // starting this one alphabetically before create2nodes..
    @Test(enabled = true, dependsOnMethods = { "testCompareSizes" })

providers/trmk-ecloud/pom.xml

@@ -118,6 +118,7 @@
                                     <goal>test</goal>
                                 </goals>
                                 <configuration>
+                                    <threadCount>1</threadCount>
                                     <systemProperties>
                                         <property>
                                             <name>test.trmk-ecloud.endpoint</name>