You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2019/08/30 15:23:53 UTC

[brooklyn-server] branch master updated (9308bf1 -> 0cb5402)

This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git.


    from 9308bf1  This closes #1062
     new 0964eee  First draft supporting shell.env for WinRm
     new 8efb1c9  set env vars on winrm executions, except for installing
     new 9361fa6  create and use install/run dirs on windows
     new 0cb5402  This closes #1064

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../camp/brooklyn/AbstractWindowsYamlTest.java     |   5 +
 .../camp/brooklyn/WindowsYamlLiveTest.java         |  91 ++++++++++----
 .../brooklyn/location/ssh/CanResolveOnBoxDir.java  |   9 +-
 .../brooklyn/location/ssh/SshMachineLocation.java  |  23 +++-
 .../base/AbstractSoftwareProcessDriver.java        | 134 +++++++++++++++++++--
 .../base/AbstractSoftwareProcessSshDriver.java     | 126 ++-----------------
 .../base/AbstractSoftwareProcessWinRmDriver.java   |  80 ++++++------
 .../software/base/VanillaWindowsProcess.java       |  41 +++++++
 .../base/VanillaWindowsProcessWinRmDriver.java     |  11 +-
 .../lifecycle/MachineLifecycleEffectorTasks.java   |  23 +---
 .../base/lifecycle/WinRmExecuteHelper.java         |   7 ++
 .../base/test/location/WindowsTestFixture.java     |  28 +++--
 .../location/winrm/WinRmMachineLocation.java       |  17 ++-
 .../util/core/internal/winrm/WinRmTool.java        |   3 +
 .../core/internal/winrm/winrm4j/Winrm4jTool.java   |   8 +-
 15 files changed, 386 insertions(+), 220 deletions(-)
 copy rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/HasConfig.java => core/src/main/java/org/apache/brooklyn/location/ssh/CanResolveOnBoxDir.java (80%)


[brooklyn-server] 03/04: create and use install/run dirs on windows

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 9361fa64a999490b9b67fe748f3a25dab8317732
Author: Alex Heneveld <al...@cloudsoftcorp.com>
AuthorDate: Tue Aug 27 00:33:00 2019 +0200

    create and use install/run dirs on windows
---
 .../camp/brooklyn/WindowsYamlLiveTest.java         |  35 +++---
 .../brooklyn/location/ssh/CanResolveOnBoxDir.java  |  27 +++++
 .../brooklyn/location/ssh/SshMachineLocation.java  |  23 +++-
 .../base/AbstractSoftwareProcessDriver.java        | 123 +++++++++++++++++++--
 .../base/AbstractSoftwareProcessSshDriver.java     | 100 -----------------
 .../base/AbstractSoftwareProcessWinRmDriver.java   |  26 ++---
 .../software/base/VanillaWindowsProcess.java       |  41 +++++++
 .../lifecycle/MachineLifecycleEffectorTasks.java   |  23 +---
 .../location/winrm/WinRmMachineLocation.java       |  17 ++-
 .../core/internal/winrm/winrm4j/Winrm4jTool.java   |   8 +-
 10 files changed, 257 insertions(+), 166 deletions(-)

diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java
index 6e1a0a1..974f9f6 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java
@@ -38,6 +38,7 @@ import org.apache.brooklyn.entity.software.base.test.location.WindowsTestFixture
 import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
 import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.util.core.task.TaskPredicates;
+import org.apache.brooklyn.util.text.StringEscapes.BashStringEscapes;
 import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
 import org.apache.brooklyn.util.text.StringPredicates;
 import org.apache.brooklyn.util.text.Strings;
