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

c8d: Add --platform flag to history, save and load #5331

Merged
merged 3 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions cli/command/image/history.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,20 @@ package image
import (
"context"

"github.com/containerd/platforms"
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/completion"
"github.com/docker/cli/cli/command/formatter"
flagsHelper "github.com/docker/cli/cli/flags"
"github.com/docker/docker/api/types/image"
"github.com/pkg/errors"
"github.com/spf13/cobra"
)

type historyOptions struct {
image string
image string
platform string

human bool
quiet bool
Expand Down Expand Up @@ -45,12 +48,24 @@ func NewHistoryCommand(dockerCli command.Cli) *cobra.Command {
flags.BoolVarP(&opts.quiet, "quiet", "q", false, "Only show image IDs")
flags.BoolVar(&opts.noTrunc, "no-trunc", false, "Don't truncate output")
flags.StringVar(&opts.format, "format", "", flagsHelper.FormatHelp)
flags.StringVar(&opts.platform, "platform", "", `Show history for the given platform. Formatted as "os[/arch[/variant]]" (e.g., "linux/amd64")`)
_ = flags.SetAnnotation("platform", "version", []string{"1.48"})

_ = cmd.RegisterFlagCompletionFunc("platform", completion.Platforms)
return cmd
}

func runHistory(ctx context.Context, dockerCli command.Cli, opts historyOptions) error {
history, err := dockerCli.Client().ImageHistory(ctx, opts.image, image.HistoryOptions{})
var options image.HistoryOptions
if opts.platform != "" {
p, err := platforms.Parse(opts.platform)
if err != nil {
return errors.Wrap(err, "invalid platform")
}
options.Platform = &p
}

history, err := dockerCli.Client().ImageHistory(ctx, opts.image, options)
if err != nil {
return err
}
Expand Down
18 changes: 18 additions & 0 deletions cli/command/image/history_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import (

"github.com/docker/cli/internal/test"
"github.com/docker/docker/api/types/image"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
"gotest.tools/v3/golden"
)

Expand All @@ -33,6 +35,11 @@ func TestNewHistoryCommandErrors(t *testing.T) {
return []image.HistoryResponseItem{{}}, errors.Errorf("something went wrong")
},
},
{
name: "invalid platform",
args: []string{"--platform", "<invalid>", "arg1"},
expectedError: `invalid platform`,
},
}
for _, tc := range testCases {
tc := tc
Expand Down Expand Up @@ -89,6 +96,17 @@ func TestNewHistoryCommandSuccess(t *testing.T) {
}}, nil
},
},
{
name: "platform",
args: []string{"--platform", "linux/amd64", "image:tag"},
imageHistoryFunc: func(img string, options image.HistoryOptions) ([]image.HistoryResponseItem, error) {
assert.Check(t, is.DeepEqual(ocispec.Platform{OS: "linux", Architecture: "amd64"}, *options.Platform))
return []image.HistoryResponseItem{{
ID: "1234567890123456789",
Created: time.Now().Unix(),
}}, nil
},
},
}
for _, tc := range testCases {
tc := tc
Expand Down
23 changes: 18 additions & 5 deletions cli/command/image/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"io"

"github.com/containerd/platforms"
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/completion"
Expand All @@ -15,8 +16,9 @@ import (
)

type loadOptions struct {
input string
quiet bool
input string
quiet bool
platform string
}

// NewLoadCommand creates a new `docker load` command
Expand All @@ -40,7 +42,10 @@ func NewLoadCommand(dockerCli command.Cli) *cobra.Command {

flags.StringVarP(&opts.input, "input", "i", "", "Read from tar archive file, instead of STDIN")
flags.BoolVarP(&opts.quiet, "quiet", "q", false, "Suppress the load output")
flags.StringVar(&opts.platform, "platform", "", `Load only the given platform variant. Formatted as "os[/arch[/variant]]" (e.g., "linux/amd64")`)
_ = flags.SetAnnotation("platform", "version", []string{"1.48"})

_ = cmd.RegisterFlagCompletionFunc("platform", completion.Platforms)
return cmd
}

Expand All @@ -63,12 +68,20 @@ func runLoad(ctx context.Context, dockerCli command.Cli, opts loadOptions) error
return errors.Errorf("requested load from stdin, but stdin is empty")
}

var loadOpts image.LoadOptions
var options image.LoadOptions
if opts.quiet || !dockerCli.Out().IsTerminal() {
loadOpts.Quiet = true
options.Quiet = true
}

response, err := dockerCli.Client().ImageLoad(ctx, input, loadOpts)
if opts.platform != "" {
p, err := platforms.Parse(opts.platform)
if err != nil {
return errors.Wrap(err, "invalid platform")
}
options.Platform = &p
}

