feat: support '--detach-keys' for 'nerdctl (run|start)'

cmd/nerdctl/compose_start.go

@@ -109,7 +109,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, false, client, ""); err != nil {
 				return err
 			}
 			info, err := c.Info(ctx, containerd.WithoutRefreshedMetadata)

cmd/nerdctl/container_run.go

@@ -22,7 +22,6 @@ import (
 	"runtime"
 
 	"github.com/containerd/console"
-	"github.com/containerd/containerd"
 	"github.com/containerd/nerdctl/pkg/api/types"
 	"github.com/containerd/nerdctl/pkg/clientutil"
 	"github.com/containerd/nerdctl/pkg/cmd/container"
@@ -91,6 +90,7 @@ func setCreateFlags(cmd *cobra.Command) {
 	})
 	cmd.Flags().String("stop-signal", "SIGTERM", "Signal to stop a container")
 	cmd.Flags().Int("stop-timeout", 0, "Timeout (in seconds) to stop a container")
+	cmd.Flags().String("detach-keys", consoleutil.DefaultDetachKeys, "Override the default detach keys")
 
 	// #region for init process
 	cmd.Flags().Bool("init", false, "Run an init process inside the container, Default to use tini")
@@ -282,6 +282,10 @@ func processCreateCommandFlagsInRun(cmd *cobra.Command) (opt types.ContainerCrea
 	if err != nil {
 		return
 	}
+	opt.DetachKeys, err = cmd.Flags().GetString("detach-keys")
+	if err != nil {
+		return
+	}
 	return opt, nil
 }
 
@@ -358,25 +362,12 @@ func runAction(cmd *cobra.Command, args []string) error {
 		return err
 	}
 	logURI := lab[labels.LogURI]
-	task, err := taskutil.NewTask(ctx, client, c, false, createOpt.Interactive, createOpt.TTY, createOpt.Detach, con, logURI)
+	detachC := make(chan struct{})
+	task, err := taskutil.NewTask(ctx, client, c, false, createOpt.Interactive, createOpt.TTY, createOpt.Detach,
+		con, logURI, createOpt.DetachKeys, detachC)
 	if err != nil {
 		return err
 	}
-	var statusC <-chan containerd.ExitStatus
-	if !createOpt.Detach {
-		defer func() {
-			if createOpt.Rm {
-				if _, taskDeleteErr := task.Delete(ctx); taskDeleteErr != nil {
-					logrus.Error(taskDeleteErr)
-				}
-			}
-		}()
-		statusC, err = task.Wait(ctx)
-		if err != nil {
-			return err
-		}
-	}
-
 	if err := task.Start(ctx); err != nil {
 		return err
 	}
@@ -390,16 +381,41 @@ func runAction(cmd *cobra.Command, args []string) error {
 			logrus.WithError(err).Error("console resize")
 		}
 	} else {
-		sigc := signalutil.ForwardAllSignals(ctx, task)
-		defer signalutil.StopCatch(sigc)
+		sigC := signalutil.ForwardAllSignals(ctx, task)
+		defer signalutil.StopCatch(sigC)
 	}
-	status := <-statusC
-	code, _, err := status.Result()
+
+	statusC, err := task.Wait(ctx)
 	if err != nil {
 		return err
 	}
-	if code != 0 {
-		return errutil.NewExitCoderErr(int(code))
+	select {
+	// io.Wait() would return when either 1) the user detaches from the container OR 2) the container is about to exit.
+	//
+	// If we replace the `select` block with io.Wait() and
+	// directly use task.Status() to check the status of the container after io.Wait() returns,
+	// it can still be running even though the container is about to exit (somehow especially for Windows).
+	//
+	// As a result, we need a separate detachC to distinguish from the 2 cases mentioned above.
+	case <-detachC:
+		io := task.IO()
+		if io == nil {
+			return errors.New("got a nil IO from the task")
+		}
+		io.Wait()
+	case status := <-statusC:
+		if createOpt.Rm {
+			if _, taskDeleteErr := task.Delete(ctx); taskDeleteErr != nil {
+				logrus.Error(taskDeleteErr)
+			}
+		}
+		code, _, err := status.Result()
+		if err != nil {
+			return err
+		}
+		if code != 0 {
+			return errutil.NewExitCoderErr(int(code))
+		}
 	}
 	return nil
 }

