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

docker.inside() breaks container environment.

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Major Major
    • docker-workflow-plugin
    • None
    • docker-workflow 1.10

      We have PATH set on our build slaves (via configure -> Node properties -> Environment Variables) that won't work in most docker containers.  Specifically, we use /usr/local/bin:/a/bin.  Note how /usr/bin and /bin aren't in the PATH (which is where cat usually is located.

      Normally, the docker.inside() command's docker run isn't setting the -e PATH=... flag.  But every so often it uses the PATH from the build slave configuration which causes the docker run to not find cat

       

      docker: Error response from daemon: oci runtime error: container_linux.go:247: starting container process caused "exec: \"cat\": executable file not found in $PATH".

       

      I was able to track this down by modifying docker-workflow to not mask the PATH: https://github.com/jenkinsci/docker-workflow-plugin/pull/96

      I don't know what the expected behavior (overriding the PATH on a container seems like a "Bad Idea", since the container may-or-may-not be in LSB/POSIX layout) is supposed to be.

      It is clear that this random flipping between behaviors is bad.

          [JENKINS-43590] docker.inside() breaks container environment.

          Christian Höltje created issue -

          Christian Höltje added a comment - - edited

           

          I found the code at https://github.com/jenkinsci/docker-workflow-plugin/blob/master/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java#L126-L128

          EnvVars envReduced = new EnvVars(env);
          EnvVars envHost = computer.getEnvironment();
          envReduced.entrySet().removeAll(envHost.entrySet());

           

          I'm not sure that .removeAll is working or maybe the computer.getEnvironment() isn't working correctly?

          Christian Höltje added a comment - - edited   I found the code at  https://github.com/jenkinsci/docker-workflow-plugin/blob/master/src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java#L126-L128 EnvVars envReduced = new EnvVars(env); EnvVars envHost = computer.getEnvironment(); envReduced.entrySet().removeAll(envHost.entrySet());   I'm not sure that .removeAll is working or maybe the computer.getEnvironment() isn't working correctly?

          Given that the PATH is very dependent on the container (e.g. the container may not conform to LSB or POSIX layout) I think it should always be unset from the envReduced environment.

          In addition, probably some other environment variables should be black listed as well, such as LD_LIBRARY_PATH and friends.

          Christian Höltje added a comment - Given that the PATH is very dependent on the container (e.g. the container may not conform to LSB or POSIX layout) I think it should always be unset from the envReduced environment. In addition, probably some other environment variables should be black listed as well, such as LD_LIBRARY_PATH and friends.

          Christian Höltje added a comment - - edited

          I made a custom build of docker-workflow which blacklists PATH (and a bunch of the LD_) environment variables:

          https://github.com/jenkinsci/docker-workflow-plugin/pull/97

          This fixed my issues, but I still don't understand how the code mentioned in my initial comment could fail randomly.

          Christian Höltje added a comment - - edited I made a custom build of docker-workflow which blacklists PATH (and a bunch of the LD_ ) environment variables: https://github.com/jenkinsci/docker-workflow-plugin/pull/97 This fixed my issues, but I still don't understand how the code mentioned in my initial comment could fail randomly.
          Christian Höltje made changes -
          Priority Original: Minor [ 4 ] New: Major [ 3 ]
          Christian Höltje made changes -
          Summary Original: docker.inside()'s -e flags are random New: docker.inside() ENV is semi-random

           

          When trying to use the latest code in master there were similar problems with the docker exec calls made for sh() as well.

          https://github.com/jenkinsci/docker-workflow-plugin/pull/91

          Christian Höltje added a comment -   When trying to use the latest code in master there were similar problems with the docker exec  calls made for sh() as well. https://github.com/jenkinsci/docker-workflow-plugin/pull/91

          Jesse Glick added a comment -

          What similar problems?

          Jesse Glick added a comment - What similar problems?

          The PATH is set to the value from the host, not the container, during a docker exec (e.g. a sh step).

          This breaks, for example, the golang container, which adds /go/bin to the front of the path (which a normal host won't have).

          I'm beginning to think that the way docker.inside works should be changed:

          1. For normal Jenkinsfile}}s the docker run -{{e flag should only be used to set the Jenkins default environment variables and nothing else. e.g. BUILD_NUMBER, WORKSPACE, etc.
          2. The only environment variables passed to docker exec are new variables set on env within a docker.inside block. e.g. sh 'env'}}should return only the container's environment variables + the Jenkins default environment variables.  It should not include {{env.FOO = 'bar' if that was run outside the docker.inside block.
          3. For declarative pipelines, it should follow the above rules.

          Christian Höltje added a comment - The PATH is set to the value from the host, not the container, during a docker exec (e.g. a sh step). This breaks, for example, the golang container, which adds /go/bin to the front of the path (which a normal host won't have). I'm beginning to think that the way docker.inside works should be changed: For normal Jenkinsfile}}s the docker run -{{e flag should only be used to set the Jenkins default environment variables and nothing else. e.g. BUILD_NUMBER , WORKSPACE , etc. The only environment variables passed to docker exec are new variables set on env  within a docker.inside block. e.g. sh 'env'}}should return only the container's environment variables + the Jenkins default environment variables.  It should not include {{env.FOO = 'bar' if that was run outside the docker.inside block. For declarative pipelines, it should follow the above rules.

          Jesse Glick added a comment -

          Everything about inside should be changed. I am inclined to simply deprecate the whole feature, and possibly the whole plugin. You should run docker CLI commands you need directly. What we do with Declarative is an open question. Probably the agent docker system needs to be redesigned.

          Perhaps this was not clear from the documentation, but inside is not designed for use with images such as golang. You must use a different idiom. inside works fine for its designed purpose: a totally passive image which merely serves to preinstall some tools and libraries.

          Jesse Glick added a comment - Everything about inside should be changed. I am inclined to simply deprecate the whole feature, and possibly the whole plugin. You should run docker CLI commands you need directly. What we do with Declarative is an open question. Probably the agent docker system needs to be redesigned. Perhaps this was not clear from the documentation, but inside is not designed for use with images such as golang . You must use a different idiom. inside works fine for its designed purpose: a totally passive image which merely serves to preinstall some tools and libraries.

            Unassigned Unassigned
            docwhat Christian Höltje
            Votes:
            12 Vote for this issue
            Watchers:
            21 Start watching this issue

              Created:
              Updated: