Skip to content

Commit

Permalink
feat: support '--detach-keys' for 'nerdctl (run|start)'
Browse files Browse the repository at this point in the history
Signed-off-by: Hsing-Yu (David) Chen <davidhsingyuchen@gmail.com>
  • Loading branch information
davidhsingyuchen committed Mar 29, 2023
1 parent 933c6c3 commit cce73c3
Show file tree
Hide file tree
Showing 12 changed files with 183 additions and 45 deletions.
2 changes: 1 addition & 1 deletion cmd/nerdctl/compose_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
56 changes: 36 additions & 20 deletions cmd/nerdctl/container_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,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")
Expand Down Expand Up @@ -379,30 +380,19 @@ func runAction(cmd *cobra.Command, args []string) (err error) {
}
}

detachKeys, err := cmd.Flags().GetString("detach-keys")
if err != nil {
return err
}
lab, err := c.Labels(ctx)
if err != nil {
return err
}
logURI := lab[labels.LogURI]
task, err := taskutil.NewTask(ctx, client, c, false, flagI, flagT, flagD, con, logURI)
task, err := taskutil.NewTask(ctx, client, c, false, flagI, flagT, flagD, con, logURI, detachKeys)
if err != nil {
return err
}
var statusC <-chan containerd.ExitStatus
if !flagD {
defer func() {
if 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
}
Expand All @@ -419,13 +409,39 @@ func runAction(cmd *cobra.Command, args []string) (err error) {
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))
io := task.IO()
if io == nil {
return errors.New("got a nil IO from the task")
}
ioC := make(chan struct{})
go func() {
logrus.Debug("waiting for IO")
io.Wait()
ioC <- struct{}{}
logrus.Debug("IO done")
}()
select {
case status := <-statusC:
// Only remove the container if it exits (i.e. don't remove it if the user just detaches from it).
if 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))
}
case <-ioC:
return nil
}
return nil
}
Expand Down
19 changes: 19 additions & 0 deletions cmd/nerdctl/container_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package main

