[JENKINS-53500] Windows support for container step

README.md

@@ -321,37 +321,12 @@ slaveTemplates.dockerTemplate {
 There are cases where this implicit inheritance via nested declaration is not wanted or another explicit inheritance is preferred.
 In this case, use `inheritFrom ''` to remove any inheritance, or `inheritFrom 'otherParent'` to override it.
 
-```groovy
-    podTemplate(label: 'docker-linux', containers: [containerTemplate(image: 'docker', name: 'docker-linux', command: 'cat', ttyEnabled: true)]) {
-        // Will run on linux node
-        podTemplate(label: 'maven-windows', inheritFrom: '', nodeSelector: 'os:windows', containers: [containerTemplate(image: 'maven-windows-servercore', name: 'maven-windows', command: 'cat', ttyEnabled: true)]) {
-            // Will run on windows node without merging the docker pod
-        }
-    }
-```
-
 #### Using a different namespace
 
 There might be cases, where you need to have the agent pod run inside a different namespace than the one configured with the cloud definition.
 For example you may need the agent to run inside an `ephemeral` namespace for the sake of testing.
 For those cases you can explicitly configure a namespace either using the ui or the pipeline.
 
-#### Specifying a different shell command other than /bin/sh
-
-By default, the shell command is /bin/sh. In some case, you would like to use another shell command like /bin/bash.
-
-```groovy
-podTemplate {
-  node(POD_LABEL) {
-    stage('Run specific shell') {
-      container(name:'mycontainer', shell:'/bin/bash') {
-        sh 'echo hello world'
-      }
-    }
-  }
-}
-```
-
 ## Container Configuration
 When configuring a container in a pipeline podTemplate the following options are available:
 
@@ -567,6 +542,11 @@ containerLog 'mongodb'
 
 Also see the online help and [examples/containerLog.groovy](examples/containerLog.groovy).
 
+# Windows support
+
+You can run pods on Windows if your cluster has Windows nodes.
+See the [example](examples/windows.groovy).
+
 # Constraints
 
 Multiple containers can be defined in a pod.

examples/windows.groovy

@@ -0,0 +1,27 @@
+/*
+ * Runs a build on a Windows pod.
+ * Tested in EKS: https://docs.aws.amazon.com/eks/latest/userguide/windows-support.html
+ */
+podTemplate(yaml: '''
+apiVersion: v1
+kind: Pod
+spec:
+  containers:
+  - name: jnlp
+    image: jenkins/jnlp-agent:latest-windows
+  - name: shell
+    image: mcr.microsoft.com/powershell:preview-windowsservercore-1809
+    command:
+    - powershell
+    args:
+    - Start-Sleep
+    - 999999
+  nodeSelector:
+    beta.kubernetes.io/os: windows
+''') {
+    node(POD_LABEL) {
+        container('shell') {
+            powershell 'Get-ChildItem Env: | Sort Name'
+        }
+    }
+}

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java

@@ -211,6 +211,13 @@ public Pod build() {
             pod.getSpec().setRestartPolicy("Never");
         }
 
+        // default OS: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/
+        if ((pod.getSpec().getNodeSelector() == null || pod.getSpec().getNodeSelector().isEmpty()) &&
+                (pod.getSpec().getAffinity() == null || pod.getSpec().getAffinity().getNodeAffinity() == null)) {
+            // TODO kubernetes.io/os for 1.14+? but: https://github.com/aws/containers-roadmap/issues/542
+            pod.getSpec().setNodeSelector(Collections.singletonMap("beta.kubernetes.io/os", "linux"));
+        }
+
         // default jnlp container
         Optional<Container> jnlpOpt = pod.getSpec().getContainers().stream().filter(c -> JNLP_NAME.equals(c.getName()))
                 .findFirst();

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/Constants.java

@@ -6,7 +6,7 @@
 public class Constants {
 
     public static final String EXIT = "exit";
-    public static final String NEWLINE = "\n";
+    public static final String NEWLINE = "\n"; // seems to work even on Windows
     public static final char CTRL_C = '\u0003';
     public static final String SPACE = " ";
 }

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java

@@ -80,7 +80,6 @@ public class ContainerExecDecorator extends LauncherDecorator implements Seriali
     private static final String COOKIE_VAR = "JENKINS_SERVER_COOKIE";
 
     private static final Logger LOGGER = Logger.getLogger(ContainerExecDecorator.class.getName());
-    private static final String DEFAULT_SHELL = "sh";
 
     /**
      * stdin buffer size for commands sent to Kubernetes exec api. A low value will cause slowness in commands executed.
@@ -110,7 +109,6 @@ public ContainerExecDecorator(KubernetesClient client, String podName, String co
         this.containerName = containerName;
         this.environmentExpander = environmentExpander;
         this.ws = ws;
-        this.shell = DEFAULT_SHELL;
     }
 
     @Deprecated
@@ -224,10 +222,6 @@ public void setWs(FilePath ws) {
         this.ws = ws;
     }
 
-    public String getShell() {
-        return shell == null? DEFAULT_SHELL:shell;
-    }
-
     public void setShell(String shell) {
         this.shell = shell;
     }
@@ -294,7 +288,8 @@ private Proc doLaunch(boolean quiet, String[] cmdEnvs, OutputStream outputForCal
                 }
                 ByteArrayOutputStream error = new ByteArrayOutputStream();
 
-                String msg = "Executing shell script inside container [" + containerName + "] of pod [" + getPodName() + "]";
+                String sh = shell != null ? shell : launcher.isUnix() ? "sh" : "cmd";
+                String msg = "Executing " + sh + " script inside container " + containerName + " of pod " + getPodName();
                 LOGGER.log(Level.FINEST, msg);
                 printStream.println(msg);
 
@@ -341,7 +336,7 @@ public void onClose(int i, String s) {
 
                 ExecWatch watch;
                 try {
-                    watch = execable.exec(getShell());
+                    watch = execable.exec(sh);
                 } catch (KubernetesClientException e) {
                     if (e.getCause() instanceof InterruptedException) {
                         throw new IOException(
@@ -376,6 +371,7 @@ public void onClose(int i, String s) {
 
                 try {
                     OutputStream stdin = watch.getInput();
+                    // stdin = new TeeOutputStream(stdin, new LogTaskListener(LOGGER, Level.FINEST).getLogger());
                     if (pwd != null) {
                         // We need to get into the project workspace.
                         // The workspace is not known in advance, so we have to execute a cd command.
@@ -406,7 +402,7 @@ public void onClose(int i, String s) {
 
                     LOGGER.log(Level.FINEST, "Launching with env vars: {0}", envVars.toString());
 
-                    this.setupEnvironmentVariable(envVars, stdin);
+                    this.setupEnvironmentVariable(envVars, stdin, sh.equals("cmd"));
 
                     doExec(stdin, printStream, masks, commands);
 
@@ -431,21 +427,22 @@ public void kill(Map<String, String> modelEnvVars) throws IOException, Interrupt
 
                 int exitCode = doLaunch(
                         true, null, null, null, null,
+                        // TODO Windows
                         "sh", "-c", "kill \\`grep -l '" + COOKIE_VAR + "=" + cookie  +"' /proc/*/environ | cut -d / -f 3 \\`"
                 ).join();
 
                 getListener().getLogger().println("kill finished with exit code " + exitCode);
             }
 
-            private void setupEnvironmentVariable(EnvVars vars, OutputStream out) throws IOException {
+            private void setupEnvironmentVariable(EnvVars vars, OutputStream out, boolean windows) throws IOException {
                 for (Map.Entry<String, String> entry : vars.entrySet()) {
                     //Check that key is bash compliant.
                     if (entry.getKey().matches("[a-zA-Z_][a-zA-Z0-9_]*")) {
                             out.write(
                                     String.format(
-                                            "export %s='%s'%s",
+                                            windows ? "set %s=%s%s" : "export %s='%s'%s",
                                             entry.getKey(),
-                                            entry.getValue().replace("'", "'\\''"),
+                                            windows ? entry.getValue() : entry.getValue().replace("'", "'\\''"),
                                             NEWLINE
                                     ).getBytes(StandardCharsets.UTF_8)
                             );

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerLogStep.java

@@ -103,14 +103,9 @@ public String getDisplayName() {
             return "Get container log from Kubernetes";
         }
 
-        @Override
-        public boolean takesImplicitBlockArgument() {
-            return false;
-        }
-
         @Override
         public boolean isAdvanced() {
-            return false;
+            return true;
         }
 
         @Override

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStep.java

@@ -14,6 +14,7 @@
 
 import hudson.Extension;
 import hudson.FilePath;
+import hudson.Util;
 import hudson.model.Node;
 import hudson.model.TaskListener;
 
@@ -35,7 +36,7 @@ public String getName() {
 
     @DataBoundSetter
     public void setShell(String shell){
-        this.shell = shell;
+        this.shell = Util.fixEmpty(shell);
     }
 
     public String getShell() {
@@ -65,11 +66,6 @@ public boolean takesImplicitBlockArgument() {
             return true;
         }
 
-        @Override
-        public boolean isAdvanced() {
-            return true;
-        }
-
         @Override
         public Set<? extends Class<?>> getRequiredContext() {
             return ImmutableSet.of(Node.class, FilePath.class, TaskListener.class);

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java

@@ -312,11 +312,6 @@ public boolean takesImplicitBlockArgument() {
             return true;
         }
 
-        @Override
-        public boolean isAdvanced() {
-            return true;
-        }
-
         @Override
         public Set<? extends Class<?>> getRequiredContext() {
             return ImmutableSet.of(Run.class, TaskListener.class);

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/SecretsMasker.java

@@ -19,6 +19,7 @@
 import hudson.Extension;
 import hudson.console.LineTransformationOutputStream;
 import hudson.remoting.Channel;
+import hudson.util.LogTaskListener;
 import io.fabric8.kubernetes.api.model.Container;
 import io.fabric8.kubernetes.api.model.EnvVar;
 import io.fabric8.kubernetes.api.model.EnvVarSource;
@@ -30,6 +31,7 @@
 import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.charset.StandardCharsets;
+import java.security.GeneralSecurityException;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
@@ -158,8 +160,13 @@ protected TaskListenerDecorator get(DelegatedContext context) throws IOException
                     LOGGER.fine(() -> "looking for " + slave.getNamespace() + "/" + slave.getPodName() + "/" + containerName + " secrets named " + secretContainerKeys);
                     ByteArrayOutputStream baos = new ByteArrayOutputStream();
                     Semaphore semaphore = new Semaphore(0);
-                    try (ExecWatch exec = slave.getKubernetesCloud().connect().pods().inNamespace(slave.getNamespace()).withName(slave.getPodName()).inContainer(containerName)
-                            .writingOutput(baos).writingError(System.err).writingErrorChannel(System.err)
+                    Boolean unix = c.isUnix();
+                    if (unix == null) {
+                        return null;
+                    }
+                    try (OutputStream errs = new LogTaskListener(LOGGER, Level.FINE).getLogger();
+                         ExecWatch exec = slave.getKubernetesCloud().connect().pods().inNamespace(slave.getNamespace()).withName(slave.getPodName()).inContainer(containerName)
+                            .writingOutput(baos).writingError(errs).writingErrorChannel(errs)
                             .usingListener(new ExecListener() {
                                 @Override
                                 public void onOpen(Response response) {
@@ -173,14 +180,14 @@ public void onClose(int code, String reason) {
                                     semaphore.release();
                                 }
                             })
-                            .exec("env")) {
+                            .exec(unix ? new String[] {"env"} : new String[] {"cmd", "/c", "set"})) {
                         if (!semaphore.tryAcquire(10, TimeUnit.SECONDS)) {
                             LOGGER.fine(() -> "time out trying to find environment from " + slave.getNamespace() + "/" + slave.getPodName() + "/" + containerName);
                         }
-                    } catch (Exception x) {
+                    } catch (RuntimeException | GeneralSecurityException x) {
                         LOGGER.log(Level.FINE, "failed to find environment from " + slave.getNamespace() + "/" + slave.getPodName() + "/" + containerName, x);
                     }
-                    for (String line : baos.toString(StandardCharsets.UTF_8.name()).split("\n")) {
+                    for (String line : baos.toString(StandardCharsets.UTF_8.name()).split("\r?\n")) {
                         int equals = line.indexOf('=');
                         if (equals != -1) {
                             String key = line.substring(0, equals);

src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/help-directConnection.html

@@ -29,6 +29,7 @@
   <p>
     <b>Note:</b> In <i>Direct Connection</i> mode agents will not be able to reconnect to a restarted master if a <i>Random</i> 'TCP port for inbound agents' is configured!<br/>
     <b>Note:</b> <i>Direct Connection</i> requires a <a target="_blank" href="https://hub.docker.com/r/jenkins/jnlp-slave/tags">jnlp-slave</a> image with a version equal or higher than 3.35-5.
+    <br/><b>Note:</b> <i>Direct Connection</i> does not work with the currently available <code>jenkins/jnlp-agent:latest-windows</code> image.
   </p>
 
 </div>

src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStep/config.jelly

@@ -1,14 +1,12 @@
 <?xml version="1.0" encoding="UTF-8"?>
-
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
-    <f:entry field="cloud" title="The cloud to use">
-      <f:textbox default="kubernetes"/>
+    <f:entry field="name" title="${%Container name}">
+        <f:textbox/>
     </f:entry>
-    <f:entry field="pod" title="The name of the pod to use">
-		<f:textbox/>
-	</f:entry>
-	<f:entry field="name" title="The name of the container to select">
-		<f:textbox/>
-	</f:entry>
+    <f:advanced>
+        <f:entry field="shell" title="${%Shell}">
+            <f:textbox/>
+        </f:entry>
+    </f:advanced>
 </j:jelly>

src/main/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerStep/help-shell.html

@@ -0,0 +1,25 @@
+<p>
+    Specifies a shell which will run inside the container
+    and process requests from Jenkins to run executables,
+    set environment variables, and similar infrastructure.
+</p>
+<p>
+    This does <strong>not</strong> affect the shell used to run user code,
+    such as <code>sh</code> steps.
+    To run your scripts with a specific shell on Linux, use an interpreter line:
+</p>
+<pre><code>sh '''#!/bin/bash
+for x in {0..9}; do echo x; done
+'''</code></pre>
+<p>
+    or just use a subprocess and an externally versioned script:
+</p>
+<pre><code>sh 'bash ci.sh'</code></pre>
+<p>
+    On Windows, choose between the <code>bat</code> or <code>powershell</code> steps.
+</p>
+<p>
+    For a pod running on Linux, defaults to <code>sh</code>, which should be in <code>$PATH</code>;
+    for a pod running on Windows, defaults to <code>cmd</code>, which should be in <code>%Path%</code>.
+    Should not generally be overridden.
+</p>

src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesTestUtil.java

@@ -55,6 +55,7 @@
 import com.google.common.collect.Maps;
 
 import io.fabric8.kubernetes.api.model.NamespaceBuilder;
+import io.fabric8.kubernetes.api.model.Node;
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.PodList;
 import io.fabric8.kubernetes.api.model.Secret;
@@ -72,6 +73,7 @@
 import jenkins.model.Jenkins;
 import jenkins.model.JenkinsLocationConfiguration;
 import static org.junit.Assert.*;
+import org.junit.AssumptionViolatedException;
 import org.jvnet.hudson.test.JenkinsRule;
 
 public class KubernetesTestUtil {
@@ -156,6 +158,25 @@ public static void assumeKubernetes() throws Exception {
         }
     }
 
+    /**
+     * Verifies that we are running in a mixed cluster with Windows nodes.
+     * (The cluster is assumed to always have Linux nodes.)
+     * This means that we can run tests involving Windows agent pods.
+     * Note that running the <em>master</em> on Windows is untested.
+     */
+    public static void assumeWindows() {
+        try (KubernetesClient client = new DefaultKubernetesClient(new ConfigBuilder(Config.autoConfigure(null)).build())) {
+            for (Node n : client.nodes().list().getItems()) {
+                String os = n.getMetadata().getLabels().get("kubernetes.io/os");
+                LOGGER.info(() -> "Found node " + n.getMetadata().getName() + " running OS " + os);
+                if ("windows".equals(os)) {
+                    return;
+                }
+            }
+        }
+        throw new AssumptionViolatedException("Cluster seems to contain no Windows nodes");
+    }
+
     public static Map<String, String> getLabels(Object o, TestName name) {
         return getLabels(null, o, name);
     }