Merge pull request containers#47 from baude/terminal_attach

Fix terminal attach

.papr.yml

@@ -10,6 +10,8 @@ host:
 
 required: true
 
+timeout: 45m
+
 tests:
   # mount yum repos to inherit injected mirrors from PAPR
   - docker run --net=host --privileged -v /etc/yum.repos.d:/etc/yum.repos.d.host:ro

Makefile

@@ -112,7 +112,7 @@ testunit:
 	$(GO) test -tags "$(BUILDTAGS)" -cover $(PACKAGES)
 
 localintegration: test-binaries
-	./test/test_runner.sh ${TESTFLAGS}
+	bash -i ./test/test_runner.sh ${TESTFLAGS}
 
 binaries: conmon kpod
 

cmd/kpod/create.go

@@ -319,6 +319,14 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime) (*createConfig, er
 		blkioWeight = uint16(u)
 	}
 
+	// Because we cannot do a non-terminal attach, we need to set tty to true
+	// if detach is not false
+	// TODO Allow non-terminal attach
+	tty := c.Bool("tty")
+	if !c.Bool("detach") && !tty {
+		tty = true
+	}
+
 	config := &createConfig{
 		capAdd:         c.StringSlice("cap-add"),
 		capDrop:        c.StringSlice("cap-drop"),
@@ -387,7 +395,7 @@ func parseCreateOpts(c *cli.Context, runtime *libpod.Runtime) (*createConfig, er
 		storageOpts: c.StringSlice("storage-opt"),
 		sysctl:      sysctl,
 		tmpfs:       c.StringSlice("tmpfs"),
-		tty:         c.Bool("tty"),
+		tty:         tty,
 		user:        uid,
 		group:       gid,
 		volumes:     c.StringSlice("volume"),

cmd/kpod/run.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"sync"
 
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/libpod"
@@ -91,20 +92,38 @@ func runCmd(c *cli.Context) error {
 		libpod.WriteFile(ctr.ID(), c.String("cidfile"))
 		return nil
 	}
+
+	// Create a bool channel to track that the console socket attach
+	// is successful.
+	attached := make(chan bool)
+	// Create a waitgroup so we can sync and wait for all goroutines
+	// to finish before exiting main
+	var wg sync.WaitGroup
+
+	if !createConfig.detach {
+		// We increment the wg counter because we need to do the attach
+		wg.Add(1)
+		// Attach to the running container
+		go func() {
+			logrus.Debug("trying to attach to the container %s", ctr.ID())
+			defer wg.Done()
+			if err := ctr.Attach(false, c.String("detach-keys"), attached); err != nil {
+				logrus.Errorf("unable to attach to container %s: %q", ctr.ID(), err)
+			}
+		}()
+		if !<-attached {
+			return errors.Errorf("unable to attach to container %s", ctr.ID())
+		}
+	}
 	// Start the container
 	if err := ctr.Start(); err != nil {
 		return errors.Wrapf(err, "unable to start container %q", ctr.ID())
 	}
 	logrus.Debug("started container ", ctr.ID())
-	if createConfig.tty {
-		// Attach to the running container
-		logrus.Debug("trying to attach to the container %s", ctr.ID())
-		if err := ctr.Attach(false, c.String("detach-keys")); err != nil {
-			return errors.Wrapf(err, "unable to attach to container %s", ctr.ID())
-		}
-	} else {
+
+	if createConfig.detach {
 		fmt.Printf("%s\n", ctr.ID())
 	}
-
+	wg.Wait()
 	return nil
 }

libpod/container.go

@@ -393,7 +393,7 @@ func (c *Container) Exec(cmd []string, tty bool, stdin bool) (string, error) {
 
 // Attach attaches to a container
 // Returns fully qualified URL of streaming server for the container
-func (c *Container) Attach(noStdin bool, keys string) error {
+func (c *Container) Attach(noStdin bool, keys string, attached chan<- bool) error {
 	// Check the validity of the provided keys first
 	var err error
 	detachKeys := []byte{}
@@ -410,7 +410,7 @@ func (c *Container) Attach(noStdin bool, keys string) error {
 	}
 	resize := make(chan remotecommand.TerminalSize)
 	defer close(resize)
-	err = c.attachContainerSocket(resize, noStdin, detachKeys)
+	err = c.attachContainerSocket(resize, noStdin, detachKeys, attached)
 	if err != nil {
 		return err
 	}

libpod/container_attach.go

@@ -2,6 +2,7 @@ package libpod
 
 import (
 	"fmt"
+	"golang.org/x/crypto/ssh/terminal"
 	"io"
 	"net"
 	"os"
@@ -25,7 +26,7 @@ const (
 )
 
 // attachContainerSocket connects to the container's attach socket and deals with the IO
-func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, noStdIn bool, detachKeys []byte) error {
+func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSize, noStdIn bool, detachKeys []byte, attached chan<- bool) error {
 	inputStream := os.Stdin
 	outputStream := os.Stdout
 	errorStream := os.Stderr
@@ -38,12 +39,14 @@ func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSi
 		return errors.Errorf("no tty available for %s", c.ID())
 	}
 
-	oldTermState, err := term.SaveState(inputStream.Fd())
-	if err != nil {
-		return errors.Wrapf(err, "unable to save terminal state")
-	}
+	if terminal.IsTerminal(int(inputStream.Fd())) {
+		oldTermState, err := term.SaveState(inputStream.Fd())
+		if err != nil {
+			return errors.Wrapf(err, "unable to save terminal state")
+		}
 
-	defer term.RestoreTerminal(inputStream.Fd(), oldTermState)
+		defer term.RestoreTerminal(inputStream.Fd(), oldTermState)
+	}
 
 	// Put both input and output into raw
 	if !noStdIn {
@@ -71,6 +74,9 @@ func (c *Container) attachContainerSocket(resize <-chan remotecommand.TerminalSi
 	}
 	defer conn.Close()
 
+	// signal back that the connection was made
+	attached <- true
+
 	receiveStdoutError := make(chan error)
 	if outputStream != nil || errorStream != nil {
 		go func() {