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

tests: build multiplatform #2620

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Conversation

idnandre
Copy link
Contributor

@idnandre idnandre commented Jul 20, 2024

add test build and bake multiplatform (--platform), to improve test coverage based on #1857

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) {
Copy link
Member

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?

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?

Copy link
Member

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?)

Copy link
Member

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:

driver.OCIExporter: useContainerdSnapshotter,
driver.DockerExporter: useContainerdSnapshotter,
driver.CacheExport: useContainerdSnapshotter,
driver.MultiPlatform: useContainerdSnapshotter,

Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@thompson-shaun thompson-shaun added this to the v0.17.0 milestone Jul 23, 2024
@crazy-max crazy-max merged commit b691a10 into docker:master Jul 25, 2024
104 checks passed
@crazy-max crazy-max mentioned this pull request Jul 25, 2024
35 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants