From 25d1bc2c09e3f6e7973d6b159e49ae7f8bd7670d Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Mon, 24 Jun 2013 12:12:06 -0700 Subject: [PATCH 1/6] Fix issue when attaching stdin alone --- commands.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/commands.go b/commands.go index 47bc492909..ea45c986f0 100644 --- a/commands.go +++ b/commands.go @@ -1277,7 +1277,8 @@ func (cli *DockerCli) CmdRun(args ...string) error { if !config.AttachStdout && !config.AttachStderr { fmt.Println(out.ID) - } else { + } + if config.AttachStdin || config.AttachStdout || config.AttachStderr { if config.Tty { cli.monitorTtySize(out.ID) } From a749fb21309b147a1bb9da0838f591ba58f6d169 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Mon, 24 Jun 2013 14:15:52 -0700 Subject: [PATCH 2/6] Make DockerCli use its own stdin/out/err instead of the os.Std* --- commands.go | 153 +++++++++++++++++++++++++---------------------- commands_test.go | 10 +++- runtime_test.go | 10 +++- 3 files changed, 98 insertions(+), 75 deletions(-) diff --git a/commands.go b/commands.go index ea45c986f0..93e5eb8512 100644 --- a/commands.go +++ b/commands.go @@ -40,7 +40,7 @@ func (cli *DockerCli) getMethod(name string) (reflect.Method, bool) { } func ParseCommands(proto, addr string, args ...string) error { - cli := NewDockerCli(proto, addr) + cli := NewDockerCli(os.Stdin, os.Stdout, os.Stderr, proto, addr) if len(args) > 0 { method, exists := cli.getMethod(args[0]) @@ -64,7 +64,7 @@ func (cli *DockerCli) CmdHelp(args ...string) error { if len(args) > 0 { method, exists := cli.getMethod(args[0]) if !exists { - fmt.Println("Error: Command not found:", args[0]) + fmt.Fprintf(cli.err, "Error: Command not found: %s\n", args[0]) } else { method.Func.CallSlice([]reflect.Value{ reflect.ValueOf(cli), @@ -74,7 +74,7 @@ func (cli *DockerCli) CmdHelp(args ...string) error { } } help := fmt.Sprintf("Usage: docker [OPTIONS] COMMAND [arg...]\n -H=[tcp://%s:%d]: tcp://host:port to bind/connect to or unix://path/to/socker to use\n\nA self-sufficient runtime for linux containers.\n\nCommands:\n", DEFAULTHTTPHOST, DEFAULTHTTPPORT) - for _, command := range [][2]string{ + for _, command := range [][]string{ {"attach", "Attach to a running container"}, {"build", "Build a container from a Dockerfile"}, {"commit", "Create a new image from a container's changes"}, @@ -106,7 +106,7 @@ func (cli *DockerCli) CmdHelp(args ...string) error { } { help += fmt.Sprintf(" %-10.10s%s\n", command[0], command[1]) } - fmt.Println(help) + fmt.Fprintf(cli.err, "%s\n", help) return nil } @@ -124,7 +124,7 @@ func (cli *DockerCli) CmdInsert(args ...string) error { v.Set("url", cmd.Arg(1)) v.Set("path", cmd.Arg(2)) - if err := cli.stream("POST", "/images/"+cmd.Arg(0)+"/insert?"+v.Encode(), nil, os.Stdout); err != nil { + if err := cli.stream("POST", "/images/"+cmd.Arg(0)+"/insert?"+v.Encode(), nil, cli.out); err != nil { return err } return nil @@ -175,7 +175,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error { if cmd.Arg(0) == "-" { // As a special case, 'docker build -' will build from an empty context with the // contents of stdin as a Dockerfile - dockerfile, err := ioutil.ReadAll(os.Stdin) + dockerfile, err := ioutil.ReadAll(cli.in) if err != nil { return err } @@ -190,7 +190,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error { // FIXME: ProgressReader shouldn't be this annoyning to use if context != nil { sf := utils.NewStreamFormatter(false) - body = utils.ProgressReader(ioutil.NopCloser(context), 0, os.Stderr, sf.FormatProgress("Uploading context", "%v bytes%0.0s%0.0s"), sf) + body = utils.ProgressReader(ioutil.NopCloser(context), 0, cli.err, sf.FormatProgress("Uploading context", "%v bytes%0.0s%0.0s"), sf) } // Upload the build context v := &url.Values{} @@ -229,7 +229,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error { } // Output the result - if _, err := io.Copy(os.Stdout, resp.Body); err != nil { + if _, err := io.Copy(cli.out, resp.Body); err != nil { return err } @@ -291,25 +291,27 @@ func (cli *DockerCli) CmdLogin(args ...string) error { return nil } - var username string - var password string - var email string + var ( + username string + password string + email string + ) - fmt.Print("Username (", cli.authConfig.Username, "): ") - username = readAndEchoString(os.Stdin, os.Stdout) + fmt.Fprintf(cli.out, "Username (%s):", cli.authConfig.Username) + username = readAndEchoString(cli.in, cli.out) if username == "" { username = cli.authConfig.Username } if username != cli.authConfig.Username { - fmt.Print("Password: ") - password = readString(os.Stdin, os.Stdout) + fmt.Fprintf(cli.out, "Password: ") + password = readString(cli.in, cli.out) if password == "" { return fmt.Errorf("Error : Password Required") } - fmt.Print("Email (", cli.authConfig.Email, "): ") - email = readAndEchoString(os.Stdin, os.Stdout) + fmt.Fprintf(cli.out, "Email (%s): ", cli.authConfig.Email) + email = readAndEchoString(cli.in, cli.out) if email == "" { email = cli.authConfig.Email } @@ -343,7 +345,7 @@ func (cli *DockerCli) CmdLogin(args ...string) error { } auth.SaveConfig(cli.authConfig) if out2.Status != "" { - fmt.Println(out2.Status) + fmt.Fprintln(cli.out, "%s\n", out2.Status) } return nil } @@ -361,14 +363,14 @@ func (cli *DockerCli) CmdWait(args ...string) error { for _, name := range cmd.Args() { body, _, err := cli.call("POST", "/containers/"+name+"/wait", nil) if err != nil { - fmt.Printf("%s", err) + fmt.Fprintf(cli.err, "%s", err) } else { var out APIWait err = json.Unmarshal(body, &out) if err != nil { return err } - fmt.Println(out.StatusCode) + fmt.Fprintf(cli.out, "%s\n", out.StatusCode) } } return nil @@ -397,13 +399,13 @@ func (cli *DockerCli) CmdVersion(args ...string) error { utils.Debugf("Error unmarshal: body: %s, err: %s\n", body, err) return err } - fmt.Println("Client version:", VERSION) - fmt.Println("Server version:", out.Version) + fmt.Fprintf(cli.out, "Client version: %s\n", VERSION) + fmt.Fprintf(cli.out, "Server version: %s\n", out.Version) if out.GitCommit != "" { - fmt.Println("Git commit:", out.GitCommit) + fmt.Fprintf(cli.out, "Git commit: %s\n", out.GitCommit) } if out.GoVersion != "" { - fmt.Println("Go version:", out.GoVersion) + fmt.Fprintln(cli.out, "Go version: %s\n", out.GoVersion) } return nil } @@ -429,19 +431,19 @@ func (cli *DockerCli) CmdInfo(args ...string) error { return err } - fmt.Printf("Containers: %d\n", out.Containers) - fmt.Printf("Images: %d\n", out.Images) + fmt.Fprintf(cli.out, "Containers: %d\n", out.Containers) + fmt.Fprintf(cli.out, "Images: %d\n", out.Images) if out.Debug || os.Getenv("DEBUG") != "" { - fmt.Printf("Debug mode (server): %v\n", out.Debug) - fmt.Printf("Debug mode (client): %v\n", os.Getenv("DEBUG") != "") - fmt.Printf("Fds: %d\n", out.NFd) - fmt.Printf("Goroutines: %d\n", out.NGoroutines) + fmt.Fprintf(cli.out, "Debug mode (server): %v\n", out.Debug) + fmt.Fprintf(cli.out, "Debug mode (client): %v\n", os.Getenv("DEBUG") != "") + fmt.Fprintf(cli.out, "Fds: %d\n", out.NFd) + fmt.Fprintf(cli.out, "Goroutines: %d\n", out.NGoroutines) } if !out.MemoryLimit { - fmt.Println("WARNING: No memory limit support") + fmt.Fprintf(cli.err, "WARNING: No memory limit support\n") } if !out.SwapLimit { - fmt.Println("WARNING: No swap limit support") + fmt.Fprintf(cli.err, "WARNING: No swap limit support\n") } return nil } @@ -463,9 +465,9 @@ func (cli *DockerCli) CmdStop(args ...string) error { for _, name := range cmd.Args() { _, _, err := cli.call("POST", "/containers/"+name+"/stop?"+v.Encode(), nil) if err != nil { - fmt.Fprintf(os.Stderr, "%s", err) + fmt.Fprintf(cli.err, "%s\n", err) } else { - fmt.Println(name) + fmt.Fprintf(cli.out, "%s\n", name) } } return nil @@ -488,9 +490,9 @@ func (cli *DockerCli) CmdRestart(args ...string) error { for _, name := range cmd.Args() { _, _, err := cli.call("POST", "/containers/"+name+"/restart?"+v.Encode(), nil) if err != nil { - fmt.Fprintf(os.Stderr, "%s", err) + fmt.Fprintf(cli.err, "%s\n", err) } else { - fmt.Println(name) + fmt.Fprintf(cli.out, "%s\n", name) } } return nil @@ -509,9 +511,9 @@ func (cli *DockerCli) CmdStart(args ...string) error { for _, name := range args { _, _, err := cli.call("POST", "/containers/"+name+"/start", nil) if err != nil { - fmt.Fprintf(os.Stderr, "%s", err) + fmt.Fprintf(cli.err, "%s\n", err) } else { - fmt.Println(name) + fmt.Fprintln(cli.out, "%s\n", name) } } return nil @@ -526,30 +528,30 @@ func (cli *DockerCli) CmdInspect(args ...string) error { cmd.Usage() return nil } - fmt.Printf("[") + fmt.Fprintf(cli.out, "[") for i, name := range args { if i > 0 { - fmt.Printf(",") + fmt.Fprintf(cli.out, ",") } obj, _, err := cli.call("GET", "/containers/"+name+"/json", nil) if err != nil { obj, _, err = cli.call("GET", "/images/"+name+"/json", nil) if err != nil { - fmt.Fprintf(os.Stderr, "%s", err) + fmt.Fprintf(cli.err, "%s\n", err) continue } } indented := new(bytes.Buffer) if err = json.Indent(indented, obj, "", " "); err != nil { - fmt.Fprintf(os.Stderr, "%s", err) + fmt.Fprintf(cli.err, "%s\n", err) continue } - if _, err := io.Copy(os.Stdout, indented); err != nil { - fmt.Fprintf(os.Stderr, "%s", err) + if _, err := io.Copy(cli.out, indented); err != nil { + fmt.Fprintf(cli.err, "%s\n", err) } } - fmt.Printf("]") + fmt.Fprintf(cli.out, "]") return nil } @@ -595,7 +597,7 @@ func (cli *DockerCli) CmdRmi(args ...string) error { for _, name := range cmd.Args() { body, _, err := cli.call("DELETE", "/images/"+name, nil) if err != nil { - fmt.Fprintf(os.Stderr, "%s", err) + fmt.Fprintf(cli.err, "%s", err) } else { var outs []APIRmi err = json.Unmarshal(body, &outs) @@ -634,7 +636,7 @@ func (cli *DockerCli) CmdHistory(args ...string) error { if err != nil { return err } - w := tabwriter.NewWriter(os.Stdout, 20, 1, 3, ' ', 0) + w := tabwriter.NewWriter(cli.out, 20, 1, 3, ' ', 0) fmt.Fprintln(w, "ID\tCREATED\tCREATED BY") for _, out := range outs { @@ -710,7 +712,7 @@ func (cli *DockerCli) CmdImport(args ...string) error { v.Set("tag", tag) v.Set("fromSrc", src) - err := cli.stream("POST", "/images/create?"+v.Encode(), os.Stdin, os.Stdout) + err := cli.stream("POST", "/images/create?"+v.Encode(), cli.in, cli.out) if err != nil { return err } @@ -754,7 +756,7 @@ func (cli *DockerCli) CmdPush(args ...string) error { v := url.Values{} v.Set("registry", *registry) - if err := cli.stream("POST", "/images/"+name+"/push?"+v.Encode(), bytes.NewBuffer(buf), os.Stdout); err != nil { + if err := cli.stream("POST", "/images/"+name+"/push?"+v.Encode(), bytes.NewBuffer(buf), cli.out); err != nil { return err } return nil @@ -785,7 +787,7 @@ func (cli *DockerCli) CmdPull(args ...string) error { v.Set("tag", *tag) v.Set("registry", *registry) - if err := cli.stream("POST", "/images/create?"+v.Encode(), nil, os.Stdout); err != nil { + if err := cli.stream("POST", "/images/create?"+v.Encode(), nil, cli.out); err != nil { return err } @@ -833,7 +835,7 @@ func (cli *DockerCli) CmdImages(args ...string) error { return err } - w := tabwriter.NewWriter(os.Stdout, 20, 1, 3, ' ', 0) + w := tabwriter.NewWriter(cli.out, 20, 1, 3, ' ', 0) if !*quiet { fmt.Fprintln(w, "REPOSITORY\tTAG\tID\tCREATED\tSIZE") } @@ -919,7 +921,7 @@ func (cli *DockerCli) CmdPs(args ...string) error { if err != nil { return err } - w := tabwriter.NewWriter(os.Stdout, 20, 1, 3, ' ', 0) + w := tabwriter.NewWriter(cli.out, 20, 1, 3, ' ', 0) if !*quiet { fmt.Fprint(w, "ID\tIMAGE\tCOMMAND\tCREATED\tSTATUS\tPORTS") if *size { @@ -1013,7 +1015,7 @@ func (cli *DockerCli) CmdExport(args ...string) error { return nil } - if err := cli.stream("GET", "/containers/"+cmd.Arg(0)+"/export", nil, os.Stdout); err != nil { + if err := cli.stream("GET", "/containers/"+cmd.Arg(0)+"/export", nil, cli.out); err != nil { return err } return nil @@ -1055,10 +1057,10 @@ func (cli *DockerCli) CmdLogs(args ...string) error { return nil } - if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?logs=1&stdout=1", false, nil, os.Stdout); err != nil { + if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?logs=1&stdout=1", false, nil, cli.out); err != nil { return err } - if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?logs=1&stderr=1", false, nil, os.Stderr); err != nil { + if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?logs=1&stderr=1", false, nil, cli.err); err != nil { return err } return nil @@ -1099,7 +1101,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, os.Stdin, os.Stdout); err != nil { + if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?"+v.Encode(), container.Config.Tty, cli.in.(*os.File), cli.out); err != nil { return err } return nil @@ -1127,8 +1129,8 @@ func (cli *DockerCli) CmdSearch(args ...string) error { if err != nil { return err } - fmt.Printf("Found %d results matching your query (\"%s\")\n", len(outs), cmd.Arg(0)) - w := tabwriter.NewWriter(os.Stdout, 20, 1, 3, ' ', 0) + fmt.Fprintf(cli.out, "Found %d results matching your query (\"%s\")\n", len(outs), cmd.Arg(0)) + w := tabwriter.NewWriter(cli.out, 20, 1, 3, ' ', 0) fmt.Fprintf(w, "NAME\tDESCRIPTION\n") for _, out := range outs { desc := strings.Replace(out.Description, "\n", " ", -1) @@ -1246,7 +1248,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { if statusCode == 404 { v := url.Values{} v.Set("fromImage", config.Image) - err = cli.stream("POST", "/images/create?"+v.Encode(), nil, os.Stderr) + err = cli.stream("POST", "/images/create?"+v.Encode(), nil, cli.err) if err != nil { return err } @@ -1259,28 +1261,28 @@ func (cli *DockerCli) CmdRun(args ...string) error { return err } - out := &APIRun{} - err = json.Unmarshal(body, out) + runResult := &APIRun{} + err = json.Unmarshal(body, runResult) if err != nil { return err } - for _, warning := range out.Warnings { - fmt.Fprintln(os.Stderr, "WARNING: ", warning) + for _, warning := range runResult.Warnings { + fmt.Fprintln(cli.err, "WARNING: ", warning) } //start the container - _, _, err = cli.call("POST", "/containers/"+out.ID+"/start", nil) + _, _, err = cli.call("POST", "/containers/"+runResult.ID+"/start", nil) if err != nil { return err } if !config.AttachStdout && !config.AttachStderr { - fmt.Println(out.ID) + fmt.Fprintf(cli.out, "%s\b", runResult.ID) } if config.AttachStdin || config.AttachStdout || config.AttachStderr { if config.Tty { - cli.monitorTtySize(out.ID) + cli.monitorTtySize(runResult.ID) } v := url.Values{} @@ -1296,7 +1298,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { if config.AttachStderr { v.Set("stderr", "1") } - if err := cli.hijack("POST", "/containers/"+out.ID+"/attach?"+v.Encode(), config.Tty, os.Stdin, os.Stdout); err != nil { + if err := cli.hijack("POST", "/containers/"+runResult.ID+"/attach?"+v.Encode(), config.Tty, cli.in.(*os.File), cli.out); err != nil { utils.Debugf("Error hijack: %s", err) return err } @@ -1515,19 +1517,30 @@ func (cli *DockerCli) monitorTtySize(id string) { func Subcmd(name, signature, description string) *flag.FlagSet { flags := flag.NewFlagSet(name, flag.ContinueOnError) flags.Usage = func() { - fmt.Printf("\nUsage: docker %s %s\n\n%s\n\n", name, signature, description) + // FIXME: use custom stdout or return error + fmt.Fprintf(os.Stdout, "\nUsage: docker %s %s\n\n%s\n\n", name, signature, description) flags.PrintDefaults() } return flags } -func NewDockerCli(proto, addr string) *DockerCli { +func NewDockerCli(in io.ReadCloser, out, err io.Writer, proto, addr string) *DockerCli { authConfig, _ := auth.LoadConfig(os.Getenv("HOME")) - return &DockerCli{proto, addr, authConfig} + return &DockerCli{ + proto: proto, + addr: addr, + authConfig: authConfig, + in: in, + out: out, + err: err, + } } type DockerCli struct { proto string addr string authConfig *auth.AuthConfig + in io.ReadCloser + out io.Writer + err io.Writer } diff --git a/commands_test.go b/commands_test.go index 05ece80dac..8628082169 100644 --- a/commands_test.go +++ b/commands_test.go @@ -334,7 +334,8 @@ func TestRunDisconnectTty(t *testing.T) { t.Fatalf("/bin/cat should still be running after closing stdin (tty mode)") } } - +*/ +/* // TestAttachStdin checks attaching to stdin without stdout and stderr. // 'docker run -i -a stdin' should sends the client's stdin to the command, // then detach from it and print the container id. @@ -345,6 +346,10 @@ func TestRunAttachStdin(t *testing.T) { } defer nuke(runtime) srv := &Server{runtime: runtime} + // enableCors: false, + // lock: &sync.Mutex{}, + // pullingPool: make(map[string]struct{}), + // pushingPool: make(map[string]struct{}), stdin, stdinPipe := io.Pipe() stdout, stdoutPipe := io.Pipe() @@ -399,7 +404,8 @@ func TestRunAttachStdin(t *testing.T) { } } } - +*/ +/* // Expected behaviour, the process stays alive when the client disconnects func TestAttachDisconnect(t *testing.T) { runtime, err := newTestRuntime() diff --git a/runtime_test.go b/runtime_test.go index da037d7105..9bf8a73ef9 100644 --- a/runtime_test.go +++ b/runtime_test.go @@ -16,9 +16,13 @@ import ( "time" ) -const unitTestImageName string = "docker-ut" -const unitTestImageId string = "e9aa60c60128cad1" -const unitTestStoreBase string = "/var/lib/docker/unit-tests" +const ( + unitTestImageName = "docker-ut" + unitTestImageId = "e9aa60c60128cad1" + unitTestStoreBase = "/var/lib/docker/unit-tests" + testDaemonAddr = "127.0.0.1:4270" + testDaemonProto = "tcp" +) func nuke(runtime *Runtime) error { var wg sync.WaitGroup From 672d3a6c6cd0494a0b8eaa13674c3cb87cba029c Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Mon, 24 Jun 2013 14:52:02 -0700 Subject: [PATCH 3/6] Make term function consistent with each other --- commands.go | 15 +++++++-------- term/term.go | 12 ++++-------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/commands.go b/commands.go index 93e5eb8512..f58d498257 100644 --- a/commands.go +++ b/commands.go @@ -280,11 +280,11 @@ func (cli *DockerCli) CmdLogin(args ...string) error { return readStringOnRawTerminal(stdin, stdout, false) } - oldState, err := term.SetRawTerminal() + oldState, err := term.SetRawTerminal(os.Stdin.Fd()) if err != nil { return err } - defer term.RestoreTerminal(oldState) + defer term.RestoreTerminal(os.Stdin.Fd(), oldState) cmd := Subcmd("login", "", "Register or Login to the docker registry server") if err := cmd.Parse(args); err != nil { @@ -319,7 +319,7 @@ func (cli *DockerCli) CmdLogin(args ...string) error { password = cli.authConfig.Password email = cli.authConfig.Email } - term.RestoreTerminal(oldState) + term.RestoreTerminal(os.Stdin.Fd(), oldState) cli.authConfig.Username = username cli.authConfig.Password = password @@ -1272,13 +1272,12 @@ func (cli *DockerCli) CmdRun(args ...string) error { } //start the container - _, _, err = cli.call("POST", "/containers/"+runResult.ID+"/start", nil) - if err != nil { + if _, _, err = cli.call("POST", "/containers/"+runResult.ID+"/start", nil); err != nil { return err } if !config.AttachStdout && !config.AttachStderr { - fmt.Fprintf(cli.out, "%s\b", runResult.ID) + fmt.Fprintf(cli.out, "%s\n", runResult.ID) } if config.AttachStdin || config.AttachStdout || config.AttachStderr { if config.Tty { @@ -1457,11 +1456,11 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in *os.Fi }) if in != nil && setRawTerminal && term.IsTerminal(in.Fd()) && os.Getenv("NORAW") == "" { - oldState, err := term.SetRawTerminal() + oldState, err := term.SetRawTerminal(os.Stdin.Fd()) if err != nil { return err } - defer term.RestoreTerminal(oldState) + defer term.RestoreTerminal(os.Stdin.Fd(), oldState) } sendStdin := utils.Go(func() error { io.Copy(rwc, in) diff --git a/term/term.go b/term/term.go index 0cc91ea1b6..3f743d227f 100644 --- a/term/term.go +++ b/term/term.go @@ -38,13 +38,13 @@ func IsTerminal(fd uintptr) bool { // Restore restores the terminal connected to the given file descriptor to a // previous state. -func Restore(fd uintptr, state *State) error { +func RestoreTerminal(fd uintptr, state *State) error { _, _, err := syscall.Syscall(syscall.SYS_IOCTL, fd, uintptr(setTermios), uintptr(unsafe.Pointer(&state.termios))) return err } -func SetRawTerminal() (*State, error) { - oldState, err := MakeRaw(os.Stdin.Fd()) +func SetRawTerminal(fd uintptr) (*State, error) { + oldState, err := MakeRaw(fd) if err != nil { return nil, err } @@ -52,12 +52,8 @@ func SetRawTerminal() (*State, error) { signal.Notify(c, os.Interrupt) go func() { _ = <-c - Restore(os.Stdin.Fd(), oldState) + RestoreTerminal(fd, oldState) os.Exit(0) }() return oldState, err } - -func RestoreTerminal(state *State) { - Restore(os.Stdin.Fd(), state) -} From 873a5aa8e72102a31f34c62603a3f5976471e94a Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Mon, 24 Jun 2013 14:59:37 -0700 Subject: [PATCH 4/6] Make NewDockerCli handle terminal --- commands.go | 63 +++++++++++++++++++++++++++++++++++++------------ runtime_test.go | 7 ++++++ 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/commands.go b/commands.go index f58d498257..b84bea5be8 100644 --- a/commands.go +++ b/commands.go @@ -280,11 +280,11 @@ func (cli *DockerCli) CmdLogin(args ...string) error { return readStringOnRawTerminal(stdin, stdout, false) } - oldState, err := term.SetRawTerminal(os.Stdin.Fd()) + oldState, err := term.SetRawTerminal(cli.terminalFd) if err != nil { return err } - defer term.RestoreTerminal(os.Stdin.Fd(), oldState) + defer term.RestoreTerminal(cli.terminalFd, oldState) cmd := Subcmd("login", "", "Register or Login to the docker registry server") if err := cmd.Parse(args); err != nil { @@ -319,7 +319,7 @@ func (cli *DockerCli) CmdLogin(args ...string) error { password = cli.authConfig.Password email = cli.authConfig.Email } - term.RestoreTerminal(os.Stdin.Fd(), oldState) + term.RestoreTerminal(cli.terminalFd, oldState) cli.authConfig.Username = username cli.authConfig.Password = password @@ -1092,7 +1092,9 @@ func (cli *DockerCli) CmdAttach(args ...string) error { } if container.Config.Tty { - cli.monitorTtySize(cmd.Arg(0)) + if err := cli.monitorTtySize(cmd.Arg(0)); err != nil { + return err + } } v := url.Values{} @@ -1101,7 +1103,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.(*os.File), cli.out); err != nil { + if err := cli.hijack("POST", "/containers/"+cmd.Arg(0)+"/attach?"+v.Encode(), container.Config.Tty, cli.in, cli.out); err != nil { return err } return nil @@ -1281,7 +1283,9 @@ func (cli *DockerCli) CmdRun(args ...string) error { } if config.AttachStdin || config.AttachStdout || config.AttachStderr { if config.Tty { - cli.monitorTtySize(runResult.ID) + if err := cli.monitorTtySize(runResult.ID); err != nil { + return err + } } v := url.Values{} @@ -1297,7 +1301,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { if config.AttachStderr { v.Set("stderr", "1") } - if err := cli.hijack("POST", "/containers/"+runResult.ID+"/attach?"+v.Encode(), config.Tty, cli.in.(*os.File), cli.out); err != nil { + if err := cli.hijack("POST", "/containers/"+runResult.ID+"/attach?"+v.Encode(), config.Tty, cli.in, cli.out); err != nil { utils.Debugf("Error hijack: %s", err) return err } @@ -1428,7 +1432,7 @@ func (cli *DockerCli) stream(method, path string, in io.Reader, out io.Writer) e return nil } -func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in *os.File, out io.Writer) error { +func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.ReadCloser, out io.Writer) error { req, err := http.NewRequest(method, fmt.Sprintf("/v%g%s", APIVERSION, path), nil) if err != nil { @@ -1455,15 +1459,17 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in *os.Fi return err }) - if in != nil && setRawTerminal && term.IsTerminal(in.Fd()) && os.Getenv("NORAW") == "" { - oldState, err := term.SetRawTerminal(os.Stdin.Fd()) + if in != nil && setRawTerminal && cli.isTerminal && os.Getenv("NORAW") == "" { + oldState, err := term.SetRawTerminal(cli.terminalFd) if err != nil { return err } - defer term.RestoreTerminal(os.Stdin.Fd(), oldState) + defer term.RestoreTerminal(cli.terminalFd, oldState) } sendStdin := utils.Go(func() error { - io.Copy(rwc, in) + if in != nil { + io.Copy(rwc, in) + } if err := rwc.(*net.TCPConn).CloseWrite(); err != nil { utils.Debugf("Couldn't send EOF: %s\n", err) } @@ -1476,7 +1482,7 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in *os.Fi return err } - if !term.IsTerminal(in.Fd()) { + if !cli.isTerminal { if err := <-sendStdin; err != nil { utils.Debugf("Error sendStdin: %s", err) return err @@ -1487,7 +1493,10 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in *os.Fi } func (cli *DockerCli) resizeTty(id string) { - ws, err := term.GetWinsize(os.Stdin.Fd()) + if !cli.isTerminal { + return + } + ws, err := term.GetWinsize(cli.terminalFd) if err != nil { utils.Debugf("Error getting size: %s", err) } @@ -1499,7 +1508,10 @@ func (cli *DockerCli) resizeTty(id string) { } } -func (cli *DockerCli) monitorTtySize(id string) { +func (cli *DockerCli) monitorTtySize(id string) error { + if !cli.isTerminal { + return fmt.Errorf("Impossible to monitor size on non-tty") + } cli.resizeTty(id) c := make(chan os.Signal, 1) @@ -1511,6 +1523,7 @@ func (cli *DockerCli) monitorTtySize(id string) { } } }() + return nil } func Subcmd(name, signature, description string) *flag.FlagSet { @@ -1524,6 +1537,22 @@ func Subcmd(name, signature, description string) *flag.FlagSet { } func NewDockerCli(in io.ReadCloser, out, err io.Writer, proto, addr string) *DockerCli { + var ( + isTerminal bool = false + terminalFd uintptr + ) + + if in != nil { + if file, ok := in.(*os.File); ok { + terminalFd = file.Fd() + isTerminal = term.IsTerminal(terminalFd) + } + } + + if err == nil { + err = out + } + authConfig, _ := auth.LoadConfig(os.Getenv("HOME")) return &DockerCli{ proto: proto, @@ -1532,6 +1561,8 @@ func NewDockerCli(in io.ReadCloser, out, err io.Writer, proto, addr string) *Doc in: in, out: out, err: err, + isTerminal: isTerminal, + terminalFd: terminalFd, } } @@ -1542,4 +1573,6 @@ type DockerCli struct { in io.ReadCloser out io.Writer err io.Writer + isTerminal bool + terminalFd uintptr } diff --git a/runtime_test.go b/runtime_test.go index 9bf8a73ef9..d83808c78f 100644 --- a/runtime_test.go +++ b/runtime_test.go @@ -77,6 +77,13 @@ func init() { if err := srv.ImagePull(unitTestImageName, "", "", os.Stdout, utils.NewStreamFormatter(false), nil); err != nil { panic(err) } + + // Spawn a Daemon + go func() { + if err := ListenAndServe(testDaemonProto, testDaemonAddr, srv, os.Getenv("DEBUG") != ""); err != nil { + panic(err) + } + }() } // FIXME: test that ImagePull(json=true) send correct json output From 5190f7f33a9e36f1f5991a385f4db198e9f1b3a6 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Mon, 24 Jun 2013 18:22:02 -0700 Subject: [PATCH 5/6] Implement regression test for stdin attach --- api.go | 15 ++++++- commands.go | 6 ++- commands_test.go | 102 +++++++++++++++++++---------------------------- runtime_test.go | 23 +++++++++++ z_final_test.go | 12 ++++++ 5 files changed, 96 insertions(+), 62 deletions(-) create mode 100644 z_final_test.go diff --git a/api.go b/api.go index 3155bf5513..dd33f40a66 100644 --- a/api.go +++ b/api.go @@ -660,7 +660,20 @@ func postContainersAttach(srv *Server, version float64, w http.ResponseWriter, r if err != nil { return err } - defer in.Close() + defer func() { + if tcpc, ok := in.(*net.TCPConn); ok { + tcpc.CloseWrite() + } else { + in.Close() + } + }() + defer func() { + if tcpc, ok := out.(*net.TCPConn); ok { + tcpc.CloseWrite() + } else if closer, ok := out.(io.Closer); ok { + closer.Close() + } + }() fmt.Fprintf(out, "HTTP/1.1 200 OK\r\nContent-Type: application/vnd.docker.raw-stream\r\n\r\n") if err := srv.ContainerAttach(name, logs, stream, stdin, stdout, stderr, in, out); err != nil { diff --git a/commands.go b/commands.go index b84bea5be8..7923f84fc1 100644 --- a/commands.go +++ b/commands.go @@ -1279,8 +1279,10 @@ func (cli *DockerCli) CmdRun(args ...string) error { } if !config.AttachStdout && !config.AttachStderr { - fmt.Fprintf(cli.out, "%s\n", runResult.ID) + // Make this asynchrone in order to let the client write to stdin before having to read the ID + go fmt.Fprintf(cli.out, "%s\n", runResult.ID) } + if config.AttachStdin || config.AttachStdout || config.AttachStderr { if config.Tty { if err := cli.monitorTtySize(runResult.ID); err != nil { @@ -1301,6 +1303,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { if config.AttachStderr { v.Set("stderr", "1") } + if err := cli.hijack("POST", "/containers/"+runResult.ID+"/attach?"+v.Encode(), config.Tty, cli.in, cli.out); err != nil { utils.Debugf("Error hijack: %s", err) return err @@ -1466,6 +1469,7 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea } defer term.RestoreTerminal(cli.terminalFd, oldState) } + sendStdin := utils.Go(func() error { if in != nil { io.Copy(rwc, in) diff --git a/commands_test.go b/commands_test.go index 8628082169..87c4c02a52 100644 --- a/commands_test.go +++ b/commands_test.go @@ -3,8 +3,9 @@ package docker import ( "bufio" "fmt" + "github.com/dotcloud/docker/utils" "io" - _ "io/ioutil" + "io/ioutil" "strings" "testing" "time" @@ -59,20 +60,6 @@ func assertPipe(input, output string, r io.Reader, w io.Writer, count int) error } /*TODO -func cmdWait(srv *Server, container *Container) error { - stdout, stdoutPipe := io.Pipe() - - go func() { - srv.CmdWait(nil, stdoutPipe, container.Id) - }() - - if _, err := bufio.NewReader(stdout).ReadString('\n'); err != nil { - return err - } - // Cleanup pipes - return closeWrap(stdout, stdoutPipe) -} - func cmdImages(srv *Server, args ...string) (string, error) { stdout, stdoutPipe := io.Pipe() @@ -144,41 +131,39 @@ func TestImages(t *testing.T) { // todo: add checks for -a } +*/ // TestRunHostname checks that 'docker run -h' correctly sets a custom hostname func TestRunHostname(t *testing.T) { - runtime, err := newTestRuntime() - if err != nil { - t.Fatal(err) - } - defer nuke(runtime) - - srv := &Server{runtime: runtime} - - stdin, _ := io.Pipe() stdout, stdoutPipe := io.Pipe() + cli := NewDockerCli(nil, stdoutPipe, nil, testDaemonProto, testDaemonAddr) + defer cleanup(globalRuntime) + c := make(chan struct{}) go func() { - if err := srv.CmdRun(stdin, rcli.NewDockerLocalConn(stdoutPipe), "-h", "foobar", GetTestImage(runtime).Id, "hostname"); err != nil { + defer close(c) + if err := cli.CmdRun("-h", "foobar", unitTestImageId, "hostname"); err != nil { t.Fatal(err) } - close(c) }() - cmdOutput, err := bufio.NewReader(stdout).ReadString('\n') - if err != nil { - t.Fatal(err) - } - if cmdOutput != "foobar\n" { - t.Fatalf("'hostname' should display '%s', not '%s'", "foobar\n", cmdOutput) - } + utils.Debugf("--") + setTimeout(t, "Reading command output time out", 2*time.Second, func() { + cmdOutput, err := bufio.NewReader(stdout).ReadString('\n') + if err != nil { + t.Fatal(err) + } + if cmdOutput != "foobar\n" { + t.Fatalf("'hostname' should display '%s', not '%s'", "foobar\n", cmdOutput) + } + }) - setTimeout(t, "CmdRun timed out", 2*time.Second, func() { + setTimeout(t, "CmdRun timed out", 5*time.Second, func() { <-c - cmdWait(srv, srv.runtime.List()[0]) }) } +/* func TestRunExit(t *testing.T) { runtime, err := newTestRuntime() if err != nil { @@ -335,33 +320,26 @@ func TestRunDisconnectTty(t *testing.T) { } } */ -/* + // TestAttachStdin checks attaching to stdin without stdout and stderr. // 'docker run -i -a stdin' should sends the client's stdin to the command, // then detach from it and print the container id. func TestRunAttachStdin(t *testing.T) { - runtime, err := newTestRuntime() - if err != nil { - t.Fatal(err) - } - defer nuke(runtime) - srv := &Server{runtime: runtime} - // enableCors: false, - // lock: &sync.Mutex{}, - // pullingPool: make(map[string]struct{}), - // pushingPool: make(map[string]struct{}), stdin, stdinPipe := io.Pipe() stdout, stdoutPipe := io.Pipe() + cli := NewDockerCli(stdin, stdoutPipe, nil, testDaemonProto, testDaemonAddr) + defer cleanup(globalRuntime) + ch := make(chan struct{}) go func() { - srv.CmdRun(stdin, rcli.NewDockerLocalConn(stdoutPipe), "-i", "-a", "stdin", GetTestImage(runtime).Id, "sh", "-c", "echo hello; cat") - close(ch) + defer close(ch) + cli.CmdRun("-i", "-a", "stdin", unitTestImageId, "sh", "-c", "echo hello && cat") }() // Send input to the command, close stdin - setTimeout(t, "Write timed out", 2*time.Second, func() { + setTimeout(t, "Write timed out", 10*time.Second, func() { if _, err := stdinPipe.Write([]byte("hi there\n")); err != nil { t.Fatal(err) } @@ -370,23 +348,27 @@ func TestRunAttachStdin(t *testing.T) { } }) - container := runtime.List()[0] + container := globalRuntime.List()[0] // Check output - cmdOutput, err := bufio.NewReader(stdout).ReadString('\n') - if err != nil { - t.Fatal(err) - } - if cmdOutput != container.ShortId()+"\n" { - t.Fatalf("Wrong output: should be '%s', not '%s'\n", container.ShortId()+"\n", cmdOutput) - } + setTimeout(t, "Reading command output time out", 10*time.Second, func() { + cmdOutput, err := bufio.NewReader(stdout).ReadString('\n') + if err != nil { + t.Fatal(err) + } + if cmdOutput != container.ShortID()+"\n" { + t.Fatalf("Wrong output: should be '%s', not '%s'\n", container.ShortID()+"\n", cmdOutput) + } + }) // wait for CmdRun to return - setTimeout(t, "Waiting for CmdRun timed out", 2*time.Second, func() { + setTimeout(t, "Waiting for CmdRun timed out", 5*time.Second, func() { + // Unblock hijack end + stdout.Read([]byte{}) <-ch }) - setTimeout(t, "Waiting for command to exit timed out", 2*time.Second, func() { + setTimeout(t, "Waiting for command to exit timed out", 5*time.Second, func() { container.Wait() }) @@ -404,7 +386,7 @@ func TestRunAttachStdin(t *testing.T) { } } } -*/ + /* // Expected behaviour, the process stays alive when the client disconnects func TestAttachDisconnect(t *testing.T) { diff --git a/runtime_test.go b/runtime_test.go index d83808c78f..23b1be0257 100644 --- a/runtime_test.go +++ b/runtime_test.go @@ -24,6 +24,8 @@ const ( testDaemonProto = "tcp" ) +var globalRuntime *Runtime + func nuke(runtime *Runtime) error { var wg sync.WaitGroup for _, container := range runtime.List() { @@ -37,6 +39,23 @@ func nuke(runtime *Runtime) error { return os.RemoveAll(runtime.root) } +func cleanup(runtime *Runtime) error { + for _, container := range runtime.List() { + container.Kill() + runtime.Destroy(container) + } + images, err := runtime.graph.All() + if err != nil { + return err + } + for _, image := range images { + if image.ID != unitTestImageId { + runtime.graph.Delete(image.ID) + } + } + return nil +} + func layerArchive(tarfile string) (io.Reader, error) { // FIXME: need to close f somewhere f, err := os.Open(tarfile) @@ -64,6 +83,7 @@ func init() { if err != nil { panic(err) } + globalRuntime = runtime // Create the "Server" srv := &Server{ @@ -84,6 +104,9 @@ func init() { panic(err) } }() + + // Give some time to ListenAndServer to actually start + time.Sleep(time.Second) } // FIXME: test that ImagePull(json=true) send correct json output diff --git a/z_final_test.go b/z_final_test.go new file mode 100644 index 0000000000..78a7acf6e7 --- /dev/null +++ b/z_final_test.go @@ -0,0 +1,12 @@ +package docker + +import ( + "github.com/dotcloud/docker/utils" + "runtime" + "testing" +) + +func TestFinal(t *testing.T) { + cleanup(globalRuntime) + t.Logf("Fds: %d, Goroutines: %d", utils.GetTotalUsedFds(), runtime.NumGoroutine()) +} From 6127d757a7495df83b3d4cec53dc56b48c764023 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Tue, 25 Jun 2013 10:39:11 -0700 Subject: [PATCH 6/6] Add missing fprintf instead of printf --- commands.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/commands.go b/commands.go index 7923f84fc1..7175c07b0b 100644 --- a/commands.go +++ b/commands.go @@ -576,7 +576,7 @@ func (cli *DockerCli) CmdPort(args ...string) error { } if frontend, exists := out.NetworkSettings.PortMapping[cmd.Arg(1)]; exists { - fmt.Println(frontend) + fmt.Fprintf(cli.out, "%s\n", frontend) } else { return fmt.Errorf("Error: No private port '%s' allocated on %s", cmd.Arg(1), cmd.Arg(0)) } @@ -606,9 +606,9 @@ func (cli *DockerCli) CmdRmi(args ...string) error { } for _, out := range outs { if out.Deleted != "" { - fmt.Println("Deleted:", out.Deleted) + fmt.Fprintf(cli.out, "Deleted: %s\n", out.Deleted) } else { - fmt.Println("Untagged:", out.Untagged) + fmt.Fprintf(cli.out, "Untagged: %s\n", out.Untagged) } } } @@ -666,9 +666,9 @@ func (cli *DockerCli) CmdRm(args ...string) error { for _, name := range cmd.Args() { _, _, err := cli.call("DELETE", "/containers/"+name+"?"+val.Encode(), nil) if err != nil { - fmt.Printf("%s", err) + fmt.Fprintf(cli.err, "%s\n", err) } else { - fmt.Println(name) + fmt.Fprintf(cli.out, "%s\n", name) } } return nil @@ -688,9 +688,9 @@ func (cli *DockerCli) CmdKill(args ...string) error { for _, name := range args { _, _, err := cli.call("POST", "/containers/"+name+"/kill", nil) if err != nil { - fmt.Printf("%s", err) + fmt.Fprintf(cli.err, "%s\n", err) } else { - fmt.Println(name) + fmt.Fprintf(cli.out, "%s\n", name) } } return nil @@ -814,7 +814,7 @@ func (cli *DockerCli) CmdImages(args ...string) error { if err != nil { return err } - fmt.Printf("%s", body) + fmt.Fprintf(cli.out, "%s", body) } else { v := url.Values{} if cmd.NArg() == 1 { @@ -1000,7 +1000,7 @@ func (cli *DockerCli) CmdCommit(args ...string) error { return err } - fmt.Println(apiID.ID) + fmt.Fprintf(cli.out, "%s\n", apiID.ID) return nil } @@ -1042,7 +1042,7 @@ func (cli *DockerCli) CmdDiff(args ...string) error { return err } for _, change := range changes { - fmt.Println(change.String()) + fmt.Fprintf(cli.out, "%s\n", change.String()) } return nil }