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

Fix handling of missing image message with newer versions of Docker #3171

Merged
merged 1 commit into from Jul 13, 2017

Conversation

Projects
None yet
3 participants
@enolan
Contributor

enolan commented May 12, 2017

At some unknown version, docker inspect stopped outputting "Error: No such
image" and started outputting "Error: No such object:" when the image is
question was missing. That change broke detecting that situation.

This commit checks for either version of the message.

Please include the following checklist in your PR:

  • Any changes that could be relevant to users have been recorded in the ChangeLog.md
  • The documentation has been updated, if necessary.

Tested by running stack build in a project that needed a docker image that wasn't on this machine. Output before commit:

$ ../stack/.stack-work/install/x86_64-linux-nopie/lts-8.5/8.0.2/bin/stack build -v
Version 1.4.1, Git revision 0ddbaa7fd1cfe8ec4c8cb17364dc8f0aa6d4314d (4766 commits) x86_64 hpack-0.17.0
2017-05-11 16:44:39.636920: [debug] Checking for project config at: /home/enolan/mystuff/code/emp-pl-site/stack.yaml
@(Stack/Config.hs:949:9)
2017-05-11 16:44:39.637307: [debug] Loading project config file stack.yaml
@(Stack/Config.hs:974:13)
2017-05-11 16:44:39.641121: [debug] Run process: /usr/bin/docker --version
@(System/Process/Read.hs:306:3)
2017-05-11 16:44:39.655740: [debug] Process finished in 14ms: /usr/bin/docker --version
@(System/Process/Read.hs:306:3)
2017-05-11 16:44:39.656427: [debug] Run process: /usr/bin/docker inspect fpco/stack-build:lts-8.13
@(System/Process/Read.hs:306:3)
Running /usr/bin/docker inspect fpco/stack-build:lts-8.13 exited with ExitFailure 1

[]

Error: No such object: fpco/stack-build:lts-8.13

And after:

$ ../stack/.stack-work/install/x86_64-linux-nopie/lts-8.5/8.0.2/bin/stack build -v
Version 1.4.1, Git revision 0ddbaa7fd1cfe8ec4c8cb17364dc8f0aa6d4314d (4766 commits) x86_64 hpack-0.17.0
2017-05-11 17:00:09.534096: [debug] Checking for project config at: /home/enolan/mystuff/code/emp-pl-site/stack.yaml
@(Stack/Config.hs:949:9)
2017-05-11 17:00:09.534578: [debug] Loading project config file stack.yaml
@(Stack/Config.hs:974:13)
2017-05-11 17:00:09.539702: [debug] Run process: /usr/bin/docker --version
@(System/Process/Read.hs:306:3)
2017-05-11 17:00:09.552763: [debug] Process finished in 12ms: /usr/bin/docker --version
@(System/Process/Read.hs:306:3)
2017-05-11 17:00:09.553652: [debug] Run process: /usr/bin/docker inspect fpco/stack-build:lts-8.13
@(System/Process/Read.hs:306:3)
The Docker image referenced by your configuration file has not
been downloaded:
    fpco/stack-build:lts-8.13

Run 'stack docker pull' to download it, then try again.
Fix handling of missing image message with newer versions of Docker
At some unknown version, `docker inspect` stopped outputting "Error: No such
image" and started outputting "Error: No such object:" when the image is
question was missing. That change broke detecting that situation.

This commit checks for either version of the message.
@lwm

This comment has been minimized.

Member

lwm commented May 29, 2017

At some unknown version, docker inspect stopped outputting "Error: No such
image" and started outputting "Error: No such object:"

😒

Great that you caught this though!

So, this original string check is quite fragile then! The difference between the InspectFailedException and NotPulledException seems nice but could continue to cause problems like this? Is Error: No such object: fpco/stack-build:lts-8.13 such a bad error message after all? I don't think we lose much information if we merge those two exception types and NotPullException isn't used that much.

@enolan

This comment has been minimized.

Contributor

enolan commented May 29, 2017

Looking at it again, I think we can handle non-zero exit codes from docker inspect the same as zero ones. It returns an empty JSON list when the image is missing, which is what inspects returns anyway given that error message. The JSON output behavior should be pretty stable. It should still distinguish between missing the image and some other failure though.

I can make that change.

@lwm

This comment has been minimized.

Member

lwm commented May 29, 2017

It should still distinguish between missing the image and some other failure though.

🚀 🚀 🚀

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Jul 13, 2017

Thank you, this PR as-is improves the situation. If you'd like to make the update for non-zero exit codes, that would be great! But I'd like to get this fix in immediately.

@snoyberg snoyberg merged commit c5cba6e into commercialhaskell:master Jul 13, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment