Add support for multiple platform options in image load and save#6126
Add support for multiple platform options in image load and save#6126vvoland merged 3 commits intodocker:masterfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6126 +/- ##
==========================================
+ Coverage 55.02% 55.05% +0.03%
==========================================
Files 361 361
Lines 30152 30161 +9
==========================================
+ Hits 16591 16605 +14
+ Misses 12604 12598 -6
- Partials 957 958 +1 🚀 New features to boost your workflow:
|
Signed-off-by: Cesar Talledo <cesar.talledo@docker.com>
Signed-off-by: Cesar Talledo <cesar.talledo@docker.com>
54b967e to
8993f54
Compare
…/save. Signed-off-by: Cesar Talledo <cesar.talledo@docker.com>
| name: "with-comma-separated-platforms", | ||
| args: []string{"--platform", "linux/amd64,linux/arm64/v8,linux/riscv64"}, | ||
| imageLoadFunc: func(input io.Reader, options ...client.ImageLoadOption) (image.LoadResponse, error) { | ||
| assert.Check(t, len(options) > 0) // can be 1 or two depending on whether a terminal is attached :/ |
There was a problem hiding this comment.
Hmm I'm not sure I understand this one. How does terminal affect parsing here?
There was a problem hiding this comment.
I was simply copying a comment in the test right above this line (i.e., "with-single-platform"), but the underlying rationale for the comment comes from this code:
func runLoad(ctx context.Context, dockerCli command.Cli, opts loadOptions) error {
...
var options []client.ImageLoadOption
if opts.quiet || !dockerCli.Out().IsTerminal() {
options = append(options, client.ImageLoadWithQuiet(true))
}
...
There was a problem hiding this comment.
Ah right.. We should apply these opts to a temporary var and then check the resulting opts struct, but we can't do that because these types are unexported..
I guess the only way to test it in this situation is by checking HTTP request at the HTTP client, but that would need rewriting the test a bit.
Although, I think the best solution would be to have a debug helper on the client side that would allow introspecting for testing purposes...
There was a problem hiding this comment.
We should do that in a follow up though
|
FWIW; I had a "wip" branch to implement a multi-platform option, but it still needs some work (and a decision where we want to put it); |
|
Changed the changelog a bit, let me know if that looks good. cc @ArthurFlag |
Thanks @vvoland, LGTM. |
- What I did
Prior to this change,
docker image loadanddocker image saveaccept only a single platform via the--platformoption.This change adds support for multiple platforms using a comma-separated list passed to
--platform. E.g.:** NOTE **: Depends on the corresponding change in the Moby engine (see moby/moby#50166).
- How I did it
Updated the
--platformoption in theimage loadandimage savecommands to accept an array of platforms, as opposed to a single platform.- How to verify it
Run updated unit tests.
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)