-
Notifications
You must be signed in to change notification settings - Fork 485
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
tests: build multiplatform #2620
Conversation
Signed-off-by: idnandre <[email protected]>
cmd := buildxCmd(sb, withDir(dir), withArgs("bake"), withArgs("--set", fmt.Sprintf("*.output=type=image,name=%s,push=true", target))) | ||
out, err := cmd.CombinedOutput() | ||
|
||
if !isMobyWorker(sb) { |
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.
@crazy-max how does this check detect whether the docker daemon supports multi-arch? Having a quick look; isMobyWorker
and isMobyContainerdSnapWorker
look identical, but if docker is running with the containerd image-store enabled, it supports multi-arch?
Lines 97 to 105 in 56cb197
func isMobyWorker(sb integration.Sandbox) bool { | |
name, hasFeature := driverName(sb.Name()) | |
return name == "docker" && !hasFeature | |
} | |
func isMobyContainerdSnapWorker(sb integration.Sandbox) bool { | |
name, hasFeature := driverName(sb.Name()) | |
return name == "docker" && hasFeature | |
} |
Should we have a supportsMultiArch
check instead?
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.
OH! Nevermind; hasFeature
vs !hasFeature
🙈 I missed the !
🙈 😂
Still, maybe a supportsMultiArch
or hasMultiarchSupport
could make sense (more descriptive?)
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 is not just multiarch but also cache and oci exporters that are supported with containerd snap:
buildx/driver/docker/driver.go
Lines 92 to 95 in aa35c95
driver.OCIExporter: useContainerdSnapshotter, | |
driver.DockerExporter: useContainerdSnapshotter, | |
driver.CacheExport: useContainerdSnapshotter, | |
driver.MultiPlatform: useContainerdSnapshotter, |
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.
LGTM thanks!
add test build and bake multiplatform (--platform), to improve test coverage based on #1857