Skip to content

Support multi-arch images for Windows via ctr#5298

Merged
fuweid merged 1 commit intocontainerd:masterfrom
jsturtevant:issue-5297
Apr 12, 2021
Merged

Support multi-arch images for Windows via ctr#5298
fuweid merged 1 commit intocontainerd:masterfrom
jsturtevant:issue-5297

Conversation

@jsturtevant
Copy link
Copy Markdown
Contributor

@jsturtevant jsturtevant commented Mar 31, 2021

This refactors the cri platform code to the base containerd platform code so it can be re-used across solutions.

fixes #5297

Signed-off-by: James Sturtevant jstur@microsoft.com
@jsturtevant
support multi-arch images for windows via ctr
d9ff8eb 
Signed-off-by: James Sturtevant <jstur@microsoft.com>
@k8s-ci-robot
Copy link
Copy Markdown

k8s-ci-robot commented Mar 31, 2021

Hi @jsturtevant. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
jsturtevant
jsturtevant commented Mar 31, 2021
View reviewed changes
platforms/defaults_windows.go
return matchComparer{
defaults: Ordered(DefaultSpec(), specs.Platform{
OS: "linux",
Architecture: runtime.GOARCH,
Copy link
Copy Markdown
Contributor Author

@jsturtevant jsturtevant Mar 31, 2021

Choose a reason for hiding this comment

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

This introduces a change to the way it was done in cri before. We could alternatively keep the solution the same by adding something like DefaultForCri() and use it in the cri defaults, if preferred as a first step.
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Mar 31, 2021

Build succeeded.
@jsturtevant jsturtevant mentioned this pull request Apr 11, 2021
Open
@AkihiroSuda AkihiroSuda added this to the 1.5 milestone Apr 12, 2021
@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented Apr 12, 2021

/ok-to-test
@AkihiroSuda AkihiroSuda requested a review from fuweid April 12, 2021 04:33
fuweid
fuweid reviewed Apr 12, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

@jsturtevant could you mind to rebase master? the CI was unstable past few days. Thanks!
@fuweid fuweid merged commit 7e3fd8d into containerd:master Apr 12, 2021
This was referenced Aug 26, 2021
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test

4 participants

@jsturtevant @k8s-ci-robot @AkihiroSuda @fuweid