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.

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.

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

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.

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.

I agree that inside should be redesigned.  There is a lot that isn't ideal about it (user management, etc.).

I'm not sure why inside is inappropriate for running golang, it's just another image used to build/test code.

Either way, when 1.11 was released we accidentally upgraded to it (without my PR) and the error about 'cat' not being in the $PATH is back.

Can we accept PR #97 as a work-around until inside is redesigned, jglick?

golang has a special usage mode IIRC, with a dedicated source directory.

It doesn't have to work that way, you can build your source from an arbitrary directory.  Go just prefers it in /go/src/...

The real problem is that when you do go install <some-tool> it installs it to /go/bin, but /go/bin isn't in the $PATH because it is overridden by the build slave host's $PATH.

golang works as well as anything else does with dockerImg.inside()

Has been a long time since I tried it but I recall problems with retrieving packages, which went to a fixed location. Anyway golang has a documented usage mode which it would be better to use as is rather than relying on WithContainerStep.

Fair enough. Still doesn't address the fact that $PATH (and some other variables, like LOCALE and LANG) shouldn't be overridden inside dockerImg.inside().

Sure, this sounds like a bug offhand. Just not sure what is going to be broken by a fix. Generally every time I do anything relating to environment variables in Jenkins, the regressions start piling up!

Well, if you want to just deliberately break every thing then:

1. Use no environmental variables for run cat.

2. Have a new object created inside the block that is similar to env. Maybe call it insideEnv.

3. Use insideEnv for the docker exec.

This gives the ability to explicitly modify the environment variables from the pipeline.

Maybe an optional flag to inside to prepopulate it with the Jenkins build vars,if that can be done reliably.

I think I run into the same issue while trying to override HOME env when using .inside() since updated to docker-workflow v1.9.1:

docker.image('node:baseline-6.9.5').inside('-e HOME=/tmp') { [...]

08:39:41 $ docker run -t -d -u 1001:1001 -e HOME=/tmp [...] -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** -e ******** --entrypoint cat node:baseline-6.9.5
[Pipeline] \{
[Pipeline] sh
08:39:42 + env
08:39:42 JENKINS_HOME=/apps/jenkins/latest
08:39:42 HOME=/apps/jenkins/latest

jglick: At the very least, do you have any idea why this code isn't working?

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

...because in my case, I have PATH set on the agent/node/slave; if this code worked it would unset PATH which would at least let cat work again for me. 

Not sure offhand, would need to spend time studying it.

Generally I would treat Image.inside as “take it or leave it”—if it works as a convenience, great; if not, ignore it and use generic techniques such as calling docker run to run desired steps (and docker cp to extra any files needed for archiving or reporting).

This is an issue for me as well.  The composer docker image has the php binary in /usr/local/bin.  I've tried setting PATH via inside('-e PATH') without luck.  I was able to get around it at the moment by setting the PATH before calling the composer command.  

Fails:

docker.image("composer:latest").inside{
  sh 'composer install -a --no-dev --ignore-platform-reqs --no-scripts --no-interaction --no-ansi --no-progress'
}

Works:

docker.image("composer:latest").inside{
  sh 'PATH=$PATH:/usr/local/bin composer install -a --no-dev --ignore-platform-reqs --no-scripts --no-interaction --no-ansi --no-progress'
}

It looks like multi-stage docker builds are a way around this, and they work on a workstation the same as in Jenkins.

Note: the documentation doesn't mention this, but you can use --target= to pick which image gets tagged by the build.

Example:

docker build --target=builder --tag=build-env .

...would tag the builder from the multi-stage build so you could examine it.

Yes, though there are some limitations to that.

Code changed in jenkins
User: Christian Höltje
Path:
src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java
http://jenkins-ci.org/commit/docker-workflow-plugin/e04959ea87d391ec89965c760b9d054b73bed317
Log:
Don't pass PATH to docker containers

Docker containers usually have special PATHs; the PATH defined
in on the Agent won't make sense for the container.

JENKINS-43590 docker.inside()'s -e flags are random

I think blocking PATH isn't a correct fix.

Environment is built from various sources, including Computer.getEnvironment(), which returns node's `PATH`.

Tool installers do amend this using `PATH+` syntax

docker-pipeline should replace Computer.getEnvironment() result with the actual `env` from container, so this mechanism works as expected.

ndeloof

I didn't even know that was possible, but it makes sense.  Do we need a new issue for that?

I agree with ndeloof

This code change actually break our pipeline that passes PATH environment from hosts.

Docker container path should be an optional overwrite, instead of blocking!