Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docker docs don't document PATH setup, which has debatable edge cases #2742

Closed
Blaisorblade opened this issue Oct 26, 2016 · 1 comment

Comments

Projects
None yet
3 participants
@Blaisorblade
Copy link
Collaborator

commented Oct 26, 2016

Background:
Stack's Docker support initializes the PATH in the container using the container's default PATH, taken apparently from docker inspect. That relies on the unenforced best practice of setting a reasonable PATH in the Dockerfile:
https://docs.docker.com/engine/userguide/eng-image/dockerfile_best-practices/#/env

@dysinger just tried using stack on an image that doesn't follow this best practice (he said he generated the image via packer), and got trouble because default PATH entries like /usr/bin were not set.

Issues:

  1. If the PATH from docker inspect is empty, there's no default value (not even the "/bin:/usr/bin" hinted by man 3 execvp on Linux). Adding a reasonable standard value from some better source would help a lot (and would have avoided his issue).
  2. This was tricky to debug because the docs were incomplete. This whole process to setup PATH seems not described in the docs (https://docs.haskellstack.org/en/latest/docker_integration/), I had to rely on source code and comments. I'm not sure how much of this should be documented, but probably something should. For extra fun, running a shell in the container would usually show a reasonable PATH, set up by shell initialization, confusing debugging. For instance, this command showed a correct PATH:
    docker run -i -t --rm $curr_image sh -c 'echo $PATH'
    I wonder whether stack should fetch the PATH from this command instead of docker inspect: this would be less error-prone for users, but it could be a bad idea for some reason (I don't understand this code enough.
  3. IIUC, a value of PATH specified manually in stack.yaml overrides anything setup above, including the extra folders added by Stack, which probably causes trouble.

To see whether PATH was initialized by the Dockerfile for some container/image $NAME, one can run docker inspect -f '{{.Config.Env}}' $NAME to get the predefined environment variables, and search PATH among them.

Relevant code:

stack/src/Stack/Docker.hs

Lines 300 to 303 in 8aaef91

newPathEnv <- augmentPath
[ hostBinDirPath
, sandboxHomeDir </> $(mkRelDir ".local/bin")]
(T.pack <$> lookupImageEnv "PATH" imageEnvVars)
(set up PATH);

stack/src/Stack/Docker.hs

Lines 928 to 932 in 8aaef91

-- | Parsed @Config@ section of @docker inspect@ output.
data ImageConfig = ImageConfig
{icEnv :: [String]
,icEntrypoint :: [String]}
deriving (Show)
(comments on icEnv, from which imageEnvVars comes).

@mgsloan mgsloan added this to the P2: Should milestone Oct 26, 2016

snoyberg added a commit that referenced this issue Mar 27, 2019

@snoyberg

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

See #4664

snoyberg added a commit that referenced this issue Mar 27, 2019

@snoyberg snoyberg closed this in 49a400b Mar 28, 2019

snoyberg added a commit that referenced this issue Mar 28, 2019

Merge pull request #4664 from commercialhaskell/2742-warn-docker-miss…
…ing-path

Warn on missing PATH in Docker image (fixes #2742)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.