diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 193b1d93a9f..6fa8e272248 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -400,7 +400,7 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: fetch-depth: 1 - - uses: actions/cache@0c907a75c2c80ebcb7f088228285e798b750cf8f # v4.2.1 + - uses: actions/cache@d4323d4df104b026a6aa633fdb11d772146be0bf # v4.2.2 with: path: /root/.vagrant.d key: vagrant-${{ matrix.box }} diff --git a/cmd/nerdctl/compose/compose_start.go b/cmd/nerdctl/compose/compose_start.go index e6bd17d2cb1..9c8510a38e4 100644 --- a/cmd/nerdctl/compose/compose_start.go +++ b/cmd/nerdctl/compose/compose_start.go @@ -112,7 +112,7 @@ func startContainers(ctx context.Context, client *containerd.Client, containers } // in compose, always disable attach - if err := containerutil.Start(ctx, c, false, client, ""); err != nil { + if err := containerutil.Start(ctx, c, []string{}, client, ""); err != nil { return err } info, err := c.Info(ctx, containerd.WithoutRefreshedMetadata) diff --git a/cmd/nerdctl/container/container_create.go b/cmd/nerdctl/container/container_create.go index 5b6de8bd97a..4b619811ddf 100644 --- a/cmd/nerdctl/container/container_create.go +++ b/cmd/nerdctl/container/container_create.go @@ -19,6 +19,7 @@ package container import ( "fmt" "runtime" + "strings" "github.com/spf13/cobra" @@ -79,6 +80,24 @@ func processContainerCreateOptions(cmd *cobra.Command) (types.ContainerCreateOpt // The nerdctl create command similar to nerdctl run -d except the container is never started. // So we keep the default value of `opt.Detach` true. opt.Detach = true + + opt.Attach, err = cmd.Flags().GetStringSlice("attach") + if err != nil { + return opt, err + } + + validAttachFlag := true + for i, str := range opt.Attach { + opt.Attach[i] = strings.ToUpper(str) + + if opt.Attach[i] != "STDIN" && opt.Attach[i] != "STDOUT" && opt.Attach[i] != "STDERR" { + validAttachFlag = false + } + } + if !validAttachFlag { + return opt, fmt.Errorf("invalid stream specified with -a flag. Valid streams are STDIN, STDOUT, and STDERR") + } + opt.Restart, err = cmd.Flags().GetString("restart") if err != nil { return opt, err diff --git a/cmd/nerdctl/container/container_create_test.go b/cmd/nerdctl/container/container_create_test.go index 8b507d37fc6..5c74cc8cec2 100644 --- a/cmd/nerdctl/container/container_create_test.go +++ b/cmd/nerdctl/container/container_create_test.go @@ -18,6 +18,9 @@ package container import ( "encoding/json" + "fmt" + "runtime" + "strings" "testing" "time" @@ -127,3 +130,91 @@ func TestCreateHyperVContainer(t *testing.T) { testCase.Run(t) } + +func TestCreateWithAttach(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("create attach test is not yet implemented on Windows") + } + t.Parallel() + base := testutil.NewBase(t) + + // Test --attach stdout only + stdoutContainer := testutil.Identifier(t) + "-stdout" + t.Cleanup(func() { + base.Cmd("rm", "-f", stdoutContainer).Run() + }) + base.Cmd("create", "--name", stdoutContainer, "--attach", "stdout", + testutil.CommonImage, "sh", "-c", "echo stdout-msg && echo stderr-msg >&2").AssertOK() + base.Cmd("start", stdoutContainer).AssertOutStreamsWithFunc(func(stdout, stderr string) error { + // Verify stdout message is present + expected := "stdout-msg" + if !strings.Contains(stdout, expected) { + return fmt.Errorf("expected %q, got %q", expected, stdout) + } + // Verify stderr message is not present + if strings.Contains(stderr, "stderr-msg") { + return fmt.Errorf("stderr message was captured but should not be: %q", stderr) + } + return nil + }) + + // Test --attach stderr only + stderrContainer := testutil.Identifier(t) + "-stderr" + t.Cleanup(func() { + base.Cmd("rm", "-f", stderrContainer).Run() + }) + base.Cmd("create", "--name", stderrContainer, "--attach", "stderr", + testutil.CommonImage, "sh", "-c", "echo stdout-msg && echo stderr-msg >&2").AssertOK() + base.Cmd("start", stderrContainer).AssertOutStreamsWithFunc(func(stdout, stderr string) error { + // Verify stderr message is present + expected := "stderr-msg" + if !strings.Contains(stderr, expected) { + return fmt.Errorf("expected %q, got %q", expected, stderr) + } + // Verify stdout message is not present + if strings.Contains(stdout, "stdout-msg") { + return fmt.Errorf("stdout message was captured but should not be: %q", stderr) + } + return nil + }) + + // Test both stdout and stderr + bothContainer := testutil.Identifier(t) + "-both" + t.Cleanup(func() { + base.Cmd("rm", "-f", bothContainer).Run() + }) + base.Cmd("create", "--name", bothContainer, "--attach", "stdout", "--attach", "stderr", + testutil.CommonImage, "sh", "-c", "echo stdout-msg && echo stderr-msg >&2").AssertOK() + base.Cmd("start", bothContainer).AssertOutStreamsWithFunc(func(stdout, stderr string) error { + // Verify stdout message is present + expectedStdout := "stdout-msg" + if !strings.Contains(stdout, expectedStdout) { + return fmt.Errorf("Expected stdout message %q, got %q", expectedStdout, stdout) + } + // Verify stderr message is present + expectedStderr := "stderr-msg" + if !strings.Contains(stderr, expectedStderr) { + return fmt.Errorf("Expected stderr message %q, got %q", expectedStderr, stderr) + } + return nil + }) + + // Test start --attach + attachContainer := testutil.Identifier(t) + "-attach" + defer base.Cmd("rm", "-f", attachContainer).Run() + base.Cmd("create", "--name", attachContainer, + testutil.CommonImage, "sh", "-c", "echo stdout-msg && echo stderr-msg >&2").AssertOK() + base.Cmd("start", "--attach", attachContainer).AssertOutStreamsWithFunc(func(stdout, stderr string) error { + // Verify stdout message is present + expectedStdout := "stdout-msg" + if !strings.Contains(stdout, expectedStdout) { + return fmt.Errorf("Expected stdout message %q, got %q", expectedStdout, stdout) + } + // Verify stderr message is present + expectedStderr := "stderr-msg" + if !strings.Contains(stderr, expectedStderr) { + return fmt.Errorf("Expected stderr message %q, got %q", expectedStderr, stderr) + } + return nil + }) +} diff --git a/cmd/nerdctl/container/container_run.go b/cmd/nerdctl/container/container_run.go index 361a3e0d0d9..62a4a972ac3 100644 --- a/cmd/nerdctl/container/container_run.go +++ b/cmd/nerdctl/container/container_run.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "runtime" - "strings" "github.com/spf13/cobra" @@ -73,7 +72,6 @@ func NewRunCommand() *cobra.Command { setCreateFlags(cmd) cmd.Flags().BoolP("detach", "d", false, "Run container in background and print container ID") - cmd.Flags().StringSliceP("attach", "a", []string{}, "Attach STDIN, STDOUT, or STDERR") return cmd } @@ -285,7 +283,7 @@ func setCreateFlags(cmd *cobra.Command) { } return []string{"default"}, cobra.ShellCompDirectiveNoFileComp }) - + cmd.Flags().StringSliceP("attach", "a", []string{}, "Attach STDIN, STDOUT, or STDERR") } func processCreateCommandFlagsInRun(cmd *cobra.Command) (types.ContainerCreateOptions, error) { @@ -312,22 +310,6 @@ func processCreateCommandFlagsInRun(cmd *cobra.Command) (types.ContainerCreateOp if err != nil { return opt, err } - opt.Attach, err = cmd.Flags().GetStringSlice("attach") - if err != nil { - return opt, err - } - - validAttachFlag := true - for i, str := range opt.Attach { - opt.Attach[i] = strings.ToUpper(str) - - if opt.Attach[i] != "STDIN" && opt.Attach[i] != "STDOUT" && opt.Attach[i] != "STDERR" { - validAttachFlag = false - } - } - if !validAttachFlag { - return opt, fmt.Errorf("invalid stream specified with -a flag. Valid streams are STDIN, STDOUT, and STDERR") - } return opt, nil } diff --git a/pkg/cmd/container/create.go b/pkg/cmd/container/create.go index 671ab145ccc..394b77f4e72 100644 --- a/pkg/cmd/container/create.go +++ b/pkg/cmd/container/create.go @@ -88,6 +88,7 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa var internalLabels internalLabels internalLabels.platform = options.Platform internalLabels.namespace = options.GOptions.Namespace + internalLabels.attachStreams = options.Attach var ( id = idgen.GenerateID() @@ -624,11 +625,12 @@ func withStop(stopSignal string, stopTimeout int, ensuredImage *imgutil.EnsuredI type internalLabels struct { // labels from cmd options - namespace string - platform string - extraHosts []string - pidFile string - blkioWeight uint16 + namespace string + platform string + extraHosts []string + pidFile string + blkioWeight uint16 + attachStreams []string // labels from cmd options or automatically set name string hostname string @@ -753,6 +755,13 @@ func withInternalLabels(internalLabels internalLabels) (containerd.NewContainerO if internalLabels.rm != "" { m[labels.ContainerAutoRemove] = internalLabels.rm } + if len(internalLabels.attachStreams) > 0 { + attachJSON, err := json.Marshal(internalLabels.attachStreams) + if err != nil { + return nil, err + } + m[labels.AttachStreams] = string(attachJSON) + } if internalLabels.blkioWeight > 0 { hostConfigLabel.BlkioWeight = internalLabels.blkioWeight diff --git a/pkg/cmd/container/restart.go b/pkg/cmd/container/restart.go index 6b60b082236..91b7c7a8309 100644 --- a/pkg/cmd/container/restart.go +++ b/pkg/cmd/container/restart.go @@ -38,7 +38,7 @@ func Restart(ctx context.Context, client *containerd.Client, containers []string if err := containerutil.Stop(ctx, found.Container, options.Timeout, options.Signal); err != nil { return err } - if err := containerutil.Start(ctx, found.Container, false, client, ""); err != nil { + if err := containerutil.Start(ctx, found.Container, []string{}, client, ""); err != nil { return err } _, err := fmt.Fprintln(options.Stdout, found.Req) diff --git a/pkg/cmd/container/start.go b/pkg/cmd/container/start.go index 3d9de68cb29..b3febdad466 100644 --- a/pkg/cmd/container/start.go +++ b/pkg/cmd/container/start.go @@ -18,6 +18,7 @@ package container import ( "context" + "encoding/json" "fmt" containerd "github.com/containerd/containerd/v2/client" @@ -25,6 +26,7 @@ import ( "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/containerutil" "github.com/containerd/nerdctl/v2/pkg/idutil/containerwalker" + "github.com/containerd/nerdctl/v2/pkg/labels" ) // Start starts a list of `containers`. If attach is true, it only starts a single container. @@ -40,7 +42,16 @@ func Start(ctx context.Context, client *containerd.Client, reqs []string, option if found.MatchCount > 1 { return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req) } - if err := containerutil.Start(ctx, found.Container, options.Attach, client, options.DetachKeys); err != nil { + + var attachStreams []string + // Only get attach streams if we're dealing with a single container request + if len(reqs) == 1 { + attachStreams, err = getAttachStreams(ctx, found.Container, options) + if err != nil { + return err + } + } + if err := containerutil.Start(ctx, found.Container, attachStreams, client, options.DetachKeys); err != nil { return err } if !options.Attach { @@ -55,3 +66,40 @@ func Start(ctx context.Context, client *containerd.Client, reqs []string, option return walker.WalkAll(ctx, reqs, true) } + +func getAttachStreams(ctx context.Context, container containerd.Container, options types.ContainerStartOptions) ([]string, error) { + if options.Attach { + // If start is called with --attach option, --attach attaches to STDOUT and STDERR + // source: https://github.com/containerd/nerdctl/blob/main/docs/command-reference.md#whale-nerdctl-start + // + return []string{"STDOUT", "STDERR"}, nil + } + + // Otherwise, check if attach streams were set in container labels during container create + ctrlabels, err := container.Labels(ctx) + if err != nil { + return []string{}, err + } + + attachJSON, ok := ctrlabels[labels.AttachStreams] + if !ok { + return []string{}, nil + } + + var attachStreams []string + if err := json.Unmarshal([]byte(attachJSON), &attachStreams); err != nil { + return []string{}, err + } + + if len(attachStreams) == 0 { + return []string{}, nil + } + + for _, stream := range attachStreams { + if stream != "STDOUT" && stream != "STDERR" && stream != "STDIN" { + return []string{}, fmt.Errorf("invalid attach stream in labels: %s", stream) + } + } + + return attachStreams, nil +} diff --git a/pkg/containerutil/containerutil.go b/pkg/containerutil/containerutil.go index c929a1716c4..8b21fc718e5 100644 --- a/pkg/containerutil/containerutil.go +++ b/pkg/containerutil/containerutil.go @@ -24,6 +24,7 @@ import ( "io" "path" "path/filepath" + "slices" "strconv" "strings" "syscall" @@ -216,7 +217,7 @@ func GenerateSharingPIDOpts(ctx context.Context, targetCon containerd.Container) } // Start starts `container` with `attach` flag. If `attach` is true, it will attach to the container's stdio. -func Start(ctx context.Context, container containerd.Container, flagA bool, client *containerd.Client, detachKeys string) (err error) { +func Start(ctx context.Context, container containerd.Container, attachStreamOpt []string, client *containerd.Client, detachKeys string) (err error) { // defer the storage of start error in the dedicated label defer func() { if err != nil { @@ -245,6 +246,7 @@ func Start(ctx context.Context, container containerd.Container, flagA bool, clie return err } flagT := process.Process.Terminal + flagA := slices.Contains(attachStreamOpt, "STDOUT") || slices.Contains(attachStreamOpt, "STDERR") var con console.Console if flagA && flagT { con, err = consoleutil.Current() @@ -281,12 +283,6 @@ func Start(ctx context.Context, container containerd.Container, flagA bool, clie } } detachC := make(chan struct{}) - attachStreamOpt := []string{} - if flagA { - // In start, flagA attaches only STDOUT/STDERR - // source: https://github.com/containerd/nerdctl/blob/main/docs/command-reference.md#whale-nerdctl-start - attachStreamOpt = []string{"STDOUT", "STDERR"} - } task, err := taskutil.NewTask(ctx, client, container, attachStreamOpt, false, flagT, true, con, logURI, detachKeys, namespace, detachC) if err != nil { return err diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 0376497ad9e..21e117e467b 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -115,4 +115,7 @@ const ( // DNSSettings sets the dockercompat DNS config values DNSSetting = Prefix + "dns" + + // AttachStreams is the list of streams to attach on container start/run. + AttachStreams = Prefix + "attach-streams" )