From 2aee304c303aa5edf4f3aeb69d30d2de93555287 Mon Sep 17 00:00:00 2001 From: hejianshan Date: Mon, 3 Aug 2026 18:20:44 +0800 Subject: [PATCH] fix: add --timeout flag to container commit The hardcoded 1-hour lease expiration in nerdctl container commit can cause failures for containers with large filesystem deltas (e.g., many packages installed). When the diff.Compare operation exceeds 1 hour, the containerd GC cleans up the expired lease and its ingest data, resulting in 'context deadline exceeded' errors. Changes: - Add Timeout field to ContainerCommitOptions and commit.Opts - Modify Commit() to use configurable lease expiration: * --timeout=0 (or negative): use containerd's default 24h lease * --timeout>0: use the specified duration * --timeout defaults to 1h for backward compatibility - Add --timeout CLI flag (default: 1h) - Add tests for --timeout flag Usage: nerdctl container commit mycontainer myimage:tag # default 1h nerdctl container commit --timeout=4h ... # 4h timeout nerdctl container commit --timeout=0 ... # 24h (containerd default) Signed-off-by: Turbo Jia --- cmd/nerdctl/container/container_commit.go | 8 ++ .../container/container_commit_test.go | 76 +++++++++++++++++++ docs/command-reference.md | 1 + pkg/api/types/container_types.go | 3 + pkg/cmd/container/commit.go | 1 + pkg/imgutil/commit/commit.go | 15 +++- 6 files changed, 102 insertions(+), 2 deletions(-) diff --git a/cmd/nerdctl/container/container_commit.go b/cmd/nerdctl/container/container_commit.go index 14db2be2a0b..2218f0eb9ed 100644 --- a/cmd/nerdctl/container/container_commit.go +++ b/cmd/nerdctl/container/container_commit.go @@ -18,6 +18,7 @@ package container import ( "errors" + "time" "github.com/spf13/cobra" @@ -51,6 +52,7 @@ func CommitCommand() *cobra.Command { cmd.Flags().Bool("zstdchunked", false, "Convert the committed layer to zstd:chunked for lazy pulling") cmd.Flags().Int("zstdchunked-compression-level", 3, "zstd:chunked compression level") cmd.Flags().Int("zstdchunked-chunk-size", 0, "zstd:chunked chunk size") + cmd.Flags().Duration("timeout", 1*time.Hour, "Maximum duration for the commit operation (default 1h, 0 for containerd's default 24h)") return cmd } @@ -123,6 +125,11 @@ func commitOptions(cmd *cobra.Command) (types.ContainerCommitOptions, error) { return types.ContainerCommitOptions{}, err } + timeout, err := cmd.Flags().GetDuration("timeout") + if err != nil { + return types.ContainerCommitOptions{}, err + } + // estargz and zstdchunked are mutually exclusive if estargz && zstdchunked { return types.ContainerCommitOptions{}, errors.New("options --estargz and --zstdchunked lead to conflict, only one of them can be used") @@ -137,6 +144,7 @@ func commitOptions(cmd *cobra.Command) (types.ContainerCommitOptions, error) { Change: change, Compression: types.CompressionType(com), Format: types.ImageFormat(format), + Timeout: timeout, EstargzOptions: types.EstargzOptions{ Estargz: estargz, EstargzCompressionLevel: estargzCompressionLevel, diff --git a/cmd/nerdctl/container/container_commit_test.go b/cmd/nerdctl/container/container_commit_test.go index 68291c39714..16b015dc10a 100644 --- a/cmd/nerdctl/container/container_commit_test.go +++ b/cmd/nerdctl/container/container_commit_test.go @@ -91,6 +91,82 @@ func TestCommit(t *testing.T) { testCase.Run(t) } +func TestCommitWithTimeout(t *testing.T) { + testCase := nerdtest.Setup() + testCase.Require = require.All( + require.Not(nerdtest.Docker), + require.Not(require.Windows), + nerdtest.CGroup, + ) + + testCase.Setup = func(data test.Data, helpers test.Helpers) { + identifier := data.Identifier() + helpers.Ensure("run", "-d", "--name", identifier, testutil.CommonImage, "sleep", nerdtest.Infinity) + nerdtest.EnsureContainerStarted(helpers, identifier) + helpers.Ensure("exec", identifier, "sh", "-euxc", `echo hello-test-commit-timeout > /foo`) + } + + testCase.Cleanup = func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + helpers.Anyhow("rmi", "-f", data.Identifier()) + } + + testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand { + identifier := data.Identifier() + helpers.Ensure( + "commit", + "--timeout=2h", + "-c", `CMD ["/foo"]`, + "-c", `ENTRYPOINT ["cat"]`, + "--pause=false", + identifier, identifier, + ) + return helpers.Command("run", "--rm", identifier) + } + + testCase.Expected = test.Expects(0, nil, expect.Equals("hello-test-commit-timeout\n")) + + testCase.Run(t) +} + +func TestCommitWithTimeoutZero(t *testing.T) { + testCase := nerdtest.Setup() + testCase.Require = require.All( + require.Not(nerdtest.Docker), + require.Not(require.Windows), + nerdtest.CGroup, + ) + + testCase.Setup = func(data test.Data, helpers test.Helpers) { + identifier := data.Identifier() + helpers.Ensure("run", "-d", "--name", identifier, testutil.CommonImage, "sleep", nerdtest.Infinity) + nerdtest.EnsureContainerStarted(helpers, identifier) + helpers.Ensure("exec", identifier, "sh", "-euxc", `echo hello-test-commit-timeout0 > /foo`) + } + + testCase.Cleanup = func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + helpers.Anyhow("rmi", "-f", data.Identifier()) + } + + testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand { + identifier := data.Identifier() + helpers.Ensure( + "commit", + "--timeout=0s", + "-c", `CMD ["/foo"]`, + "-c", `ENTRYPOINT ["cat"]`, + "--pause=false", + identifier, identifier, + ) + return helpers.Command("run", "--rm", identifier) + } + + testCase.Expected = test.Expects(0, nil, expect.Equals("hello-test-commit-timeout0\n")) + + testCase.Run(t) +} + func TestZstdCommit(t *testing.T) { testCase := nerdtest.Setup() testCase.Require = require.All( diff --git a/docs/command-reference.md b/docs/command-reference.md index 9e32e1d8adf..deeeba4a6ab 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -816,6 +816,7 @@ Flags: - :whale: `-m, --message`: Commit message - :whale: `-c, --change`: Apply Dockerfile instruction to the created image (supported directives: [CMD, ENTRYPOINT]) - :whale: `-p, --pause`: Pause container during commit (default: true) +- :nerd_face: `--timeout`: Maximum duration for the commit operation (default: 1h). Set to 0 to use containerd's default lease expiration (24h). Accepts Go duration format (e.g., `2h`, `90m`, `0s`). - :nerd_face: `--compression`: Commit compression algorithm (supported values: zstd or gzip) (default: gzip) (zstd is generally better for compression ratio but might not be as widely supported) - :nerd_face: `--format`: Format of the committed image (supported values: docker or oci) (default: docker) (docker uses Docker Schema2 media types for compatibility, oci uses OCI image format media types) - :nerd_face: `--estargz`: Convert the committed layer to eStargz for lazy pulling diff --git a/pkg/api/types/container_types.go b/pkg/api/types/container_types.go index 8dd099eb53d..c6797fa384d 100644 --- a/pkg/api/types/container_types.go +++ b/pkg/api/types/container_types.go @@ -421,6 +421,9 @@ type ContainerCommitOptions struct { Compression CompressionType // Format specifies the image format for the committed image (docker or oci) Format ImageFormat + // Timeout is the maximum duration for the commit operation (lease expiration). + // Defaults to 1 hour. Set to 0 for no timeout (24h lease). + Timeout time.Duration // Embed EstargzOptions for eStargz conversion options EstargzOptions // Embed ZstdChunkedOptions for zstd:chunked conversion options diff --git a/pkg/cmd/container/commit.go b/pkg/cmd/container/commit.go index 4b447004247..67b8aefa659 100644 --- a/pkg/cmd/container/commit.go +++ b/pkg/cmd/container/commit.go @@ -51,6 +51,7 @@ func Commit(ctx context.Context, client *containerd.Client, rawRef string, req s Changes: changes, Compression: options.Compression, Format: options.Format, + Timeout: options.Timeout, EstargzOptions: options.EstargzOptions, ZstdChunkedOptions: options.ZstdChunkedOptions, } diff --git a/pkg/imgutil/commit/commit.go b/pkg/imgutil/commit/commit.go index f283a4dd290..45d3d87ab09 100644 --- a/pkg/imgutil/commit/commit.go +++ b/pkg/imgutil/commit/commit.go @@ -68,6 +68,9 @@ type Opts struct { Changes Changes Compression types.CompressionType Format types.ImageFormat + // Timeout is the maximum duration for the commit operation (lease expiration). + // Defaults to 1 hour. Set to 0 to use containerd's default (24h). + Timeout time.Duration types.EstargzOptions types.ZstdChunkedOptions } @@ -174,8 +177,16 @@ func Commit(ctx context.Context, client *containerd.Client, container containerd sn = client.SnapshotService(snName) ) - // Don't gc me and clean the dirty data after 1 hour! - ctx, done, err := client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(1*time.Hour)) + // Set lease expiration based on the configured timeout. + // The CLI flag defaults to 1h for backward compatibility. + // Set --timeout=0 to use containerd's default lease expiration (24h). + var done func(context.Context) error + if opts.Timeout <= 0 { + // Use containerd's default lease expiration (24h) by passing no opts. + ctx, done, err = client.WithLease(ctx) + } else { + ctx, done, err = client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(opts.Timeout)) + } if err != nil { return emptyDigest, fmt.Errorf("failed to create lease for commit: %w", err) }