cmd/nerdctl/container_run_linux_test.go

@@ -327,3 +327,34 @@ func TestRunWithOOMScoreAdj(t *testing.T) {
 
 	base.Cmd("run", "--rm", "--oom-score-adj", score, testutil.AlpineImage, "cat", "/proc/self/oom_score_adj").AssertOutContains(score)
 }
+
+func TestRunWithDetachKeys(t *testing.T) {
+	t.Parallel()
+
+	if testutil.GetTarget() == testutil.Docker {
+		t.Skip("When detaching from a container, for a session started with 'docker attach'" +
+			", it prints 'read escape sequence', but for one started with 'docker (run|start)', it prints nothing." +
+			" However, the flag is called '--detach-keys' in all cases" +
+			", so nerdctl prints 'read detach keys' for all cases" +
+			", and that's why this test is skipped for Docker.")
+	}
+
+	base := testutil.NewBase(t)
+	containerName := testutil.Identifier(t)
+	opts := []func(*testutil.Cmd){
+		testutil.WithStdin(testutil.NewDelayOnceReader(bytes.NewReader([]byte{1, 2}))), // https://www.physics.udel.edu/~watson/scen103/ascii.html
+	}
+	defer base.Cmd("container", "rm", "-f", containerName).AssertOK()
+	// unbuffer(1) emulates tty, which is required by `nerdctl run -t`.
+	// unbuffer(1) can be installed with `apt-get install expect`.
+	//
+	// "-p" is needed because we need unbuffer to read from stdin, and from [1]:
+	// "Normally, unbuffer does not read from stdin. This simplifies use of unbuffer in some situations.
+	//  To use unbuffer in a pipeline, use the -p flag."
+	//
+	// [1] https://linux.die.net/man/1/unbuffer
+	base.CmdWithHelper([]string{"unbuffer", "-p"}, "run", "-it", "--detach-keys=ctrl-a,ctrl-b", "--name", containerName, testutil.CommonImage).
+		CmdOption(opts...).AssertOutContains("read detach keys")
+	container := base.InspectContainer(containerName)
+	assert.Equal(base.T, container.State.Running, true)
+}

cmd/nerdctl/container_start.go

@@ -21,6 +21,7 @@ import (
 	"github.com/containerd/nerdctl/pkg/api/types"
 	"github.com/containerd/nerdctl/pkg/clientutil"
 	"github.com/containerd/nerdctl/pkg/cmd/container"
+	"github.com/containerd/nerdctl/pkg/consoleutil"
 	"github.com/spf13/cobra"
 )
 
@@ -37,6 +38,7 @@ func newStartCommand() *cobra.Command {
 
 	startCommand.Flags().SetInterspersed(false)
 	startCommand.Flags().BoolP("attach", "a", false, "Attach STDOUT/STDERR and forward signals")
+	startCommand.Flags().String("detach-keys", consoleutil.DefaultDetachKeys, "Override the default detach keys")
 
 	return startCommand
 }
@@ -50,10 +52,15 @@ func processContainerStartOptions(cmd *cobra.Command) (types.ContainerStartOptio
 	if err != nil {
 		return types.ContainerStartOptions{}, err
 	}
+	detachKeys, err := cmd.Flags().GetString("detach-keys")
+	if err != nil {
+		return types.ContainerStartOptions{}, err
+	}
 	return types.ContainerStartOptions{
-		Stdout:   cmd.OutOrStdout(),
-		GOptions: globalOptions,
-		Attach:   attach,
+		Stdout:     cmd.OutOrStdout(),
+		GOptions:   globalOptions,
+		Attach:     attach,
+		DetachKeys: detachKeys,
 	}, nil
 }
 

cmd/nerdctl/container_start_linux_test.go

