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.

Sign up for GitHub

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

Jump to bottom

Add JSON stream progress writer #11478

Merged
merged 5 commits into from
Jul 1, 2024
Merged

Add JSON stream progress writer #11478

merged 5 commits into from
Jul 1, 2024

Conversation

felixfontein
Copy link
Contributor

What I did
I added a JSON stream progress writer. This allows to write machine readable output to stderr, which makes it a lot easier to use Docker Compose from other tools, like Ansible.

Related issue
Fixes #10872.

(not mandatory) A picture of a cute animal, if possible in relation to what you did
A cute cat with rather sharp claws, watch out when petting her

@felixfontein felixfontein mentioned this pull request Feb 8, 2024
Closed
@felixfontein felixfontein force-pushed the json-progress branch 2 times, most recently from 73e9f16 to 3fec17c Compare February 8, 2024 11:02
@felixfontein
Copy link
Contributor Author

I still need to run make docs and commit the changes. Unfortunately it never comes remotely near to completion with the not so great internet connection I have on this train, so that has to wait until later...

@felixfontein felixfontein mentioned this pull request Feb 8, 2024
Closed
@felixfontein
Copy link
Contributor Author

Docs are updated, CI passes (checked in felixfontein#1), but while testing this (now that I was actually able to build this) I noticed that Docker Compose apparently expects Docker Buildx and Docker CLI itself to also have a JSON progress writer. I'm getting invalid progress mode json errors when it tries to pull images, and building also doesn't seem to work.

@felixfontein felixfontein marked this pull request as draft February 8, 2024 13:35
@felixfontein
Copy link
Contributor Author

I eventually noticed that in case of errors, the error is always printed regularly and not via the progress API. I added code in d3b85f9 that in case --progress json it also formats the error as JSON.

@felixfontein felixfontein marked this pull request as ready for review February 8, 2024 20:42
@felixfontein
Copy link
Contributor Author

This is now ready for review.

@felixfontein felixfontein mentioned this pull request Feb 13, 2024
Merged
@felixfontein felixfontein force-pushed the json-progress branch 2 times, most recently from 150e636 to 07a29dd Compare February 23, 2024 05:38
@felixfontein
Copy link
Contributor Author

It probably makes sense to set logging to JSON (logrus.SetFormatter(&logrus.JSONFormatter{}), see https://github.com/sirupsen/logrus/blob/master/README.md) to avoid random non-JSON logging messages to show up on stderr.

ndeloof
ndeloof reviewed Mar 15, 2024
View reviewed changes
cmd/compose/build.go Outdated
@@ -67,10 +67,15 @@ func (opts buildOptions) toAPIBuildOptions(services []string) (api.BuildOptions,
builderName = os.Getenv("BUILDX_BUILDER")
}

// The build backends do not have a "json" progress; use "plain" instead
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use rawjson so buildkit will render raw json solver events and you get a clean json stream for all events

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created moby/buildkit#4769 since I think it would be better if the rawjson output would write one JSON object per line, instead of using multiple lines to show a more human-readable JSON record (that's not really that human readable anyway).

@felixfontein felixfontein mentioned this pull request Mar 17, 2024
Closed
@felixfontein
Copy link
Contributor Author

@ndeloof can you take another look at this? Thanks!

@ndeloof
Copy link
Contributor

ndeloof commented Jul 1, 2024

Can you please squash your commits and rebase, I'll enable auto-merge on this PR

@ndeloof ndeloof enabled auto-merge (rebase) July 1, 2024 13:22
@ndeloof ndeloof merged commit ace69c9 into docker:main Jul 1, 2024
30 checks passed
@felixfontein
Copy link
Contributor Author

@ndeloof thanks a lot for reviewing and merging! Since you rebase-merged I assume I don't need to rebase/squash this anymore :)

@felixfontein felixfontein deleted the json-progress branch July 1, 2024 14:00
@ndeloof
Copy link
Contributor

ndeloof commented Jul 1, 2024

auto-merge applied before squash, never mind

This was referenced Jul 17, 2024
Merged
Merged
@cmrigney cmrigney mentioned this pull request Jul 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ndeloof ndeloof ndeloof approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

docker-compose - dry-run - json output
2 participants
@felixfontein @ndeloof