WinRM commands

Use executePsCommand and executeCmdCommand instead of executeScript

WinRM Script tests

Author: Valentin Aitken

brooklyn-server/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlTemplateTest.java

@@ -23,6 +23,7 @@
 
 import java.util.List;
 
+import com.google.common.collect.Iterables;
 import org.apache.brooklyn.api.entity.Application;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntitySpec;

brooklyn-server/launcher/src/test/java/org/apache/brooklyn/launcher/blueprints/Windows7zipBlueprintLiveTest.java

@@ -88,7 +88,7 @@ public void test7zip() throws Exception {
                 String password = Strings.getFirstWordAfter(winRMAddress, ":");
 
                 WinRmTool winRmTool = WinRmTool.connect(ipPort, user, password);
-                WinRmToolResponse winRmResponse = winRmTool.executePs(ImmutableList.of("(Get-Item \"C:\\\\Program Files\\\\7-Zip\\\\7z.exe\").name"));
+                WinRmToolResponse winRmResponse = winRmTool.executePs("(Get-Item \"C:\\\\Program Files\\\\7-Zip\\\\7z.exe\").name");
 
                 LOG.info("winRmResponse: code="+winRmResponse.getStatusCode()+"; out="+winRmResponse.getStdOut()+"; err="+winRmResponse.getStdErr());
                 return "7z.exe\r\n".equals(winRmResponse.getStdOut());

brooklyn-server/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java

@@ -862,7 +862,7 @@ protected MachineLocation obtainOnce(ConfigBag setup) throws NoMachinesAvailable
                         String scriptContent = ResourceUtils.create(this).getResourceAsString(setupScriptItem);
                         String script = TemplateProcessor.processTemplateContents(scriptContent, getManagementContext(), substitutions);
                         if (windows) {
-                            ((WinRmMachineLocation)machineLocation).executeCommand(ImmutableList.copyOf((script.replace("\r", "").split("\n"))));
+                            ((WinRmMachineLocation)machineLocation).executeCommand(script);
                         } else {
                             ((SshMachineLocation)machineLocation).execCommands("Customizing node " + this, ImmutableList.of(script));
                         }
@@ -2664,6 +2664,8 @@ protected LoginCredentials waitForWinRmAvailable(final ComputeService computeSer
                 public Boolean call() {
                     for (Map.Entry<WinRmMachineLocation, LoginCredentials> entry : machinesToTry.entrySet()) {
                         WinRmMachineLocation machine = entry.getKey();
+
+                        // TODO explicitely execute no retry
                         WinRmToolResponse response = machine.executeCommand(
                                 ImmutableMap.of(WinRmTool.PROP_EXEC_TRIES.getName(), 1),
                                 ImmutableList.of("echo testing"));

brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java

@@ -29,16 +29,19 @@
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 
+import com.google.common.collect.ImmutableList;
 import org.apache.brooklyn.api.entity.EntityLocal;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
 import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
 import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.sensor.Sensors;
-import org.apache.brooklyn.entity.software.base.lifecycle.NativeWindowsScriptRunner;
 import org.apache.brooklyn.entity.software.base.lifecycle.WinRmExecuteHelper;
+import org.apache.brooklyn.location.winrm.NaiveWindowsScriptRunner;
 import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.internal.winrm.WinRmScriptTool;
 import org.apache.brooklyn.util.core.internal.winrm.WinRmTool;
 import org.apache.brooklyn.util.core.internal.winrm.WinRmToolResponse;
 import org.apache.brooklyn.util.core.task.Tasks;
@@ -51,11 +54,10 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 
-public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwareProcessDriver implements NativeWindowsScriptRunner {
+public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwareProcessDriver implements NaiveWindowsScriptRunner {
     private static final Logger LOG = LoggerFactory.getLogger(AbstractSoftwareProcessWinRmDriver.class);
 
     AttributeSensor<String> WINDOWS_USERNAME = Sensors.newStringSensor("windows.username",
@@ -175,7 +177,7 @@ public int copyResource(Map<Object, Object> sshFlags, String source, String targ
         if (createParentDir) {
             createDirectory(getDirectory(target), "Creating resource directory");
         }
-        
+
         InputStream stream = null;
         try {
             Tasks.setBlockingDetails("retrieving resource "+source+" for copying across");
@@ -204,15 +206,15 @@ protected void createDirectory(String directoryName, String summaryForLogging) {
     }
 
     @Override
-    public Integer executeNativeOrPsCommand(Map flags, String regularCommand, String powerShellCommand, String phase, Boolean allowNoOp) {
+    public Integer executeNativeOrPsCommand(Map flags, String regularCommand, final String powerShellCommand, String summaryForLogging, Boolean allowNoOp) {
         if (Strings.isBlank(regularCommand) && Strings.isBlank(powerShellCommand)) {
             if (allowNoOp) {
                 return new WinRmToolResponse("", "", 0).getStatusCode();
             } else {
-                throw new IllegalStateException(String.format("Exactly one of cmd or psCmd must be set for %s of %s", phase, entity));
+                throw new IllegalStateException(String.format("Exactly one of cmd or psCmd must be set for %s of %s", summaryForLogging, entity));
             }
         } else if (!Strings.isBlank(regularCommand) && !Strings.isBlank(powerShellCommand)) {
-            throw new IllegalStateException(String.format("%s and %s cannot both be set for %s of %s", regularCommand, powerShellCommand, phase, entity));
+            throw new IllegalStateException(String.format("%s and %s cannot both be set for %s of %s", regularCommand, powerShellCommand, summaryForLogging, entity));
         }
 
         ByteArrayOutputStream stdIn = new ByteArrayOutputStream();
@@ -236,7 +238,7 @@ public Integer executeNativeOrPsCommand(Map flags, String regularCommand, String
 
         WinRmToolResponse response;
         if (Strings.isBlank(regularCommand)) {
-            response = getLocation().executePsScript(ImmutableList.of(powerShellCommand));
+            response = getLocation().executePsScript(powerShellCommand);
         } else {
             response = getLocation().executeCommand(ImmutableList.of(regularCommand));
         }
@@ -249,6 +251,24 @@ public Integer executeNativeOrPsCommand(Map flags, String regularCommand, String
         return response.getStatusCode();
     }
 
+    public int executeNativeOrPsScript(Map flags, List<String> regularCommands, final List<String> powerShellCommands, String summaryForLogging, Boolean allowNoOp) {
+        if ((regularCommands == null || regularCommands.size() == 0) && (powerShellCommands == null || powerShellCommands.size() == 0)) {
+            if (allowNoOp) {
+                return new WinRmToolResponse("", "", 0).getStatusCode();
+            } else {
+                throw new IllegalStateException(String.format("Exactly one of cmd or psCmd must be set for %s", summaryForLogging));
+            }
+        }
+
+        MutableMap<String, Object> scriptProps = MutableMap.<String, Object>of(WinRmScriptTool.PROP_SUMMARY.getName(), summaryForLogging);
+        WinRmScriptTool scriptTool = new WinRmScriptTool(scriptProps, this);
+        if (regularCommands == null || regularCommands.size() == 0) {
+            return scriptTool.execPsScript(flags, powerShellCommands);
+        } else {
+            return scriptTool.execNativeScript(flags, regularCommands);
+        }
+    }
+
     private void writeToStream(ByteArrayOutputStream stream, String string) {
         try {
             stream.write(string.getBytes());
@@ -257,18 +277,30 @@ private void writeToStream(ByteArrayOutputStream stream, String string) {
         }
     }
 
-    public int execute(List<String> script) {
+    public int execute(String script) {
         return getLocation().executeCommand(script).getStatusCode();
     }
 
-    public int executePsScriptNoRetry(List<String> psScript) {
-        return getLocation().executePsScript(ImmutableMap.of(WinRmTool.PROP_EXEC_TRIES, 1), psScript).getStatusCode();
+    public int executePsCommandNoRetry(String psCommand) {
+        return getLocation().executePsScript(ImmutableMap.of(WinRmTool.PROP_EXEC_TRIES, 1), ImmutableList.of(psCommand)).getStatusCode();
+    }
+
+    public int executeCommandNoRetry(String command) {
+        return getLocation().executeCommand(ImmutableMap.of(WinRmTool.PROP_EXEC_TRIES, 1), ImmutableList.of(command)).getStatusCode();
     }
 
     public int executePsScript(List<String> psScript) {
         return getLocation().executePsScript(psScript).getStatusCode();
     }
 
+    public WinRmToolResponse executeCommand(String cmdCommand) {
+        return getLocation().executeCommand(cmdCommand);
+    }
+
+    public WinRmToolResponse executePsScript(String psCommand) {
+        return getLocation().executePsScript(psCommand);
+    }
+
     public int copyTo(File source, String destination) {
         return getLocation().copyTo(source, destination);
     }
@@ -278,12 +310,7 @@ public int copyTo(InputStream source, String destination) {
     }
 
     public void rebootAndWait() {
-        try {
-            executePsScriptNoRetry(ImmutableList.of("Restart-Computer -Force"));
-        } catch (Exception e) {
-            // Restarting the computer will cause the command to fail; ignore the exception and continue
-            Exceptions.propagateIfFatal(e);
-        }
+        executePsCommandNoRetry("Restart-Computer -Force");
         waitForWinRmStatus(false, entity.getConfig(VanillaWindowsProcess.REBOOT_BEGUN_TIMEOUT));
         waitForWinRmStatus(true, entity.getConfig(VanillaWindowsProcess.REBOOT_COMPLETED_TIMEOUT)).getWithError();
     }
@@ -298,7 +325,7 @@ private ReferenceWithError<Boolean> waitForWinRmStatus(final boolean requiredSta
             @Override
             public Boolean call() throws Exception {
                 try {
-                    return (execute(ImmutableList.of("hostname")) == 0) == requiredStatus;
+                    return (execute("hostname") == 0) == requiredStatus;
                 } catch (Exception e) {
                     return !requiredStatus;
                 }

brooklyn-server/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/WinRmExecuteHelper.java

@@ -32,6 +32,7 @@
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.mgmt.TaskQueueingContext;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
+import org.apache.brooklyn.location.winrm.NaiveWindowsScriptRunner;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
 import org.apache.brooklyn.util.core.task.TaskBuilder;
 import org.apache.brooklyn.util.core.task.Tasks;
@@ -51,7 +52,7 @@ public class WinRmExecuteHelper {
 
     private Task<Integer> task;
 
-    protected final NativeWindowsScriptRunner runner;
+    protected final NaiveWindowsScriptRunner runner;
     public final String summary;
 
     private String command;
@@ -62,7 +63,7 @@ public class WinRmExecuteHelper {
     protected Predicate<? super Integer> resultCodeCheck = Predicates.alwaysTrue();
     protected ByteArrayOutputStream stdout, stderr, stdin;
 
-    public WinRmExecuteHelper(NativeWindowsScriptRunner runner, String summary) {
+    public WinRmExecuteHelper(NaiveWindowsScriptRunner runner, String summary) {
         this.runner = runner;
         this.summary = summary;
     }

brooklyn-server/software/base/src/test/java/org/apache/brooklyn/entity/software/base/VanillaWindowsProcessWinrmExitStatusLiveTest.java

@@ -34,6 +34,7 @@
 import org.apache.brooklyn.entity.software.base.test.location.WindowsTestFixture;
 import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
 import org.apache.brooklyn.test.EntityTestUtils;
+import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -147,6 +148,23 @@ public void testExecPsWithZeroExitCodes() {
         EntityTestUtils.assertAttributeEqualsEventually(entity, Attributes.SERVICE_UP, false);
     }
 
+    @Test(groups = "Live", expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "Execution failed, invalid result 1.*")
+    public void testExecPsWithExitCodesWithMultiLineCommands() throws Throwable {
+        try {
+            VanillaWindowsProcess entity = app.createAndManageChild(EntitySpec.create(VanillaWindowsProcess.class)
+                    .configure(
+                            VanillaWindowsProcess.INSTALL_POWERSHELL_COMMAND,
+                        "exit 1")
+                    .configure(VanillaWindowsProcess.LAUNCH_POWERSHELL_COMMAND, "Write-Host launch")
+                    .configure(VanillaWindowsProcess.CHECK_RUNNING_POWERSHELL_COMMAND, "Write-Host checkrunning")
+                    .configure(VanillaWindowsProcess.STOP_POWERSHELL_COMMAND, "Write-Host stop"));
+
+            app.start(ImmutableList.of(machine));
+        } catch (Exception e) {
+            throw Exceptions.getFirstInteresting(e);
+        }
+    }
+
     @Test(groups = "Live")
     public void testPreInstallNonZeroExitCode() {
         runExecNonZeroExitCode("pre-install-command");