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

build: fix writing correct image ID with -q #1844

Merged
merged 2 commits into from
May 26, 2023

Conversation

tonistiigi
Copy link
Member

fixes #1828

There is no reason why one can't make a docker tarball or
load to Docker instance from remote driver.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Container driver wrote manifest digest that had a
mismatch with --iidfile output.

When --iidfile was set the --metadata-file was not
written.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi added this to the v0.11.0 milestone May 26, 2023
@tonistiigi tonistiigi requested a review from jedevc May 26, 2023 07:22
@@ -90,7 +90,7 @@ func (d *Driver) Client(ctx context.Context) (*client.Client, error) {
func (d *Driver) Features() map[driver.Feature]bool {
return map[driver.Feature]bool{
driver.OCIExporter: true,
driver.DockerExporter: false,
driver.DockerExporter: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand why this was false.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👀 yeah, I'm not sure either. Looks like it didn't get caught in #1078, my bad.

@crazy-max
Copy link
Member

This reminds me of #989 to set the correct digest when image is pushed with moby but the other way around for this case.

Comment on lines +104 to +107
if sb.Name() == "remote" {
// there is no Docker atm to load the image
outFlag += ",dest=" + targetDir + "/image.tar"
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

🎉 tests!

I think we should probably find a way to hook into the unsupportedFeatures property of buildkit's Backend implementation, so that we could expose the driver features there, so we could rework this using integration.CheckFeatureCompat(t, sb, driver.DockerExporter).

I'll open a tracking issue for this, I'll take a look after the release.

@jedevc jedevc requested a review from crazy-max May 26, 2023 09:02
@jedevc jedevc merged commit 354ccc9 into docker:master May 26, 2023
57 checks passed
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.

Output with --quiet --load flags is the hash of a manifest that is not loaded into the daemon
3 participants