From 649357f14ae05568be0d2b601db97f7c1de0557c Mon Sep 17 00:00:00 2001 From: zhangyue Date: Sun, 21 Oct 2018 17:44:26 +0800 Subject: [PATCH] bugfix: start cli setRawMode when in tty mode and Terminal Signed-off-by: zhangyue --- cli/start.go | 35 +++++++++++++++++++++++++++-------- test/cli_start_test.go | 13 +++++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/cli/start.go b/cli/start.go index 13959d82a..41ecfa96b 100644 --- a/cli/start.go +++ b/cli/start.go @@ -60,7 +60,6 @@ func (s *StartCommand) addFlags() { func (s *StartCommand) runStart(args []string) error { ctx := context.Background() apiClient := s.cli.Client() - // attach to io. if s.attach || s.stdin { var wait chan struct{} @@ -69,17 +68,28 @@ func (s *StartCommand) runStart(args []string) error { return fmt.Errorf("cannot start and attach multiple containers at once") } - in, out, err := setRawMode(s.stdin, false) + container := args[0] + c, err := apiClient.ContainerGet(ctx, container) if err != nil { - return fmt.Errorf("failed to set raw mode") + return err } - defer func() { - if err := restoreMode(in, out); err != nil { - fmt.Fprintf(os.Stderr, "failed to restore term mode") + + if err := checkTty(s.attach, c.Config.Tty, os.Stdout.Fd()); err != nil { + return err + } + + if c.Config.Tty { + in, out, err := setRawMode(s.stdin, false) + if err != nil { + return fmt.Errorf("failed to set raw mode") } - }() + defer func() { + if err := restoreMode(in, out); err != nil { + fmt.Fprintf(os.Stderr, "failed to restore term mode") + } + }() + } - container := args[0] conn, br, err := apiClient.ContainerAttach(ctx, container, s.stdin) if err != nil { return fmt.Errorf("failed to attach container: %v", err) @@ -175,6 +185,15 @@ func restoreMode(in, out *terminal.State) error { return nil } +// CheckTty checks if we are trying to attach to a container tty +// from a non-tty client input stream, and if so, returns an error. +func checkTty(attachStdin, ttyMode bool, fd uintptr) error { + if ttyMode && attachStdin && !terminal.IsTerminal(int(fd)) { + return errors.New("the input device is not a TTY") + } + return nil +} + // startExample shows examples in start command, and is used in auto-generated cli docs. func startExample() string { return `$ pouch ps -a diff --git a/test/cli_start_test.go b/test/cli_start_test.go index 6d1e3305e..d60694ae4 100644 --- a/test/cli_start_test.go +++ b/test/cli_start_test.go @@ -15,6 +15,7 @@ import ( "github.com/go-check/check" "github.com/gotestyourself/gotestyourself/icmd" "github.com/kr/pty" + "github.com/stretchr/testify/assert" ) // PouchStartSuite is the test suite for start CLI. @@ -324,6 +325,18 @@ func (suite *PouchStartSuite) TestStartFromCheckpoint(c *check.C) { command.PouchRun("start", "--checkpoint-dir", tmpDir, "--checkpoint", checkpoint, restoredContainer).Assert(c, icmd.Success) } +// TestStartWithTty tests running container with -tty flag and attach stdin in a non-tty client. +func (suite *PouchStartSuite) TestStartWithTty(c *check.C) { + name := "TestStartWithTty" + res := command.PouchRun("create", "-t", "--name", name, busyboxImage, "/bin/sh", "-c", "while true;do echo hello;done") + defer DelContainerForceMultyTime(c, name) + res.Assert(c, icmd.Success) + + attachRes := command.PouchRun("start", "-a", "-i", name) + errString := attachRes.Stderr() + assert.Equal(c, errString, "Error: the input device is not a TTY\n") +} + // TestStartMultiContainers tries to start more than one container. func (suite *PouchStartSuite) TestStartMultiContainers(c *check.C) { containernames := []string{"TestStartMultiContainer-1", "TestStartMultiContainer-2"}