response, err := dockerCli.Client().ImageLoad(ctx, input, options)
if err != nil {
return err
}
Expand Down
18 changes: 18 additions & 0 deletions cli/command/image/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import (

"github.com/docker/cli/internal/test"
"github.com/docker/docker/api/types/image"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
"gotest.tools/v3/golden"
)

Expand Down Expand Up @@ -40,6 +42,14 @@ func TestNewLoadCommandErrors(t *testing.T) {
return image.LoadResponse{}, errors.Errorf("something went wrong")
},
},
{
name: "invalid platform",
args: []string{"--platform", "<invalid>"},
expectedError: `invalid platform`,
imageLoadFunc: func(input io.Reader, options image.LoadOptions) (image.LoadResponse, error) {
return image.LoadResponse{}, nil
},
},
}
for _, tc := range testCases {
tc := tc
Expand Down Expand Up @@ -96,6 +106,14 @@ func TestNewLoadCommandSuccess(t *testing.T) {
return image.LoadResponse{Body: io.NopCloser(strings.NewReader("Success"))}, nil
},
},
{
name: "with platform",
args: []string{"--platform", "linux/amd64"},
imageLoadFunc: func(input io.Reader, options image.LoadOptions) (image.LoadResponse, error) {
assert.Check(t, is.DeepEqual(ocispec.Platform{OS: "linux", Architecture: "amd64"}, *options.Platform))
return image.LoadResponse{Body: io.NopCloser(strings.NewReader("Success"))}, nil
},
},
}
for _, tc := range testCases {
tc := tc
Expand Down
20 changes: 17 additions & 3 deletions cli/command/image/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"io"

"github.com/containerd/platforms"
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/completion"
Expand All @@ -13,8 +14,9 @@ import (
)

type saveOptions struct {
images []string
output string
images []string
output string
platform string
}

// NewSaveCommand creates a new `docker save` command
Expand All @@ -38,7 +40,10 @@ func NewSaveCommand(dockerCli command.Cli) *cobra.Command {
flags := cmd.Flags()

flags.StringVarP(&opts.output, "output", "o", "", "Write to a file, instead of STDOUT")
flags.StringVar(&opts.platform, "platform", "", `Save only the given platform variant. Formatted as "os[/arch[/variant]]" (e.g., "linux/amd64")`)
_ = flags.SetAnnotation("platform", "version", []string{"1.48"})

_ = cmd.RegisterFlagCompletionFunc("platform", completion.Platforms)
return cmd
}

Expand All @@ -52,7 +57,16 @@ func RunSave(ctx context.Context, dockerCli command.Cli, opts saveOptions) error
return errors.Wrap(err, "failed to save image")
}

responseBody, err := dockerCli.Client().ImageSave(ctx, opts.images, image.SaveOptions{})
var options image.SaveOptions
if opts.platform != "" {
p, err := platforms.Parse(opts.platform)
if err != nil {
return errors.Wrap(err, "invalid platform")
}
options.Platform = &p
}

