diff --git a/cmd/nerdctl/create.go b/cmd/nerdctl/create.go index b0811f17da7..7a5c9f063c0 100644 --- a/cmd/nerdctl/create.go +++ b/cmd/nerdctl/create.go @@ -70,7 +70,12 @@ func createAction(cmd *cobra.Command, args []string) error { } defer cancel() - container, gc, err := createContainer(ctx, cmd, client, args, platform, false, false, true) + flagT, err := cmd.Flags().GetBool("tty") + if err != nil { + return err + } + + container, gc, err := createContainer(ctx, cmd, client, args, platform, false, flagT, true) if err != nil { if gc != nil { gc() diff --git a/cmd/nerdctl/create_linux_test.go b/cmd/nerdctl/create_linux_test.go index 693788729da..fb1268fb43a 100644 --- a/cmd/nerdctl/create_linux_test.go +++ b/cmd/nerdctl/create_linux_test.go @@ -18,10 +18,12 @@ package main import ( "fmt" + "runtime" "testing" "github.com/containerd/nerdctl/pkg/testutil" "github.com/containerd/nerdctl/pkg/testutil/nettestutil" + "gotest.tools/v3/assert" ) func TestCreate(t *testing.T) { @@ -106,3 +108,29 @@ func TestCreateWithMACAddress(t *testing.T) { } } } + +func TestCreateWithTty(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("json-file log driver is not yet implemented on Windows") + } + base := testutil.NewBase(t) + imageName := testutil.CommonImage + withoutTtyContainerName := "without-terminal-" + testutil.Identifier(t) + withTtyContainerName := "with-terminal-" + testutil.Identifier(t) + + // without -t, fail + base.Cmd("create", "--name", withoutTtyContainerName, imageName, "stty").AssertOK() + base.Cmd("start", withoutTtyContainerName).AssertOK() + defer base.Cmd("container", "rm", "-f", withoutTtyContainerName).AssertOK() + base.Cmd("logs", withoutTtyContainerName).AssertCombinedOutContains("stty: standard input: Not a tty") + withoutTtyContainer := base.InspectContainer(withoutTtyContainerName) + assert.Equal(base.T, 1, withoutTtyContainer.State.ExitCode) + + // with -t, success + base.Cmd("create", "-t", "--name", withTtyContainerName, imageName, "stty").AssertOK() + base.Cmd("start", withTtyContainerName).AssertOK() + defer base.Cmd("container", "rm", "-f", withTtyContainerName).AssertOK() + base.Cmd("logs", withTtyContainerName).AssertCombinedOutContains("speed 38400 baud; line = 0;") + withTtyContainer := base.InspectContainer(withTtyContainerName) + assert.Equal(base.T, 0, withTtyContainer.State.ExitCode) +} diff --git a/cmd/nerdctl/run.go b/cmd/nerdctl/run.go index 20aa7761b73..8d0e73d26f3 100644 --- a/cmd/nerdctl/run.go +++ b/cmd/nerdctl/run.go @@ -98,7 +98,7 @@ func setCreateFlags(cmd *cobra.Command) { // No "-h" alias for "--help", because "-h" for "--hostname". cmd.Flags().Bool("help", false, "show help") - cmd.Flags().BoolP("tty", "t", false, "(Currently -t needs to correspond to -i)") + cmd.Flags().BoolP("tty", "t", false, "Allocate a pseudo-TTY") cmd.Flags().BoolP("interactive", "i", false, "Keep STDIN open even if not attached") cmd.Flags().String("restart", "no", `Restart policy to apply when a container exits (implemented values: "no"|"always")`) cmd.RegisterFlagCompletionFunc("restart", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { @@ -322,7 +322,7 @@ func runAction(cmd *cobra.Command, args []string) error { } var con console.Console - if flagT { + if flagT && !flagD { con = console.Current() defer con.Reset() if err := con.SetRaw(); err != nil { @@ -336,7 +336,7 @@ func runAction(cmd *cobra.Command, args []string) error { } logURI := lab[labels.LogURI] - task, err := taskutil.NewTask(ctx, client, container, flagI, flagT, flagD, con, logURI) + task, err := taskutil.NewTask(ctx, client, container, false, flagI, flagT, flagD, con, logURI) if err != nil { return err } @@ -477,9 +477,6 @@ func createContainer(ctx context.Context, cmd *cobra.Command, client *containerd } if flagT { - if flagD { - return nil, nil, errors.New("currently flag -t and -d cannot be specified together (FIXME)") - } opts = append(opts, oci.WithTTY) } @@ -491,8 +488,14 @@ func createContainer(ctx context.Context, cmd *cobra.Command, client *containerd internalLabels.mountPoints = mountPoints opts = append(opts, mountOpts...) + // always set internalLabels.logURI + // to support restart the container that run with "-it", like + // + // 1, nerdctl run --name demo -it imagename + // 2, ctrl + c to stop demo container + // 3, nerdctl start/restart demo var logURI string - if flagD { + { // json-file is the built-in and default log driver for nerdctl logDriver, err := cmd.Flags().GetString("log-driver") if err != nil { diff --git a/cmd/nerdctl/run_test.go b/cmd/nerdctl/run_test.go index 58ce840be10..3f1e05ecf56 100644 --- a/cmd/nerdctl/run_test.go +++ b/cmd/nerdctl/run_test.go @@ -449,3 +449,27 @@ COPY --from=builder /go/src/logger/logger / assert.Check(t, strings.Contains(log, "foo")) assert.Check(t, strings.Contains(log, "bar")) } + +func TestRunWithTtyAndDetached(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("json-file log driver is not yet implemented on Windows") + } + base := testutil.NewBase(t) + imageName := testutil.CommonImage + withoutTtyContainerName := "without-terminal-" + testutil.Identifier(t) + withTtyContainerName := "with-terminal-" + testutil.Identifier(t) + + // without -t, fail + base.Cmd("run", "-d", "--name", withoutTtyContainerName, imageName, "stty").AssertOK() + defer base.Cmd("container", "rm", "-f", withoutTtyContainerName).AssertOK() + base.Cmd("logs", withoutTtyContainerName).AssertCombinedOutContains("stty: standard input: Not a tty") + withoutTtyContainer := base.InspectContainer(withoutTtyContainerName) + assert.Equal(base.T, 1, withoutTtyContainer.State.ExitCode) + + // with -t, success + base.Cmd("run", "-d", "-t", "--name", withTtyContainerName, imageName, "stty").AssertOK() + defer base.Cmd("container", "rm", "-f", withTtyContainerName).AssertOK() + base.Cmd("logs", withTtyContainerName).AssertCombinedOutContains("speed 38400 baud; line = 0;") + withTtyContainer := base.InspectContainer(withTtyContainerName) + assert.Equal(base.T, 0, withTtyContainer.State.ExitCode) +} diff --git a/cmd/nerdctl/start.go b/cmd/nerdctl/start.go index 147ca221d31..f2ce82ad035 100644 --- a/cmd/nerdctl/start.go +++ b/cmd/nerdctl/start.go @@ -21,19 +21,17 @@ import ( "encoding/json" "errors" "fmt" - "net/url" - "os" "runtime" "strings" "github.com/containerd/containerd" - "github.com/containerd/containerd/cio" "github.com/containerd/containerd/cmd/ctr/commands" "github.com/containerd/containerd/oci" "github.com/containerd/nerdctl/pkg/formatter" "github.com/containerd/nerdctl/pkg/idutil/containerwalker" "github.com/containerd/nerdctl/pkg/labels" "github.com/containerd/nerdctl/pkg/netutil/nettype" + "github.com/containerd/nerdctl/pkg/taskutil" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" @@ -116,23 +114,13 @@ func startContainer(ctx context.Context, container containerd.Container, flagA b return err } - taskCIO := cio.NullIO - - // Choosing the user selected option over the labels - if flagA { - taskCIO = cio.NewCreator(cio.WithStreams(os.Stdin, os.Stdout, os.Stderr)) - } - if logURIStr := lab[labels.LogURI]; logURIStr != "" { - logURI, err := url.Parse(logURIStr) - if err != nil { - return err - } - if flagA { - logrus.Debug("attaching output instead of using the log-uri") - } else { - taskCIO = cio.LogURI(logURI) - } + process, err := container.Spec(ctx) + if err != nil { + return err } + flagT := process.Process.Terminal + + logURI := lab[labels.LogURI] cStatus := formatter.ContainerStatus(ctx, container) if cStatus == "Up" { @@ -147,7 +135,7 @@ func startContainer(ctx context.Context, container containerd.Container, flagA b logrus.WithError(err).Debug("failed to delete old task") } } - task, err := container.NewTask(ctx, taskCIO) + task, err := taskutil.NewTask(ctx, client, container, flagA, false, flagT, true, nil, logURI) if err != nil { return err } diff --git a/pkg/taskutil/taskutil.go b/pkg/taskutil/taskutil.go index 2066a2d6329..f6c35f30dbc 100644 --- a/pkg/taskutil/taskutil.go +++ b/pkg/taskutil/taskutil.go @@ -36,9 +36,40 @@ import ( ) // NewTask is from https://github.com/containerd/containerd/blob/v1.4.3/cmd/ctr/commands/tasks/tasks_unix.go#L70-L108 -func NewTask(ctx context.Context, client *containerd.Client, container containerd.Container, flagI, flagT, flagD bool, con console.Console, logURI string) (containerd.Task, error) { +func NewTask(ctx context.Context, client *containerd.Client, container containerd.Container, + flagA, flagI, flagT, flagD bool, con console.Console, logURI string) (containerd.Task, error) { var ioCreator cio.Creator - if flagT { + if flagA { + logrus.Debug("attaching output instead of using the log-uri") + if flagT { + ioCreator = cio.NewCreator(cio.WithStreams(os.Stdin, os.Stdout, os.Stderr), cio.WithTerminal) + } else { + ioCreator = cio.NewCreator(cio.WithStreams(os.Stdin, os.Stdout, os.Stderr)) + } + + } else if flagT && flagD { + u, err := url.Parse(logURI) + if err != nil { + return nil, err + } + + var args []string + for k, vs := range u.Query() { + args = append(args, k) + if len(vs) > 0 { + args = append(args, vs[0]) + } + } + + // args[0]: _NERDCTL_INTERNAL_LOGGING + // args[1]: /var/lib/nerdctl/1935db59 + if len(args) != 2 { + return nil, errors.New("parse logging path error") + } + ioCreator = cio.TerminalBinaryIO(u.Path, map[string]string{ + args[0]: args[1], + }) + } else if flagT && !flagD { if con == nil { return nil, errors.New("got nil con with flagT=true") }