-
Notifications
You must be signed in to change notification settings - Fork 14
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
Process and display build and push logs #450
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! I have a few small nits but the biggest question is what's the goroutine doing? It just smells like a race to me but perhaps it isn't - I did not dive into the underlying API too much.
Buildkit sends build logs with the id of `moby.buildkit.trace` to the `aux` field of jsonmessage. This commit adds logic akin to that in `https://github.com/docker/docker-ce` to properly decode and parse the buildkit build logs.
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like https://github.com/moby/buildkit/blob/b78713cdd127b359b40ae0658c36cbab1ed28d60/session/session.go#L135 s.Close waits for termination.
Wanted to ask for tests here but since it's log redirection feature perhaps OK to elide. Was wondering about
The buildkit support is covered by existing test suite yes? |
Yes - the default is now Buildkit, meaning our tests will build images in Buildkit mode. 👍 |
This PR leverages Pulumi's
host.LogStatus
function to display Docker image logs "ephemerally", i.e. in the "Info" status column on the Pulumi terminal output.Additionally, this change uses Docker's
JSONMessage
package to parse the information given by the Docker build process.Using this package allows us to uniformly process all logs sent by Docker and have reasonable baseline expectations for informational fields.
The local implementation leans strongly on the logic in
jsonmessage.Display
to extract all relevant information.The result is that the user can watch Docker build logs in the Pulumi Info box.
Additional changes made:
jsonmessage
'saux
field, as well as encrypted.Fixes #441.