responseBody, err := dockerCli.Client().ImageSave(ctx, opts.images, options)
if err != nil {
return err
}
Expand Down
16 changes: 16 additions & 0 deletions cli/command/image/save_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/docker/cli/internal/test"
"github.com/docker/docker/api/types/image"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
Expand Down Expand Up @@ -51,6 +52,11 @@ func TestNewSaveCommandErrors(t *testing.T) {
args: []string{"-o", "/dev/null", "arg1"},
expectedError: "failed to save image: invalid output path: \"/dev/null\" must be a directory or a regular file",
},
{
name: "invalid platform",
args: []string{"--platform", "<invalid>", "arg1"},
expectedError: `invalid platform`,
},
}
for _, tc := range testCases {
tc := tc
Expand Down Expand Up @@ -95,6 +101,16 @@ func TestNewSaveCommandSuccess(t *testing.T) {
return io.NopCloser(strings.NewReader("")), nil
},
},
{
args: []string{"--platform", "linux/amd64", "arg1"},
isTerminal: false,
imageSaveFunc: func(images []string, options image.SaveOptions) (io.ReadCloser, error) {
assert.Assert(t, is.Len(images, 1))
assert.Check(t, is.Equal("arg1", images[0]))
assert.Check(t, is.DeepEqual(ocispec.Platform{OS: "linux", Architecture: "amd64"}, *options.Platform))
return io.NopCloser(strings.NewReader("")), nil
},
},
}
for _, tc := range testCases {
tc := tc
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
IMAGE CREATED CREATED BY SIZE COMMENT
123456789012 Less than a second ago 0B
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Success
1 change: 1 addition & 0 deletions docs/reference/commandline/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Show the history of an image
| `--format` | `string` | | Format output using a custom template:<br>'table': Print output in table format with column headers (default)<br>'table TEMPLATE': Print output in table format using the given Go template<br>'json': Print in JSON format<br>'TEMPLATE': Print output using the given Go template.<br>Refer to https://docs.docker.com/go/formatting/ for more information about formatting output with templates |
| `-H`, `--human` | `bool` | `true` | Print sizes and dates in human readable format |
| `--no-trunc` | `bool` | | Don't truncate output |
| `--platform` | `string` | | Show history for the given platform. Formatted as `os[/arch[/variant]]` (e.g., `linux/amd64`) |
| `-q`, `--quiet` | `bool` | | Only show image IDs |


Expand Down
64 changes: 58 additions & 6 deletions docs/reference/commandline/image_history.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ Show the history of an image

### Options

| Name | Type | Default | Description |
|:----------------------|:---------|:--------|:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| [`--format`](#format) | `string` | | Format output using a custom template:<br>'table': Print output in table format with column headers (default)<br>'table TEMPLATE': Print output in table format using the given Go template<br>'json': Print in JSON format<br>'TEMPLATE': Print output using the given Go template.<br>Refer to https://docs.docker.com/go/formatting/ for more information about formatting output with templates |
| `-H`, `--human` | `bool` | `true` | Print sizes and dates in human readable format |
| `--no-trunc` | `bool` | | Don't truncate output |
| `-q`, `--quiet` | `bool` | | Only show image IDs |
| Name | Type | Default | Description |
|:--------------------------|:---------|:--------|:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| [`--format`](#format) | `string` | | Format output using a custom template:<br>'table': Print output in table format with column headers (default)<br>'table TEMPLATE': Print output in table format using the given Go template<br>'json': Print in JSON format<br>'TEMPLATE': Print output using the given Go template.<br>Refer to https://docs.docker.com/go/formatting/ for more information about formatting output with templates |
| `-H`, `--human` | `bool` | `true` | Print sizes and dates in human readable format |
| `--no-trunc` | `bool` | | Don't truncate output |
| [`--platform`](#platform) | `string` | | Show history for the given platform. Formatted as `os[/arch[/variant]]` (e.g., `linux/amd64`) |
| `-q`, `--quiet` | `bool` | | Only show image IDs |


<!---MARKER_GEN_END-->
Expand Down Expand Up @@ -76,3 +77,54 @@ $ docker history --format "{{.ID}}: {{.CreatedSince}}" busybox
f6e427c148a7: 4 weeks ago
<missing>: 4 weeks ago
```

### <a name="platform"></a> Show history for a specific platform (--platform)

The `--platform` option allows you to specify which platform variant to show
history for if multiple platforms are present. By default, `docker history`
shows the history for the daemon's native platform or if not present, the
first available platform.
Copy link
Contributor

Choose a reason for hiding this comment

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

the first available platform found in alphabetical order?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think this relates a bit to the other comments above; I kept it somewhat "on the surface here" because I also didn't want to document it as a strict behavior. We need to define some of that in more details; "first available" may also be "best match", which may not be "alphabetical", i.e., if amd64 and arm64 are present, this should pick arm64 as a default (on arm), and amd64 (on x86).

So perhaps even "first available" is wrong already.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me if we want to define this a bit better somewhere else (or leave the road open to a better definition at a later point, without adding to user expectations)


If the local image store has multiple platform variants of an image, the
`--platform` option selects which variant to show the history for. An error
is produced if the given platform is not present in the local image cache.

The platform option takes the `os[/arch[/variant]]` format; for example,
`linux/amd64` or `linux/arm64/v8`. Architecture and variant are optional,
and if omitted falls back to the daemon's defaults.


The following example pulls the RISC-V variant of the `alpine:latest` image
and shows its history.


```console
$ docker image pull --quiet --platform=linux/riscv64 alpine
docker.io/library/alpine:latest

$ docker image history --platform=linux/s390x alpine
IMAGE CREATED CREATED BY SIZE COMMENT
beefdbd8a1da 3 weeks ago /bin/sh -c #(nop) CMD ["/bin/sh"] 0B
<missing> 3 weeks ago /bin/sh -c #(nop) ADD file:ba2637314e600db5a… 8.46MB
```

The following example attempts to show the history for a platform variant of
`alpine:latest` that doesn't exist in the local image store, resulting in
an error.

```console
$ docker image ls --tree
IMAGE ID DISK USAGE CONTENT SIZE IN USE
alpine:latest beefdbd8a1da 10.6MB 3.37MB
├─ linux/riscv64 80cde017a105 10.6MB 3.37MB
├─ linux/amd64 33735bd63cf8 0B 0B
├─ linux/arm/v6 50f635c8b04d 0B 0B
├─ linux/arm/v7 f2f82d424957 0B 0B
├─ linux/arm64/v8 9cee2b382fe2 0B 0B
├─ linux/386 b3e87f642f5c 0B 0B
├─ linux/ppc64le c7a6800e3dc5 0B 0B
└─ linux/s390x 2b5b26e09ca2 0B 0B

$ docker image history --platform=linux/s390x alpine
Error response from daemon: image with reference alpine:latest was found but does not match the specified platform: wanted linux/s390x
```
Loading
Loading