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 2022/07/20 00:19:46 UTC

[brooklyn-server] 02/02: add new SimpleProcessTaskFactory interface, fix error in container bashScript arg

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 ccc632229cdf4f8a6e8066d542ad186192053cb8
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Wed Jul 20 00:11:32 2022 +0100

    add new SimpleProcessTaskFactory interface, fix error in container bashScript arg
    
    new shared interface can be used for containers and processes.
    Function switched from guava to java.util.
---
 .../core/effector/ssh/SshEffectorTasks.java        |  2 +-
 .../brooklyn/util/core/task/ssh/SshTasks.java      |  2 +-
 .../ssh/internal/AbstractSshExecTaskFactory.java   |  2 +-
 .../task/ssh/internal/PlainSshExecTaskFactory.java |  3 +-
 .../util/core/task/system/ProcessTaskFactory.java  | 54 +++++++++++-----------
 .../util/core/task/system/ProcessTaskStub.java     |  3 +-
 .../util/core/task/system/ProcessTaskWrapper.java  |  5 +-
 .../core/task/system/SimpleProcessTaskFactory.java | 41 ++++++++++++++++
 .../internal/AbstractProcessTaskFactory.java       | 15 ++++--
 .../system/internal/SystemProcessTaskFactory.java  |  1 +
 .../tasks/kubectl/ContainerTaskFactory.java        | 43 +++++++++++------
 .../brooklyn/tasks/kubectl/ContainerTaskTest.java  |  8 ++--
 .../location/winrm/PlainWinRmExecTaskFactory.java  |  3 +-
 13 files changed, 123 insertions(+), 59 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/effector/ssh/SshEffectorTasks.java b/core/src/main/java/org/apache/brooklyn/core/effector/ssh/SshEffectorTasks.java
index d101b5c4a0..bf6f0b783b 100644
--- a/core/src/main/java/org/apache/brooklyn/core/effector/ssh/SshEffectorTasks.java
+++ b/core/src/main/java/org/apache/brooklyn/core/effector/ssh/SshEffectorTasks.java
@@ -21,6 +21,7 @@ package org.apache.brooklyn.core.effector.ssh;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
 
 import javax.annotation.Nullable;
 
@@ -60,7 +61,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.Beta;
-import com.google.common.base.Function;
 import com.google.common.collect.Maps;
 
 /**
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/SshTasks.java b/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/SshTasks.java
index fd6a6e399a..d732177051 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/SshTasks.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/SshTasks.java
@@ -22,6 +22,7 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
 
 import javax.annotation.Nullable;
 
@@ -59,7 +60,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.annotations.Beta;
-import com.google.common.base.Function;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/internal/AbstractSshExecTaskFactory.java b/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/internal/AbstractSshExecTaskFactory.java
index 170040bc54..e61bd7cd01 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/internal/AbstractSshExecTaskFactory.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/internal/AbstractSshExecTaskFactory.java
@@ -35,7 +35,7 @@ import org.apache.commons.io.output.TeeOutputStream;
 import org.apache.commons.io.output.WriterOutputStream;
 
 // cannot be (cleanly) instantiated due to nested generic self-referential type; however trivial subclasses do allow it 
-public abstract class AbstractSshExecTaskFactory<T extends AbstractProcessTaskFactory<T,RET>,RET> extends AbstractProcessTaskFactory<T,RET> implements ProcessTaskFactory<RET> {
+public abstract class AbstractSshExecTaskFactory<T extends AbstractProcessTaskFactory<T,RET>,RET> extends AbstractProcessTaskFactory<T,RET> {
     
     /** constructor where machine will be added later */
     public AbstractSshExecTaskFactory(String ...commands) {
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/internal/PlainSshExecTaskFactory.java b/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/internal/PlainSshExecTaskFactory.java
index 3da6d6f9ce..2decc6c11a 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/internal/PlainSshExecTaskFactory.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/ssh/internal/PlainSshExecTaskFactory.java
@@ -19,12 +19,11 @@
 package org.apache.brooklyn.util.core.task.ssh.internal;
 
 import java.util.List;
+import java.util.function.Function;
 
 import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
 
-import com.google.common.base.Function;
-
 /** the "Plain" class exists purely so we can massage return types for callers' convenience */
 public class PlainSshExecTaskFactory<RET> extends AbstractSshExecTaskFactory<PlainSshExecTaskFactory<RET>,RET> {
     /** constructor where machine will be added later */
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskFactory.java b/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskFactory.java
index b0878c3020..b7e8e3aab9 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskFactory.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskFactory.java
@@ -18,48 +18,50 @@
  */
 package org.apache.brooklyn.util.core.task.system;
 
-import java.util.Map;
-
+import com.google.common.annotations.Beta;
 import org.apache.brooklyn.api.location.MachineLocation;
-import org.apache.brooklyn.api.mgmt.TaskFactory;
 import org.apache.brooklyn.config.ConfigKey;
-import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.util.core.internal.ssh.SshTool;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskStub.ScriptReturnType;
 
-import com.google.common.annotations.Beta;
-import com.google.common.base.Function;
+import java.util.Map;
+import java.util.function.Function;
+
+public interface ProcessTaskFactory<T> extends SimpleProcessTaskFactory<ProcessTaskFactory<T>,ProcessTaskWrapper<?>,T,ProcessTaskWrapper<T>> {
+    ProcessTaskFactory<T> machine(MachineLocation machine);
+    ProcessTaskFactory<T> add(String ...commandsToAdd);
+    ProcessTaskFactory<T> add(Iterable<String> commandsToAdd);
+    ProcessTaskFactory<T> requiringExitCodeZero();
+    ProcessTaskFactory<T> requiringExitCodeZero(String extraErrorMessage);
+    @Override ProcessTaskFactory<T> allowingNonZeroExitCode();
+
+    ProcessTaskFactory<String> requiringZeroAndReturningStdout();
+    ProcessTaskFactory<Boolean> returningIsExitCodeZero();
+    <RET2> ProcessTaskFactory<RET2> returning(ScriptReturnType type);
+    @Override ProcessTaskFactory<String> returningStdout();
+    @Override ProcessTaskFactory<Integer> returningExitCodeAllowingNonZero();
+    @Override <TT2> ProcessTaskFactory<TT2> returning(Function<ProcessTaskWrapper<?>, TT2> resultTransformation);
 
-public interface ProcessTaskFactory<T> extends TaskFactory<ProcessTaskWrapper<T>> {
-    public ProcessTaskFactory<T> machine(MachineLocation machine);
-    public ProcessTaskFactory<T> add(String ...commandsToAdd);
-    public ProcessTaskFactory<T> add(Iterable<String> commandsToAdd);
-    public ProcessTaskFactory<T> requiringExitCodeZero();
-    public ProcessTaskFactory<T> requiringExitCodeZero(String extraErrorMessage);
-    public ProcessTaskFactory<T> allowingNonZeroExitCode();
-    public ProcessTaskFactory<String> requiringZeroAndReturningStdout();
-    public ProcessTaskFactory<Boolean> returningIsExitCodeZero();
-    public <RET2> ProcessTaskFactory<RET2> returning(ScriptReturnType type);
-    public <RET2> ProcessTaskFactory<RET2> returning(Function<ProcessTaskWrapper<?>, RET2> resultTransformation);
-    public ProcessTaskFactory<T> runAsCommand();
-    public ProcessTaskFactory<T> runAsScript();
-    public ProcessTaskFactory<T> runAsRoot();
-    public ProcessTaskFactory<T> environmentVariable(String key, String val);
-    public ProcessTaskFactory<T> environmentVariables(Map<String,String> vars);
-    public ProcessTaskFactory<T> summary(String summary);
+    //public <RET2> ProcessTaskFactory<T0,RET2> returning(Function<ProcessTaskWrapper<T0>, RET2> resultTransformation);
+    ProcessTaskFactory<T> runAsCommand();
+    ProcessTaskFactory<T> runAsScript();
+    ProcessTaskFactory<T> runAsRoot();
+    @Override ProcessTaskFactory<T> environmentVariable(String key, String val);
+    @Override ProcessTaskFactory<T> environmentVariables(Map<String,String> vars);
+    @Override ProcessTaskFactory<T> summary(String summary);
     
     /** allows setting config-key based properties for specific underlying tools */
     @Beta
-    public <V> ProcessTaskFactory<T> configure(ConfigKey<V> key, V value);
+    <V> ProcessTaskFactory<T> configure(ConfigKey<V> key, V value);
 
     /** allows setting config-key/flag based properties for specific underlying tools;
      * but note that if any are prefixed with {@link SshTool#BROOKLYN_CONFIG_KEY_PREFIX}
      * these should normally be filtered out */
     @Beta
-    public ProcessTaskFactory<T> configure(Map<?,?> flags);
+    ProcessTaskFactory<T> configure(Map<?,?> flags);
 
     /** adds a listener which will be notified of (otherwise) successful completion,
      * typically used to invalidate the result (ie throw exception, to promote a string in the output to an exception);
      * invoked even if return code is zero, so a better error can be thrown */
-    public ProcessTaskFactory<T> addCompletionListener(Function<ProcessTaskWrapper<?>, Void> function);
+    ProcessTaskFactory<T> addCompletionListener(Function<ProcessTaskWrapper<?>, Void> function);
 }
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskStub.java b/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskStub.java
index 549ba91d31..471fe54965 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskStub.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskStub.java
@@ -21,14 +21,13 @@ package org.apache.brooklyn.util.core.task.system;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Function;
 
 import org.apache.brooklyn.api.location.MachineLocation;
-import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.text.Strings;
 
-import com.google.common.base.Function;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskWrapper.java b/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskWrapper.java
index 03ca591c0a..4185ca2923 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskWrapper.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/system/ProcessTaskWrapper.java
@@ -21,6 +21,7 @@ package org.apache.brooklyn.util.core.task.system;
 import java.io.ByteArrayOutputStream;
 import java.io.OutputStream;
 import java.util.concurrent.Callable;
+import java.util.function.Function;
 
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.mgmt.TaskWrapper;
@@ -35,9 +36,7 @@ import org.apache.brooklyn.util.text.Strings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Function;
-
-/** Wraps a fully constructed process task, and allows callers to inspect status. 
+/** Wraps a fully constructed process task, and allows callers to inspect status.
  * Note that methods in here such as {@link #getStdout()} will return partially completed streams while the task is ongoing
  * (and exit code will be null). You can {@link #block()} or {@link #get()} as conveniences on the underlying {@link #getTask()}. */ 
 public abstract class ProcessTaskWrapper<RET> extends ProcessTaskStub implements TaskWrapper<RET> {
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/system/SimpleProcessTaskFactory.java b/core/src/main/java/org/apache/brooklyn/util/core/task/system/SimpleProcessTaskFactory.java
new file mode 100644
index 0000000000..6b4831d2d4
--- /dev/null
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/system/SimpleProcessTaskFactory.java
@@ -0,0 +1,41 @@
+/*
+ * 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.util.core.task.system;
+
+import org.apache.brooklyn.api.mgmt.TaskAdaptable;
+import org.apache.brooklyn.api.mgmt.TaskFactory;
+
+import java.util.Map;
+import java.util.function.Function;
+
+public interface SimpleProcessTaskFactory<SELF extends SimpleProcessTaskFactory<SELF,T0,T,TT>,T0,T,TT extends TaskAdaptable<T>> extends TaskFactory<TT> {
+
+    SELF summary(String summary);
+
+    SELF environmentVariable(String key, String val);
+    SELF environmentVariables(Map<String,String> vars);
+
+    SELF allowingNonZeroExitCode();
+
+    SimpleProcessTaskFactory<?,T0,String,?> returningStdout();
+    SimpleProcessTaskFactory<?,T0,Integer,?> returningExitCodeAllowingNonZero();
+
+    <T2> SimpleProcessTaskFactory<?,T0,T2,?> returning(Function<T0, T2> resultTransformation);
+
+}
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/AbstractProcessTaskFactory.java b/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/AbstractProcessTaskFactory.java
index d463779ba3..f1a22e6e8e 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/AbstractProcessTaskFactory.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/AbstractProcessTaskFactory.java
@@ -20,13 +20,13 @@ package org.apache.brooklyn.util.core.task.system.internal;
 
 import java.util.Arrays;
 import java.util.Map;
+import java.util.function.Function;
 
 import org.apache.brooklyn.api.location.MachineLocation;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
-import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.util.core.task.TaskBuilder;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskFactory;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskStub;
@@ -34,7 +34,6 @@ import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
 import org.apache.brooklyn.util.stream.Streams;
 import org.apache.brooklyn.util.text.Strings;
 
-import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Iterables;
 
@@ -54,7 +53,7 @@ public abstract class AbstractProcessTaskFactory<T extends AbstractProcessTaskFa
     protected void markDirty() {
         dirty = true;
     }
-    
+
     @Override
     public T add(String ...commandsToAdd) {
         markDirty();
@@ -114,6 +113,16 @@ public abstract class AbstractProcessTaskFactory<T extends AbstractProcessTaskFa
         return this.<String>returning(ScriptReturnType.STDOUT_STRING);
     }
 
+    @Override
+    public ProcessTaskFactory<String> returningStdout() {
+        return returning(ScriptReturnType.STDOUT_STRING);
+    }
+
+    @Override
+    public ProcessTaskFactory<Integer> returningExitCodeAllowingNonZero() {
+        return allowingNonZeroExitCode().returning(ScriptReturnType.EXIT_CODE);
+    }
+
     @Override
     @SuppressWarnings("unchecked")
     public <RET2> ProcessTaskFactory<RET2> returning(ScriptReturnType type) {
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/SystemProcessTaskFactory.java b/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/SystemProcessTaskFactory.java
index 56be6cfc6f..3ecbbe6458 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/SystemProcessTaskFactory.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/SystemProcessTaskFactory.java
@@ -21,6 +21,7 @@ package org.apache.brooklyn.util.core.task.system.internal;
 import java.io.File;
 
 import org.apache.brooklyn.api.location.MachineLocation;
+import org.apache.brooklyn.util.core.task.system.SimpleProcessTaskFactory;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.apache.brooklyn.core.config.Sanitizer;
diff --git a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskFactory.java b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskFactory.java
index 40d01a47d7..45b17058fa 100644
--- a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskFactory.java
+++ b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskFactory.java
@@ -21,7 +21,6 @@ package org.apache.brooklyn.tasks.kubectl;
 import com.google.common.collect.Lists;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.Task;
-import org.apache.brooklyn.api.mgmt.TaskFactory;
 import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
 import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.EntityInitializers;
@@ -38,6 +37,7 @@ import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskFactory;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskStub;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
+import org.apache.brooklyn.util.core.task.system.SimpleProcessTaskFactory;
 import org.apache.brooklyn.util.core.task.system.internal.SystemProcessTaskFactory;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.text.Strings;
@@ -60,7 +60,7 @@ import java.util.stream.Collectors;
 
 import static org.apache.brooklyn.tasks.kubectl.ContainerCommons.*;
 
-public class ContainerTaskFactory<T extends ContainerTaskFactory<T,RET>,RET> implements TaskFactory<Task<RET>> {
+public class ContainerTaskFactory<T extends ContainerTaskFactory<T,RET>,RET> implements SimpleProcessTaskFactory<T,ContainerTaskFactory.ContainerTaskResult,RET, Task<RET>> {
 
     private static final Logger LOG = LoggerFactory.getLogger(ContainerTaskFactory.class);
 
@@ -88,7 +88,7 @@ public class ContainerTaskFactory<T extends ContainerTaskFactory<T,RET>,RET> imp
                         if (!argumentsCfg.isEmpty()) LOG.warn("Ignoring 'args' "+argumentsCfg+" because bashScript is set");
 
                         commandsCfg = MutableList.of("/bin/bash", "-c");
-                        List<Object> argumentsCfgO = bashScript instanceof Iterable ? MutableList.copyOf((Iterable) commandsCfg) : MutableList.of(bashScript);
+                        List<Object> argumentsCfgO = bashScript instanceof Iterable ? MutableList.copyOf((Iterable) bashScript) : MutableList.of(bashScript);
                         argumentsCfg = MutableList.of(argumentsCfgO.stream().map(x -> ""+x).collect(Collectors.joining("\n")));
                     }
 
@@ -126,9 +126,9 @@ public class ContainerTaskFactory<T extends ContainerTaskFactory<T,RET>,RET> imp
                             .withImage(containerImage)
                             .withImagePullPolicy(containerImagePullPolicy)
                             .withName(containerName)
+                            .withCommand(Lists.newArrayList(commandsCfg))
                             .withArgs(argumentsCfg)
                             .withEnv(env)
-                            .withCommand(Lists.newArrayList(commandsCfg))
                             .withVolumeMounts(volumeMounts)
                             .withVolumes(volumes)
                             .withWorkingDir(workingDir)
@@ -263,7 +263,7 @@ public class ContainerTaskFactory<T extends ContainerTaskFactory<T,RET>,RET> imp
                             else result.mainExitCode = -1;
 
                             if (result.mainExitCode!=0 && config.get(REQUIRE_EXIT_CODE_ZERO)) {
-                                LOG.info("Failed container job "+namespace+" (exit code "+result.mainExitCode+") output: "+result.mainExitCode);
+                                LOG.info("Failed container job "+namespace+" (exit code "+result.mainExitCode+") output: "+result.mainStdout);
                                 throw new IllegalStateException("Non-zero exit code (" + result.mainExitCode + ") disallowed");
                             }
 
@@ -298,26 +298,39 @@ public class ContainerTaskFactory<T extends ContainerTaskFactory<T,RET>,RET> imp
     }
     public T bashScriptCommands(String firstCommandAndArgs, String ...otherCommandAndArgs) { return bashScriptCommands(MutableList.of(firstCommandAndArgs).appendAll(Arrays.asList(otherCommandAndArgs))); }
     public T image(String image) { config.put(CONTAINER_IMAGE, image); return self(); }
-    public T allowNonZeroExitCode() { return allowNonZeroExitCode(true); }
-    public T allowNonZeroExitCode(boolean allowNonZero) { config.put(REQUIRE_EXIT_CODE_ZERO, !allowNonZero); return self(); }
+    public T allowingNonZeroExitCode() { return allowingNonZeroExitCode(true); }
+    public T allowingNonZeroExitCode(boolean allowNonZero) { config.put(REQUIRE_EXIT_CODE_ZERO, !allowNonZero); return self(); }
     public T imagePullPolicy(PullPolicy policy) { config.put(CONTAINER_IMAGE_PULL_POLICY, policy); return self(); }
-    public T env(Map<String,?> map) {
+    @Override
+    public T environmentVariables(Map<String,String> map) {
+        return environmentVariablesRaw(map);
+    }
+    public T environmentVariablesRaw(Map<String,?> map) {
         config.put(BrooklynConfigKeys.SHELL_ENVIRONMENT, MutableMap.copyOf( map ) );
         return self();
     }
-    public T env(String key, Object val) {
-        return env(MutableMap.copyOf( config.get(BrooklynConfigKeys.SHELL_ENVIRONMENT) ).add(key, val));
+
+    @Override
+    public T environmentVariable(String key, String val) {
+        return this.environmentVariableRaw(key, (Object)val);
     }
-    public <RET2,T2 extends ContainerTaskFactory<T2,RET2>> T2 returning(Function<ContainerTaskResult,RET2> conversion) {
-        T2 result = (T2) self();
+    public T environmentVariableRaw(String key, Object val) {
+        return environmentVariablesRaw(MutableMap.copyOf( config.get(BrooklynConfigKeys.SHELL_ENVIRONMENT) ).add(key, val));
+    }
+
+    @Override
+    public <RET2> ContainerTaskFactory<?,RET2> returning(Function<ContainerTaskResult,RET2> conversion) {
+        ContainerTaskFactory<?,RET2> result = (ContainerTaskFactory<?,RET2>) self();
         result.returnConversion = conversion;
         return result;
     }
-    public <T2 extends ContainerTaskFactory<T2,String>> T2 returningStdout() {
+    @Override
+    public ContainerTaskFactory<?,String> returningStdout() {
         return returning(ContainerTaskResult::getMainStdout);
     }
-    public <T2 extends ContainerTaskFactory<T2,Integer>> T2 returningExitCode() {
-        return returning(ContainerTaskResult::getMainExitCode);
+    @Override
+    public ContainerTaskFactory<?,Integer> returningExitCodeAllowingNonZero() {
+        return allowingNonZeroExitCode().returning(ContainerTaskResult::getMainExitCode);
     }
 
     /** specify the namespace to use, and whether to create or delete it. by default a randomly generated namespace is used and always cleaned up,
diff --git a/software/base/src/test/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskTest.java b/software/base/src/test/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskTest.java
index fa9c61df1c..828f9be068 100644
--- a/software/base/src/test/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskTest.java
+++ b/software/base/src/test/java/org/apache/brooklyn/tasks/kubectl/ContainerTaskTest.java
@@ -53,7 +53,7 @@ public class ContainerTaskTest extends BrooklynAppUnitTestSupport {
                 .imagePullPolicy(PullPolicy.IF_NOT_PRESENT)
                 .timeout(Duration.TWO_MINUTES)
                 .image("perl")
-                .command( "/bin/bash", "-c","echo 'hello test'" )
+                .command( "/bin/bash", "-c", "echo 'hello test'" )
                 .newTask();
 
         DynamicTasks.queueIfPossible(containerTask).orSubmitAsync(entity);
@@ -71,7 +71,7 @@ public class ContainerTaskTest extends BrooklynAppUnitTestSupport {
                 .imagePullPolicy(PullPolicy.IF_NOT_PRESENT)
                 .timeout(Duration.TWO_MINUTES)
                 .image("perl")
-                .env("test_name", "EnvTest")
+                .environmentVariable("test_name", "EnvTest")
                 .bashScriptCommands("echo hello ${test_name}" )
                 .newTask();
 
@@ -110,7 +110,7 @@ public class ContainerTaskTest extends BrooklynAppUnitTestSupport {
                 .timeout(Duration.TWO_MINUTES)
                 .image("perl")
                 .command( "/bin/bash", "-c","echo 'hello test' && exit 42" )
-                .allowNonZeroExitCode()
+                .allowingNonZeroExitCode()
                 .newTask();
         DynamicTasks.queueIfPossible(containerTask).orSubmitAsync(entity);
 
@@ -132,7 +132,7 @@ public class ContainerTaskTest extends BrooklynAppUnitTestSupport {
                 .timeout(Duration.TWO_MINUTES)
                 .image("perl")
                 .bashScriptCommands("echo starting", "sleep 6", "echo done", "exit 42", "echo ignored")
-                .allowNonZeroExitCode()
+                .allowingNonZeroExitCode()
                 .newTask();
         DynamicTasks.queueIfPossible(containerTask).orSubmitAsync(entity);
 
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/PlainWinRmExecTaskFactory.java b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/PlainWinRmExecTaskFactory.java
index 7d719ddc0a..5c04659fac 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/PlainWinRmExecTaskFactory.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/PlainWinRmExecTaskFactory.java
@@ -19,9 +19,10 @@
 package org.apache.brooklyn.location.winrm;
 
 import com.google.common.annotations.Beta;
-import com.google.common.base.Function;
 import java.io.ByteArrayOutputStream;
 import java.io.OutputStreamWriter;
+import java.util.function.Function;
+
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.util.core.internal.winrm.winrm4j.PrettyXmlWriter;
 import org.apache.brooklyn.util.core.task.TaskBuilder;