From acd9c668647d273488772bfcb06a0f1a44dfb411 Mon Sep 17 00:00:00 2001 From: baude Date: Wed, 8 Nov 2017 15:14:33 -0600 Subject: [PATCH] Fix terminal attach Re-order the startup of a new container via run from initialize > start > attach to initialize > attach > start. This fixes output when running: kpod run -i -t IMAGE command and kpod run IMAGE command Signed-off-by: baude --- .papr.yml | 2 ++ Makefile | 2 +- cmd/kpod/create.go | 10 +++++++++- cmd/kpod/run.go | 35 +++++++++++++++++++++++++++-------- libpod/container.go | 4 ++-- libpod/container_attach.go | 18 ++++++++++++------ 6 files changed, 53 insertions(+), 18 deletions(-) diff --git a/.papr.yml b/.papr.yml index d522194116..3414051b3e 100644 --- a/.papr.yml +++ b/.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 diff --git a/Makefile b/Makefile index e6d44e03e8..06d558dfc2 100644 --- a/Makefile +++ b/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 diff --git a/cmd/kpod/create.go b/cmd/kpod/create.go index bbfeaebaed..0a0e31c406 100644 --- a/cmd/kpod/create.go +++ b/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"), diff --git a/cmd/kpod/run.go b/cmd/kpod/run.go index 84da12e6a1..87ff2d0351 100644 --- a/cmd/kpod/run.go +++ b/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 } diff --git a/libpod/container.go b/libpod/container.go index 7c2f921f38..d93efda979 100644 --- a/libpod/container.go +++ b/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 } diff --git a/libpod/container_attach.go b/libpod/container_attach.go index e308df4a44..8c1c98fe53 100644 --- a/libpod/container_attach.go +++ b/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() {