@@ -0,0 +1,69 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package main
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"github.com/containerd/nerdctl/pkg/testutil"
+	"gotest.tools/v3/assert"
+)
+
+func TestStartDetachKeys(t *testing.T) {
+	t.Parallel()
+
+	if testutil.GetTarget() == testutil.Docker {
+		t.Skip("When detaching from a container, for a session started with 'docker attach'" +
+			", it prints 'read escape sequence', but for one started with 'docker (run|start)', it prints nothing." +
+			" However, the flag is called '--detach-keys' in all cases" +
+			", so nerdctl prints 'read detach keys' for all cases" +
+			", and that's why this test is skipped for Docker.")
+	}
+
+	base := testutil.NewBase(t)
+	containerName := testutil.Identifier(t)
+
+	defer base.Cmd("container", "rm", "-f", containerName).AssertOK()
+	opts := []func(*testutil.Cmd){
+		// If NewDelayOnceReader is not used,
+		// the container state will be Created instead of Exited.
+		// Maybe `unbuffer` exits too early in that case?
+		testutil.WithStdin(testutil.NewDelayOnceReader(strings.NewReader("exit\n"))),
+	}
+	// unbuffer(1) emulates tty, which is required by `nerdctl run -t`.
+	// unbuffer(1) can be installed with `apt-get install expect`.
+	//
+	// "-p" is needed because we need unbuffer to read from stdin, and from [1]:
+	// "Normally, unbuffer does not read from stdin. This simplifies use of unbuffer in some situations.
+	//  To use unbuffer in a pipeline, use the -p flag."
+	//
+	// [1] https://linux.die.net/man/1/unbuffer
+	base.CmdWithHelper([]string{"unbuffer", "-p"}, "run", "-it", "--name", containerName, testutil.CommonImage).
+		CmdOption(opts...).AssertOK()
+	container := base.InspectContainer(containerName)
+	assert.Equal(base.T, container.State.Running, false)
+
+	opts = []func(*testutil.Cmd){
+		testutil.WithStdin(testutil.NewDelayOnceReader(bytes.NewReader([]byte{1, 2}))), // https://www.physics.udel.edu/~watson/scen103/ascii.html
+	}
+	base.CmdWithHelper([]string{"unbuffer", "-p"}, "start", "-a", "--detach-keys=ctrl-a,ctrl-b", containerName).
+		CmdOption(opts...).AssertOutContains("read detach keys")
+	container = base.InspectContainer(containerName)
+	assert.Equal(base.T, container.State.Running, true)
+}

docs/command-reference.md

@@ -150,6 +150,7 @@ Basic flags:
 - :whale: `--uts=(host)` : UTS namespace to use
 - :whale: `--stop-signal`: Signal to stop a container (default "SIGTERM")
 - :whale: `--stop-timeout`: Timeout (in seconds) to stop a container
+- :whale: `--detach-keys`: Override the default detach keys
 
 Platform flags:
 
@@ -370,7 +371,7 @@ IPFS flags:
 - :nerd_face: `--ipfs-address`: Multiaddr of IPFS API (default uses `$IPFS_PATH` env variable if defined or local directory `~/.ipfs`)
 
 Unimplemented `docker run` flags:
-    `--attach`, `--blkio-weight-device`, `--cpu-rt-*`, `--detach-keys`, `--device-*`,
+    `--attach`, `--blkio-weight-device`, `--cpu-rt-*`, `--device-*`,
     `--disable-content-trust`, `--domainname`, `--expose`, `--health-*`, `--ip6`, `--isolation`, `--no-healthcheck`,
     `--link*`, `--mac-address`, `--publish-all`, `--sig-proxy`, `--storage-opt`,
     `--userns`, `--volume-driver`, `--volumes-from`
@@ -538,8 +539,9 @@ Usage: `nerdctl start [OPTIONS] CONTAINER [CONTAINER...]`
 Flags:
 
 - :whale: `-a, --attach`: Attach STDOUT/STDERR and forward signals
+- :whale: `--detach-keys`: Override the default detach keys
 
-Unimplemented `docker start` flags: `--checkpoint`, `--checkpoint-dir`, `--detach-keys`, `--interactive`
+Unimplemented `docker start` flags: `--checkpoint`, `--checkpoint-dir`, `--interactive`
 
 ### :whale: nerdctl restart
 

go.mod