import (
"bytes"
"errors"
"fmt"
"os"
Expand Down Expand Up @@ -467,3 +468,21 @@ func TestRunWithTtyAndDetached(t *testing.T) {
withTtyContainer := base.InspectContainer(withTtyContainerName)
assert.Equal(base.T, 0, withTtyContainer.State.ExitCode)
}

func TestRunWithDetachKeys(t *testing.T) {
t.Parallel()

base := testutil.NewBase(t)
if testutil.GetTarget() == testutil.Nerdctl {
testutil.RequireDaemonVersion(base, ">= 1.6.0-0")
}

containerName := testutil.Identifier(t)
opts := []func(*testutil.Cmd){
testutil.WithStdin(bytes.NewReader([]byte{17, 16})), // https://www.physics.udel.edu/~watson/scen103/ascii.html
}
base.Cmd("run", "-it", "--detach-keys=ctrl-q,ctrl-p", "--name", containerName, testutil.CommonImage).CmdOption(opts...).AssertOK()
defer base.Cmd("container", "rm", "-f", containerName).AssertOK()
container := base.InspectContainer(containerName)
assert.Equal(base.T, container.State.Running, true)
}
6 changes: 4 additions & 2 deletions docs/command-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down Expand Up @@ -366,7 +367,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`
Expand Down Expand Up @@ -534,8 +535,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

Expand Down
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ require (
github.com/fluent/fluent-logger-golang v1.9.0
github.com/hashicorp/go-multierror v1.1.1
github.com/ipfs/go-cid v0.4.0
github.com/klauspost/compress v1.16.3
github.com/mattn/go-isatty v0.0.18
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.0.0-20221205130635-1aeaba878587
github.com/opencontainers/go-digest v1.0.0
github.com/opencontainers/image-spec v1.1.0-rc2.0.20221005185240-3a7f492d3f1b
github.com/opencontainers/runtime-spec v1.1.0-rc.1
Expand All @@ -62,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/v3 v3.0.1 // indirect
github.com/containerd/fifo v1.1.0 // indirect
Expand All @@ -84,7 +87,6 @@ require (
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/imdario/mergo v0.3.14 // indirect
github.com/inconshreveable/mousetrap v1.0.1 // indirect
github.com/klauspost/compress v1.16.3
github.com/klauspost/cpuid/v2 v2.1.1 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-shellwords v1.0.12 // indirect
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -335,6 +336,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=
Expand Down Expand Up @@ -720,6 +722,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.0.0-20221205130635-1aeaba878587 h1:HfkjXDfhgVaN5rmueG8cL8KKeFNecRCXFhaJ2qZ5SKA=
github.com/moby/term v0.0.0-20221205130635-1aeaba878587/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=
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/types/container_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/container/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/container/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
64 changes: 64 additions & 0 deletions pkg/consoleutil/detach.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
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 consoleutil

import (
"errors"
"fmt"
"io"

"github.com/moby/term"
)

const DefaultDetachKeys = "ctrl-p,ctrl-q"

type detachableStdin struct {
stdin io.Reader
closer func()
}

// NewDetachableStdin returns a new TTY proxy reader
// which wraps stdin and detects when the specified detach keys are read,
// in which case closer will be called.
func NewDetachableStdin(stdin io.Reader, keys string, closer func()) (io.Reader, error) {
if len(keys) == 0 {
keys = DefaultDetachKeys
}
b, err := term.ToBytes(keys)
if err != nil {
return nil, fmt.Errorf("failed to convert the detach keys to bytes: %w", err)
}
return &detachableStdin{
stdin: term.NewEscapeProxy(stdin, b),
closer: closer,
}, nil
}

func (ds *detachableStdin) Read(p []byte) (int, error) {
n, err := ds.stdin.Read(p)
var eerr term.EscapeError
if errors.As(err, &eerr) {
if ds.closer != nil {
ds.closer()
}
// TODO (before merging this PR): Return a nil error instead after https://github.com/containerd/containerd/issues/8326 is fixed.
// Returning an error for now to avoid debug logs overflowing my screen.
// return n, nil
return n, err
}
return n, err
}
43 changes: 27 additions & 16 deletions pkg/containerutil/containerutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package containerutil

import (
"context"
"errors"
"fmt"
"io"
"path"
Expand Down Expand Up @@ -198,7 +199,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) (err error) {
func Start(ctx context.Context, container containerd.Container, flagA bool, client *containerd.Client, detachKeys string) (err error) {
// defer the storage of start error in the dedicated label
defer func() {
if err != nil {
Expand Down Expand Up @@ -247,23 +248,14 @@ func Start(ctx context.Context, container containerd.Container, flagA bool, clie
logrus.WithError(err).Debug("failed to delete old task")
}
}
task, err := taskutil.NewTask(ctx, client, container, flagA, false, flagT, true, con, logURI)
task, err := taskutil.NewTask(ctx, client, container, flagA, false, flagT, true, con, logURI, detachKeys)
if err != nil {
return err
}

var statusC <-chan containerd.ExitStatus
if flagA {
statusC, err = task.Wait(ctx)
if err != nil {
return err
}
}

if err := task.Start(ctx); err != nil {
return err
}

if !flagA {
return nil
}
Expand All @@ -272,16 +264,35 @@ func Start(ctx context.Context, container containerd.Container, flagA bool, clie
logrus.WithError(err).Error("console resize")
}
}

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))
io := task.IO()
if io == nil {
return errors.New("got a nil IO from the task")
}
ioC := make(chan struct{})
go func() {
logrus.Debug("waiting for IO")
io.Wait()
ioC <- struct{}{}
logrus.Debug("IO done")
}()
select {
case status := <-statusC:
code, _, err := status.Result()
if err != nil {
return err
}
if code != 0 {
return errutil.NewExitCoderErr(int(code))
}
case <-ioC:
return nil
}
return nil
}
Expand Down
Loading

0 comments on commit cce73c3

Please sign in to comment.