From 8fbc569d3194670356be0e7c74ec409cb0b68623 Mon Sep 17 00:00:00 2001 From: Zhang Wei Date: Mon, 4 Apr 2016 15:32:26 +0800 Subject: [PATCH 1/2] vendor engine-api 8924d6900370b4c7e7984b Signed-off-by: Zhang Wei Upstream-commit: e1a4332c13d0bdc5e960102a4f0dfd8e1fb6ffaf Component: engine --- components/engine/hack/vendor.sh | 2 +- .../engine-api/client/container_wait.go | 2 +- .../docker/engine-api/client/hijack.go | 22 +++++++++++++++---- .../engine-api/client/transport/transport.go | 13 ----------- .../docker/engine-api/types/client.go | 2 +- .../engine-api/types/container/host_config.go | 9 ++++---- .../docker/engine-api/types/stats.go | 2 +- 7 files changed, 27 insertions(+), 25 deletions(-) diff --git a/components/engine/hack/vendor.sh b/components/engine/hack/vendor.sh index 27d2023034..851019dc0d 100755 --- a/components/engine/hack/vendor.sh +++ b/components/engine/hack/vendor.sh @@ -24,7 +24,7 @@ clone git golang.org/x/net 47990a1ba55743e6ef1affd3a14e5bac8553615d https://gith clone git golang.org/x/sys eb2c74142fd19a79b3f237334c7384d5167b1b46 https://github.com/golang/sys.git clone git github.com/docker/go-units 651fc226e7441360384da338d0fd37f2440ffbe3 clone git github.com/docker/go-connections v0.2.0 -clone git github.com/docker/engine-api e37a82dfcea64559ca6a581776253c01d83357d9 +clone git github.com/docker/engine-api 8924d6900370b4c7e7984be5adc61f50a80d7537 clone git github.com/RackSec/srslog 259aed10dfa74ea2961eddd1d9847619f6e98837 clone git github.com/imdario/mergo 0.2.1 diff --git a/components/engine/vendor/src/github.com/docker/engine-api/client/container_wait.go b/components/engine/vendor/src/github.com/docker/engine-api/client/container_wait.go index 8736b0dbb5..c26ff3f378 100644 --- a/components/engine/vendor/src/github.com/docker/engine-api/client/container_wait.go +++ b/components/engine/vendor/src/github.com/docker/engine-api/client/container_wait.go @@ -8,7 +8,7 @@ import ( "github.com/docker/engine-api/types" ) -// ContainerWait pauses execution util a container exits. +// ContainerWait pauses execution until a container exits. // It returns the API status code as response of its readiness. func (cli *Client) ContainerWait(ctx context.Context, containerID string) (int, error) { resp, err := cli.post(ctx, "/containers/"+containerID+"/wait", nil, nil, nil) diff --git a/components/engine/vendor/src/github.com/docker/engine-api/client/hijack.go b/components/engine/vendor/src/github.com/docker/engine-api/client/hijack.go index 8102d481b4..dbd91ef629 100644 --- a/components/engine/vendor/src/github.com/docker/engine-api/client/hijack.go +++ b/components/engine/vendor/src/github.com/docker/engine-api/client/hijack.go @@ -46,8 +46,7 @@ func (cli *Client) postHijacked(ctx context.Context, path string, query url.Valu req.Header.Set("Connection", "Upgrade") req.Header.Set("Upgrade", "tcp") - tlsConfig := cli.transport.TLSConfig() - conn, err := dial(cli.proto, cli.addr, tlsConfig) + conn, err := dial(cli.proto, cli.addr, cli.transport.TLSConfig()) if err != nil { if strings.Contains(err.Error(), "connection refused") { return types.HijackedResponse{}, fmt.Errorf("Cannot connect to the Docker daemon. Is 'docker daemon' running on this host?") @@ -69,11 +68,11 @@ func (cli *Client) postHijacked(ctx context.Context, path string, query url.Valu defer clientconn.Close() // Server hijacks the connection, error 'connection closed' expected - clientconn.Do(req) + _, err = clientconn.Do(req) rwc, br := clientconn.Hijack() - return types.HijackedResponse{Conn: rwc, Reader: br}, nil + return types.HijackedResponse{Conn: rwc, Reader: br}, err } func tlsDial(network, addr string, config *tls.Config) (net.Conn, error) { @@ -126,6 +125,21 @@ func tlsDialWithDialer(dialer *net.Dialer, network, addr string, config *tls.Con tcpConn.SetKeepAlivePeriod(30 * time.Second) } + colonPos := strings.LastIndex(addr, ":") + if colonPos == -1 { + colonPos = len(addr) + } + hostname := addr[:colonPos] + + // If no ServerName is set, infer the ServerName + // from the hostname we're connecting to. + if config.ServerName == "" { + // Make a copy to avoid polluting argument or default. + c := *config + c.ServerName = hostname + config = &c + } + conn := tls.Client(rawConn, config) if timeout == 0 { diff --git a/components/engine/vendor/src/github.com/docker/engine-api/client/transport/transport.go b/components/engine/vendor/src/github.com/docker/engine-api/client/transport/transport.go index 9e0095f3e4..ff28af1855 100644 --- a/components/engine/vendor/src/github.com/docker/engine-api/client/transport/transport.go +++ b/components/engine/vendor/src/github.com/docker/engine-api/client/transport/transport.go @@ -4,7 +4,6 @@ package transport import ( "fmt" "net/http" - "strings" "github.com/docker/go-connections/sockets" ) @@ -35,10 +34,6 @@ func NewTransportWithHTTP(proto, addr string, client *http.Client) (Client, erro } } - if transport.TLSClientConfig != nil && transport.TLSClientConfig.ServerName == "" { - transport.TLSClientConfig.ServerName = hostname(addr) - } - return &apiTransport{ Client: client, tlsInfo: &tlsInfo{transport.TLSClientConfig}, @@ -59,12 +54,4 @@ func defaultTransport(proto, addr string) *http.Transport { return tr } -func hostname(addr string) string { - colonPos := strings.LastIndex(addr, ":") - if colonPos == -1 { - return addr - } - return addr[:colonPos] -} - var _ Client = &apiTransport{} diff --git a/components/engine/vendor/src/github.com/docker/engine-api/types/client.go b/components/engine/vendor/src/github.com/docker/engine-api/types/client.go index 00fc97054e..975a5bb639 100644 --- a/components/engine/vendor/src/github.com/docker/engine-api/types/client.go +++ b/components/engine/vendor/src/github.com/docker/engine-api/types/client.go @@ -103,7 +103,7 @@ func (h *HijackedResponse) Close() { h.Conn.Close() } -// CloseWriter is an interface that implement structs +// CloseWriter is an interface that implements structs // that close input streams to prevent from writing. type CloseWriter interface { CloseWrite() error diff --git a/components/engine/vendor/src/github.com/docker/engine-api/types/container/host_config.go b/components/engine/vendor/src/github.com/docker/engine-api/types/container/host_config.go index 0b2af65145..7ad634b6da 100644 --- a/components/engine/vendor/src/github.com/docker/engine-api/types/container/host_config.go +++ b/components/engine/vendor/src/github.com/docker/engine-api/types/container/host_config.go @@ -236,10 +236,11 @@ type Resources struct { Ulimits []*units.Ulimit // List of ulimits to be set in the container // Applicable to Windows - CPUCount int64 `json:"CpuCount"` // CPU count - CPUPercent int64 `json:"CpuPercent"` // CPU percent - MaximumIOps uint64 // Maximum IOps for the container system drive - MaximumIOBps uint64 // Maximum IO in bytes per second for the container system drive + CPUCount int64 `json:"CpuCount"` // CPU count + CPUPercent int64 `json:"CpuPercent"` // CPU percent + IOMaximumIOps uint64 // Maximum IOps for the container system drive + IOMaximumBandwidth uint64 // Maximum IO in bytes per second for the container system drive + NetworkMaximumBandwidth uint64 // Maximum bandwidth of the network endpoint in bytes per second } // UpdateConfig holds the mutable attributes of a Container. diff --git a/components/engine/vendor/src/github.com/docker/engine-api/types/stats.go b/components/engine/vendor/src/github.com/docker/engine-api/types/stats.go index 55081ae4d2..1434033410 100644 --- a/components/engine/vendor/src/github.com/docker/engine-api/types/stats.go +++ b/components/engine/vendor/src/github.com/docker/engine-api/types/stats.go @@ -8,7 +8,7 @@ import "time" type ThrottlingData struct { // Number of periods with throttling active Periods uint64 `json:"periods"` - // Number of periods when the container hit its throttling limit. + // Number of periods when the container hits its throttling limit. ThrottledPeriods uint64 `json:"throttled_periods"` // Aggregate time the container was throttled for in nanoseconds. ThrottledTime uint64 `json:"throttled_time"` From 97016757fb1d32ee1a599ce8aff7b2c5b8f58198 Mon Sep 17 00:00:00 2001 From: Zhang Wei Date: Wed, 23 Mar 2016 19:34:47 +0800 Subject: [PATCH 2/2] Let client print error when speicify wrong detach keys Fix #21064 Let client print error message explicitly when user specifies wrong detach keys. Signed-off-by: Zhang Wei Upstream-commit: 91e5bb954131904af150b06bd22b007559a8ce27 Component: engine --- components/engine/api/client/attach.go | 14 ++++++-- components/engine/api/client/run.go | 18 +++++++--- components/engine/api/client/start.go | 16 ++++++--- .../engine/api/server/httputils/errors.go | 23 ++++++++---- .../router/container/container_routes.go | 35 +++++++++---------- .../engine/api/types/backend/backend.go | 2 +- components/engine/daemon/attach.go | 12 ++++++- components/engine/daemon/exec.go | 3 +- .../integration-cli/docker_api_attach_test.go | 15 +++++--- .../docker_cli_run_unix_test.go | 32 +++++++++++++++++ 10 files changed, 126 insertions(+), 44 deletions(-) diff --git a/components/engine/api/client/attach.go b/components/engine/api/client/attach.go index 144d47efb6..e9e1a891c2 100644 --- a/components/engine/api/client/attach.go +++ b/components/engine/api/client/attach.go @@ -3,6 +3,7 @@ package client import ( "fmt" "io" + "net/http/httputil" "golang.org/x/net/context" @@ -66,9 +67,12 @@ func (cli *DockerCli) CmdAttach(args ...string) error { defer signal.StopCatch(sigc) } - resp, err := cli.client.ContainerAttach(context.Background(), options) - if err != nil { - return err + resp, errAttach := cli.client.ContainerAttach(context.Background(), options) + if errAttach != nil && errAttach != httputil.ErrPersistEOF { + // ContainerAttach returns an ErrPersistEOF (connection closed) + // means server met an error and put it in Hijacked connection + // keep the error and read detailed error message from hijacked connection later + return errAttach } defer resp.Close() @@ -90,6 +94,10 @@ func (cli *DockerCli) CmdAttach(args ...string) error { return err } + if errAttach != nil { + return errAttach + } + _, status, err := getExitCode(cli, options.ContainerID) if err != nil { return err diff --git a/components/engine/api/client/run.go b/components/engine/api/client/run.go index ec0a4ea856..88160965bf 100644 --- a/components/engine/api/client/run.go +++ b/components/engine/api/client/run.go @@ -3,6 +3,7 @@ package client import ( "fmt" "io" + "net/http/httputil" "os" "runtime" "strings" @@ -205,19 +206,26 @@ func (cli *DockerCli) CmdRun(args ...string) error { DetachKeys: cli.configFile.DetachKeys, } - resp, err := cli.client.ContainerAttach(context.Background(), options) - if err != nil { - return err + resp, errAttach := cli.client.ContainerAttach(context.Background(), options) + if errAttach != nil && errAttach != httputil.ErrPersistEOF { + // ContainerAttach returns an ErrPersistEOF (connection closed) + // means server met an error and put it in Hijacked connection + // keep the error and read detailed error message from hijacked connection later + return errAttach } ctx, cancelFun = context.WithCancel(context.Background()) errCh = promise.Go(func() error { - return cli.holdHijackedConnection(ctx, config.Tty, in, out, stderr, resp) + errHijack := cli.holdHijackedConnection(ctx, config.Tty, in, out, stderr, resp) + if errHijack == nil { + return errAttach + } + return errHijack }) } if *flAutoRemove { defer func() { - if err := cli.removeContainer(createResponse.ID, true, false, false); err != nil { + if err := cli.removeContainer(createResponse.ID, true, false, true); err != nil { fmt.Fprintf(cli.err, "%v\n", err) } }() diff --git a/components/engine/api/client/start.go b/components/engine/api/client/start.go index 0d141da88c..68274ca66a 100644 --- a/components/engine/api/client/start.go +++ b/components/engine/api/client/start.go @@ -3,6 +3,7 @@ package client import ( "fmt" "io" + "net/http/httputil" "os" "strings" @@ -94,14 +95,21 @@ func (cli *DockerCli) CmdStart(args ...string) error { in = cli.in } - resp, err := cli.client.ContainerAttach(context.Background(), options) - if err != nil { - return err + resp, errAttach := cli.client.ContainerAttach(context.Background(), options) + if errAttach != nil && errAttach != httputil.ErrPersistEOF { + // ContainerAttach return an ErrPersistEOF (connection closed) + // means server met an error and put it in Hijacked connection + // keep the error and read detailed error message from hijacked connection + return errAttach } defer resp.Close() ctx, cancelFun := context.WithCancel(context.Background()) cErr := promise.Go(func() error { - return cli.holdHijackedConnection(ctx, c.Config.Tty, in, cli.out, cli.err, resp) + errHijack := cli.holdHijackedConnection(ctx, c.Config.Tty, in, cli.out, cli.err, resp) + if errHijack == nil { + return errAttach + } + return errHijack }) // 3. Start the container. diff --git a/components/engine/api/server/httputils/errors.go b/components/engine/api/server/httputils/errors.go index c6b0a6b2d8..92b615b8fc 100644 --- a/components/engine/api/server/httputils/errors.go +++ b/components/engine/api/server/httputils/errors.go @@ -24,11 +24,11 @@ type inputValidationError interface { IsValidationError() bool } -// WriteError decodes a specific docker error and sends it in the response. -func WriteError(w http.ResponseWriter, err error) { - if err == nil || w == nil { - logrus.WithFields(logrus.Fields{"error": err, "writer": w}).Error("unexpected HTTP error handling") - return +// GetHTTPErrorStatusCode retrieve status code from error message +func GetHTTPErrorStatusCode(err error) int { + if err == nil { + logrus.WithFields(logrus.Fields{"error": err}).Error("unexpected HTTP error handling") + return http.StatusInternalServerError } var statusCode int @@ -65,5 +65,16 @@ func WriteError(w http.ResponseWriter, err error) { statusCode = http.StatusInternalServerError } - http.Error(w, errMsg, statusCode) + return statusCode +} + +// WriteError decodes a specific docker error and sends it in the response. +func WriteError(w http.ResponseWriter, err error) { + if err == nil || w == nil { + logrus.WithFields(logrus.Fields{"error": err, "writer": w}).Error("unexpected HTTP error handling") + return + } + + statusCode := GetHTTPErrorStatusCode(err) + http.Error(w, err.Error(), statusCode) } diff --git a/components/engine/api/server/router/container/container_routes.go b/components/engine/api/server/router/container/container_routes.go index a02e826d04..114c71271e 100644 --- a/components/engine/api/server/router/container/container_routes.go +++ b/components/engine/api/server/router/container/container_routes.go @@ -15,7 +15,6 @@ import ( "github.com/docker/docker/api/types/backend" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/signal" - "github.com/docker/docker/pkg/term" "github.com/docker/engine-api/types" "github.com/docker/engine-api/types/container" "github.com/docker/engine-api/types/filters" @@ -408,15 +407,7 @@ func (s *containerRouter) postContainersAttach(ctx context.Context, w http.Respo containerName := vars["name"] _, upgrade := r.Header["Upgrade"] - - keys := []byte{} detachKeys := r.FormValue("detachKeys") - if detachKeys != "" { - keys, err = term.ToBytes(detachKeys) - if err != nil { - logrus.Warnf("Invalid escape keys provided (%s) using default : ctrl-p ctrl-q", detachKeys) - } - } hijacker, ok := w.(http.Hijacker) if !ok { @@ -452,11 +443,24 @@ func (s *containerRouter) postContainersAttach(ctx context.Context, w http.Respo UseStderr: httputils.BoolValue(r, "stderr"), Logs: httputils.BoolValue(r, "logs"), Stream: httputils.BoolValue(r, "stream"), - DetachKeys: keys, + DetachKeys: detachKeys, MuxStreams: true, } - return s.backend.ContainerAttach(containerName, attachConfig) + if err = s.backend.ContainerAttach(containerName, attachConfig); err != nil { + logrus.Errorf("Handler for %s %s returned error: %v", r.Method, r.URL.Path, err) + // Remember to close stream if error happens + conn, _, errHijack := hijacker.Hijack() + if errHijack == nil { + statusCode := httputils.GetHTTPErrorStatusCode(err) + statusText := http.StatusText(statusCode) + fmt.Fprintf(conn, "HTTP/1.1 %d %s\r\nContent-Type: application/vnd.docker.raw-stream\r\n\r\n%s\r\n", statusCode, statusText, err.Error()) + httputils.CloseStreams(conn) + } else { + logrus.Errorf("Error Hijacking: %v", err) + } + } + return nil } func (s *containerRouter) wsContainersAttach(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { @@ -465,15 +469,8 @@ func (s *containerRouter) wsContainersAttach(ctx context.Context, w http.Respons } containerName := vars["name"] - var keys []byte var err error detachKeys := r.FormValue("detachKeys") - if detachKeys != "" { - keys, err = term.ToBytes(detachKeys) - if err != nil { - logrus.Warnf("Invalid escape keys provided (%s) using default : ctrl-p ctrl-q", detachKeys) - } - } done := make(chan struct{}) started := make(chan struct{}) @@ -499,7 +496,7 @@ func (s *containerRouter) wsContainersAttach(ctx context.Context, w http.Respons GetStreams: setupStreams, Logs: httputils.BoolValue(r, "logs"), Stream: httputils.BoolValue(r, "stream"), - DetachKeys: keys, + DetachKeys: detachKeys, UseStdin: true, UseStdout: true, UseStderr: true, diff --git a/components/engine/api/types/backend/backend.go b/components/engine/api/types/backend/backend.go index f93f99ed5e..c7b4f01757 100644 --- a/components/engine/api/types/backend/backend.go +++ b/components/engine/api/types/backend/backend.go @@ -18,7 +18,7 @@ type ContainerAttachConfig struct { UseStderr bool Logs bool Stream bool - DetachKeys []byte + DetachKeys string // Used to signify that streams are multiplexed and therefore need a StdWriter to encode stdout/sderr messages accordingly. // TODO @cpuguy83: This shouldn't be needed. It was only added so that http and websocket endpoints can use the same function, and the websocket function was not using a stdwriter prior to this change... diff --git a/components/engine/daemon/attach.go b/components/engine/daemon/attach.go index 79e9cd51da..73ae907401 100644 --- a/components/engine/daemon/attach.go +++ b/components/engine/daemon/attach.go @@ -11,10 +11,20 @@ import ( "github.com/docker/docker/daemon/logger" "github.com/docker/docker/errors" "github.com/docker/docker/pkg/stdcopy" + "github.com/docker/docker/pkg/term" ) // ContainerAttach attaches to logs according to the config passed in. See ContainerAttachConfig. func (daemon *Daemon) ContainerAttach(prefixOrName string, c *backend.ContainerAttachConfig) error { + keys := []byte{} + var err error + if c.DetachKeys != "" { + keys, err = term.ToBytes(c.DetachKeys) + if err != nil { + return fmt.Errorf("Invalid escape keys (%s) provided", c.DetachKeys) + } + } + container, err := daemon.GetContainer(prefixOrName) if err != nil { return err @@ -48,7 +58,7 @@ func (daemon *Daemon) ContainerAttach(prefixOrName string, c *backend.ContainerA stderr = errStream } - if err := daemon.containerAttach(container, stdin, stdout, stderr, c.Logs, c.Stream, c.DetachKeys); err != nil { + if err := daemon.containerAttach(container, stdin, stdout, stderr, c.Logs, c.Stream, keys); err != nil { fmt.Fprintf(outStream, "Error attaching: %s\n", err) } return nil diff --git a/components/engine/daemon/exec.go b/components/engine/daemon/exec.go index be06845c68..2160fe4e1d 100644 --- a/components/engine/daemon/exec.go +++ b/components/engine/daemon/exec.go @@ -101,7 +101,8 @@ func (d *Daemon) ContainerExecCreate(config *types.ExecConfig) (string, error) { if config.DetachKeys != "" { keys, err = term.ToBytes(config.DetachKeys) if err != nil { - logrus.Warnf("Wrong escape keys provided (%s, error: %s) using default : ctrl-p ctrl-q", config.DetachKeys, err.Error()) + err = fmt.Errorf("Invalid escape keys (%s) provided", config.DetachKeys) + return "", err } } diff --git a/components/engine/integration-cli/docker_api_attach_test.go b/components/engine/integration-cli/docker_api_attach_test.go index 352079fc02..c0fa675bb0 100644 --- a/components/engine/integration-cli/docker_api_attach_test.go +++ b/components/engine/integration-cli/docker_api_attach_test.go @@ -67,11 +67,18 @@ func (s *DockerSuite) TestGetContainersAttachWebsocket(c *check.C) { // regression gh14320 func (s *DockerSuite) TestPostContainersAttachContainerNotFound(c *check.C) { - status, body, err := sockRequest("POST", "/containers/doesnotexist/attach", nil) - c.Assert(status, checker.Equals, http.StatusNotFound) + req, client, err := newRequestClient("POST", "/containers/doesnotexist/attach", nil, "") c.Assert(err, checker.IsNil) - expected := "No such container: doesnotexist\n" - c.Assert(string(body), checker.Contains, expected) + + resp, err := client.Do(req) + // connection will shutdown, err should be "persistent connection closed" + c.Assert(err, checker.NotNil) // Server shutdown connection + + body, err := readBody(resp.Body) + c.Assert(err, checker.IsNil) + c.Assert(resp.StatusCode, checker.Equals, http.StatusNotFound) + expected := "No such container: doesnotexist\r\n" + c.Assert(string(body), checker.Equals, expected) } func (s *DockerSuite) TestGetContainersWsAttachContainerNotFound(c *check.C) { diff --git a/components/engine/integration-cli/docker_cli_run_unix_test.go b/components/engine/integration-cli/docker_cli_run_unix_test.go index fe7cc6977a..518c00adbf 100644 --- a/components/engine/integration-cli/docker_cli_run_unix_test.go +++ b/components/engine/integration-cli/docker_cli_run_unix_test.go @@ -197,6 +197,38 @@ func (s *DockerSuite) TestRunAttachDetachFromFlag(c *check.C) { c.Assert(running, checker.Equals, "true", check.Commentf("expected container to still be running")) } +// TestRunDetach checks attaching and detaching with the escape sequence specified via flags. +func (s *DockerSuite) TestRunAttachDetachFromInvalidFlag(c *check.C) { + name := "attach-detach" + dockerCmd(c, "run", "--name", name, "-itd", "busybox", "top") + c.Assert(waitRun(name), check.IsNil) + + // specify an invalid detach key, container will ignore it and use default + cmd := exec.Command(dockerBinary, "attach", "--detach-keys='ctrl-A,a'", name) + stdout, err := cmd.StdoutPipe() + if err != nil { + c.Fatal(err) + } + cpty, tty, err := pty.Open() + if err != nil { + c.Fatal(err) + } + defer cpty.Close() + cmd.Stdin = tty + if err := cmd.Start(); err != nil { + c.Fatal(err) + } + + bufReader := bufio.NewReader(stdout) + out, err := bufReader.ReadString('\n') + if err != nil { + c.Fatal(err) + } + // it should print a warning to indicate the detach key flag is invalid + errStr := "Invalid escape keys (ctrl-A,a) provided" + c.Assert(strings.TrimSpace(out), checker.Equals, errStr) +} + // TestRunDetach checks attaching and detaching with the escape sequence specified via config file. func (s *DockerSuite) TestRunAttachDetachFromConfig(c *check.C) { keyCtrlA := []byte{1}