@@ -151,13 +152,13 @@ public class WindowsYamlLiveTest extends AbstractWindowsYamlTest {
     public void testPowershellMinimalist() throws Exception {
         Map<String, String> cmds = ImmutableMap.<String, String>builder()
                 .put("myarg", "myval")
-                .put("launch.powershell.command", "\"& c:\\\\exit0.ps1\"")
-                .put("checkRunning.powershell.command", "\"& c:\\\\exit0.bat\"")
+                .put("launch.powershell.command", JavaStringEscapes.wrapJavaString("& \"$Env:INSTALL_DIR\\exit0.ps1\""))
+                .put("checkRunning.powershell.command", JavaStringEscapes.wrapJavaString("& \"$Env:INSTALL_DIR\\exit0.bat\""))
                 .build();
         
         Map<String, List<String>> stdouts = ImmutableMap.of();
         
-        runWindowsApp(cmds, stdouts, null);
+        runWindowsApp(cmds, stdouts, true, null);
     }
     
     @Test(groups="Live")
@@ -182,7 +183,7 @@ public class WindowsYamlLiveTest extends AbstractWindowsYamlTest {
                 .put("winrm: pre-launch-command.*", ImmutableList.of("myval"))
                 .build();
         
-        runWindowsApp(cmds, stdouts, null);
+        runWindowsApp(cmds, stdouts, false, null);
     }
     
     @Test(groups="Live")
@@ -207,7 +208,7 @@ public class WindowsYamlLiveTest extends AbstractWindowsYamlTest {
                 .put("winrm: pre-launch-command.*", ImmutableList.of("myval"))
                 .build();
         
-        runWindowsApp(cmds, stdouts, null);
+        runWindowsApp(cmds, stdouts, false, null);
     }
     
     @Test(groups="Live")
@@ -227,7 +228,7 @@ public class WindowsYamlLiveTest extends AbstractWindowsYamlTest {
         
         Map<String, List<String>> stdouts = ImmutableMap.of();
         
-        runWindowsApp(cmds, stdouts, "winrm: pre-install-command.*");
+        runWindowsApp(cmds, stdouts, false, "winrm: pre-install-command.*");
     }
     
     // FIXME Failing to match the expected exception, but looks fine! Needs more investigation.
@@ -248,7 +249,7 @@ public class WindowsYamlLiveTest extends AbstractWindowsYamlTest {
         
         Map<String, List<String>> stdouts = ImmutableMap.of();
         
-        runWindowsApp(cmds, stdouts, "winrm: is-running-command.*");
+        runWindowsApp(cmds, stdouts, false, "winrm: is-running-command.*");
     }
     
     // FIXME Needs more work to get the stop's task that failed, so can assert got the right error message
@@ -269,29 +270,29 @@ public class WindowsYamlLiveTest extends AbstractWindowsYamlTest {
         
         Map<String, List<String>> stdouts = ImmutableMap.of();
         
-        runWindowsApp(cmds, stdouts, "winrm: stop-command.*");
+        runWindowsApp(cmds, stdouts, false, "winrm: stop-command.*");
     }
     
-    protected void runWindowsApp(Map<String, String> commands, Map<String, List<String>> stdouts, String taskRegexFailed) throws Exception {
+    protected void runWindowsApp(Map<String, String> commands, Map<String, List<String>> stdouts, boolean useInstallDir, String taskRegexFailed) throws Exception {
         String cmdFailed = (taskRegexFailed == null) ? null : TASK_REGEX_TO_COMMAND.get(taskRegexFailed);
         
         List<String> yaml = Lists.newArrayList();
         yaml.addAll(yamlLocation);
+        String prefix = useInstallDir ? "" : "c:\\";
         yaml.addAll(ImmutableList.of(
                 "services:",
                 "- type: org.apache.brooklyn.entity.software.base.VanillaWindowsProcess",
                 "  brooklyn.config:",
-                "    onbox.base.dir.skipResolution: true",
                 "    templates.preinstall:",
                 "      classpath://org/apache/brooklyn/camp/brooklyn/echoFreemarkerMyarg.bat: c:\\echoFreemarkerMyarg.bat",
                 "      classpath://org/apache/brooklyn/camp/brooklyn/echoFreemarkerMyarg.ps1: c:\\echoFreemarkerMyarg.ps1",
                 "    files.preinstall:",
-                "      classpath://org/apache/brooklyn/camp/brooklyn/echoArg.bat: c:\\echoArg.bat",
-                "      classpath://org/apache/brooklyn/camp/brooklyn/echoMyArg.ps1: c:\\echoMyArg.ps1",
-                "      classpath://org/apache/brooklyn/camp/brooklyn/exit0.bat: c:\\exit0.bat",
-                "      classpath://org/apache/brooklyn/camp/brooklyn/exit1.bat: c:\\exit1.bat",
-                "      classpath://org/apache/brooklyn/camp/brooklyn/exit0.ps1: c:\\exit0.ps1",
-                "      classpath://org/apache/brooklyn/camp/brooklyn/exit1.ps1: c:\\exit1.ps1",
+                "      classpath://org/apache/brooklyn/camp/brooklyn/echoArg.bat: "+prefix+"echoArg.bat",
+                "      classpath://org/apache/brooklyn/camp/brooklyn/echoMyArg.ps1: "+prefix+"echoMyArg.ps1",
+                "      classpath://org/apache/brooklyn/camp/brooklyn/exit0.bat: "+prefix+"exit0.bat",
+                "      classpath://org/apache/brooklyn/camp/brooklyn/exit1.bat: "+prefix+"exit1.bat",
+                "      classpath://org/apache/brooklyn/camp/brooklyn/exit0.ps1: "+prefix+"exit0.ps1",
+                "      classpath://org/apache/brooklyn/camp/brooklyn/exit1.ps1: "+prefix+"exit1.ps1",
                 ""));
         
         for (Map.Entry<String, String> entry : commands.entrySet()) {
@@ -363,7 +364,7 @@ public class WindowsYamlLiveTest extends AbstractWindowsYamlTest {
     
     private void assertPhaseStreamEquals(Entity entity, String phase, String stream, Predicate<String> check) {
         Optional<Task<?>> t = findTaskOrSubTask(entity, TaskPredicates.displayNameSatisfies(StringPredicates.startsWith("winrm: "+phase)));
-        Asserts.assertThat(BrooklynTaskTags.stream(t.get(), stream).getStreamContentsAbbreviated().trim(), check);
+        Asserts.assertThat(BrooklynTaskTags.stream(t.get(), stream).streamContents.get().trim(), check);
     }
 
     @Override
diff --git a/core/src/main/java/org/apache/brooklyn/location/ssh/CanResolveOnBoxDir.java b/core/src/main/java/org/apache/brooklyn/location/ssh/CanResolveOnBoxDir.java
new file mode 100644
index 0000000..a53a319
--- /dev/null
+++ b/core/src/main/java/org/apache/brooklyn/location/ssh/CanResolveOnBoxDir.java
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.location.ssh;
+
+import org.apache.brooklyn.api.entity.Entity;
+
+public interface CanResolveOnBoxDir {
+
+    String resolveOnBoxDirFor(Entity entity, String unresolvedPath);
+
+}
diff --git a/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java b/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
index 909cc2b..ada35d0 100644
--- a/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
+++ b/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
@@ -44,6 +44,7 @@ import java.util.concurrent.TimeUnit;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.location.MachineDetails;
 import org.apache.brooklyn.api.location.MachineLocation;
 import org.apache.brooklyn.api.location.PortRange;
@@ -57,6 +58,7 @@ import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.config.ConfigUtils;
 import org.apache.brooklyn.core.config.MapConfigKey;
 import org.apache.brooklyn.core.config.Sanitizer;
+import org.apache.brooklyn.core.effector.ssh.SshEffectorTasks;
 import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
 import org.apache.brooklyn.core.location.AbstractMachineLocation;
 import org.apache.brooklyn.core.location.BasicMachineDetails;
@@ -80,8 +82,10 @@ import org.apache.brooklyn.util.core.internal.ssh.SshException;
 import org.apache.brooklyn.util.core.internal.ssh.SshTool;
 import org.apache.brooklyn.util.core.internal.ssh.sshj.SshjTool;
 import org.apache.brooklyn.util.core.mutex.WithMutexes;
+import org.apache.brooklyn.util.core.task.DynamicTasks;
 import org.apache.brooklyn.util.core.task.ScheduledTask;
 import org.apache.brooklyn.util.core.task.Tasks;
+import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
 import org.apache.brooklyn.util.core.task.system.internal.ExecWithLoggingHelpers;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.KeyTransformingLoadingCache.KeyTransformingSameTypeLoadingCache;
@@ -128,7 +132,7 @@ import com.google.common.reflect.TypeToken;
  * Additionally there are routines to copyTo, copyFrom; and installTo (which tries a curl, and falls back to copyTo
  * in event the source is accessible by the caller only).
  */
-public class SshMachineLocation extends AbstractMachineLocation implements MachineLocation, PortSupplier, WithMutexes, Closeable {
+public class SshMachineLocation extends AbstractMachineLocation implements MachineLocation, PortSupplier, WithMutexes, Closeable, CanResolveOnBoxDir {
 
     private static final Logger LOG = LoggerFactory.getLogger(SshMachineLocation.class);
     private static final Logger logSsh = LoggerFactory.getLogger(BrooklynLogging.SSH_IO);
@@ -1040,4 +1044,21 @@ public class SshMachineLocation extends AbstractMachineLocation implements Machi
         return mutexes().hasMutex(mutexId);
     }
 
+    @Override
+    public String resolveOnBoxDirFor(Entity entity, String unresolvedPath) {
+        ProcessTaskWrapper<Integer> baseTask = SshEffectorTasks.ssh(
+            BashCommands.alternatives("mkdir -p \"${BASE_DIR}\"",
+                BashCommands.chain(
+                    BashCommands.sudo("mkdir -p \"${BASE_DIR}\""),
+                    BashCommands.sudo("chown "+getUser()+" \"${BASE_DIR}\""))),
+            "cd ~",
+            "cd ${BASE_DIR}",
+            "echo BASE_DIR_RESULT':'`pwd`:BASE_DIR_RESULT")
+            .environmentVariable("BASE_DIR", unresolvedPath)
+            .requiringExitCodeZero()
+            .summary("initializing on-box base dir "+unresolvedPath).newTask();
+        DynamicTasks.queueIfPossible(baseTask).orSubmitAsync(entity);
+        return Strings.getFragmentBetween(baseTask.block().getStdout(), "BASE_DIR_RESULT:", ":BASE_DIR_RESULT");
+    }
+
 }
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java
index b7748df..f945aeb 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java
@@ -47,6 +47,7 @@ import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
 import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.entity.lifecycle.Lifecycle;
 import org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic;
+import org.apache.brooklyn.core.feed.ConfigToAttributes;
 import org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks;
 import org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks.CloseableLatch;
 import org.apache.brooklyn.util.collections.MutableMap;
@@ -58,6 +59,7 @@ import org.apache.brooklyn.util.core.text.TemplateProcessor;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.os.Os;
 import org.apache.brooklyn.util.stream.ReaderInputStream;
+import org.apache.brooklyn.util.text.Strings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -77,6 +79,13 @@ public abstract class AbstractSoftwareProcessDriver implements SoftwareProcessDr
     protected final ResourceUtils resource;
     protected final Location location;
 
+    // we cache these for efficiency and in case the entity becomes unmanaged
+    private volatile String installDir;
+    private volatile String runDir;
+    private volatile String expandedInstallDir;
+
+    private final Object installDirSetupMutex = new Object();
+
     public AbstractSoftwareProcessDriver(EntityLocal entity, Location location) {
         this.entity = checkNotNull(entity, "entity");
         this.location = checkNotNull(location, "location");
@@ -441,6 +450,10 @@ public abstract class AbstractSoftwareProcessDriver implements SoftwareProcessDr
         }
     }
 
+    protected String mergePaths(String ...s) {
+        return Os.mergePathsUnix(s);
+    }
+    
     private void applyFnToResourcesAppendToList(
             Map<String, String> resources, final Function<SourceAndDestination, Task<?>> function,
             String destinationParentDir, final List<TaskAdaptable<?>> tasks) {
@@ -448,7 +461,7 @@ public abstract class AbstractSoftwareProcessDriver implements SoftwareProcessDr
         for (Map.Entry<String, String> entry : resources.entrySet()) {
             final String source = checkNotNull(entry.getKey(), "Missing source for resource");
             String target = checkNotNull(entry.getValue(), "Missing destination for resource");
-            final String destination = Os.isAbsolutish(target) ? target : Os.mergePathsUnix(destinationParentDir, target);
+            final String destination = Os.isAbsolutish(target) ? target : mergePaths(destinationParentDir, target);
 
             // if source is a directory then copy all files underneath.
             // e.g. /tmp/a/{b,c/d}, source = /tmp/a, destination = dir/a/b and dir/a/c/d.
@@ -463,7 +476,7 @@ public abstract class AbstractSoftwareProcessDriver implements SoftwareProcessDr
                         public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
                             if (attrs.isRegularFile()) {
                                 Path relativePath = file.subpath(startElements, file.getNameCount());
-                                tasks.add(function.apply(new SourceAndDestination(file.toString(), Os.mergePathsUnix(destination, relativePath.toString()))));
+                                tasks.add(function.apply(new SourceAndDestination(file.toString(), mergePaths(destination, relativePath.toString()))));
                             }
                             return FileVisitResult.CONTINUE;
                         }
@@ -562,18 +575,19 @@ public abstract class AbstractSoftwareProcessDriver implements SoftwareProcessDr
     }
 
     /**
-     * @param template URI of file to template and copy, e.g. file://.., http://.., classpath://..
+     * @param templateUrl URI of file to template and copy, e.g. file://.., http://.., classpath://..
      * @param target Destination on server.
      * @param extraSubstitutions Extra substitutions for the templater to use, for example
      *               "foo" -> "bar", and in a template ${foo}.
      * @return The exit code of the SSH command run.
      */
-    public int copyTemplate(String template, String target, boolean createParent, Map<String, ?> extraSubstitutions) {
-        String data = processTemplate(template, extraSubstitutions);
+    public int copyTemplate(String templateUrl, String target, boolean createParent, Map<String, ?> extraSubstitutions) {
+        log.debug("Processing template "+templateUrl+" and copying to "+target+" on "+getLocation()+" for "+getEntity());
+        String data = processTemplate(templateUrl, extraSubstitutions);
         return copyResource(MutableMap.<Object,Object>of(), new StringReader(data), target, createParent);
     }
 
-    public abstract int copyResource(Map<Object,Object> sshFlags, String source, String target, boolean createParentDir);
+    public abstract int copyResource(Map<Object,Object> sshFlags, String sourceUrl, String target, boolean createParentDir);
 
     public abstract int copyResource(Map<Object,Object> sshFlags, InputStream source, String target, boolean createParentDir);
 
@@ -595,8 +609,8 @@ public abstract class AbstractSoftwareProcessDriver implements SoftwareProcessDr
         return copyResource(MutableMap.of(), resource, target);
     }
 
-    public int copyResource(String resource, String target, boolean createParentDir) {
-        return copyResource(MutableMap.of(), resource, target, createParentDir);
+    public int copyResource(String resourceUrl, String target, boolean createParentDir) {
+        return copyResource(MutableMap.of(), resourceUrl, target, createParentDir);
     }
 
     @SuppressWarnings({ "rawtypes", "unchecked" })
@@ -677,6 +691,95 @@ public abstract class AbstractSoftwareProcessDriver implements SoftwareProcessDr
         return envSerializer.serialize(env);
     }
 
-    public abstract String getRunDir();
-    public abstract String getInstallDir();
+
+    protected void setInstallDir(String installDir) {
+        this.installDir = installDir;
+        entity.sensors().set(SoftwareProcess.INSTALL_DIR, installDir);
+    }
+
+    public String getInstallDir() {
+        if (installDir != null) return installDir;
+
+        String existingVal = getEntity().getAttribute(SoftwareProcess.INSTALL_DIR);
+        if (Strings.isNonBlank(existingVal)) { // e.g. on rebind
+            installDir = existingVal;
+            return installDir;
+        }
+
+        synchronized (installDirSetupMutex) {
+            // previously we looked at sensor value, but we shouldn't as it might have been converted from the config key value
+            // *before* we computed the install label, or that label may have changed since previous install; now force a recompute
+            setInstallLabel();
+
+            // set it null first so that we force a recompute
+            setInstallDir(null);
+            setInstallDir(Os.tidyPath(ConfigToAttributes.apply(getEntity(), SoftwareProcess.INSTALL_DIR)));
+            return installDir;
+        }
+    }
+
+    protected void setInstallLabel() {
+        if (((EntityInternal)getEntity()).config().getLocalRaw(SoftwareProcess.INSTALL_UNIQUE_LABEL).isPresentAndNonNull()) return;
+        getEntity().config().set(SoftwareProcess.INSTALL_UNIQUE_LABEL,
+            getEntity().getEntityType().getSimpleName()+
+            (Strings.isNonBlank(getVersion()) ? "_"+getVersion() : "")+
+            (Strings.isNonBlank(getInstallLabelExtraSalt()) ? "_"+getInstallLabelExtraSalt() : "") );
+    }
+
+    /** allows subclasses to return extra salt (ie unique hash)
+     * for cases where install dirs need to be distinct e.g. based on extra plugins being placed in the install dir;
+     * {@link #setInstallLabel()} uses entity-type simple name and version already
+     * <p>
+     * this salt should not be too long and must not contain invalid path chars.
+     * a hash code of other relevant info is not a bad choice.
+     **/
+    protected String getInstallLabelExtraSalt() {
+        return null;
+    }
+
+    protected void setRunDir(String runDir) {
+        this.runDir = runDir;
+        entity.sensors().set(SoftwareProcess.RUN_DIR, runDir);
+    }
+
+    public String getRunDir() {
+        if (runDir != null) return runDir;
+
+        String existingVal = getEntity().getAttribute(SoftwareProcess.RUN_DIR);
+        if (Strings.isNonBlank(existingVal)) { // e.g. on rebind
+            runDir = existingVal;
+            return runDir;
+        }
+
+        setRunDir(Os.tidyPath(ConfigToAttributes.apply(getEntity(), SoftwareProcess.RUN_DIR)));
+        return runDir;
+    }
+
+    public void setExpandedInstallDir(String val) {
+        String oldVal = getEntity().getAttribute(SoftwareProcess.EXPANDED_INSTALL_DIR);
+        if (Strings.isNonBlank(oldVal) && !oldVal.equals(val)) {
+            log.info("Resetting expandedInstallDir (to "+val+" from "+oldVal+") for "+getEntity());
+        }
+
+        expandedInstallDir = val;
+        getEntity().sensors().set(SoftwareProcess.EXPANDED_INSTALL_DIR, val);
+    }
+
+    public String getExpandedInstallDir() {
+        if (expandedInstallDir != null) return expandedInstallDir;
+
+        String existingVal = getEntity().getAttribute(SoftwareProcess.EXPANDED_INSTALL_DIR);
+        if (Strings.isNonBlank(existingVal)) { // e.g. on rebind
+            expandedInstallDir = existingVal;
+            return expandedInstallDir;
+        }
+
+        String untidiedVal = ConfigToAttributes.apply(getEntity(), SoftwareProcess.EXPANDED_INSTALL_DIR);
+        if (Strings.isNonBlank(untidiedVal)) {
+            setExpandedInstallDir(Os.tidyPath(untidiedVal));
+            return expandedInstallDir;
+        } else {
+            throw new IllegalStateException("expandedInstallDir is null; most likely install was not called for "+getEntity());
+        }
+    }
 }
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
index 9ff370c..376577b 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
@@ -81,13 +81,6 @@ public abstract class AbstractSoftwareProcessSshDriver extends AbstractSoftwareP
     public static final Logger log = LoggerFactory.getLogger(AbstractSoftwareProcessSshDriver.class);
     public static final Logger logSsh = LoggerFactory.getLogger(BrooklynLogging.SSH_IO);
 
-    // we cache these for efficiency and in case the entity becomes unmanaged
-    private volatile String installDir;
-    private volatile String runDir;
-    private volatile String expandedInstallDir;
-
-    private final Object installDirSetupMutex = new Object();
-
     protected volatile DownloadResolver resolver;
 
     @Override
@@ -130,99 +123,6 @@ public abstract class AbstractSoftwareProcessSshDriver extends AbstractSoftwareP
         return (SshMachineLocation) super.getLocation();
     }
 
-    protected void setInstallDir(String installDir) {
-        this.installDir = installDir;
-        entity.sensors().set(SoftwareProcess.INSTALL_DIR, installDir);
-    }
-
-    @Override
-    public String getInstallDir() {
-        if (installDir != null) return installDir;
-
-        String existingVal = getEntity().getAttribute(SoftwareProcess.INSTALL_DIR);
-        if (Strings.isNonBlank(existingVal)) { // e.g. on rebind
-            installDir = existingVal;
-            return installDir;
-        }
-
-        synchronized (installDirSetupMutex) {
-            // previously we looked at sensor value, but we shouldn't as it might have been converted from the config key value
-            // *before* we computed the install label, or that label may have changed since previous install; now force a recompute
-            setInstallLabel();
-
-            // set it null first so that we force a recompute
-            setInstallDir(null);
-            setInstallDir(Os.tidyPath(ConfigToAttributes.apply(getEntity(), SoftwareProcess.INSTALL_DIR)));
-            return installDir;
-        }
-    }
-
-    protected void setInstallLabel() {
-        if (((EntityInternal)getEntity()).config().getLocalRaw(SoftwareProcess.INSTALL_UNIQUE_LABEL).isPresentAndNonNull()) return;
-        getEntity().config().set(SoftwareProcess.INSTALL_UNIQUE_LABEL,
-            getEntity().getEntityType().getSimpleName()+
-            (Strings.isNonBlank(getVersion()) ? "_"+getVersion() : "")+
-            (Strings.isNonBlank(getInstallLabelExtraSalt()) ? "_"+getInstallLabelExtraSalt() : "") );
-    }
-
-    /** allows subclasses to return extra salt (ie unique hash)
-     * for cases where install dirs need to be distinct e.g. based on extra plugins being placed in the install dir;
-     * {@link #setInstallLabel()} uses entity-type simple name and version already
-     * <p>
-     * this salt should not be too long and must not contain invalid path chars.
-     * a hash code of other relevant info is not a bad choice.
-     **/
-    protected String getInstallLabelExtraSalt() {
-        return null;
-    }
-
-    protected void setRunDir(String runDir) {
-        this.runDir = runDir;
-        entity.sensors().set(SoftwareProcess.RUN_DIR, runDir);
-    }
-
-    @Override
-    public String getRunDir() {
-        if (runDir != null) return runDir;
-
-        String existingVal = getEntity().getAttribute(SoftwareProcess.RUN_DIR);
-        if (Strings.isNonBlank(existingVal)) { // e.g. on rebind
-            runDir = existingVal;
-            return runDir;
-        }
-
-        setRunDir(Os.tidyPath(ConfigToAttributes.apply(getEntity(), SoftwareProcess.RUN_DIR)));
-        return runDir;
-    }
-
-    public void setExpandedInstallDir(String val) {
-        String oldVal = getEntity().getAttribute(SoftwareProcess.EXPANDED_INSTALL_DIR);
-        if (Strings.isNonBlank(oldVal) && !oldVal.equals(val)) {
-            log.info("Resetting expandedInstallDir (to "+val+" from "+oldVal+") for "+getEntity());
-        }
-
-        expandedInstallDir = val;
-        getEntity().sensors().set(SoftwareProcess.EXPANDED_INSTALL_DIR, val);
-    }
-
-    public String getExpandedInstallDir() {
-        if (expandedInstallDir != null) return expandedInstallDir;
-
-        String existingVal = getEntity().getAttribute(SoftwareProcess.EXPANDED_INSTALL_DIR);
-        if (Strings.isNonBlank(existingVal)) { // e.g. on rebind
-            expandedInstallDir = existingVal;
-            return expandedInstallDir;
-        }
-
-        String untidiedVal = ConfigToAttributes.apply(getEntity(), SoftwareProcess.EXPANDED_INSTALL_DIR);
-        if (Strings.isNonBlank(untidiedVal)) {
-            setExpandedInstallDir(Os.tidyPath(untidiedVal));
-            return expandedInstallDir;
-        } else {
-            throw new IllegalStateException("expandedInstallDir is null; most likely install was not called for "+getEntity());
-        }
-    }
-
     public SshMachineLocation getMachine() { return getLocation(); }
     public String getHostname() { return entity.getAttribute(Attributes.HOSTNAME); }
     public String getAddress() { return entity.getAttribute(Attributes.ADDRESS); }
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java
index 0303313..eca10a1 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java
@@ -58,7 +58,6 @@ 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 {
     private static final Logger LOG = LoggerFactory.getLogger(AbstractSoftwareProcessWinRmDriver.class);
@@ -74,6 +73,10 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
         entity.sensors().set(WINDOWS_PASSWORD, location.config().get(WinRmMachineLocation.PASSWORD));
     }
 
+    protected String mergePaths(String ...s) {
+        return super.mergePaths(s).replaceAll("/", "\\\\");
+    }
+
     protected WinRmExecuteHelper newScript(String command, String psCommand, String phase, String taskNamePrefix) {
         return newScript(command, psCommand, phase, taskNamePrefix, null);
     }
@@ -284,28 +287,17 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
     }
 
     @Override
-    public String getRunDir() {
-        // TODO: This needs to be tidied, and read from the appropriate flags (if set)
-        return "$HOME\\brooklyn-managed-processes\\apps\\" + entity.getApplicationId() + "\\entities\\" + getEntityVersionLabel()+"_"+entity.getId();
-    }
-
-    @Override
-    public String getInstallDir() {
-        // TODO: This needs to be tidied, and read from the appropriate flags (if set)
-        return "$HOME\\brooklyn-managed-processes\\installs\\" + entity.getApplicationId() + "\\" + getEntityVersionLabel()+"_"+entity.getId();
-    }
-
-    @Override
-    public int copyResource(Map<Object, Object> sshFlags, String source, String target, boolean createParentDir) {
+    public int copyResource(Map<Object, Object> sshFlags, String sourceUrl, String target, boolean createParentDir) {
         if (createParentDir) {
             createDirectory(getDirectory(target), "Creating resource directory");
         }
 
         InputStream stream = null;
         try {
-            Tasks.setBlockingDetails("retrieving resource "+source+" for copying across");
-            stream = resource.getResourceFromUrl(source);
-            Tasks.setBlockingDetails("copying resource "+source+" to server");
+            Tasks.setBlockingDetails("retrieving resource "+sourceUrl+" for copying across");
+            stream = resource.getResourceFromUrl(sourceUrl);
+            Tasks.setBlockingDetails("copying resource "+sourceUrl+" to server");
+            LOG.debug("Copying "+sourceUrl+" to "+target+" on "+getLocation()+" for "+getEntity());
             return copyTo(stream, target);
         } catch (Exception e) {
             throw Exceptions.propagate(e);
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaWindowsProcess.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaWindowsProcess.java
index e772f24..7879d58 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaWindowsProcess.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaWindowsProcess.java
@@ -30,12 +30,53 @@ import org.apache.brooklyn.api.sensor.AttributeSensor;
 import org.apache.brooklyn.config.ConfigInheritance;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
+import org.apache.brooklyn.core.sensor.AttributeSensorAndConfigKey;
 import org.apache.brooklyn.core.sensor.Sensors;
+import org.apache.brooklyn.core.sensor.TemplatedStringAttributeSensorAndConfigKey;
+import org.apache.brooklyn.util.core.flags.SetFromFlag;
 import org.apache.brooklyn.util.time.Duration;
 
 @Catalog(name="Vanilla Windows Process", description="A basic Windows entity configured with scripts, e.g. for launch, check-running and stop")
 @ImplementedBy(VanillaWindowsProcessImpl.class)
 public interface VanillaWindowsProcess extends AbstractVanillaProcess {
+
+    @SetFromFlag("installDir")
+    AttributeSensorAndConfigKey<String,String> INSTALL_DIR = new TemplatedStringAttributeSensorAndConfigKey(
+        "install.dir", 
+        "Directory in which this software will be installed (if downloading/unpacking artifacts explicitly); uses FreeMarker templating format",
+        "${" +
+        "config['"+BrooklynConfigKeys.ONBOX_BASE_DIR.getName()+"']!" +
+        "config['"+BrooklynConfigKeys.BROOKLYN_DATA_DIR.getName()+"']!" +
+        "'ERROR-ONBOX_BASE_DIR-not-set'" +
+        "}" +
+        "\\" +
+        "installs\\" +
+        // the  var??  tests if it exists, passing value to ?string(if_present,if_absent)
+        // the ! provides a default value afterwards, which is never used, but is required for parsing
+        // when the config key is not available;
+        // thus the below prefers the install.unique_label, but falls back to simple name
+        // plus a version identifier *if* the version is explicitly set
+        "${(config['install.unique_label']??)?string(config['install.unique_label']!'X'," +
+        "(entity.entityType.simpleName)+" +
+        "((config['install.version']??)?string('_'+(config['install.version']!'X'),''))" +
+        ")}");
+
+    @SetFromFlag("runDir")
+    AttributeSensorAndConfigKey<String,String> RUN_DIR = new TemplatedStringAttributeSensorAndConfigKey(
+        "run.dir", 
+        "Directory from which this software to be run; uses FreeMarker templating format",
+        "${" +
+        "config['"+BrooklynConfigKeys.ONBOX_BASE_DIR.getName()+"']!" +
+        "config['"+BrooklynConfigKeys.BROOKLYN_DATA_DIR.getName()+"']!" +
+        "'ERROR-ONBOX_BASE_DIR-not-set'" +
+        "}" +
+        "\\" +
+        "apps\\${entity.applicationId}\\" +
+        "entities\\${entity.entityType.simpleName}_" +
+        "${entity.id}");
+
+    
     // 3389 is RDP; 5985 is WinRM (3389 isn't used by Brooklyn, but useful for the end-user subsequently)
     ConfigKey<Collection<Integer>> REQUIRED_OPEN_LOGIN_PORTS = ConfigKeys.newConfigKeyWithDefault(
             SoftwareProcess.REQUIRED_OPEN_LOGIN_PORTS,
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
index 26c9ac4..721c3a0 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
@@ -44,7 +44,6 @@ import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.config.Sanitizer;
 import org.apache.brooklyn.core.effector.EffectorBody;
 import org.apache.brooklyn.core.effector.Effectors;
-import org.apache.brooklyn.core.effector.ssh.SshEffectorTasks;
 import org.apache.brooklyn.core.entity.Attributes;
 import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
 import org.apache.brooklyn.core.entity.Entities;
@@ -74,6 +73,7 @@ import org.apache.brooklyn.entity.software.base.SoftwareProcess.StopSoftwarePara
 import org.apache.brooklyn.entity.software.base.SoftwareProcess.StopSoftwareParameters.StopMode;
 import org.apache.brooklyn.entity.stock.EffectorStartableImpl.StartParameters;
 import org.apache.brooklyn.location.localhost.LocalhostMachineProvisioningLocation;
+import org.apache.brooklyn.location.ssh.CanResolveOnBoxDir;
 import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
@@ -81,14 +81,11 @@ import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.core.task.ValueResolverIterator;
-import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.net.UserAndHostAndPort;
 import org.apache.brooklyn.util.os.Os;
 import org.apache.brooklyn.util.repeat.Repeater;
-import org.apache.brooklyn.util.ssh.BashCommands;
-import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -561,6 +558,7 @@ public abstract class MachineLifecycleEffectorTasks {
         if (base==null) base = machine.getConfig(BrooklynConfigKeys.BROOKLYN_DATA_DIR);
         if (base==null) base = entity.getManagementContext().getConfig().getConfig(BrooklynConfigKeys.BROOKLYN_DATA_DIR);
         if (base==null) base = "~/brooklyn-managed-processes";
+        
         if (base.equals("~")) base=".";
         if (base.startsWith("~/")) base = "."+base.substring(1);
 
@@ -569,21 +567,8 @@ public abstract class MachineLifecycleEffectorTasks {
             if (log.isDebugEnabled()) log.debug("Skipping on-box base dir resolution for "+entity+" at "+machine);
             if (!Os.isAbsolutish(base)) base = "~/"+base;
             resolvedBase = Os.tidyPath(base);
-        } else if (machine instanceof SshMachineLocation) {
-            SshMachineLocation ms = (SshMachineLocation)machine;
-            ProcessTaskWrapper<Integer> baseTask = SshEffectorTasks.ssh(
-                BashCommands.alternatives("mkdir -p \"${BASE_DIR}\"",
-                    BashCommands.chain(
-                        BashCommands.sudo("mkdir -p \"${BASE_DIR}\""),
-                        BashCommands.sudo("chown "+ms.getUser()+" \"${BASE_DIR}\""))),
-                "cd ~",
-                "cd ${BASE_DIR}",
-                "echo BASE_DIR_RESULT':'`pwd`:BASE_DIR_RESULT")
-                .environmentVariable("BASE_DIR", base)
-                .requiringExitCodeZero()
-                .summary("initializing on-box base dir "+base).newTask();
-            DynamicTasks.queueIfPossible(baseTask).orSubmitAsync(entity);
-            resolvedBase = Strings.getFragmentBetween(baseTask.block().getStdout(), "BASE_DIR_RESULT:", ":BASE_DIR_RESULT");
+        } else if (machine instanceof CanResolveOnBoxDir) {
+            resolvedBase = ((CanResolveOnBoxDir)machine).resolveOnBoxDirFor(entity, base);
         }
         if (resolvedBase==null) {
             if (!Os.isAbsolutish(base)) base = "~/"+base;
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinRmMachineLocation.java b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinRmMachineLocation.java
index 9471096..0fd93b7 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinRmMachineLocation.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinRmMachineLocation.java
@@ -32,6 +32,7 @@ import java.util.Set;
 
 import javax.annotation.Nullable;
 
+import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.location.MachineDetails;
 import org.apache.brooklyn.api.location.MachineLocation;
 import org.apache.brooklyn.api.location.OsDetails;
@@ -40,19 +41,24 @@ import org.apache.brooklyn.config.ConfigKey.HasConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.config.ConfigUtils;
 import org.apache.brooklyn.core.config.Sanitizer;
+import org.apache.brooklyn.core.effector.ssh.SshEffectorTasks;
 import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
 import org.apache.brooklyn.core.location.AbstractMachineLocation;
 import org.apache.brooklyn.core.location.access.PortForwardManager;
 import org.apache.brooklyn.core.location.access.PortForwardManagerLocationResolver;
 import org.apache.brooklyn.core.mgmt.ManagementContextInjectable;
+import org.apache.brooklyn.location.ssh.CanResolveOnBoxDir;
 import org.apache.brooklyn.util.core.ClassLoaderUtils;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.core.internal.ssh.SshTool;
 import org.apache.brooklyn.util.core.internal.winrm.WinRmTool;
 import org.apache.brooklyn.util.core.internal.winrm.WinRmToolResponse;
 import org.apache.brooklyn.util.core.internal.winrm.winrm4j.Winrm4jTool;
+import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.ssh.BashCommands;
 import org.apache.brooklyn.util.stream.Streams;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.commons.codec.binary.Base64;
@@ -71,7 +77,7 @@ import com.google.common.collect.Iterables;
 import com.google.common.net.HostAndPort;
 import com.google.common.reflect.TypeToken;
 
-public class WinRmMachineLocation extends AbstractMachineLocation implements MachineLocation {
+public class WinRmMachineLocation extends AbstractMachineLocation implements MachineLocation, CanResolveOnBoxDir {
 
     private static final Logger LOG = LoggerFactory.getLogger(WinRmMachineLocation.class);
 
@@ -492,4 +498,13 @@ public class WinRmMachineLocation extends AbstractMachineLocation implements Mac
 //        ));
     }
 
+    @Override
+    public String resolveOnBoxDirFor(Entity entity, String unresolvedPath) {
+        // TODO this is simplistic, writes at c:\ for HOME
+        if (unresolvedPath.startsWith("./") || unresolvedPath.startsWith("~/")) {
+            unresolvedPath = "C:\\"+unresolvedPath.substring(2);
+        }
+        return unresolvedPath.replaceAll("/", "\\");
+    }
+
 }
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/Winrm4jTool.java b/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/Winrm4jTool.java
index 2ea8318..b7ff665 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/Winrm4jTool.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/winrm4j/Winrm4jTool.java
@@ -144,7 +144,12 @@ public class Winrm4jTool implements org.apache.brooklyn.util.core.internal.winrm
             byte[] inputData = new byte[chunkSize];
             int bytesRead;
             int expectedFileSize = 0;
+            int i=0;
             while ((bytesRead = source.read(inputData)) > 0) {
+                i++;
+                
+                LOG.debug("Copying chunk "+i+" to "+destination+" on "+host);
+                
                 byte[] chunk;
                 if (bytesRead == chunkSize) {
                     chunk = inputData;
@@ -156,7 +161,8 @@ public class Winrm4jTool implements org.apache.brooklyn.util.core.internal.winrm
                         " -value ([System.Convert]::FromBase64String(\"" + new String(Base64.encodeBase64(chunk)) + "\"))}"));
                 expectedFileSize += bytesRead;
             }
-
+            LOG.debug("Finished copying to "+destination+" on "+host);
+            
             return new org.apache.brooklyn.util.core.internal.winrm.WinRmToolResponse("", "", 0);
         } catch (java.io.IOException e) {
             throw propagate(e, "Failed copying to server at "+destination);


[brooklyn-server] 01/04: First draft supporting shell.env for WinRm

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 0964eeea1ec303dddfa970747cac258e843470e0
Author: Martin Harris <gi...@nakomis.com>
AuthorDate: Mon Aug 26 16:38:41 2019 +0100

    First draft supporting shell.env for WinRm
---
 .../entity/software/base/AbstractSoftwareProcessDriver.java   | 11 +++++++++++
 .../software/base/AbstractSoftwareProcessSshDriver.java       | 10 ----------
 .../software/base/AbstractSoftwareProcessWinRmDriver.java     | 10 ++++++++++
 .../entity/software/base/lifecycle/WinRmExecuteHelper.java    |  6 ++++++
 .../apache/brooklyn/util/core/internal/winrm/WinRmTool.java   |  4 ++++
 5 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java
index b582776..b7748df 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java
@@ -51,6 +51,7 @@ import org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffect
 import org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks.CloseableLatch;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.ResourceUtils;
+import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.core.text.TemplateProcessor;
@@ -666,6 +667,16 @@ public abstract class AbstractSoftwareProcessDriver implements SoftwareProcessDr
         return getEntity().config().get(SoftwareProcess.SUGGESTED_VERSION);
     }
 
+    /**
+     * The environment variables to be set when executing the commands (for install, run, check running, etc).
+     * @see SoftwareProcess#SHELL_ENVIRONMENT
+     */
+    public Map<String, String> getShellEnvironment() {
+        Map<String, Object> env = entity.getConfig(SoftwareProcess.SHELL_ENVIRONMENT);
+        ShellEnvironmentSerializer envSerializer = new ShellEnvironmentSerializer(((EntityInternal)entity).getManagementContext());
+        return envSerializer.serialize(env);
+    }
+
     public abstract String getRunDir();
     public abstract String getInstallDir();
 }
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
index a6b0def..d1118ad 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
@@ -379,16 +379,6 @@ public abstract class AbstractSoftwareProcessSshDriver extends AbstractSoftwareP
     }
 
     /**
-     * The environment variables to be set when executing the commands (for install, run, check running, etc).
-     * @see SoftwareProcess#SHELL_ENVIRONMENT
-     */
-    public Map<String, String> getShellEnvironment() {
-        Map<String, Object> env = entity.getConfig(SoftwareProcess.SHELL_ENVIRONMENT);
-        ShellEnvironmentSerializer envSerializer = new ShellEnvironmentSerializer(((EntityInternal)entity).getManagementContext());
-        return envSerializer.serialize(env);
-    }
-
-    /**
      * @param sshFlags Extra flags to be used when making an SSH connection to the entity's machine.
      *                 If the map contains the key {@link #IGNORE_ENTITY_SSH_FLAGS} then only the
      *                 given flags are used. Otherwise, the given flags are combined with (and take
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java
index b7d23ea..e125918 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java
@@ -29,8 +29,10 @@ import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 
+import javax.annotation.Nullable;
 import javax.xml.ws.WebServiceException;
 
+import com.google.common.base.Function;
 import org.apache.brooklyn.api.entity.EntityLocal;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
@@ -83,10 +85,18 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
     }
 
     protected WinRmExecuteHelper newScript(String command, String psCommand, String phase, String ntDomain) {
+        Map<String, String> environment = (Map)getEntity().getConfig(WinRmTool.SHELL_ENVIRONMENT);
+    
+        if (environment == null) {
+            // Important only to call getShellEnvironment() if env was not supplied; otherwise it
+            // could cause us to resolve config (e.g. block for attributeWhenReady) too early.
+            environment = getShellEnvironment();
+        }
         return newScript(phase)
                 .setNtDomain(ntDomain)
                 .setCommand(command)
                 .setPsCommand(psCommand)
+                .setEnv(environment)
                 .failOnNonZeroResultCode()
                 .gatherOutput();
     }
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/WinRmExecuteHelper.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/WinRmExecuteHelper.java
index 5fe5824..ecfa3bd 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/WinRmExecuteHelper.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/WinRmExecuteHelper.java
@@ -59,6 +59,7 @@ public class WinRmExecuteHelper {
     private String domain;
     private String command;
     private String psCommand;
+    private Map<String, String> env;
 
     @SuppressWarnings("rawtypes")
     protected final Map flags = new LinkedHashMap();
@@ -97,6 +98,11 @@ public class WinRmExecuteHelper {
         return this;
     }
 
+    public WinRmExecuteHelper setEnv(Map<String, String> env) {
+        this.env = env;
+        return this;
+    }
+
     /**
      * indicates that the script should acquire the given mutexId on the given mutexSupport
      * and maintain it for the duration of script execution;
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/WinRmTool.java b/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/WinRmTool.java
index 5706489..dff6e07 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/WinRmTool.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/WinRmTool.java
@@ -30,6 +30,7 @@ import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.config.MapConfigKey;
 import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
+import org.apache.brooklyn.util.core.flags.SetFromFlag;
 import org.apache.brooklyn.util.time.Duration;
 
 import com.google.common.annotations.Beta;
@@ -90,6 +91,9 @@ public interface WinRmTool {
             "Can be used to pass additional custom data to the WinrmTool, which is especially useful " +
                     "if writing a bespoke tool implementation");
 
+    @SetFromFlag("env")
+    MapConfigKey<Object> SHELL_ENVIRONMENT = BrooklynConfigKeys.SHELL_ENVIRONMENT;
+
     /**
      * @deprecated since 0.9.0; use {@link #executeCommand(List)} to avoid ambiguity between native command and power shell.
      */


[brooklyn-server] 04/04: This closes #1064

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 0cb54020da34819cc8f7dbfb59fce34b859e4eb3
Merge: 9308bf1 9361fa6
Author: Alex Heneveld <al...@cloudsoftcorp.com>
AuthorDate: Fri Aug 30 16:23:40 2019 +0100

    This closes #1064

 .../camp/brooklyn/AbstractWindowsYamlTest.java     |   5 +
 .../camp/brooklyn/WindowsYamlLiveTest.java         |  91 ++++++++++----
 .../brooklyn/location/ssh/CanResolveOnBoxDir.java  |  27 +++++
 .../brooklyn/location/ssh/SshMachineLocation.java  |  23 +++-
 .../base/AbstractSoftwareProcessDriver.java        | 134 +++++++++++++++++++--
 .../base/AbstractSoftwareProcessSshDriver.java     | 126 ++-----------------
 .../base/AbstractSoftwareProcessWinRmDriver.java   |  80 ++++++------
 .../software/base/VanillaWindowsProcess.java       |  41 +++++++
 .../base/VanillaWindowsProcessWinRmDriver.java     |  11 +-
 .../lifecycle/MachineLifecycleEffectorTasks.java   |  23 +---
 .../base/lifecycle/WinRmExecuteHelper.java         |   7 ++
 .../base/test/location/WindowsTestFixture.java     |  28 +++--
 .../location/winrm/WinRmMachineLocation.java       |  17 ++-
 .../util/core/internal/winrm/WinRmTool.java        |   3 +
 .../core/internal/winrm/winrm4j/Winrm4jTool.java   |   8 +-
 15 files changed, 409 insertions(+), 215 deletions(-)


[brooklyn-server] 02/04: set env vars on winrm executions, except for installing

Posted by he...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 8efb1c9ebef9f0383e6c8c3c43977afb4dd3660c
Author: Alex Heneveld <al...@cloudsoftcorp.com>
AuthorDate: Mon Aug 26 21:45:54 2019 +0200

    set env vars on winrm executions, except for installing
---
 .../camp/brooklyn/AbstractWindowsYamlTest.java     |  5 ++
 .../camp/brooklyn/WindowsYamlLiveTest.java         | 58 ++++++++++++++++--
 .../base/AbstractSoftwareProcessSshDriver.java     | 16 ++---
 .../base/AbstractSoftwareProcessWinRmDriver.java   | 68 ++++++++++++----------
 .../base/VanillaWindowsProcessWinRmDriver.java     | 11 +++-
 .../base/lifecycle/WinRmExecuteHelper.java         |  1 +
 .../base/test/location/WindowsTestFixture.java     | 28 ++++++---
 .../util/core/internal/winrm/WinRmTool.java        |  5 +-
 8 files changed, 137 insertions(+), 55 deletions(-)

diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractWindowsYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractWindowsYamlTest.java
index f23bec4..4d9f07c 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractWindowsYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractWindowsYamlTest.java
@@ -27,6 +27,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.HasTaskChildren;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
@@ -109,6 +110,10 @@ public abstract class AbstractWindowsYamlTest extends AbstractYamlTest {
         return (stream != null) ? stream.streamContents.get() : null;
     }
 
+    protected Optional<Task<?>> findTaskOrSubTask(Entity entity, Predicate<? super Task<?>> matcher) {
+        return findTaskOrSubTask(BrooklynTaskTags.getTasksInEntityContext(mgmt().getExecutionManager(), entity), matcher);
+    }
+    
     protected Optional<Task<?>> findTaskOrSubTask(Iterable<? extends Task<?>> tasks, Predicate<? super Task<?>> matcher) {
         List<String> taskNames = Lists.newArrayList();
         Optional<Task<?>> result = findTaskOrSubTaskImpl(tasks, matcher, taskNames);
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java
index 7c38647..6e1a0a1 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WindowsYamlLiveTest.java
@@ -25,14 +25,20 @@ import java.util.Map;
 
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.location.MachineProvisioningLocation;
+import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.core.entity.Attributes;
 import org.apache.brooklyn.core.entity.Dumper;
 import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.EntityAsserts;
+import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.entity.software.base.SoftwareProcess;
 import org.apache.brooklyn.entity.software.base.VanillaWindowsProcess;
 import org.apache.brooklyn.entity.software.base.test.location.WindowsTestFixture;
 import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
+import org.apache.brooklyn.test.Asserts;
+import org.apache.brooklyn.util.core.task.TaskPredicates;
+import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
 import org.apache.brooklyn.util.text.StringPredicates;
 import org.apache.brooklyn.util.text.Strings;
 import org.slf4j.Logger;
@@ -44,8 +50,12 @@ import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Joiner;
+import com.google.common.base.Optional;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 
 /**
@@ -54,6 +64,8 @@ import com.google.common.collect.Lists;
 @Test
 public class WindowsYamlLiveTest extends AbstractWindowsYamlTest {
     
+    // set EXISTING_WINDOWS_TEST_USER_PASS_HOST_ENV_VAR as per WindowsTestFixture to re-use existing machines
+    
     // TODO Remove duplication of assertStreams and VanillaWindowsProcessWinrmStreamsLiveTest.assertStreams
     
     private static final Logger log = LoggerFactory.getLogger(WindowsYamlLiveTest.class);
@@ -88,16 +100,16 @@ public class WindowsYamlLiveTest extends AbstractWindowsYamlTest {
         
         location = WindowsTestFixture.setUpWindowsLocation(mgmt());
         machine = location.obtain(ImmutableMap.of());
-        String ip = machine.getAddress().getHostAddress();
-        String password = machine.config().get(WinRmMachineLocation.PASSWORD);
 
         yamlLocation = ImmutableList.of(
                 "location:",
                 "  byon:",
                 "    hosts:",
-                "    - winrm: "+ip+":5985",
-                "      password: \""+password.replace("\"", "\\\"") + "\"",
-                "      user: Administrator",
+                "    - winrm: "+machine.getAddress().getHostAddress() 
+                        // this is the default, probably not necessary but kept for posterity 
+                        +":5985",
+                "      password: "+JavaStringEscapes.wrapJavaString(machine.config().get(WinRmMachineLocation.PASSWORD)),
+                "      user: "+machine.config().get(WinRmMachineLocation.USER),
                 "      osFamily: windows");
     }
 
@@ -318,6 +330,42 @@ public class WindowsYamlLiveTest extends AbstractWindowsYamlTest {
         }
     }
 
+    @Test(groups="Live")
+    public void testEnvVarResolution() throws Exception {
+        List<String> yaml = Lists.newArrayList();
+        yaml.addAll(yamlLocation);
+        String in = "%KEY1%: %ADDR_RESOLVED%";
+        yaml.addAll(ImmutableList.of(
+            "services:",
+            "  - type: org.apache.brooklyn.entity.software.base.VanillaWindowsProcess", 
+            "    brooklyn.config:", 
+            "      install.command: "+JavaStringEscapes.wrapJavaString("echo "+in), 
+            "      customize.command: "+JavaStringEscapes.wrapJavaString("echo "+in), 
+            "      launch.command: "+JavaStringEscapes.wrapJavaString("echo "+in), 
+            "      stop.command: echo true", 
+            "      checkRunning.command: echo true", 
+            "      shell.env:", 
+            "        KEY1: Address", 
+            "        ADDR_RESOLVED: $brooklyn:attributeWhenReady(\"host.address\")"));
+
+        app = createAndStartApplication(Joiner.on("\n").join(yaml));
+        waitForApplicationTasks(app);
+        log.info("App started:");
+        Dumper.dumpInfo(app);
+        
+        
+        Entity win = Iterables.getOnlyElement(app.getChildren());
+        String out = "Address: "+win.sensors().get(SoftwareProcess.ADDRESS);
+        assertPhaseStreamEquals(win, "install", "stdout", Predicates.equalTo(in));
+        assertPhaseStreamEquals(win, "customize", "stdout", Predicates.equalTo(out));
+        assertPhaseStreamEquals(win, "launch", "stdout", Predicates.equalTo(out));
+    }
+    
+    private void assertPhaseStreamEquals(Entity entity, String phase, String stream, Predicate<String> check) {
+        Optional<Task<?>> t = findTaskOrSubTask(entity, TaskPredicates.displayNameSatisfies(StringPredicates.startsWith("winrm: "+phase)));
+        Asserts.assertThat(BrooklynTaskTags.stream(t.get(), stream).getStreamContentsAbbreviated().trim(), check);
+    }
+
     @Override
     protected Logger getLogger() {
         return log;
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
index d1118ad..9ff370c 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
@@ -493,6 +493,8 @@ public abstract class AbstractSoftwareProcessSshDriver extends AbstractSoftwareP
     public static final String RESTARTING = "restarting";
 
     public static final String PID_FILENAME = "pid.txt";
+    static final String INSTALL_DIR_ENV_VAR = "INSTALL_DIR";
+    static final String RUN_DIR_ENV_VAR = "RUN_DIR";
 
     /* Flags */
 
@@ -578,9 +580,9 @@ public abstract class AbstractSoftwareProcessSshDriver extends AbstractSoftwareP
                 final String mutexId = "installation lock at host";
                 s.useMutex(getLocation().mutexes(), mutexId, "installing "+elvis(entity,this));
                 s.header.append(
-                        "export INSTALL_DIR=\""+getInstallDir()+"\"",
-                        "mkdir -p $INSTALL_DIR",
-                        "cd $INSTALL_DIR",
+                        "export "+INSTALL_DIR_ENV_VAR+"=\""+getInstallDir()+"\"",
+                        "mkdir -p $"+INSTALL_DIR_ENV_VAR,
+                        "cd $"+INSTALL_DIR_ENV_VAR,
                         "test -f BROOKLYN && exit 0"
                         );
 
@@ -592,10 +594,10 @@ public abstract class AbstractSoftwareProcessSshDriver extends AbstractSoftwareP
             }
             if (ImmutableSet.of(CUSTOMIZING, LAUNCHING, CHECK_RUNNING, STOPPING, KILLING, RESTARTING).contains(phase)) {
                 s.header.append(
-                        "export INSTALL_DIR=\""+getInstallDir()+"\"",
-                        "export RUN_DIR=\""+getRunDir()+"\"",
-                        "mkdir -p $RUN_DIR",
-                        "cd $RUN_DIR"
+                        "export "+INSTALL_DIR_ENV_VAR+"=\""+getInstallDir()+"\"",
+                        "export "+RUN_DIR_ENV_VAR+"=\""+getRunDir()+"\"",
+                        "mkdir -p $"+RUN_DIR_ENV_VAR,
+                        "cd $"+RUN_DIR_ENV_VAR
                         );
             }
         }
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java
index e125918..0303313 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessWinRmDriver.java
@@ -29,10 +29,8 @@ import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 
-import javax.annotation.Nullable;
 import javax.xml.ws.WebServiceException;
 
-import com.google.common.base.Function;
 import org.apache.brooklyn.api.entity.EntityLocal;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
@@ -43,6 +41,7 @@ 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.WinRmMachineLocation;
+import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.internal.winrm.WinRmTool;
 import org.apache.brooklyn.util.core.internal.winrm.WinRmToolResponse;
 import org.apache.brooklyn.util.core.mutex.WithMutexes;
@@ -75,37 +74,37 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
         entity.sensors().set(WINDOWS_PASSWORD, location.config().get(WinRmMachineLocation.PASSWORD));
     }
 
-    /** @see #newScript(Map, String) */
-    protected WinRmExecuteHelper newScript(String phase) {
-        return newScript(Maps.<String, Object>newLinkedHashMap(), phase);
+    protected WinRmExecuteHelper newScript(String command, String psCommand, String phase, String taskNamePrefix) {
+        return newScript(command, psCommand, phase, taskNamePrefix, null);
     }
 
-    protected WinRmExecuteHelper newScript(String command, String psCommand, String phase) {
-        return newScript(command, psCommand, phase, null);
-    }
-
-    protected WinRmExecuteHelper newScript(String command, String psCommand, String phase, String ntDomain) {
-        Map<String, String> environment = (Map)getEntity().getConfig(WinRmTool.SHELL_ENVIRONMENT);
-    
-        if (environment == null) {
-            // Important only to call getShellEnvironment() if env was not supplied; otherwise it
-            // could cause us to resolve config (e.g. block for attributeWhenReady) too early.
-            environment = getShellEnvironment();
-        }
-        return newScript(phase)
-                .setNtDomain(ntDomain)
+    protected WinRmExecuteHelper newScript(String command, String psCommand, String phase, String taskNamePrefix, String ntDomain) {
+        WinRmExecuteHelper result = newEmptyScript(taskNamePrefix);
+        result.setNtDomain(ntDomain)
                 .setCommand(command)
                 .setPsCommand(psCommand)
-                .setEnv(environment)
                 .failOnNonZeroResultCode()
                 .gatherOutput();
+        
+        Map<String, String> env = MutableMap.of();
+        env.put("INSTALL_DIR", getInstallDir());
+        if (AbstractSoftwareProcessSshDriver.INSTALLING.equals(phase)) {
+            // don't set shell env during this phase; otherwise it could cause us 
+            // to resolve config (e.g. block for attributeWhenReady) too early; instead just give install dir
+        } else {
+            env.put("RUN_DIR", getRunDir());
+            env.putAll(getShellEnvironment());
+        }
+        result.setEnv(env);
+        
+        return result;
     }
 
-    protected WinRmExecuteHelper newScript(Map<String, ?> flags, String phase) {
+    protected WinRmExecuteHelper newEmptyScript(String taskNamePrefix) {
         if (!Entities.isManaged(getEntity()))
-            throw new IllegalStateException(getEntity() + " is no longer managed; cannot create script to run here (" + phase + ")");
+            throw new IllegalStateException(getEntity() + " is no longer managed; cannot create script to run here (" + taskNamePrefix + ")");
 
-        WinRmExecuteHelper s = new WinRmExecuteHelper(this, phase + " " + elvis(entity, this));
+        WinRmExecuteHelper s = new WinRmExecuteHelper(this, taskNamePrefix + " " + elvis(entity, this));
         return s;
     }
 
@@ -115,6 +114,7 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
             newScript(
                     getEntity().getConfig(BrooklynConfigKeys.PRE_INSTALL_COMMAND),
                     getEntity().getConfig(VanillaWindowsProcess.PRE_INSTALL_POWERSHELL_COMMAND),
+                    AbstractSoftwareProcessSshDriver.INSTALLING,
                     "pre-install-command")
                 .useMutex(getLocation().mutexes(), "installation lock at host", "installing "+elvis(entity,this))
                 .execute();
@@ -135,6 +135,7 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
             newScript(
                     getEntity().getConfig(BrooklynConfigKeys.POST_INSTALL_COMMAND),
                     getEntity().getConfig(VanillaWindowsProcess.POST_INSTALL_POWERSHELL_COMMAND),
+                    AbstractSoftwareProcessSshDriver.INSTALLING,
                     "post-install-command")
             .useMutex(getLocation().mutexes(), "installation lock at host", "installing "+elvis(entity,this))
             .execute();
@@ -147,6 +148,7 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
             executeCommandInTask(
                     getEntity().getConfig(BrooklynConfigKeys.PRE_CUSTOMIZE_COMMAND),
                     getEntity().getConfig(VanillaWindowsProcess.PRE_CUSTOMIZE_POWERSHELL_COMMAND),
+                    AbstractSoftwareProcessSshDriver.CUSTOMIZING,
                     "pre-customize-command");
         }
     }
@@ -157,6 +159,7 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
             executeCommandInTask(
                     getEntity().getConfig(BrooklynConfigKeys.POST_CUSTOMIZE_COMMAND),
                     getEntity().getConfig(VanillaWindowsProcess.POST_CUSTOMIZE_POWERSHELL_COMMAND),
+                    AbstractSoftwareProcessSshDriver.CUSTOMIZING,
                     "post-customize-command");
         }
     }
@@ -167,6 +170,7 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
             executeCommandInTask(
                     getEntity().getConfig(BrooklynConfigKeys.PRE_LAUNCH_COMMAND),
                     getEntity().getConfig(VanillaWindowsProcess.PRE_LAUNCH_POWERSHELL_COMMAND),
+                    AbstractSoftwareProcessSshDriver.LAUNCHING,
                     "pre-launch-command");
         }
     }
