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

Add --metadata-file flag to output build metadata #2095

Merged
merged 1 commit into from May 1, 2021
Merged

Add --metadata-file flag to output build metadata #2095

merged 1 commit into from May 1, 2021

Conversation

rittneje
Copy link
Contributor

Output build metadata (such as image digest) to a file based on a flag to buildctl.
Fixes #1158.

buildctl build --frontend=dockerfile.v0 --metadata-file meta.json --local context=. --local dockerfile=.. --output type=image,name=foo:latest,push=false
{"containerimage.config.digest":"sha256:9ba453cfcf085ebe34d7e3bc0cd13e02038361693a53d8663ddca8f83e26447d","containerimage.digest":"sha256:475f3a5ec8513c157acc20aacf3c872afb3b508ed40f9e9fb519d31897b7a3f0","image.name":"foo:latest"}

@tonistiigi @AkihiroSuda This is based on the comments from #1315.

Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

LGTM but can you add a test. You can also modify an existing test and add an extra check.

@rittneje
Copy link
Contributor Author

@tonistiigi Sure I can try to add something. Also, do you understand the test failure here? Not sure how these changes would have affected it.

        --- FAIL: TestClientGatewayIntegration/TestClientGatewayExecError/worker=containerd/only_rootfs (1.10s)
            build_test.go:1216: 
                	Error Trace:	build_test.go:1216
                	Error:      	Not equal: 
                	            	expected: "su6iub2plufj65normatzcstv"
                	            	actual  : ""
                	            	
                	            	Diff:
                	            	--- Expected
                	            	+++ Actual
                	            	@@ -1 +1 @@
                	            	-su6iub2plufj65normatzcstv
                	            	+
                	Test:       	TestClientGatewayIntegration/TestClientGatewayExecError/worker=containerd/only_rootfs

@rittneje
Copy link
Contributor Author

@tonistiigi Actually, I took a look at the existing unit tests. They are pretty complicated, so I'm not entirely sure what I'm supposed to do. Can you provide some additional guidance on this?

@tonistiigi
Copy link
Member

You can base it on https://github.com/moby/buildkit/blob/master/client/client_test.go#L1783-L1821 for example. No need to push in your case.

Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

On second look, I wonder though if client pkg is the correct place for it. Solve already returns this value so it is kind of duplicated to have side-effect and write it to a file in there as well. I think it might be more suited directly in buildctl pkg.

cmd/buildctl/build_test.go Outdated Show resolved Hide resolved
cmd/buildctl/build.go Outdated Show resolved Hide resolved
Signed-off-by: Jesse Rittner <rittneje@gmail.com>
@rittneje
Copy link
Contributor Author

rittneje commented May 1, 2021

@tonistiigi Apologies for the churn. I was not able to run the tests locally before due to docker/cli#2989. The test I added is passing now.

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

Successfully merging this pull request may close these issues.

Add some way to get the sha256 digest of the image
2 participants