-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
ctr pull should unpack for default platform when transfer service is used #11086
Conversation
Signed-off-by: Jin Dong <[email protected]>
hi @dmcgowan could you PTAL this PR when you get some time? thanks. This makes |
sopts = append(sopts, image.WithUnpack(platform, cliContext.String("snapshotter"))) | ||
allPlatforms := cliContext.Bool("all-platforms") | ||
if len(p) > 0 && allPlatforms { | ||
return errors.New("cannot specify both --platform and --all-platforms") |
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 probably out-of-scope for this PR, but are there other subcommands using all-platforms
that would benefit from this check?
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.
Other commands just ignore --platform
s if both are specified, e.g., in Fetch
:
containerd/cmd/ctr/commands/content/fetch.go
Lines 137 to 143 in e514bae
if !cliContext.Bool("all-platforms") { | |
p := cliContext.StringSlice("platform") | |
if len(p) == 0 { | |
p = append(p, platforms.DefaultString()) | |
} | |
config.Platforms = p | |
} |
I think error early is better than ignore one flag, but am okay to keep it the same with other commands
/cherrypick release/2.0 |
@djdongjin: new pull request created: #11139 In response to this:
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-sigs/prow repository. |
Currently if neither
--all-platforms
nor--platform
is specified inctr i pull
, it will pull using the default platform, but skip the unpack step (if using transfer service). However, we should apply thedefault platform
to unpack as well.