Merge pull request #4284 from crosbymichael/refactor-tty

Move tty into drivers

container.go

@@ -10,10 +10,8 @@ import (
 	"github.com/dotcloud/docker/graphdriver"
 	"github.com/dotcloud/docker/links"
 	"github.com/dotcloud/docker/nat"
-	"github.com/dotcloud/docker/pkg/term"
 	"github.com/dotcloud/docker/runconfig"
 	"github.com/dotcloud/docker/utils"
-	"github.com/kr/pty"
 	"io"
 	"io/ioutil"
 	"log"
@@ -61,7 +59,6 @@ type Container struct {
 	stderr    *utils.WriteBroadcaster
 	stdin     io.ReadCloser
 	stdinPipe io.WriteCloser
-	ptyMaster io.Closer
 
 	runtime *Runtime
 
@@ -213,56 +210,6 @@ func (container *Container) generateEnvConfig(env []string) error {
 	return nil
 }
 
-func (container *Container) setupPty() error {
-	ptyMaster, ptySlave, err := pty.Open()
-	if err != nil {
-		return err
-	}
-	container.ptyMaster = ptyMaster
-	container.command.Stdout = ptySlave
-	container.command.Stderr = ptySlave
-	container.command.Console = ptySlave.Name()
-
-	// Copy the PTYs to our broadcasters
-	go func() {
-		defer container.stdout.CloseWriters()
-		utils.Debugf("startPty: begin of stdout pipe")
-		io.Copy(container.stdout, ptyMaster)
-		utils.Debugf("startPty: end of stdout pipe")
-	}()
-
-	// stdin
-	if container.Config.OpenStdin {
-		container.command.Stdin = ptySlave
-		container.command.SysProcAttr.Setctty = true
-		go func() {
-			defer container.stdin.Close()
-			utils.Debugf("startPty: begin of stdin pipe")
-			io.Copy(ptyMaster, container.stdin)
-			utils.Debugf("startPty: end of stdin pipe")
-		}()
-	}
-	return nil
-}
-
-func (container *Container) setupStd() error {
-	container.command.Stdout = container.stdout
-	container.command.Stderr = container.stderr
-	if container.Config.OpenStdin {
-		stdin, err := container.command.StdinPipe()
-		if err != nil {
-			return err
-		}
-		go func() {
-			defer stdin.Close()
-			utils.Debugf("start: begin of stdin pipe")
-			io.Copy(stdin, container.stdin)
-			utils.Debugf("start: end of stdin pipe")
-		}()
-	}
-	return nil
-}
-
 func (container *Container) Attach(stdin io.ReadCloser, stdinCloser io.Closer, stdout io.Writer, stderr io.Writer) chan error {
 	var cStdout, cStderr io.ReadCloser
 
@@ -593,17 +540,6 @@ func (container *Container) Start() (err error) {
 	}
 	container.waitLock = make(chan struct{})
 
-	// Setuping pipes and/or Pty
-	var setup func() error
-	if container.Config.Tty {
-		setup = container.setupPty
-	} else {
-		setup = container.setupStd
-	}
-	if err := setup(); err != nil {
-		return err
-	}
-
 	callbackLock := make(chan struct{})
 	callback := func(command *execdriver.Command) {
 		container.State.SetRunning(command.Pid())
@@ -843,7 +779,8 @@ func (container *Container) monitor(callback execdriver.StartCallback) error {
 		populateCommand(container)
 		err = container.runtime.RestoreCommand(container)
 	} else {
-		exitCode, err = container.runtime.Run(container, callback)
+		pipes := execdriver.NewPipes(container.stdin, container.stdout, container.stderr, container.Config.OpenStdin)
+		exitCode, err = container.runtime.Run(container, pipes, callback)
 	}
 
 	if err != nil {
@@ -887,7 +824,6 @@ func (container *Container) cleanup() {
 			link.Disable()
 		}
 	}
-
 	if container.Config.OpenStdin {
 		if err := container.stdin.Close(); err != nil {
 			utils.Errorf("%s: Error close stdin: %s", container.ID, err)
@@ -899,10 +835,9 @@ func (container *Container) cleanup() {
 	if err := container.stderr.CloseWriters(); err != nil {
 		utils.Errorf("%s: Error close stderr: %s", container.ID, err)
 	}
-
+	if container.command.Terminal != nil {
-	if container.ptyMaster != nil {
+		if err := container.command.Terminal.Close(); err != nil {
-		if err := container.ptyMaster.Close(); err != nil {
+			utils.Errorf("%s: Error closing terminal: %s", container.ID, err)
-			utils.Errorf("%s: Error closing Pty master: %s", container.ID, err)
 		}
 	}
 
@@ -994,11 +929,7 @@ func (container *Container) Wait() int {
 }
 
 func (container *Container) Resize(h, w int) error {
-	pty, ok := container.ptyMaster.(*os.File)
+	return container.command.Terminal.Resize(h, w)
-	if !ok {
-		return fmt.Errorf("ptyMaster does not have Fd() method")
-	}
-	return term.SetWinsize(pty.Fd(), &term.Winsize{Height: uint16(h), Width: uint16(w)})
 }
 
 func (container *Container) ExportRw() (archive.Archive, error) {
@@ -1202,11 +1133,9 @@ func (container *Container) Exposes(p nat.Port) bool {
 }
 
 func (container *Container) GetPtyMaster() (*os.File, error) {
-	if container.ptyMaster == nil {
+	ttyConsole, ok := container.command.Terminal.(execdriver.TtyTerminal)
+	if !ok {
 		return nil, ErrNoTTY
 	}
-	if pty, ok := container.ptyMaster.(*os.File); ok {
+	return ttyConsole.Master(), nil
-		return pty, nil
-	}
-	return nil, ErrNotATTY
 }

execdriver/chroot/driver.go

@@ -37,7 +37,7 @@ func NewDriver() (*driver, error) {
 	return &driver{}, nil
 }
 
-func (d *driver) Run(c *execdriver.Command, startCallback execdriver.StartCallback) (int, error) {
+func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (int, error) {
 	params := []string{
 		"chroot",
 		c.Rootfs,

execdriver/driver.go

@@ -2,6 +2,8 @@ package execdriver
 
 import (
 	"errors"
+	"io"
+	"os"
 	"os/exec"
 )
 
@@ -57,8 +59,20 @@ type Info interface {
 	IsRunning() bool
 }
 
+// Terminal in an interface for drivers to implement
+// if they want to support Close and Resize calls from
+// the core
+type Terminal interface {
+	io.Closer
+	Resize(height, width int) error
+}
+
+type TtyTerminal interface {
+	Master() *os.File
+}
+
 type Driver interface {
-	Run(c *Command, startCallback StartCallback) (int, error) // Run executes the process and blocks until the process exits and returns the exit code
+	Run(c *Command, pipes *Pipes, startCallback StartCallback) (int, error) // Run executes the process and blocks until the process exits and returns the exit code
 	Kill(c *Command, sig int) error
 	Restore(c *Command) error                     // Wait and try to re-attach on an out of process command
 	Name() string                                 // Driver name
@@ -82,7 +96,6 @@ type Resources struct {
 }
 
 // Process wrapps an os/exec.Cmd to add more metadata
-// TODO: Rename to Command
 type Command struct {
 	exec.Cmd `json:"-"`
 
@@ -100,7 +113,8 @@ type Command struct {
 	Config     []string   `json:"config"`  //  generic values that specific drivers can consume
 	Resources  *Resources `json:"resources"`
 
-	Console string `json:"-"`
+	Terminal Terminal `json:"-"` // standard or tty terminal
+	Console  string   `json:"-"` // dev/console path
 }
 
 // Return the pid of the process

execdriver/lxc/driver.go

@@ -76,7 +76,10 @@ func (d *driver) Name() string {
 	return fmt.Sprintf("%s-%s", DriverName, version)
 }
 
-func (d *driver) Run(c *execdriver.Command, startCallback execdriver.StartCallback) (int, error) {
+func (d *driver) Run(c *execdriver.Command, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (int, error) {
+	if err := SetTerminal(c, pipes); err != nil {
+		return -1, err
+	}
 	configPath, err := d.generateLXCConfig(c)
 	if err != nil {
 		return -1, err

execdriver/lxc/term.go

@@ -0,0 +1,126 @@
+package lxc
+
+import (
+	"github.com/dotcloud/docker/execdriver"
+	"github.com/dotcloud/docker/pkg/term"
+	"github.com/kr/pty"
+	"io"
+	"os"
+)
+
+func SetTerminal(command *execdriver.Command, pipes *execdriver.Pipes) error {
+	var (
+		term execdriver.Terminal
+		err  error
+	)
+	if command.Tty {
+		term, err = NewTtyConsole(command, pipes)
+	} else {
+		term, err = NewStdConsole(command, pipes)
+	}
+	if err != nil {
+		return err
+	}
+	command.Terminal = term
+	return nil
+}
+
+type TtyConsole struct {
+	master *os.File
+	slave  *os.File
+}
+
+func NewTtyConsole(command *execdriver.Command, pipes *execdriver.Pipes) (*TtyConsole, error) {
+	ptyMaster, ptySlave, err := pty.Open()
+	if err != nil {
+		return nil, err
+	}
+	tty := &TtyConsole{
+		master: ptyMaster,
+		slave:  ptySlave,
+	}
+	if err := tty.attach(command, pipes); err != nil {
+		tty.Close()
+		return nil, err
+	}
+	return tty, nil
+}
+
+func (t *TtyConsole) Master() *os.File {
+	return t.master
+}
+
+func (t *TtyConsole) Resize(h, w int) error {
+	return term.SetWinsize(t.master.Fd(), &term.Winsize{Height: uint16(h), Width: uint16(w)})
+}
+
+func (t *TtyConsole) attach(command *execdriver.Command, pipes *execdriver.Pipes) error {
+	command.Stdout = t.slave
+	command.Stderr = t.slave
+	command.Console = t.slave.Name()
+
+	go func() {
+		if wb, ok := pipes.Stdout.(interface {
+			CloseWriters() error
+		}); ok {
+			defer wb.CloseWriters()
+		}
+		io.Copy(pipes.Stdout, t.master)
+	}()
+
+	if pipes.Stdin != nil {
+		command.Stdin = t.slave
+		command.SysProcAttr.Setctty = true
+
+		go func() {
+			defer pipes.Stdin.Close()
+			io.Copy(t.master, pipes.Stdin)
+		}()
+	}
+	return nil
+}
+
+func (t *TtyConsole) Close() error {
+	t.slave.Close()
+	return t.master.Close()
+}
+
+type StdConsole struct {
+}
+
+func NewStdConsole(command *execdriver.Command, pipes *execdriver.Pipes) (*StdConsole, error) {
+	std := &StdConsole{}
+
+	if err := std.attach(command, pipes); err != nil {
+		return nil, err
+	}
+	return std, nil
+}
+
+func (s *StdConsole) attach(command *execdriver.Command, pipes *execdriver.Pipes) error {
+	command.Stdout = pipes.Stdout
+	command.Stderr = pipes.Stderr
+
+	if pipes.Stdin != nil {
+		stdin, err := command.StdinPipe()
+		if err != nil {
+			return err
+		}
+
+		go func() {
+			defer stdin.Close()
+			io.Copy(stdin, pipes.Stdin)
+		}()
+	}
+	return nil
+}
+
+func (s *StdConsole) Resize(h, w int) error {
+	// we do not need to reside a non tty
+	return nil
+}
+
+func (s *StdConsole) Close() error {
+	// nothing to close here
+	return nil
+}

execdriver/pipes.go

@@ -0,0 +1,23 @@
+package execdriver
+
+import (
+	"io"
+)
+
+// Pipes is a wrapper around a containers output for
+// stdin, stdout, stderr
+type Pipes struct {
+	Stdin          io.ReadCloser
+	Stdout, Stderr io.Writer
+}
+
+func NewPipes(stdin io.ReadCloser, stdout, stderr io.Writer, useStdin bool) *Pipes {
+	p := &Pipes{
+		Stdout: stdout,
+		Stderr: stderr,
+	}
+	if useStdin {
+		p.Stdin = stdin
+	}
+	return p
+}

runtime.go

@@ -812,8 +812,8 @@ func (runtime *Runtime) Diff(container *Container) (archive.Archive, error) {
 	}), nil
 }
 
-func (runtime *Runtime) Run(c *Container, startCallback execdriver.StartCallback) (int, error) {
+func (runtime *Runtime) Run(c *Container, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (int, error) {
-	return runtime.execDriver.Run(c.command, startCallback)
+	return runtime.execDriver.Run(c.command, pipes, startCallback)
 }
 
 func (runtime *Runtime) Kill(c *Container, sig int) error {