Merge pull request #123 from ndeloof/JENKINS-33510

[FIX JENKINS-33510] exec --workdir has been introduced in docker 17.12
jenkinsci · Jan 8, 2018 · 5aa1db2 · 5aa1db2
2 parents ed59809 + ba96994
commit 5aa1db2
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 7 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java b/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java
@@ -185,7 +185,7 @@ public static class Execution extends AbstractStepExecutionImpl {
             DockerFingerprints.addRunFacet(dockerClient.getContainerRecord(env, container), run);
             ImageAction.add(step.image, run);
             getContext().newBodyInvoker().
-                    withContext(BodyInvoker.mergeLauncherDecorators(getContext().get(LauncherDecorator.class), new Decorator(container, envHost, ws, toolName))).
+                    withContext(BodyInvoker.mergeLauncherDecorators(getContext().get(LauncherDecorator.class), new Decorator(container, envHost, ws, toolName, dockerVersion))).
                     withCallback(new Callback(container, toolName)).
                     start();
             return false;
@@ -212,12 +212,16 @@ private static class Decorator extends LauncherDecorator implements Serializable
         private final String[] envHost;
         private final String ws;
         private final @CheckForNull String toolName;
+        private final boolean hasEnv;
+        private final boolean hasWorkdir;
 
-        Decorator(String container, EnvVars envHost, String ws, String toolName) {
+        Decorator(String container, EnvVars envHost, String ws, String toolName, VersionNumber dockerVersion) {
             this.container = container;
             this.envHost = Util.mapToEnv(envHost);
             this.ws = ws;
             this.toolName = toolName;
+            this.hasEnv = dockerVersion != null && dockerVersion.compareTo(new VersionNumber("1.13.0")) >= 0;
+            this.hasWorkdir = dockerVersion != null && dockerVersion.compareTo(new VersionNumber("17.12")) >= 0;
         }
 
         @Override public Launcher decorate(final Launcher launcher, final Node node) {
@@ -229,13 +233,18 @@ private static class Decorator extends LauncherDecorator implements Serializable
                     } catch (InterruptedException x) {
                         throw new IOException(x);
                     }
-                    List<String> prefix = new ArrayList<>(Arrays.asList(executable, "exec", container, "env"));
+                    List<String> prefix = new ArrayList<>(Arrays.asList(executable, "exec"));
                     if (ws != null) {
                         FilePath cwd = starter.pwd();
                         if (cwd != null) {
                             String path = cwd.getRemote();
                             if (!path.equals(ws)) {
-                                launcher.getListener().getLogger().println("JENKINS-33510: working directory will be " + ws + " not " + path);
+                                if (hasWorkdir) {
+                                    prefix.add("--workdir");
+                                    prefix.add(path);
+                                } else {
+                                    launcher.getListener().getLogger().println("Docker version is older than 17.12, working directory will be " + ws + " not " + path);
+                                }
                             }
                         }
                     } // otherwise we are loading an old serialized Decorator
@@ -250,7 +259,17 @@ private static class Decorator extends LauncherDecorator implements Serializable
                         }
                     }
                     LOGGER.log(Level.FINE, "(exec) reduced environment: {0}", envReduced);
-                    prefix.addAll(envReduced);
+                    if (hasEnv) {
+                        for (String e : envReduced) {
+                            prefix.add("--env");
+                            prefix.add(e);
+                        }
+                        prefix.add(container);
+                    } else {
+                        prefix.add(container);
+                        prefix.add("env");
+                        prefix.addAll(envReduced);
+                    }
 
                     // Adapted from decorateByPrefix:
                     starter.cmds().addAll(0, prefix);

diff --git a/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java b/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java
@@ -32,6 +32,8 @@
 import java.io.File;
 import java.util.Collections;
 import java.util.logging.Level;
+
+import hudson.util.VersionNumber;
 import org.apache.commons.fileupload.FileItem;
 import org.apache.commons.io.FileUtils;
 import org.hamcrest.Matchers;
@@ -256,12 +258,11 @@ public class WithContainerStepTest {
         });
     }
 
-    @Ignore("TODO no fix yet")
     @Issue("JENKINS-33510")
     @Test public void cd() throws Exception {
         story.addStep(new Statement() {
             @Override public void evaluate() throws Throwable {
-                DockerTestUtil.assumeDocker();
+                DockerTestUtil.assumeDocker(new VersionNumber("17.12"));
                 WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "prj");
                 p.setDefinition(new CpsFlowDefinition(
                     "node {\n" +