@@ -177,6 +181,7 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
             executeCommandInTask(
                     getEntity().getConfig(BrooklynConfigKeys.POST_LAUNCH_COMMAND),
                     getEntity().getConfig(VanillaWindowsProcess.POST_LAUNCH_POWERSHELL_COMMAND),
+                    AbstractSoftwareProcessSshDriver.LAUNCHING,
                     "post-launch-command");
         }
     }
@@ -235,12 +240,12 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
         return getLocation();
     }
 
-    protected int executeCommandInTask(String command, String psCommand, String phase) {
-        return executeCommandInTask(command, psCommand, phase, null);
+    protected int executeCommandInTask(String command, String psCommand, String phase, String taskNamePrefix) {
+        return executeCommandInTask(command, psCommand, phase, taskNamePrefix, null);
     }
 
-    protected int executeCommandInTask(String command, String psCommand, String phase, String ntDomain) {
-        WinRmExecuteHelper helper = newScript(command, psCommand, phase, ntDomain);
+    protected int executeCommandInTask(String command, String psCommand, String phase, String taskNamePrefix, String ntDomain) {
+        WinRmExecuteHelper helper = newScript(command, psCommand, phase, taskNamePrefix, ntDomain);
         return helper.execute();
     }
 
@@ -324,15 +329,15 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
     }
 
     @Override
-    public Integer executeNativeOrPsCommand(Map flags, String regularCommand, String powerShellCommand, String phase, Boolean allowNoOp) {
+    public Integer executeNativeOrPsCommand(Map flags, String regularCommand, String powerShellCommand, String summary, 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", summary, 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, summary, entity));
         }
 
         ByteArrayOutputStream stdIn = new ByteArrayOutputStream();
@@ -360,6 +365,9 @@ public abstract class AbstractSoftwareProcessWinRmDriver extends AbstractSoftwar
         if (flags.get(WinRmTool.COMPUTER_NAME) != null) {
             winrmProps.put(WinRmTool.COMPUTER_NAME, flags.get(WinRmTool.COMPUTER_NAME));
         }
+        if (flags.get(WinRmTool.ENVIRONMENT)!=null) {
+            winrmProps.put(WinRmTool.ENVIRONMENT, flags.get(WinRmTool.ENVIRONMENT));
+        }
 
         if (Strings.isBlank(regularCommand)) {
             response = getLocation().executePsScript(winrmProps.build(), ImmutableList.of(powerShellCommand));
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaWindowsProcessWinRmDriver.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaWindowsProcessWinRmDriver.java
index d901242..5058ddd 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaWindowsProcessWinRmDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/VanillaWindowsProcessWinRmDriver.java
@@ -50,7 +50,7 @@ public class VanillaWindowsProcessWinRmDriver extends AbstractSoftwareProcessWin
         // TODO: At some point in the future, this should probably be refactored to get the name of the machine in WinRmMachineLocation and set it as the hostname sensor
         String hostname = null;
         if (entity.getConfig(VanillaWindowsProcess.INSTALL_REBOOT_REQUIRED)) {
-            WinRmExecuteHelper checkHostnameTask = newScript("Checking hostname")
+            WinRmExecuteHelper checkHostnameTask = newEmptyScript("Checking hostname")
                     .setCommand("hostname")
                     .failOnNonZeroResultCode()
                     .gatherOutput();
@@ -62,7 +62,7 @@ public class VanillaWindowsProcessWinRmDriver extends AbstractSoftwareProcessWin
         if(Strings.isNonBlank(getEntity().getConfig(VanillaWindowsProcess.INSTALL_COMMAND)) || Strings.isNonBlank(getEntity().getConfig(VanillaWindowsProcess.INSTALL_POWERSHELL_COMMAND))) {
             String cmd = getEntity().getConfig(VanillaWindowsProcess.INSTALL_COMMAND);
             String psCmd = getEntity().getConfig(VanillaWindowsProcess.INSTALL_POWERSHELL_COMMAND);
-            newScript(cmd, psCmd, "install-command", hostname)
+            newScript(cmd, psCmd, AbstractSoftwareProcessSshDriver.INSTALLING, "installing-command", hostname)
                     .useMutex(getLocation().mutexes(), "installation lock at host", "installing "+elvis(entity,this))
                     .execute();
         }
@@ -77,6 +77,7 @@ public class VanillaWindowsProcessWinRmDriver extends AbstractSoftwareProcessWin
             executeCommandInTask(
                     getEntity().getConfig(VanillaWindowsProcess.CUSTOMIZE_COMMAND),
                     getEntity().getConfig(VanillaWindowsProcess.CUSTOMIZE_POWERSHELL_COMMAND),
+                    AbstractSoftwareProcessSshDriver.CUSTOMIZING,
                     "customize-command");
         }
         if (entity.getConfig(VanillaWindowsProcess.CUSTOMIZE_REBOOT_REQUIRED)) {
@@ -91,6 +92,7 @@ public class VanillaWindowsProcessWinRmDriver extends AbstractSoftwareProcessWin
             executeCommandInTask(
                     getEntity().getConfig(VanillaWindowsProcess.LAUNCH_COMMAND),
                     getEntity().getConfig(VanillaWindowsProcess.LAUNCH_POWERSHELL_COMMAND),
+                    AbstractSoftwareProcessSshDriver.LAUNCHING,
                     "launch-command");
         }
     }
@@ -101,7 +103,9 @@ public class VanillaWindowsProcessWinRmDriver extends AbstractSoftwareProcessWin
         try {
             exitCode = executeCommandInTask(
                     getEntity().getConfig(VanillaWindowsProcess.CHECK_RUNNING_COMMAND),
-                    getEntity().getConfig(VanillaWindowsProcess.CHECK_RUNNING_POWERSHELL_COMMAND), "is-running-command");
+                    getEntity().getConfig(VanillaWindowsProcess.CHECK_RUNNING_POWERSHELL_COMMAND), 
+                    AbstractSoftwareProcessSshDriver.CHECK_RUNNING,
+                    "is-running-command");
         } catch (Exception e) {
             Throwable interestingCause = findExceptionCausedByWindowsRestart(e);
             if (interestingCause != null) {
@@ -122,6 +126,7 @@ public class VanillaWindowsProcessWinRmDriver extends AbstractSoftwareProcessWin
         executeCommandInTask(
                 getEntity().getConfig(VanillaWindowsProcess.STOP_COMMAND),
                 getEntity().getConfig(VanillaWindowsProcess.STOP_POWERSHELL_COMMAND),
+                AbstractSoftwareProcessSshDriver.STOPPING,
                 "stop-command");
     }
 
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/WinRmExecuteHelper.java b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/WinRmExecuteHelper.java
index ecfa3bd..c7f9285 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/WinRmExecuteHelper.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/WinRmExecuteHelper.java
@@ -196,6 +196,7 @@ public class WinRmExecuteHelper {
                 flags.put("err", stderr);
             }
             flags.put(WinRmTool.COMPUTER_NAME, domain);
+            if (env!=null) flags.put(WinRmTool.ENVIRONMENT, env);
             result = runner.executeNativeOrPsCommand(flags, command, psCommand, summary, false);
             if (!resultCodeCheck.apply(result)) {
                 throw logWithDetailsAndThrow(format("Execution failed, invalid result %s for %s", result, summary), null);
diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/location/WindowsTestFixture.java b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/location/WindowsTestFixture.java
index 07ccdef..1a744c0 100644
--- a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/location/WindowsTestFixture.java
+++ b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/test/location/WindowsTestFixture.java
@@ -27,12 +27,17 @@ import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
 import org.apache.brooklyn.location.jclouds.JcloudsLocation;
 import org.apache.brooklyn.location.winrm.WinRmMachineLocation;
 import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.text.Strings;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 
 public class WindowsTestFixture {
 
+    /** Can be configured as `user:pass@host` to allow use of a pre-existing fixed winrm target;
+     * if not, will provision in AWS */
+    public static final String EXISTING_WINDOWS_TEST_USER_PASS_HOST_ENV_VAR = "EXISTING_WINDOWS_TEST_USER_PASS_HOST_ENV_VAR";
+    
     public static MachineProvisioningLocation<WinRmMachineLocation> setUpWindowsLocation(ManagementContext mgmt) throws Exception {
         return setUpWindowsLocation(mgmt, ImmutableMap.<String, Object>of());
     }
@@ -41,8 +46,17 @@ public class WindowsTestFixture {
     public static MachineProvisioningLocation<WinRmMachineLocation> setUpWindowsLocation(ManagementContext mgmt, Map<String, ?> props) throws Exception {
         // Commented out / unused code included here to make it easy to supply a 
         // pre-existing Windows VM for use in a bunch of different tests.
-//        return (MachineProvisioningLocation<WinRmMachineLocation>) newByonLocation((ManagementContextInternal) mgmt);
-        return (MachineProvisioningLocation<WinRmMachineLocation>) newJcloudsLocation((ManagementContextInternal) mgmt, props);
+        String userPassAtHost = System.getenv(EXISTING_WINDOWS_TEST_USER_PASS_HOST_ENV_VAR);
+        if (Strings.isBlank(userPassAtHost)) {
+            return (MachineProvisioningLocation<WinRmMachineLocation>) newJcloudsLocation((ManagementContextInternal) mgmt, props);
+        } else {
+            return (MachineProvisioningLocation<WinRmMachineLocation>) newByonLocation((ManagementContextInternal) mgmt,
+                MutableMap.of(
+                        "winrm", userPassAtHost.split("@")[1],
+                        "password", userPassAtHost.split(":")[1].split("@")[0],
+                        "user", userPassAtHost.split(":")[0]
+                    ));
+        }
     }
     
     private static MachineProvisioningLocation<?> newJcloudsLocation(ManagementContextInternal mgmt, Map<String, ?> props) {
@@ -62,17 +76,17 @@ public class WindowsTestFixture {
                 .build());
     }
     
-    @SuppressWarnings("unused")
     private static MachineProvisioningLocation<?> newByonLocation(ManagementContextInternal mgmt, Map<String, ?> props) {
         Map<String, Object> config = new LinkedHashMap<>();
-        config.put("hosts", "52.12.211.123:5985");
+        config.put("useJcloudsSshInit", "false");
+        config.put("byonIdentity", "123");
         config.put("osFamily", "windows");
+        // these are overwritten by the map
         config.put("winrm", "52.12.211.123:5985");
         config.put("user", "Administrator");
         config.put("password", "pa55w0rd");
-        config.put("useJcloudsSshInit", "false");
-        config.put("byonIdentity", "123");
         config.putAll(props);
-        return (MachineProvisioningLocation<?>) mgmt.getLocationRegistry().getLocationManaged("byon", config);
+        
+        return (MachineProvisioningLocation<?>) mgmt.getLocationRegistry().getLocationManaged("byon", ImmutableMap.of("hosts", ImmutableList.of(config)));
     }
 }
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/WinRmTool.java b/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/WinRmTool.java
index dff6e07..1d9d2b9 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/WinRmTool.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/util/core/internal/winrm/WinRmTool.java
@@ -49,6 +49,8 @@ public interface WinRmTool {
     ConfigKey<Integer> PROP_PORT = ConfigKeys.newIntegerConfigKey("port", "WinRM port to use when connecting to the remote machine");
     ConfigKey<Boolean> USE_HTTPS_WINRM = ConfigKeys.newBooleanConfigKey("winrm.useHttps", "The parameter tells the machine sensors whether the winrm port is over https. If the parameter is true then 5986 will be used as a winrm port.", false);
     ConfigKey<Integer> RETRIES_OF_NETWORK_FAILURES = ConfigKeys.newIntegerConfigKey("retriesOfNetworkFailures", "The parameter sets the number of retries for connection failures. If you use high value, consider taking care for the machine's network.", 4);
+    
+    @SetFromFlag("env")
     ConfigKey<Map<String,String>> ENVIRONMENT = MapConfigKey.builder(new TypeToken<Map<String,String>>() {})
             .name("winrm.environment")
             .description("WinRM Environment variables").build();
@@ -91,9 +93,6 @@ public interface WinRmTool {
             "Can be used to pass additional custom data to the WinrmTool, which is especially useful " +
                     "if writing a bespoke tool implementation");
 
-    @SetFromFlag("env")
-    MapConfigKey<Object> SHELL_ENVIRONMENT = BrooklynConfigKeys.SHELL_ENVIRONMENT;
-
     /**
      * @deprecated since 0.9.0; use {@link #executeCommand(List)} to avoid ambiguity between native command and power shell.
      */