From 8ae5348a51ab6026db29009f109bb3724908f0aa Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Mon, 9 Sep 2013 18:14:23 -0700 Subject: [PATCH] Split stdout stderr for non-tty commands --- api_test.go | 5 ++- commands.go | 32 ++++++++++++------- server.go | 12 +++++-- utils/utils.go | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 16 deletions(-) diff --git a/api_test.go b/api_test.go index 837a20c24..fd736d5c9 100644 --- a/api_test.go +++ b/api_test.go @@ -445,7 +445,7 @@ func TestGetContainersChanges(t *testing.T) { } func TestGetContainersTop(t *testing.T) { - t.Skip("Fixme. Skipping test for now. Reported error when testing using dind: 'api_test.go:527: Expected 2 processes, found 0.'") + t.Skip("Fixme. Skipping test for now. Reported error when testing using dind: 'api_test.go:527: Expected 2 processes, found 0.'") runtime, err := newTestRuntime() if err != nil { t.Fatal(err) @@ -566,7 +566,6 @@ func TestPostCommit(t *testing.T) { srv := &Server{runtime: runtime} - // Create a container and remove a file container, err := runtime.Create( &Config{ @@ -952,7 +951,7 @@ func TestPostContainersAttach(t *testing.T) { }) setTimeout(t, "read/write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + if err := assertPipe("hello\n", string(utils.Stdout)+"hello", stdout, stdinPipe, 15); err != nil { t.Fatal(err) } }) diff --git a/commands.go b/commands.go index 00eeaf388..7f6f4cc0b 100644 --- a/commands.go +++ b/commands.go @@ -1219,7 +1219,7 @@ func (cli *DockerCli) CmdLogs(args ...string) error { return nil } - if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?logs=1&stdout=1&stderr=1", false, nil, cli.out); err != nil { + if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?logs=1&stdout=1&stderr=1", false, nil, cli.out, cli.err); err != nil { return err } return nil @@ -1262,7 +1262,7 @@ func (cli *DockerCli) CmdAttach(args ...string) error { v.Set("stdout", "1") v.Set("stderr", "1") - if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?"+v.Encode(), container.Config.Tty, cli.in, cli.out); err != nil { + if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?"+v.Encode(), container.Config.Tty, cli.in, cli.out, cli.err); err != nil { return err } return nil @@ -1526,7 +1526,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { v := url.Values{} v.Set("logs", "1") v.Set("stream", "1") - var out io.Writer + var out, stderr io.Writer if config.AttachStdin { v.Set("stdin", "1") @@ -1537,7 +1537,11 @@ func (cli *DockerCli) CmdRun(args ...string) error { } if config.AttachStderr { v.Set("stderr", "1") - out = cli.out + if config.Tty { + stderr = cli.out + } else { + stderr = cli.err + } } signals := make(chan os.Signal, 1) @@ -1551,7 +1555,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { } }() - if err := cli.hijack("POST", "/containers/"+runResult.ID+"/attach?"+v.Encode(), config.Tty, cli.in, out); err != nil { + if err := cli.hijack("POST", "/containers/"+runResult.ID+"/attach?"+v.Encode(), config.Tty, cli.in, out, stderr); err != nil { utils.Debugf("Error hijack: %s", err) return err } @@ -1718,7 +1722,7 @@ func (cli *DockerCli) stream(method, path string, in io.Reader, out io.Writer, h return nil } -func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.ReadCloser, out io.Writer) error { +func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.ReadCloser, stdout, stderr io.Writer) error { req, err := http.NewRequest(method, fmt.Sprintf("/v%g%s", APIVERSION, path), nil) if err != nil { @@ -1744,10 +1748,16 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea rwc, br := clientconn.Hijack() defer rwc.Close() - var receiveStdout (chan error) - if out != nil { - receiveStdout = utils.Go(func() error { - _, err := io.Copy(out, br) + var receiveStdout chan error + + if stdout != nil { + receiveStdout = utils.Go(func() (err error) { + // When TTY is ON, use regular copy + if setRawTerminal { + _, err = io.Copy(stdout, br) + } else { + _, err = utils.StdCopy(stdout, stderr, br) + } utils.Debugf("[hijack] End of stdout") return err }) @@ -1779,7 +1789,7 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea return nil }) - if out != nil { + if stdout != nil { if err := <-receiveStdout; err != nil { utils.Debugf("Error receiveStdout: %s", err) return err diff --git a/server.go b/server.go index 322d8fe93..c796c216e 100644 --- a/server.go +++ b/server.go @@ -1244,10 +1244,18 @@ func (srv *Server) ContainerAttach(name string, logs, stream, stdin, stdout, std cStdinCloser = in } if stdout { - cStdout = out + if container.Config.Tty { + cStdout = out + } else { + cStdout = utils.NewStdWriter(out, utils.Stdout) + } } if stderr { - cStderr = out + if container.Config.Tty { + cStderr = out + } else { + cStderr = utils.NewStdWriter(out, utils.Stderr) + } } <-container.Attach(cStdin, cStdinCloser, cStdout, cStderr) diff --git a/utils/utils.go b/utils/utils.go index d417690c0..18b695a3e 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -1021,3 +1021,90 @@ type StatusError struct { func (e *StatusError) Error() string { return fmt.Sprintf("Status: %d", e.Status) } + +type StdType []byte + +const StdWriterPrefixLen = 8 + +var ( + Stdin StdType = StdType("\001 stdin\002") + Stdout StdType = StdType("\001stdout\002") + Stderr StdType = StdType("\001stderr\002") +) + +type StdWriter struct { + io.Writer + prefix []byte +} + +func (w *StdWriter) Write(buf []byte) (n int, err error) { + n, err = w.Writer.Write(append(w.prefix, buf...)) + if n >= len(buf)+StdWriterPrefixLen { + n -= StdWriterPrefixLen + } + return n, err +} + +// NewStdWriter instanciate a new Writer based on the given type `t`. +// the utils package contains the valid parametres for `t`: +func NewStdWriter(w io.Writer, t StdType) *StdWriter { + if len(t) != StdWriterPrefixLen { + return nil + } + return &StdWriter{ + Writer: w, + prefix: []byte(t), + } +} + +// StdCopy is a modified version of io.Copy. +// +// StdCopy copies from src to dstout or dsterr until either EOF is reached +// on src or an error occurs. It returns the number of bytes +// copied and the first error encountered while copying, if any. +// +// A successful Copy returns err == nil, not err == EOF. +// Because Copy is defined to read from src until EOF, it does +// not treat an EOF from Read as an error to be reported. +// +// The source needs to be writter via StdWriter, dstout or dsterr is selected +// based on the prefix added by StdWriter +func StdCopy(dstout, dsterr io.Writer, src io.Reader) (written int64, err error) { + var ( + buf = make([]byte, 32*1024) + nw int + ew error + ) + + for { + nr, er := src.Read(buf) + if nr > 0 { + if bytes.Compare(buf[:StdWriterPrefixLen], Stdout) == 0 { + nw, ew = dstout.Write(buf[StdWriterPrefixLen:nr]) + } else if bytes.Compare(buf[:StdWriterPrefixLen], Stderr) == 0 { + nw, ew = dsterr.Write(buf[StdWriterPrefixLen:nr]) + } else if bytes.Compare(buf[:StdWriterPrefixLen], Stdin) == 0 { + nw, ew = dstout.Write(buf[StdWriterPrefixLen:nr]) + } + if nw > 0 { + written += int64(nw) + } + if ew != nil { + err = ew + break + } + if nr-StdWriterPrefixLen != nw { + err = io.ErrShortWrite + break + } + } + if er == io.EOF { + break + } + if er != nil { + err = er + break + } + } + return written, err +}