Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-40101

Different behavior between debian container using docker.inside

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Minor Minor
    • docker-workflow-plugin
    • None
    • Jenkins 2.19.3 on a Debian 8, remote slave agents running on empty Debian 8 with only docker

      Hi,

      I have a weird behavior when I try to use the docker image debian:wheezy. Whatever I try to do inside this container, it looks like docker manager don't wait for the command to be proceed.

      Here is the Jenkinsfile I'm using:

      #!groovy
      stage('run') {
          node('docker') {
              docker.image('debian:wheezy').inside {
                  sh "sleep 30s"
              }
          }
      }
      

      The image is well pulled and a container is well ran but when it comes to do the sleep command, well, it just pass and return a bad exit code and output

      ERROR: script returned exit code -1
      

      If you replace debian:wheezy with anything else like debian:jessie or ubuntu:whatever it works.

      Basically, the container don't seems to have any issue by itself but, when used on a Jenkinsfile pipeline using this plugin its seems to be broken. Actually I don't know if the issue is the plugin or the container: enlight me please !

      PS: Thx for your amazing job <3

          [JENKINS-40101] Different behavior between debian container using docker.inside

          Andras Kovi added a comment -

          Interesting, this error manifests only with the 'wheezy' image. 'jessie' works fine.

          Andras Kovi added a comment - Interesting, this error manifests only with the 'wheezy' image. 'jessie' works fine.

          Jesse Glick added a comment -

          Reproduced in a test. No idea yet what the issue is.

          Jesse Glick added a comment - Reproduced in a test. No idea yet what the issue is.

          Code changed in jenkins
          User: Jesse Glick
          Path:
          src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java
          http://jenkins-ci.org/commit/docker-workflow-plugin/528e48af56739f6f60ad79e157e4bddd7f1ea503
          Log:
          JENKINS-40101 Reproduced bug in test.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java http://jenkins-ci.org/commit/docker-workflow-plugin/528e48af56739f6f60ad79e157e4bddd7f1ea503 Log: JENKINS-40101 Reproduced bug in test.

          Code changed in jenkins
          User: Jesse Glick
          Path:
          src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java
          http://jenkins-ci.org/commit/docker-workflow-plugin/2f2486a16bf20a9838624c88fdd03f134f87dec2
          Log:
          Merge pull request #89 from jglick/wheezy-JENKINS-40101

          JENKINS-40101 Reproduced bug in test

          Compare: https://github.com/jenkinsci/docker-workflow-plugin/compare/67767f5a9a6f...2f2486a16bf2

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java http://jenkins-ci.org/commit/docker-workflow-plugin/2f2486a16bf20a9838624c88fdd03f134f87dec2 Log: Merge pull request #89 from jglick/wheezy- JENKINS-40101 JENKINS-40101 Reproduced bug in test Compare: https://github.com/jenkinsci/docker-workflow-plugin/compare/67767f5a9a6f...2f2486a16bf2

          I think I tracked this down here: http://stackoverflow.com/questions/43835347/jenkins-docker-pipeline-exit-code-1/43879172#43879172

           

          There is an assumption with inside() that all containers will have `ps` installed. withRun does not display this issue. Jessie has ps installed by default, wheezy doesn't.

           

          Here's a Jenkinsfile to show off the issue:

          node("docker") {
          
          // Weezy that also ran... apt-get update && apt-get install -y procps
          def wheezy_image = docker.image("smalone/weezy-ps-test")
          wheezy_image.pull()
          wheezy_image.inside {
          sh 'sleep 2'
          }
          
          // Base image for weezy-ps-test that has no ps installed using withRun() instead of inside()
          wheezy_image = docker.image("debian:wheezy")
          wheezy_image.pull()
          wheezy_image.withRun { c ->
          sh 'sleep 2'
          }
          
          // Base image for weezy-ps-test that has no ps installed
          wheezy_image = docker.image("debian:wheezy")
          wheezy_image.pull()
          wheezy_image.inside {
          sh 'sleep 2'
          }
          }

          Spencer Malone added a comment - I think I tracked this down here: http://stackoverflow.com/questions/43835347/jenkins-docker-pipeline-exit-code-1/43879172#43879172   There is an assumption with inside() that all containers will have `ps` installed. withRun does not display this issue. Jessie has ps installed by default, wheezy doesn't.   Here's a Jenkinsfile to show off the issue: node( "docker" ) { // Weezy that also ran... apt-get update && apt-get install -y procps def wheezy_image = docker.image( "smalone/weezy-ps-test" ) wheezy_image.pull() wheezy_image.inside { sh 'sleep 2' } // Base image for weezy-ps-test that has no ps installed using withRun() instead of inside() wheezy_image = docker.image( "debian:wheezy" ) wheezy_image.pull() wheezy_image.withRun { c -> sh 'sleep 2' } // Base image for weezy-ps-test that has no ps installed wheezy_image = docker.image( "debian:wheezy" ) wheezy_image.pull() wheezy_image.inside { sh 'sleep 2' } }

          Jesse Glick added a comment -

          I see. I had actually started work on an alternate implementation that did not use ps but never had a strong reason to continue. FTR my sketch:

          diff --git a/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java b/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java
          index fd90744..75d0fee 100644
          --- a/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java
          +++ b/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java
          @@ -52,6 +52,8 @@ import java.util.logging.Logger;
           import javax.annotation.Nonnull;
           
           import hudson.util.VersionNumber;
          +import java.util.regex.Matcher;
          +import java.util.regex.Pattern;
           import org.jenkinsci.plugins.docker.commons.fingerprint.DockerFingerprints;
           import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl;
           import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
          @@ -182,25 +184,12 @@ public class WithContainerStep extends AbstractStepImpl {
                           }
                           @Override public void kill(Map<String,String> modelEnvVars) throws IOException, InterruptedException {
                               ByteArrayOutputStream baos = new ByteArrayOutputStream();
          -                    if (getInner().launch().cmds("docker", "exec", container, "ps", "-A", "-o", "pid,command", "e").stdout(baos).quiet(true).join() != 0) {
          +                    // Similar information to $(ps -A -o pid,command e) but more reliable formatting:
          +                    if (getInner().launch().cmds("docker", "exec", container, "sh", "-c", "for f in /proc/*/environ; do echo $f; cat $f; echo; done").stdout(baos).quiet(true).join() != 0) {
                                   throw new IOException("failed to run ps");
                               }
          -                    List<String> pids = new ArrayList<String>();
          -                    LINE: for (String line : baos.toString().split("\n")) {
          -                        for (Map.Entry<String,String> entry : modelEnvVars.entrySet()) {
          -                            // TODO this is imprecise: false positive when argv happens to match KEY=value even if environment does not. Cf. trick in BourneShellScript.
          -                            if (!line.contains(entry.getKey() + "=" + entry.getValue())) {
          -                                continue LINE;
          -                            }
          -                        }
          -                        line = line.trim();
          -                        int spc = line.indexOf(' ');
          -                        if (spc == -1) {
          -                            continue;
          -                        }
          -                        pids.add(line.substring(0, spc));
          -                    }
          -                    LOGGER.log(Level.FINE, "killing {0}", pids);
          +                    List<String> pids = pidsMatching(baos.toString(), modelEnvVars);
          +                    LOGGER.log(Level.INFO, "killing {0}", pids);
                               if (!pids.isEmpty()) {
                                   List<String> cmds = new ArrayList<String>(Arrays.asList("docker", "exec", container, "kill"));
                                   cmds.addAll(pids);
          @@ -214,6 +203,17 @@ public class WithContainerStep extends AbstractStepImpl {
           
               }
           
          +    private static final Pattern OUTPUT = Pattern.compile("^/proc/(\\d+)/environ\n(.+?)\\000$", Pattern.MULTILINE | Pattern.DOTALL);
          +    static List<String> pidsMatching(String output, Map<String,String> modelEnvVars) {
          +        List<String> pids = new ArrayList<String>();
          +        Matcher m = OUTPUT.matcher(output);
          +        while (m.find()) {
          +            String[] env = m.group(2).split("\\000");
          +            System.err.println("TODO for " + m.group(1) + " found " + Arrays.toString(env));
          +        }
          +        return pids;
          +    }
          +
               private static class Callback extends BodyExecutionCallback {
           
                   private static final long serialVersionUID = 1;
          diff --git a/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java b/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java
          index 607537a..aab2e7a 100644
          --- a/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java
          +++ b/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java
          @@ -25,6 +25,7 @@ package org.jenkinsci.plugins.docker.workflow;
           
           import hudson.model.Result;
           import hudson.tools.ToolProperty;
          +import java.util.Arrays;
           import java.util.Collections;
           import org.jenkinsci.plugins.docker.commons.tools.DockerTool;
           import org.jenkinsci.plugins.workflow.BuildWatcher;
          @@ -34,6 +35,7 @@ import org.jenkinsci.plugins.workflow.job.WorkflowJob;
           import org.jenkinsci.plugins.workflow.job.WorkflowRun;
           import org.jenkinsci.plugins.workflow.steps.StepConfigTester;
           import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
          +import static org.junit.Assert.*;
           import org.junit.ClassRule;
           import org.junit.Test;
           import org.junit.Rule;
          @@ -79,6 +81,7 @@ public class WithContainerStepTest {
               @Test public void stop() {
                   story.addStep(new Statement() {
                       @Override public void evaluate() throws Throwable {
          +                assertEquals(Arrays.asList("2", "3"), WithContainerStep.pidsMatching("/proc/1/environ\nFOO=bar\000\n/proc/2/environ\nCOOKIE=killme\000FOO=baz\000\n/proc/3/environ\nFOO=quux\000COOKIE=killme\000\n/proc/4/environ\nCOOKIE=donotkillme\000\n", Collections.singletonMap("COOKIE", "killme")));
                           DockerTestUtil.assumeDocker();
                           WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
                           p.setDefinition(new CpsFlowDefinition(
          

          Jesse Glick added a comment - I see. I had actually started work on an alternate implementation that did not use ps but never had a strong reason to continue. FTR my sketch: diff --git a/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java b/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java index fd90744..75d0fee 100644 --- a/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java +++ b/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java @@ -52,6 +52,8 @@ import java.util.logging.Logger; import javax.annotation.Nonnull; import hudson.util.VersionNumber; + import java.util.regex.Matcher; + import java.util.regex.Pattern; import org.jenkinsci.plugins.docker.commons.fingerprint.DockerFingerprints; import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl; import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl; @@ -182,25 +184,12 @@ public class WithContainerStep extends AbstractStepImpl { } @Override public void kill(Map< String , String > modelEnvVars) throws IOException, InterruptedException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); - if (getInner().launch().cmds( "docker" , "exec" , container, "ps" , "-A" , "-o" , "pid,command" , "e" ).stdout(baos).quiet( true ).join() != 0) { + // Similar information to $(ps -A -o pid,command e) but more reliable formatting: + if (getInner().launch().cmds( "docker" , "exec" , container, "sh" , "-c" , " for f in /proc/*/environ; do echo $f; cat $f; echo; done" ).stdout(baos).quiet( true ).join() != 0) { throw new IOException( "failed to run ps" ); } - List< String > pids = new ArrayList< String >(); - LINE: for ( String line : baos.toString().split( "\n" )) { - for (Map.Entry< String , String > entry : modelEnvVars.entrySet()) { - // TODO this is imprecise: false positive when argv happens to match KEY=value even if environment does not. Cf. trick in BourneShellScript. - if (!line.contains(entry.getKey() + "=" + entry.getValue())) { - continue LINE; - } - } - line = line.trim(); - int spc = line.indexOf( ' ' ); - if (spc == -1) { - continue ; - } - pids.add(line.substring(0, spc)); - } - LOGGER.log(Level.FINE, "killing {0}" , pids); + List< String > pids = pidsMatching(baos.toString(), modelEnvVars); + LOGGER.log(Level.INFO, "killing {0}" , pids); if (!pids.isEmpty()) { List< String > cmds = new ArrayList< String >(Arrays.asList( "docker" , "exec" , container, "kill" )); cmds.addAll(pids); @@ -214,6 +203,17 @@ public class WithContainerStep extends AbstractStepImpl { } + private static final Pattern OUTPUT = Pattern.compile( "^/proc/(\\d+)/environ\n(.+?)\\000$" , Pattern.MULTILINE | Pattern.DOTALL); + static List< String > pidsMatching( String output, Map< String , String > modelEnvVars) { + List< String > pids = new ArrayList< String >(); + Matcher m = OUTPUT.matcher(output); + while (m.find()) { + String [] env = m.group(2).split( "\\000" ); + System .err.println( "TODO for " + m.group(1) + " found " + Arrays.toString(env)); + } + return pids; + } + private static class Callback extends BodyExecutionCallback { private static final long serialVersionUID = 1; diff --git a/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java b/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java index 607537a..aab2e7a 100644 --- a/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java @@ -25,6 +25,7 @@ package org.jenkinsci.plugins.docker.workflow; import hudson.model.Result; import hudson.tools.ToolProperty; + import java.util.Arrays; import java.util.Collections; import org.jenkinsci.plugins.docker.commons.tools.DockerTool; import org.jenkinsci.plugins.workflow.BuildWatcher; @@ -34,6 +35,7 @@ import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.steps.StepConfigTester; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; + import static org.junit.Assert.*; import org.junit.ClassRule; import org.junit.Test; import org.junit.Rule; @@ -79,6 +81,7 @@ public class WithContainerStepTest { @Test public void stop() { story.addStep( new Statement() { @Override public void evaluate() throws Throwable { + assertEquals(Arrays.asList( "2" , "3" ), WithContainerStep.pidsMatching( "/proc/1/environ\nFOO=bar\000\n/proc/2/environ\nCOOKIE=killme\000FOO=baz\000\n/proc/3/environ\nFOO=quux\000COOKIE=killme\000\n/proc/4/environ\nCOOKIE=donotkillme\000\n" , Collections.singletonMap( "COOKIE" , "killme" ))); DockerTestUtil.assumeDocker(); WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p" ); p.setDefinition( new CpsFlowDefinition(

          We ran into this as well. Essentially we're going to add ps to our base images but we'll have to be vigilant about making sure that people who use our system don't use images without `ps` or they'll run into this. It'd be nice if either the fix jglick mentioned were implemented or at least there was some diagnostic messages to help the user. Might be something I could poke at if I get time. 

          Justin Harringa added a comment - We ran into this as well. Essentially we're going to add ps to our base images but we'll have to be vigilant about making sure that people who use our system don't use images without `ps` or they'll run into this. It'd be nice if either the fix jglick mentioned were implemented or at least there was some diagnostic messages to help the user. Might be something I could poke at if I get time. 

          Jesse Glick added a comment -

          Distinct from JENKINS-47791 I think, as this is a different use of ps.

          Jesse Glick added a comment - Distinct from  JENKINS-47791 I think, as this is a different use of ps .

            Unassigned Unassigned
            clementgautier Clement Gautier
            Votes:
            8 Vote for this issue
            Watchers:
            13 Start watching this issue

              Created:
              Updated: