bugfix: start cli setRawMode when in tty mode and Terminal

Signed-off-by: zhangyue <[email protected]>

Author: zhangyue

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

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"}