-
Notifications
You must be signed in to change notification settings - Fork 509
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
Conversation
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 <[email protected]>
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 <[email protected]>
@@ -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, |
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.
I don't understand why this was false.
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.
👀 yeah, I'm not sure either. Looks like it didn't get caught in #1078, my bad.
This reminds me of #989 to set the correct digest when image is pushed with moby but the other way around for this case. |
if sb.Name() == "remote" { | ||
// there is no Docker atm to load the image | ||
outFlag += ",dest=" + targetDir + "/image.tar" | ||
} |
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.
🎉 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.
fixes #1828