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

fix: distinguish docker build errors from streaming errors #6910

Conversation

briandealwis
Copy link
Member

In trying to reproduce #6126, I noticed that docker build errors are oddly prefixed with "unable to stream build output:".

Canceled build for leeroy-app
unable to stream build output: The command '/bin/sh -c exit 1' returned a non-zero code: 1. Please fix the Dockerfile and try again..

It turns out that the Docker code for parsing and interpreting the JSON message stream from the Docker daemon parses build failures as a JSONError object, which behaves like an error. This PR changes our error-handling code to return such errors directly.

@codecov
Copy link

codecov bot commented Nov 25, 2021

Codecov Report

Merging #6910 (7cc83fb) into main (290280e) will decrease coverage by 1.64%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6910      +/-   ##
==========================================
- Coverage   70.48%   68.84%   -1.65%     
==========================================
  Files         515      551      +36     
  Lines       23150    25299    +2149     
==========================================
+ Hits        16317    17416    +1099     
- Misses       5776     6708     +932     
- Partials     1057     1175     +118     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/deploy.go 52.00% <ø> (-1.85%) ⬇️
cmd/skaffold/app/cmd/dev.go 84.61% <0.00%> (ø)
cmd/skaffold/app/cmd/render.go 36.66% <0.00%> (-4.72%) ⬇️
cmd/skaffold/skaffold.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/inspect_tests.go 62.50% <14.28%> (-1.14%) ⬇️
cmd/skaffold/app/cmd/fix.go 68.85% <40.00%> (-7.62%) ⬇️
cmd/skaffold/app/cmd/lint.go 42.85% <42.85%> (ø)
cmd/skaffold/app/cmd/find_configs.go 48.88% <50.00%> (+0.24%) ⬆️
cmd/skaffold/app/skaffold.go 76.19% <70.00%> (-8.43%) ⬇️
cmd/skaffold/app/cmd/inspect_build_env.go 65.11% <75.00%> (+6.39%) ⬆️
... and 170 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e5439d...7cc83fb. Read the comment docs.

@briandealwis
Copy link
Member Author

What's odd here is that this error can appear long after the output started. Maybe docker reports: is a better prefix?

@briandealwis briandealwis added the !! blocked !! this issue/PR is blocked by another issue label Dec 1, 2021
tejal29 and others added 2 commits December 10, 2021 05:03
Co-authored-by: Brian de Alwis <bdealwis@google.com>
@tejal29
Copy link
Member

tejal29 commented Dec 10, 2021

Not sure, why blocked label was added. Feel free to remove it!

@briandealwis briandealwis removed the !! blocked !! this issue/PR is blocked by another issue label Dec 10, 2021
@briandealwis briandealwis merged commit 5933db1 into GoogleContainerTools:main Dec 10, 2021
@briandealwis briandealwis deleted the docker-streaming-error-attribution branch December 10, 2021 17:39
@briandealwis
Copy link
Member Author

I had it blocked pending the additional message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants