Merge pull request jenkinsci#86 from jglick/hang-JENKINS-37719

[JENKINS-37719] Apply a timeout to all Docker CLI operations

Author: jglick

pom.xml

@@ -4,7 +4,7 @@
     <parent>
         <groupId>org.jenkins-ci.plugins</groupId>
         <artifactId>plugin</artifactId>
-        <version>2.17</version>
+        <version>2.21</version>
         <relativePath/>
     </parent>
     <artifactId>docker-workflow</artifactId>
@@ -52,37 +52,37 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-cps</artifactId>
-            <version>2.17</version>
+            <version>2.25</version>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-durable-task-step</artifactId>
-            <version>2.4</version>
+            <version>2.8</version>
         </dependency>
         <dependency> <!-- StepConfigTester -->
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-step-api</artifactId>
             <classifier>tests</classifier>
-            <version>2.2</version>
+            <version>2.7</version>
             <scope>test</scope>
         </dependency>
         <dependency> <!-- SemaphoreStep -->
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-support</artifactId>
             <classifier>tests</classifier>
-            <version>2.1</version>
+            <version>2.12</version>
             <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-job</artifactId>
-            <version>2.3</version>
+            <version>2.9</version>
             <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-basic-steps</artifactId>
-            <version>2.0</version>
+            <version>2.3</version>
             <scope>test</scope>
         </dependency>
         <dependency>

src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java

@@ -57,6 +57,7 @@
 import javax.annotation.Nonnull;
 
 import hudson.util.VersionNumber;
+import java.util.concurrent.TimeUnit;
 import javax.annotation.CheckForNull;
 import org.jenkinsci.plugins.docker.commons.fingerprint.DockerFingerprints;
 import org.jenkinsci.plugins.docker.commons.tools.DockerTool;
@@ -247,7 +248,7 @@ private static class Decorator extends LauncherDecorator implements Serializable
                 @Override public void kill(Map<String,String> modelEnvVars) throws IOException, InterruptedException {
                     ByteArrayOutputStream baos = new ByteArrayOutputStream();
                     String executable = getExecutable();
-                    if (getInner().launch().cmds(executable, "exec", container, "ps", "-A", "-o", "pid,command", "e").stdout(baos).quiet(true).join() != 0) {
+                    if (getInner().launch().cmds(executable, "exec", container, "ps", "-A", "-o", "pid,command", "e").stdout(baos).quiet(true).start().joinWithTimeout(10, TimeUnit.SECONDS, listener) != 0) {
                         throw new IOException("failed to run ps");
                     }
                     List<String> pids = new ArrayList<String>();
@@ -269,7 +270,7 @@ private static class Decorator extends LauncherDecorator implements Serializable
                     if (!pids.isEmpty()) {
                         List<String> cmds = new ArrayList<>(Arrays.asList(executable, "exec", container, "kill"));
                         cmds.addAll(pids);
-                        if (getInner().launch().cmds(cmds).quiet(true).join() != 0) {
+                        if (getInner().launch().cmds(cmds).quiet(true).start().joinWithTimeout(10, TimeUnit.SECONDS, listener) != 0) {
                             throw new IOException("failed to run kill");
                         }
                     }

src/main/java/org/jenkinsci/plugins/docker/workflow/client/DockerClient.java

@@ -45,6 +45,7 @@
 import java.util.Map;
 import java.util.List;
 import java.util.Arrays;
+import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
@@ -264,7 +265,7 @@ private LaunchResult launch(@CheckForNull @Nonnull EnvVars launchEnv, boolean qu
         LaunchResult result = new LaunchResult();
         ByteArrayOutputStream out = new ByteArrayOutputStream();
         ByteArrayOutputStream err = new ByteArrayOutputStream();
-        result.setStatus(procStarter.quiet(quiet).cmds(args).envs(launchEnv).stdout(out).stderr(err).join());
+        result.setStatus(procStarter.quiet(quiet).cmds(args).envs(launchEnv).stdout(out).stderr(err).start().joinWithTimeout(10, TimeUnit.SECONDS, launcher.getListener()));
         final String charsetName = Charset.defaultCharset().name();
         result.setOut(out.toString(charsetName));
         result.setErr(err.toString(charsetName));
@@ -278,10 +279,10 @@ private LaunchResult launch(@CheckForNull @Nonnull EnvVars launchEnv, boolean qu
      */
     public String whoAmI() throws IOException, InterruptedException {
         ByteArrayOutputStream userId = new ByteArrayOutputStream();
-        launcher.launch().cmds("id", "-u").quiet(true).stdout(userId).join();
+        launcher.launch().cmds("id", "-u").quiet(true).stdout(userId).start().joinWithTimeout(10, TimeUnit.SECONDS, launcher.getListener());
 
         ByteArrayOutputStream groupId = new ByteArrayOutputStream();
-        launcher.launch().cmds("id", "-g").quiet(true).stdout(groupId).join();
+        launcher.launch().cmds("id", "-g").quiet(true).stdout(groupId).start().joinWithTimeout(10, TimeUnit.SECONDS, launcher.getListener());
 
         final String charsetName = Charset.defaultCharset().name();
         return String.format("%s:%s", userId.toString(charsetName).trim(), groupId.toString(charsetName).trim());

src/test/java/org/jenkinsci/plugins/docker/workflow/DockerTestUtil.java

@@ -30,6 +30,7 @@
 import org.junit.Assume;
 
 import java.io.IOException;
+import java.util.concurrent.TimeUnit;
 
 import static org.hamcrest.Matchers.is;
 import org.jenkinsci.plugins.docker.commons.tools.DockerTool;
@@ -42,12 +43,14 @@ public class DockerTestUtil {
     public static void assumeDocker() throws Exception {
         Launcher.LocalLauncher localLauncher = new Launcher.LocalLauncher(StreamTaskListener.NULL);
         try {
-            Assume.assumeThat("Docker working", localLauncher.launch().cmds(DockerTool.getExecutable(null, null, null, null), "ps").join(), is(0));
+            Assume.assumeThat("Docker working", localLauncher.launch().cmds(DockerTool.getExecutable(null, null, null, null), "ps").start().joinWithTimeout(10, TimeUnit.SECONDS, localLauncher.getListener()), is(0));
         } catch (IOException x) {
             Assume.assumeNoException("have Docker installed", x);
         }
         DockerClient dockerClient = new DockerClient(localLauncher, null, null);
         Assume.assumeFalse("Docker version not < 1.3", dockerClient.version().isOlderThan(new VersionNumber("1.3")));
     }
+
+    private DockerTestUtil() {}
 
 }

src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java

@@ -31,8 +31,10 @@
 import hudson.tools.ToolProperty;
 import java.io.File;
 import java.util.Collections;
+import java.util.logging.Level;
 import org.apache.commons.fileupload.FileItem;
 import org.apache.commons.io.FileUtils;
+import org.hamcrest.Matchers;
 import org.jenkinsci.lib.configprovider.ConfigProvider;
 import org.jenkinsci.lib.configprovider.model.Config;
 import org.jenkinsci.plugins.configfiles.custom.CustomConfig;
@@ -43,6 +45,7 @@
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.jenkinsci.plugins.workflow.steps.StepConfigTester;
 import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
+import org.junit.Assume;
 import org.junit.ClassRule;
 import org.junit.Ignore;
 import org.junit.Test;
@@ -51,13 +54,15 @@
 import org.junit.runners.model.Statement;
 import org.jvnet.hudson.test.BuildWatcher;
 import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.LoggerRule;
 import org.jvnet.hudson.test.RestartableJenkinsRule;
 
 public class WithContainerStepTest {
 
     @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher();
     @Rule public RestartableJenkinsRule story = new RestartableJenkinsRule();
     @Rule public TemporaryFolder tmp = new TemporaryFolder();
+    @Rule public LoggerRule logging = new LoggerRule();
 
     @Test public void configRoundTrip() {
         story.addStep(new Statement() {
@@ -90,6 +95,34 @@ public class WithContainerStepTest {
         });
     }
 
+    @Issue("JENKINS-37719")
+    @Test public void hungDaemon() {
+        story.addStep(new Statement() {
+            @Override public void evaluate() throws Throwable {
+                DockerTestUtil.assumeDocker();
+                Assume.assumeThat("we are in an interactive environment and can pause dockerd", new ProcessBuilder("sudo", "pgrep", "dockerd").inheritIO().start().waitFor(), Matchers.is(0));
+                logging.record("org.jenkinsci.plugins.workflow.support.concurrent.Timeout", Level.FINE); // TODO use Timeout.class when workflow-support 2.13+
+                WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "prj");
+                p.setDefinition(new CpsFlowDefinition(
+                    "node {\n" +
+                    "  timeout(time: 20, unit: 'SECONDS') {\n" +
+                    "    withDockerContainer('httpd:2.4.12') {\n" +
+                    "      sh 'sleep infinity'\n" +
+                    "    }\n" +
+                    "  }\n" +
+                    "}", true));
+                WorkflowRun b = p.scheduleBuild2(0).waitForStart();
+                story.j.waitForMessage("+ sleep infinity", b);
+                Assume.assumeThat(new ProcessBuilder("sudo", "killall", "-STOP", "dockerd").inheritIO().start().waitFor(), Matchers.is(0));
+                try {
+                    story.j.assertBuildStatus(Result.ABORTED, story.j.waitForCompletion(b));
+                } finally {
+                    Assume.assumeThat(new ProcessBuilder("sudo", "killall", "-CONT", "dockerd").inheritIO().start().waitFor(), Matchers.is(0));
+                }
+            }
+        });
+    }
+
     @Test public void stop() {
         story.addStep(new Statement() {
             @Override public void evaluate() throws Throwable {