@@ -37,6 +37,7 @@ require (
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/moby/sys/mount v0.3.3
 	github.com/moby/sys/signal v0.7.0
+	github.com/moby/term v0.5.0
 	github.com/opencontainers/go-digest v1.0.0
 	github.com/opencontainers/image-spec v1.1.0-rc3
 	github.com/opencontainers/runtime-spec v1.1.0-rc.2
@@ -63,6 +64,7 @@ require (
 require (
 	github.com/AdaLogics/go-fuzz-headers v0.0.0-20230106234847-43070de90fa1 // indirect
 	github.com/AdamKorcz/go-118-fuzz-build v0.0.0-20221215162035-5330a85ea652 // indirect
+	github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
 	github.com/cilium/ebpf v0.9.1 // indirect
 	github.com/containerd/cgroups v1.1.0 // indirect
 	github.com/containerd/fifo v1.1.0 // indirect

go.sum

@@ -47,6 +47,7 @@ github.com/AdamKorcz/go-118-fuzz-build v0.0.0-20221215162035-5330a85ea652/go.mod
 github.com/Azure/azure-sdk-for-go v16.2.1+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
 github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
 github.com/Azure/go-ansiterm v0.0.0-20210608223527-2377c96fe795/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
+github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 h1:UQHMgLO+TxOElx5B5HZ4hJQsoJ/PvUvKRhJHDQXO8P8=
 github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E=
 github.com/Azure/go-autorest v10.8.1+incompatible/go.mod h1:r+4oMnoxhatjLLJ6zxSWATqVooLgysK6ZNox3g/xq24=
 github.com/Azure/go-autorest v14.2.0+incompatible/go.mod h1:r+4oMnoxhatjLLJ6zxSWATqVooLgysK6ZNox3g/xq24=
@@ -336,6 +337,7 @@ github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46t
 github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
+github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY=
 github.com/cyphar/filepath-securejoin v0.2.2/go.mod h1:FpkQEhXnPnOthhzymB7CGsFk2G9VLXONKD9G7QGMM+4=
 github.com/cyphar/filepath-securejoin v0.2.3 h1:YX6ebbZCZP7VkM3scTTokDgBL2TY741X51MTk3ycuNI=
 github.com/cyphar/filepath-securejoin v0.2.3/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4=
@@ -723,6 +725,8 @@ github.com/moby/sys/symlink v0.2.0 h1:tk1rOM+Ljp0nFmfOIBtlV3rTDlWOwFRhjEeAhZB0nZ
 github.com/moby/sys/symlink v0.2.0/go.mod h1:7uZVF2dqJjG/NsClqul95CqKOBRQyYSNnJ6BMgR/gFs=
 github.com/moby/term v0.0.0-20200312100748-672ec06f55cd/go.mod h1:DdlQx2hp0Ss5/fLikoLlEeIYiATotOjgB//nb973jeo=
 github.com/moby/term v0.0.0-20210610120745-9d4ed1856297/go.mod h1:vgPCkQMyxTZ7IDy8SXRufE172gr8+K/JE/7hHFxHW3A=
+github.com/moby/term v0.5.0 h1:xt8Q1nalod/v7BqbG21f8mQPqH+xAaC9C3N3wfWbVP0=
+github.com/moby/term v0.5.0/go.mod h1:8FzsFHVUBGZdbDsJw/ot+X+d5HLUbvklYLJ9uGfcI3Y=
 github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=

pkg/api/types/container_types.go

@@ -28,6 +28,8 @@ type ContainerStartOptions struct {
 	GOptions GlobalCommandOptions
 	// Attach specifies whether to attach to the container's stdio.
 	Attach bool
+	// The key sequence for detaching a container.
+	DetachKeys string
 }
 
 // ContainerKillOptions specifies options for `nerdctl (container) kill`.
@@ -59,6 +61,8 @@ type ContainerCreateOptions struct {
 	TTY bool
 	// Detach runs container in background and print container ID
 	Detach bool
+	// The key sequence for detaching a container.
+	DetachKeys string
 	// Restart specifies the policy to apply when a container exits
 	Restart string
 	// Rm specifies whether to remove the container automatically when it exits

pkg/cmd/container/restart.go

@@ -37,7 +37,7 @@ func Restart(ctx context.Context, client *containerd.Client, containers []string
 			if err := containerutil.Stop(ctx, found.Container, options.Timeout); err != nil {
 				return err
 			}
-			if err := containerutil.Start(ctx, found.Container, false, client); err != nil {
+			if err := containerutil.Start(ctx, found.Container, false, client, ""); err != nil {
 				return err
 			}
 			_, err := fmt.Fprintf(options.Stdout, "%s\n", found.Req)

pkg/cmd/container/start.go

@@ -39,7 +39,7 @@ 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); err != nil {
+			if err := containerutil.Start(ctx, found.Container, options.Attach, client, options.DetachKeys); err != nil {
 				return err
 			}
 			if !options.Attach {