Add image squash command #3756
Add image squash command #3756
Conversation
weapons97
force-pushed
the
main
branch
4 times, most recently
from
b7756fe to
9599d0a
Compare
| cmd.Flags().StringP("message", "m", "", "Commit message") | ||
| } | ||
|
|
||
| func NewSquashCommand() *cobra.Command { |
There was a problem hiding this comment.
https://github.com/containerd/nerdctl/blob/main/docs/command-reference.md needs to be updated
cmd/nerdctl/main.go
Outdated
| @@ -295,6 +295,7 @@ Config file ($NERDCTL_TOML): %s | |||
| image.NewTagCommand(), | |||
| image.NewRmiCommand(), | |||
| image.NewHistoryCommand(), | |||
| image.NewSquashCommand(), | |||
There was a problem hiding this comment.
Not a fan of inflating the number of the top-level commands
pkg/cmd/image/squash.go
Outdated
| } | ||
| } | ||
|
|
||
| // copied from github.com/containerd/containerd/rootfs/apply.go |
There was a problem hiding this comment.
Should be a permanent URL with the git tag (or the commit hash)
There was a problem hiding this comment.
The commit hash has been added.
There was a problem hiding this comment.
Suggest to use a full URL directly, e.g., https://github.com/containerd/containerd/blob/v2.0.1/pkg/rootfs/apply.go#L180
weapons97
force-pushed
the
main
branch
11 times, most recently
from
df967a2 to
b6077c1
Compare
Signed-off-by: weipeng <[email protected]>
cmd/nerdctl/image/image_squash.go
Outdated
| // NewSquashCommand returns a new `squash` command to compress the number of layers of the image | ||
| func NewSquashCommand() *cobra.Command { | ||
| var squashCommand = &cobra.Command{ | ||
| Use: "squash [flags] SOURCE_IMAGE TAG_IMAGE", |
There was a problem hiding this comment.
nit: TAG_IMAGE -> TARGET_IMAGE
| testCase := nerdtest.Setup() | ||
|
|
||
| require := test.Require( | ||
| test.Linux, |
cmd/nerdctl/image/image_squash.go
Outdated
| ) | ||
|
|
||
| func addSquashFlags(cmd *cobra.Command) { | ||
| cmd.Flags().IntP("layer-count", "c", 0, "The number of layers that can be compressed") |
There was a problem hiding this comment.
does this flag specify squashing the last N (N=layer-count) layers? If so I suggest renaming it to last-n-layer.
Or rename it to from-layer, meaning squashing all layers starting from from-layer (and --from-layer=0 means squash all layers)
There was a problem hiding this comment.
Thank you, I have changed it to last-n-layer.
cmd/nerdctl/image/image_squash.go
Outdated
|
|
||
| func addSquashFlags(cmd *cobra.Command) { | ||
| cmd.Flags().IntP("layer-count", "c", 0, "The number of layers that can be compressed") | ||
| cmd.Flags().StringP("layer-digest", "d", "", "The digest of the layer to be compressed") |
There was a problem hiding this comment.
It is indeed unnecessary, so I have removed this flag.
cmd/nerdctl/image/image_squash.go
Outdated
| cmd.Flags().StringP("author", "a", "", `Author (e.g., "nerdctl contributor <[email protected]>")`) | ||
| cmd.Flags().StringP("message", "m", "", "Commit message") |
There was a problem hiding this comment.
nit: maybe use something like author="nerdctl" and message="generated by nerdctl squash" as default values.
docs/command-reference.md
Outdated
| @@ -994,6 +994,25 @@ Flags: | |||
| - `--platform=<PLATFORM>` : Convert content for a specific platform | |||
| - `--all-platforms` : Convert content for all platforms (default: false) | |||
|
|
|||
| ### :nerd_face: nerdctl image squash | |||
|
|
|||
| Squash an image layers. | |||
There was a problem hiding this comment.
suggest to add some details, e.g., Squash all layers starting from from-layer into a single layer.
pkg/cmd/image/squash.go
Outdated
| } | ||
| } | ||
|
|
||
| // copied from github.com/containerd/containerd/rootfs/apply.go |
There was a problem hiding this comment.
Suggest to use a full URL directly, e.g., https://github.com/containerd/containerd/blob/v2.0.1/pkg/rootfs/apply.go#L180
Signed-off-by: weipeng <[email protected]>
| layerN, err := cmd.Flags().GetInt("last-n-layer") | ||
| if err != nil { | ||
| return options, err | ||
| } |
There was a problem hiding this comment.
can we add a check to make sure layerN > 0?
| if matchCount < 1 { | ||
| return fmt.Errorf("%s: not found", option.SourceImageRef) | ||
| } |
There was a problem hiding this comment.
what's the behavior if matchCount > 1?
| return err | ||
| } | ||
| remainingLayerCount := len(img.manifest.Layers) - len(sLayers) | ||
| // Don't gc me and clean the dirty data after 1 hour! |
There was a problem hiding this comment.
why 1 hour? after squash we should just remove the temp data and keep the final new data forever?
| } | ||
|
|
||
| clientImage := containerd.NewImage(sr.client, containerImage) | ||
| manifest, _, err := imgutil.ReadManifest(ctx, clientImage) |
There was a problem hiding this comment.
what if we pass in an image index instead of manifest? seems index is not supported? can we add a check to return an error if the image is not a manifest.
This pr adds the squash image command.
The related issue is #3252.