diff --git a/components/engine/api/server/router/container/backend.go b/components/engine/api/server/router/container/backend.go index 205a127be9..d51ed81776 100644 --- a/components/engine/api/server/router/container/backend.go +++ b/components/engine/api/server/router/container/backend.go @@ -44,7 +44,7 @@ type stateBackend interface { ContainerStop(name string, seconds *int) error ContainerUnpause(name string) error ContainerUpdate(name string, hostConfig *container.HostConfig) (container.ContainerUpdateOKBody, error) - ContainerWait(ctx context.Context, name string, untilRemoved bool) (<-chan *containerpkg.StateStatus, error) + ContainerWait(ctx context.Context, name string, condition containerpkg.WaitCondition) (<-chan containerpkg.StateStatus, error) } // monitorBackend includes functions to implement to provide containers monitoring functionality. diff --git a/components/engine/api/server/router/container/container.go b/components/engine/api/server/router/container/container.go index 2243845542..24c3224ee8 100644 --- a/components/engine/api/server/router/container/container.go +++ b/components/engine/api/server/router/container/container.go @@ -59,7 +59,7 @@ func (r *containerRouter) initRoutes() { router.NewPostRoute("/containers/{name:.*}/restart", r.postContainersRestart), router.NewPostRoute("/containers/{name:.*}/start", r.postContainersStart), router.NewPostRoute("/containers/{name:.*}/stop", r.postContainersStop), - router.NewPostRoute("/containers/{name:.*}/wait", r.postContainersWait), + router.NewPostRoute("/containers/{name:.*}/wait", r.postContainersWait, router.WithCancel), router.NewPostRoute("/containers/{name:.*}/resize", r.postContainersResize), router.NewPostRoute("/containers/{name:.*}/attach", r.postContainersAttach), router.NewPostRoute("/containers/{name:.*}/copy", r.postContainersCopy), // Deprecated since 1.8, Errors out since 1.12 diff --git a/components/engine/api/server/router/container/container_routes.go b/components/engine/api/server/router/container/container_routes.go index bab7e9c095..0032fea7aa 100644 --- a/components/engine/api/server/router/container/container_routes.go +++ b/components/engine/api/server/router/container/container_routes.go @@ -16,6 +16,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/versions" + containerpkg "github.com/docker/docker/container" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/signal" "golang.org/x/net/context" @@ -283,14 +284,47 @@ func (s *containerRouter) postContainersUnpause(ctx context.Context, w http.Resp } func (s *containerRouter) postContainersWait(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - waitC, err := s.backend.ContainerWait(ctx, vars["name"], false) + // Behavior changed in version 1.30 to handle wait condition and to + // return headers immediately. + version := httputils.VersionFromContext(ctx) + legacyBehavior := versions.LessThan(version, "1.30") + + // The wait condition defaults to "not-running". + waitCondition := containerpkg.WaitConditionNotRunning + if !legacyBehavior { + if err := httputils.ParseForm(r); err != nil { + return err + } + switch container.WaitCondition(r.Form.Get("condition")) { + case container.WaitConditionNextExit: + waitCondition = containerpkg.WaitConditionNextExit + case container.WaitConditionRemoved: + waitCondition = containerpkg.WaitConditionRemoved + } + } + + // Note: the context should get canceled if the client closes the + // connection since this handler has been wrapped by the + // router.WithCancel() wrapper. + waitC, err := s.backend.ContainerWait(ctx, vars["name"], waitCondition) if err != nil { return err } + w.Header().Set("Content-Type", "application/json") + + if !legacyBehavior { + // Write response header immediately. + w.WriteHeader(http.StatusOK) + if flusher, ok := w.(http.Flusher); ok { + flusher.Flush() + } + } + + // Block on the result of the wait operation. status := <-waitC - return httputils.WriteJSON(w, http.StatusOK, &container.ContainerWaitOKBody{ + return json.NewEncoder(w).Encode(&container.ContainerWaitOKBody{ StatusCode: int64(status.ExitCode()), }) } diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index f79509c3e9..10cf361d47 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -4270,6 +4270,11 @@ paths: required: true description: "ID or name of the container" type: "string" + - name: "condition" + in: "query" + description: "Wait until a container state reaches the given condition, either 'not-running' (default), 'next-exit', or 'removed'." + type: "string" + default: "not-running" tags: ["Container"] /containers/{id}: delete: diff --git a/components/engine/api/types/container/waitcondition.go b/components/engine/api/types/container/waitcondition.go new file mode 100644 index 0000000000..64820fe358 --- /dev/null +++ b/components/engine/api/types/container/waitcondition.go @@ -0,0 +1,22 @@ +package container + +// WaitCondition is a type used to specify a container state for which +// to wait. +type WaitCondition string + +// Possible WaitCondition Values. +// +// WaitConditionNotRunning (default) is used to wait for any of the non-running +// states: "created", "exited", "dead", "removing", or "removed". +// +// WaitConditionNextExit is used to wait for the next time the state changes +// to a non-running state. If the state is currently "created" or "exited", +// this would cause Wait() to block until either the container runs and exits +// or is removed. +// +// WaitConditionRemoved is used to wait for the container to be removed. +const ( + WaitConditionNotRunning WaitCondition = "not-running" + WaitConditionNextExit WaitCondition = "next-exit" + WaitConditionRemoved WaitCondition = "removed" +) diff --git a/components/engine/builder/builder.go b/components/engine/builder/builder.go index f79ba6cac1..cc7c25955a 100644 --- a/components/engine/builder/builder.go +++ b/components/engine/builder/builder.go @@ -50,7 +50,7 @@ type Backend interface { // ContainerStart starts a new container ContainerStart(containerID string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error // ContainerWait stops processing until the given container is stopped. - ContainerWait(ctx context.Context, name string, untilRemoved bool) (<-chan *containerpkg.StateStatus, error) + ContainerWait(ctx context.Context, name string, condition containerpkg.WaitCondition) (<-chan containerpkg.StateStatus, error) // ContainerCreateWorkdir creates the workdir ContainerCreateWorkdir(containerID string) error diff --git a/components/engine/builder/dockerfile/internals.go b/components/engine/builder/dockerfile/internals.go index 606d9513de..c433e22f28 100644 --- a/components/engine/builder/dockerfile/internals.go +++ b/components/engine/builder/dockerfile/internals.go @@ -4,7 +4,6 @@ package dockerfile // non-contiguous functionality. Please read the comments. import ( - "context" "crypto/sha256" "encoding/hex" "fmt" @@ -24,6 +23,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/builder" "github.com/docker/docker/builder/remotecontext" + containerpkg "github.com/docker/docker/container" "github.com/docker/docker/pkg/httputils" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/jsonmessage" @@ -597,7 +597,7 @@ func (b *Builder) run(cID string, cmd []string) (err error) { return err } - waitC, err := b.docker.ContainerWait(context.Background(), cID, false) + waitC, err := b.docker.ContainerWait(b.clientCtx, cID, containerpkg.WaitConditionNotRunning) if err != nil { // Unable to begin waiting for container. close(finished) diff --git a/components/engine/builder/dockerfile/mockbackend_test.go b/components/engine/builder/dockerfile/mockbackend_test.go index f61ccd5e65..3c273c71bc 100644 --- a/components/engine/builder/dockerfile/mockbackend_test.go +++ b/components/engine/builder/dockerfile/mockbackend_test.go @@ -2,13 +2,13 @@ package dockerfile import ( "io" - "time" "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" "github.com/docker/docker/api/types/container" "github.com/docker/docker/builder" + containerpkg "github.com/docker/docker/container" "github.com/docker/docker/image" "golang.org/x/net/context" ) @@ -54,8 +54,8 @@ func (m *MockBackend) ContainerStart(containerID string, hostConfig *container.H return nil } -func (m *MockBackend) ContainerWait(containerID string, timeout time.Duration) (int, error) { - return 0, nil +func (m *MockBackend) ContainerWait(ctx context.Context, containerID string, condition containerpkg.WaitCondition) (<-chan containerpkg.StateStatus, error) { + return nil, nil } func (m *MockBackend) ContainerCreateWorkdir(containerID string) error { diff --git a/components/engine/client/container_wait.go b/components/engine/client/container_wait.go index 93212c70ee..edfa5d3c85 100644 --- a/components/engine/client/container_wait.go +++ b/components/engine/client/container_wait.go @@ -2,25 +2,83 @@ package client import ( "encoding/json" + "net/url" "golang.org/x/net/context" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/versions" ) -// 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) (int64, error) { - resp, err := cli.post(ctx, "/containers/"+containerID+"/wait", nil, nil, nil) +// ContainerWait waits until the specified continer is in a certain state +// indicated by the given condition, either "not-running" (default), +// "next-exit", or "removed". +// +// If this client's API version is beforer 1.30, condition is ignored and +// ContainerWait will return immediately with the two channels, as the server +// will wait as if the condition were "not-running". +// +// If this client's API version is at least 1.30, ContainerWait blocks until +// the request has been acknowledged by the server (with a response header), +// then returns two channels on which the caller can wait for the exit status +// of the container or an error if there was a problem either beginning the +// wait request or in getting the response. This allows the caller to +// sychronize ContainerWait with other calls, such as specifying a +// "next-exit" condition before issuing a ContainerStart request. +func (cli *Client) ContainerWait(ctx context.Context, containerID string, condition container.WaitCondition) (<-chan container.ContainerWaitOKBody, <-chan error) { + if versions.LessThan(cli.ClientVersion(), "1.30") { + return cli.legacyContainerWait(ctx, containerID) + } + + resultC := make(chan container.ContainerWaitOKBody) + errC := make(chan error) + + query := url.Values{} + query.Set("condition", string(condition)) + + resp, err := cli.post(ctx, "/containers/"+containerID+"/wait", query, nil, nil) if err != nil { - return -1, err - } - defer ensureReaderClosed(resp) - - var res container.ContainerWaitOKBody - if err := json.NewDecoder(resp.body).Decode(&res); err != nil { - return -1, err + defer ensureReaderClosed(resp) + errC <- err + return resultC, errC } - return res.StatusCode, nil + go func() { + defer ensureReaderClosed(resp) + var res container.ContainerWaitOKBody + if err := json.NewDecoder(resp.body).Decode(&res); err != nil { + errC <- err + return + } + + resultC <- res + }() + + return resultC, errC +} + +// legacyContainerWait returns immediately and doesn't have an option to wait +// until the container is removed. +func (cli *Client) legacyContainerWait(ctx context.Context, containerID string) (<-chan container.ContainerWaitOKBody, <-chan error) { + resultC := make(chan container.ContainerWaitOKBody) + errC := make(chan error) + + go func() { + resp, err := cli.post(ctx, "/containers/"+containerID+"/wait", nil, nil, nil) + if err != nil { + errC <- err + return + } + defer ensureReaderClosed(resp) + + var res container.ContainerWaitOKBody + if err := json.NewDecoder(resp.body).Decode(&res); err != nil { + errC <- err + return + } + + resultC <- res + }() + + return resultC, errC } diff --git a/components/engine/client/container_wait_test.go b/components/engine/client/container_wait_test.go index 9300bc0a54..7b8c9f0964 100644 --- a/components/engine/client/container_wait_test.go +++ b/components/engine/client/container_wait_test.go @@ -20,12 +20,14 @@ func TestContainerWaitError(t *testing.T) { client := &Client{ client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")), } - code, err := client.ContainerWait(context.Background(), "nothing") - if err == nil || err.Error() != "Error response from daemon: Server error" { - t.Fatalf("expected a Server Error, got %v", err) - } - if code != -1 { - t.Fatalf("expected a status code equal to '-1', got %d", code) + resultC, errC := client.ContainerWait(context.Background(), "nothing", "") + select { + case result := <-resultC: + t.Fatalf("expected to not get a wait result, got %d", result.StatusCode) + case err := <-errC: + if err.Error() != "Error response from daemon: Server error" { + t.Fatalf("expected a Server Error, got %v", err) + } } } @@ -49,12 +51,14 @@ func TestContainerWait(t *testing.T) { }), } - code, err := client.ContainerWait(context.Background(), "container_id") - if err != nil { + resultC, errC := client.ContainerWait(context.Background(), "container_id", "") + select { + case err := <-errC: t.Fatal(err) - } - if code != 15 { - t.Fatalf("expected a status code equal to '15', got %d", code) + case result := <-resultC: + if result.StatusCode != 15 { + t.Fatalf("expected a status code equal to '15', got %d", result.StatusCode) + } } } @@ -63,8 +67,8 @@ func ExampleClient_ContainerWait_withTimeout() { defer cancel() client, _ := NewEnvClient() - _, err := client.ContainerWait(ctx, "container_id") - if err != nil { + _, errC := client.ContainerWait(ctx, "container_id", "") + if err := <-errC; err != nil { log.Fatal(err) } } diff --git a/components/engine/client/interface.go b/components/engine/client/interface.go index bccfc7b2ca..2cae92b2f6 100644 --- a/components/engine/client/interface.go +++ b/components/engine/client/interface.go @@ -63,7 +63,7 @@ type ContainerAPIClient interface { ContainerTop(ctx context.Context, container string, arguments []string) (container.ContainerTopOKBody, error) ContainerUnpause(ctx context.Context, container string) error ContainerUpdate(ctx context.Context, container string, updateConfig container.UpdateConfig) (container.ContainerUpdateOKBody, error) - ContainerWait(ctx context.Context, container string) (int64, error) + ContainerWait(ctx context.Context, container string, condition container.WaitCondition) (<-chan container.ContainerWaitOKBody, <-chan error) CopyFromContainer(ctx context.Context, container, srcPath string) (io.ReadCloser, types.ContainerPathStat, error) CopyToContainer(ctx context.Context, container, path string, content io.Reader, options types.CopyToContainerOptions) error ContainersPrune(ctx context.Context, pruneFilters filters.Args) (types.ContainersPruneReport, error) diff --git a/components/engine/container/state.go b/components/engine/container/state.go index 240ffbfda7..027377f4e3 100644 --- a/components/engine/container/state.go +++ b/components/engine/container/state.go @@ -45,22 +45,15 @@ type StateStatus struct { err error } -func newStateStatus(ec int, err error) *StateStatus { - return &StateStatus{ - exitCode: ec, - err: err, - } -} - // ExitCode returns current exitcode for the state. -func (ss *StateStatus) ExitCode() int { - return ss.exitCode +func (s StateStatus) ExitCode() int { + return s.exitCode } // Err returns current error for the state. Returns nil if the container had // exited on its own. -func (ss *StateStatus) Err() error { - return ss.err +func (s StateStatus) Err() error { + return s.err } // NewState creates a default state object with a fresh channel for state changes. @@ -165,45 +158,55 @@ func IsValidStateString(s string) bool { return true } -func (s *State) isStopped() bool { - // The state is not considered "stopped" if it is either "created", - // "running", or "paused". - switch s.StateString() { - case "created", "running", "paused": - return false - default: - return true - } -} +// WaitCondition is an enum type for different states to wait for. +type WaitCondition int -// Wait waits until the continer is in a "stopped" state. A context can be used -// for cancelling the request or controlling timeouts. If untilRemoved is true, -// Wait will block until the SetRemoved() method has been called. Wait must be -// called without holding the state lock. Returns a channel which can be used -// to receive the result. If the container exited on its own, the result's Err() method wil be nil and -// its ExitCode() method will return the conatiners exit code, otherwise, the -// results Err() method will return an error indicating why the wait operation -// failed. -func (s *State) Wait(ctx context.Context, untilRemoved bool) <-chan *StateStatus { +// Possible WaitCondition Values. +// +// WaitConditionNotRunning (default) is used to wait for any of the non-running +// states: "created", "exited", "dead", "removing", or "removed". +// +// WaitConditionNextExit is used to wait for the next time the state changes +// to a non-running state. If the state is currently "created" or "exited", +// this would cause Wait() to block until either the container runs and exits +// or is removed. +// +// WaitConditionRemoved is used to wait for the container to be removed. +const ( + WaitConditionNotRunning WaitCondition = iota + WaitConditionNextExit + WaitConditionRemoved +) + +// Wait waits until the continer is in a certain state indicated by the given +// condition. A context must be used for cancelling the request, controlling +// timeouts, and avoiding goroutine leaks. Wait must be called without holding +// the state lock. Returns a channel from which the caller will receive the +// result. If the container exited on its own, the result's Err() method will +// be nil and its ExitCode() method will return the conatiners exit code, +// otherwise, the results Err() method will return an error indicating why the +// wait operation failed. +func (s *State) Wait(ctx context.Context, condition WaitCondition) <-chan StateStatus { s.Lock() defer s.Unlock() - if !untilRemoved && s.isStopped() { - // We are not waiting for removal and the container is already - // in a stopped state so just return the current state. - result := newStateStatus(s.ExitCode(), s.Err()) + if condition == WaitConditionNotRunning && !s.Running { + // Buffer so we can put it in the channel now. + resultC := make(chan StateStatus, 1) - // Buffer so we don't block putting it in the channel. - resultC := make(chan *StateStatus, 1) - resultC <- result + // Send the current status. + resultC <- StateStatus{ + exitCode: s.ExitCode(), + err: s.Err(), + } return resultC } - // The waitStop chan will remain nil if we are waiting for removal, in - // which case it would block forever. + // If we are waiting only for removal, the waitStop channel should + // remain nil and block forever. var waitStop chan struct{} - if !untilRemoved { + if condition < WaitConditionRemoved { waitStop = s.waitStop } @@ -212,20 +215,26 @@ func (s *State) Wait(ctx context.Context, untilRemoved bool) <-chan *StateStatus // actually stopped. waitRemove := s.waitRemove - resultC := make(chan *StateStatus) + resultC := make(chan StateStatus) go func() { select { case <-ctx.Done(): // Context timeout or cancellation. - resultC <- newStateStatus(-1, ctx.Err()) + resultC <- StateStatus{ + exitCode: -1, + err: ctx.Err(), + } return case <-waitStop: case <-waitRemove: } s.Lock() - result := newStateStatus(s.ExitCode(), s.Err()) + result := StateStatus{ + exitCode: s.ExitCode(), + err: s.Err(), + } s.Unlock() resultC <- result @@ -365,8 +374,3 @@ func (s *State) Err() error { } return nil } - -// Error returns current error for the state. -func (s *State) Error() string { - return s.ErrorMsg -} diff --git a/components/engine/container/state_test.go b/components/engine/container/state_test.go index 903d2a786b..2a90e5541d 100644 --- a/components/engine/container/state_test.go +++ b/components/engine/container/state_test.go @@ -31,21 +31,33 @@ func TestIsValidHealthString(t *testing.T) { func TestStateRunStop(t *testing.T) { s := NewState() - // An initial wait (in "created" state) should block until the - // container has started and exited. It shouldn't take more than 100 - // milliseconds. - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) - defer cancel() - initialWait := s.Wait(ctx, false) - - // Begin another wait for the final removed state. It should complete + // Begin another wait with WaitConditionRemoved. It should complete // within 200 milliseconds. - ctx, cancel = context.WithTimeout(context.Background(), 200*time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) defer cancel() - removalWait := s.Wait(ctx, true) + removalWait := s.Wait(ctx, WaitConditionRemoved) // Full lifecycle two times. for i := 1; i <= 2; i++ { + // A wait with WaitConditionNotRunning should return + // immediately since the state is now either "created" (on the + // first iteration) or "exited" (on the second iteration). It + // shouldn't take more than 50 milliseconds. + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + // Expectx exit code to be i-1 since it should be the exit + // code from the previous loop or 0 for the created state. + if status := <-s.Wait(ctx, WaitConditionNotRunning); status.ExitCode() != i-1 { + t.Fatalf("ExitCode %v, expected %v, err %q", status.ExitCode(), i-1, status.Err()) + } + + // A wait with WaitConditionNextExit should block until the + // container has started and exited. It shouldn't take more + // than 100 milliseconds. + ctx, cancel = context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + initialWait := s.Wait(ctx, WaitConditionNextExit) + // Set the state to "Running". s.Lock() s.SetRunning(i, true) @@ -62,10 +74,12 @@ func TestStateRunStop(t *testing.T) { t.Fatalf("ExitCode %v, expected 0", s.ExitCode()) } - // Async wait up to 50 milliseconds for the exit status. - ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + // Now that it's running, a wait with WaitConditionNotRunning + // should block until we stop the container. It shouldn't take + // more than 100 milliseconds. + ctx, cancel = context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() - exitWait := s.Wait(ctx, false) + exitWait := s.Wait(ctx, WaitConditionNotRunning) // Set the state to "Exited". s.Lock() @@ -83,30 +97,15 @@ func TestStateRunStop(t *testing.T) { t.Fatalf("Pid %v, expected 0", s.Pid) } + // Receive the initialWait result. + if status := <-initialWait; status.ExitCode() != i { + t.Fatalf("ExitCode %v, expected %v, err %q", status.ExitCode(), i, status.Err()) + } + // Receive the exitWait result. - status := <-exitWait - if status.ExitCode() != i { + if status := <-exitWait; status.ExitCode() != i { t.Fatalf("ExitCode %v, expected %v, err %q", status.ExitCode(), i, status.Err()) } - - // A repeated call to Wait() should not block at this point. - ctx, cancel = context.WithTimeout(context.Background(), 10*time.Millisecond) - defer cancel() - exitWait = s.Wait(ctx, false) - - status = <-exitWait - if status.ExitCode() != i { - t.Fatalf("ExitCode %v, expected %v, err %q", status.ExitCode(), i, status.Err()) - } - - if i == 1 { - // Make sure our initial wait also succeeds. - status = <-initialWait - if status.ExitCode() != i { - // Should have the exit code from this first loop. - t.Fatalf("Initial wait exitCode %v, expected %v, err %q", status.ExitCode(), i, status.Err()) - } - } } // Set the state to dead and removed. @@ -114,8 +113,7 @@ func TestStateRunStop(t *testing.T) { s.SetRemoved() // Wait for removed status or timeout. - status := <-removalWait - if status.ExitCode() != 2 { + if status := <-removalWait; status.ExitCode() != 2 { // Should have the final exit code from the loop. t.Fatalf("Removal wait exitCode %v, expected %v, err %q", status.ExitCode(), 2, status.Err()) } @@ -124,10 +122,14 @@ func TestStateRunStop(t *testing.T) { func TestStateTimeoutWait(t *testing.T) { s := NewState() + s.Lock() + s.SetRunning(0, true) + s.Unlock() + // Start a wait with a timeout. ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() - waitC := s.Wait(ctx, false) + waitC := s.Wait(ctx, WaitConditionNotRunning) // It should timeout *before* this 200ms timer does. select { @@ -145,7 +147,6 @@ func TestStateTimeoutWait(t *testing.T) { } s.Lock() - s.SetRunning(0, true) s.SetStopped(&ExitStatus{ExitCode: 0}) s.Unlock() @@ -153,7 +154,7 @@ func TestStateTimeoutWait(t *testing.T) { // immediately. ctx, cancel = context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() - waitC = s.Wait(ctx, false) + waitC = s.Wait(ctx, WaitConditionNotRunning) select { case <-time.After(200 * time.Millisecond): diff --git a/components/engine/container/stream/attach.go b/components/engine/container/stream/attach.go index c3a630d975..5f63f88f39 100644 --- a/components/engine/container/stream/attach.go +++ b/components/engine/container/stream/attach.go @@ -8,17 +8,11 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/pkg/promise" + "github.com/docker/docker/pkg/term" ) var defaultEscapeSequence = []byte{16, 17} // ctrl-p, ctrl-q -// DetachError is special error which returned in case of container detach. -type DetachError struct{} - -func (DetachError) Error() string { - return "detached from container" -} - // AttachConfig is the config struct used to attach a client to a stream's stdio type AttachConfig struct { // Tells the attach copier that the stream's stdin is a TTY and to look for @@ -173,62 +167,11 @@ func (c *Config) CopyStreams(ctx context.Context, cfg *AttachConfig) chan error }) } -// ttyProxy is used only for attaches with a TTY. It is used to proxy -// stdin keypresses from the underlying reader and look for the passed in -// escape key sequence to signal a detach. -type ttyProxy struct { - escapeKeys []byte - escapeKeyPos int - r io.Reader -} - -func (r *ttyProxy) Read(buf []byte) (int, error) { - nr, err := r.r.Read(buf) - - preserve := func() { - // this preserves the original key presses in the passed in buffer - nr += r.escapeKeyPos - preserve := make([]byte, 0, r.escapeKeyPos+len(buf)) - preserve = append(preserve, r.escapeKeys[:r.escapeKeyPos]...) - preserve = append(preserve, buf...) - r.escapeKeyPos = 0 - copy(buf[0:nr], preserve) - } - - if nr != 1 || err != nil { - if r.escapeKeyPos > 0 { - preserve() - } - return nr, err - } - - if buf[0] != r.escapeKeys[r.escapeKeyPos] { - if r.escapeKeyPos > 0 { - preserve() - } - return nr, nil - } - - if r.escapeKeyPos == len(r.escapeKeys)-1 { - return 0, DetachError{} - } - - // Looks like we've got an escape key, but we need to match again on the next - // read. - // Store the current escape key we found so we can look for the next one on - // the next read. - // Since this is an escape key, make sure we don't let the caller read it - // If later on we find that this is not the escape sequence, we'll add the - // keys back - r.escapeKeyPos++ - return nr - r.escapeKeyPos, nil -} - func copyEscapable(dst io.Writer, src io.ReadCloser, keys []byte) (written int64, err error) { if len(keys) == 0 { keys = defaultEscapeSequence } - pr := &ttyProxy{escapeKeys: keys, r: src} + pr := term.NewEscapeProxy(src, keys) defer src.Close() return io.Copy(dst, pr) diff --git a/components/engine/daemon/attach.go b/components/engine/daemon/attach.go index 4476598c4d..a892799529 100644 --- a/components/engine/daemon/attach.go +++ b/components/engine/daemon/attach.go @@ -162,7 +162,7 @@ func (daemon *Daemon) containerAttach(c *container.Container, cfg *stream.Attach if c.Config.StdinOnce && !c.Config.Tty { // Wait for the container to stop before returning. - waitChan := c.Wait(context.Background(), false) + waitChan := c.Wait(context.Background(), container.WaitConditionNotRunning) defer func() { _ = <-waitChan // Ignore returned exit code. }() @@ -171,7 +171,7 @@ func (daemon *Daemon) containerAttach(c *container.Container, cfg *stream.Attach ctx := c.InitAttachContext() err := <-c.StreamConfig.CopyStreams(ctx, cfg) if err != nil { - if _, ok := err.(stream.DetachError); ok { + if _, ok := err.(term.EscapeError); ok { daemon.LogContainerEvent(c, "detach") } else { logrus.Errorf("attach failed with error: %v", err) diff --git a/components/engine/daemon/cluster/executor/backend.go b/components/engine/daemon/cluster/executor/backend.go index d163caadbe..2578a93c5a 100644 --- a/components/engine/daemon/cluster/executor/backend.go +++ b/components/engine/daemon/cluster/executor/backend.go @@ -40,7 +40,7 @@ type Backend interface { DeactivateContainerServiceBinding(containerName string) error UpdateContainerServiceConfig(containerName string, serviceConfig *clustertypes.ServiceConfig) error ContainerInspectCurrent(name string, size bool) (*types.ContainerJSON, error) - ContainerWait(ctx context.Context, name string, untilRemoved bool) (<-chan *containerpkg.StateStatus, error) + ContainerWait(ctx context.Context, name string, condition containerpkg.WaitCondition) (<-chan containerpkg.StateStatus, error) ContainerRm(name string, config *types.ContainerRmConfig) error ContainerKill(name string, sig uint64) error SetContainerDependencyStore(name string, store exec.DependencyGetter) error diff --git a/components/engine/daemon/cluster/executor/container/adapter.go b/components/engine/daemon/cluster/executor/container/adapter.go index 66e5d7fc29..fd2ef354a6 100644 --- a/components/engine/daemon/cluster/executor/container/adapter.go +++ b/components/engine/daemon/cluster/executor/container/adapter.go @@ -338,8 +338,8 @@ func (c *containerAdapter) events(ctx context.Context) <-chan events.Message { return eventsq } -func (c *containerAdapter) wait(ctx context.Context) (<-chan *containerpkg.StateStatus, error) { - return c.backend.ContainerWait(ctx, c.container.nameOrID(), false) +func (c *containerAdapter) wait(ctx context.Context) (<-chan containerpkg.StateStatus, error) { + return c.backend.ContainerWait(ctx, c.container.nameOrID(), containerpkg.WaitConditionNotRunning) } func (c *containerAdapter) shutdown(ctx context.Context) error { diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index 7d02f21c5a..6889c10956 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -291,16 +291,16 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { return nil } -func killProcessDirectly(container *container.Container) error { +func killProcessDirectly(cntr *container.Container) error { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() // Block until the container to stops or timeout. - status := <-container.Wait(ctx, false) + status := <-cntr.Wait(ctx, container.WaitConditionNotRunning) if status.Err() != nil { // Ensure that we don't kill ourselves - if pid := container.GetPID(); pid != 0 { - logrus.Infof("Container %s failed to exit within 10 seconds of kill - trying direct SIGKILL", stringid.TruncateID(container.ID)) + if pid := cntr.GetPID(); pid != 0 { + logrus.Infof("Container %s failed to exit within 10 seconds of kill - trying direct SIGKILL", stringid.TruncateID(cntr.ID)) if err := syscall.Kill(pid, 9); err != nil { if err != syscall.ESRCH { return err diff --git a/components/engine/daemon/daemon.go b/components/engine/daemon/daemon.go index 537c17c0ae..041319da81 100644 --- a/components/engine/daemon/daemon.go +++ b/components/engine/daemon/daemon.go @@ -779,7 +779,7 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error { defer cancel() // Wait with timeout for container to exit. - if status := <-c.Wait(ctx, false); status.Err() != nil { + if status := <-c.Wait(ctx, container.WaitConditionNotRunning); status.Err() != nil { logrus.Debugf("container %s failed to exit in %d second of SIGTERM, sending SIGKILL to force", c.ID, stopTimeout) sig, ok := signal.SignalMap["KILL"] if !ok { @@ -790,7 +790,7 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error { } // Wait for exit again without a timeout. // Explicitly ignore the result. - _ = <-c.Wait(context.Background(), false) + _ = <-c.Wait(context.Background(), container.WaitConditionNotRunning) return status.Err() } } @@ -801,7 +801,7 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error { // Wait without timeout for the container to exit. // Ignore the result. - _ = <-c.Wait(context.Background(), false) + _ = <-c.Wait(context.Background(), container.WaitConditionNotRunning) return nil } diff --git a/components/engine/daemon/exec.go b/components/engine/daemon/exec.go index 1622a9cd3c..72d01c8c23 100644 --- a/components/engine/daemon/exec.go +++ b/components/engine/daemon/exec.go @@ -253,7 +253,7 @@ func (d *Daemon) ContainerExecStart(ctx context.Context, name string, stdin io.R return fmt.Errorf("context cancelled") case err := <-attachErr: if err != nil { - if _, ok := err.(stream.DetachError); !ok { + if _, ok := err.(term.EscapeError); !ok { return fmt.Errorf("exec attach failed with error: %v", err) } d.LogContainerEvent(c, "exec_detach") diff --git a/components/engine/daemon/inspect.go b/components/engine/daemon/inspect.go index 06858223f5..4eb1d091d3 100644 --- a/components/engine/daemon/inspect.go +++ b/components/engine/daemon/inspect.go @@ -153,7 +153,7 @@ func (daemon *Daemon) getInspectData(container *container.Container) (*types.Con Dead: container.State.Dead, Pid: container.State.Pid, ExitCode: container.State.ExitCode(), - Error: container.State.Error(), + Error: container.State.ErrorMsg, StartedAt: container.State.StartedAt.Format(time.RFC3339Nano), FinishedAt: container.State.FinishedAt.Format(time.RFC3339Nano), Health: containerHealth, diff --git a/components/engine/daemon/kill.go b/components/engine/daemon/kill.go index 423a666b22..72e716d098 100644 --- a/components/engine/daemon/kill.go +++ b/components/engine/daemon/kill.go @@ -9,7 +9,7 @@ import ( "time" "github.com/Sirupsen/logrus" - "github.com/docker/docker/container" + containerpkg "github.com/docker/docker/container" "github.com/docker/docker/pkg/signal" ) @@ -55,7 +55,7 @@ func (daemon *Daemon) ContainerKill(name string, sig uint64) error { // to send the signal. An error is returned if the container is paused // or not running, or if there is a problem returned from the // underlying kill command. -func (daemon *Daemon) killWithSignal(container *container.Container, sig int) error { +func (daemon *Daemon) killWithSignal(container *containerpkg.Container, sig int) error { logrus.Debugf("Sending kill signal %d to container %s", sig, container.ID) container.Lock() defer container.Unlock() @@ -111,7 +111,7 @@ func (daemon *Daemon) killWithSignal(container *container.Container, sig int) er } // Kill forcefully terminates a container. -func (daemon *Daemon) Kill(container *container.Container) error { +func (daemon *Daemon) Kill(container *containerpkg.Container) error { if !container.IsRunning() { return errNotRunning{container.ID} } @@ -135,7 +135,7 @@ func (daemon *Daemon) Kill(container *container.Container) error { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - if status := <-container.Wait(ctx, false); status.Err() != nil { + if status := <-container.Wait(ctx, containerpkg.WaitConditionNotRunning); status.Err() != nil { return err } } @@ -150,13 +150,13 @@ func (daemon *Daemon) Kill(container *container.Container) error { // Wait for exit with no timeout. // Ignore returned status. - _ = <-container.Wait(context.Background(), false) + _ = <-container.Wait(context.Background(), containerpkg.WaitConditionNotRunning) return nil } // killPossibleDeadProcess is a wrapper around killSig() suppressing "no such process" error. -func (daemon *Daemon) killPossiblyDeadProcess(container *container.Container, sig int) error { +func (daemon *Daemon) killPossiblyDeadProcess(container *containerpkg.Container, sig int) error { err := daemon.killWithSignal(container, sig) if err == syscall.ESRCH { e := errNoSuchProcess{container.GetPID(), sig} @@ -166,6 +166,6 @@ func (daemon *Daemon) killPossiblyDeadProcess(container *container.Container, si return err } -func (daemon *Daemon) kill(c *container.Container, sig int) error { +func (daemon *Daemon) kill(c *containerpkg.Container, sig int) error { return daemon.containerd.Signal(c.ID, sig) } diff --git a/components/engine/daemon/stop.go b/components/engine/daemon/stop.go index 4fd1e40c1e..6a4776d155 100644 --- a/components/engine/daemon/stop.go +++ b/components/engine/daemon/stop.go @@ -8,7 +8,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/api/errors" - "github.com/docker/docker/container" + containerpkg "github.com/docker/docker/container" ) // ContainerStop looks for the given container and terminates it, @@ -41,7 +41,7 @@ func (daemon *Daemon) ContainerStop(name string, seconds *int) error { // process to exit. If a negative duration is given, Stop will wait // for the initial signal forever. If the container is not running Stop returns // immediately. -func (daemon *Daemon) containerStop(container *container.Container, seconds int) error { +func (daemon *Daemon) containerStop(container *containerpkg.Container, seconds int) error { if !container.IsRunning() { return nil } @@ -64,7 +64,7 @@ func (daemon *Daemon) containerStop(container *container.Container, seconds int) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - if status := <-container.Wait(ctx, false); status.Err() != nil { + if status := <-container.Wait(ctx, containerpkg.WaitConditionNotRunning); status.Err() != nil { logrus.Infof("Container failed to stop after sending signal %d to the process, force killing", stopSignal) if err := daemon.killPossiblyDeadProcess(container, 9); err != nil { return err @@ -76,12 +76,12 @@ func (daemon *Daemon) containerStop(container *container.Container, seconds int) ctx, cancel := context.WithTimeout(context.Background(), time.Duration(seconds)*time.Second) defer cancel() - if status := <-container.Wait(ctx, false); status.Err() != nil { + if status := <-container.Wait(ctx, containerpkg.WaitConditionNotRunning); status.Err() != nil { logrus.Infof("Container %v failed to exit within %d seconds of signal %d - using the force", container.ID, seconds, stopSignal) // 3. If it doesn't, then send SIGKILL if err := daemon.Kill(container); err != nil { // Wait without a timeout, ignore result. - _ = <-container.Wait(context.Background(), false) + _ = <-container.Wait(context.Background(), containerpkg.WaitConditionNotRunning) logrus.Warn(err) // Don't return error because we only care that container is stopped, not what function stopped it } } diff --git a/components/engine/daemon/wait.go b/components/engine/daemon/wait.go index 884911ab49..76c16b9efc 100644 --- a/components/engine/daemon/wait.go +++ b/components/engine/daemon/wait.go @@ -5,18 +5,18 @@ import ( "golang.org/x/net/context" ) -// ContainerWait stops processing until the given container is stopped or -// removed (if untilRemoved is true). If the container is not found, a nil +// ContainerWait waits until the given container is in a certain state +// indicated by the given condition. If the container is not found, a nil // channel and non-nil error is returned immediately. If the container is // found, a status result will be sent on the returned channel once the wait // condition is met or if an error occurs waiting for the container (such as a -// context timeout or cancellation). On a successful stop, the exit code of the +// context timeout or cancellation). On a successful wait, the exit code of the // container is returned in the status with a non-nil Err() value. -func (daemon *Daemon) ContainerWait(ctx context.Context, name string, untilRemoved bool) (<-chan *container.StateStatus, error) { - container, err := daemon.GetContainer(name) +func (daemon *Daemon) ContainerWait(ctx context.Context, name string, condition container.WaitCondition) (<-chan container.StateStatus, error) { + cntr, err := daemon.GetContainer(name) if err != nil { return nil, err } - return container.Wait(ctx, untilRemoved), nil + return cntr.Wait(ctx, condition), nil } diff --git a/components/engine/docs/api/version-history.md b/components/engine/docs/api/version-history.md index ce274e3489..3891d35c04 100644 --- a/components/engine/docs/api/version-history.md +++ b/components/engine/docs/api/version-history.md @@ -28,6 +28,7 @@ keywords: "API, Docker, rcli, REST, documentation" the swarm, the desired CA key for the swarm (if not using an external certificate), and an optional parameter to force swarm to generate and rotate to a new CA certificate/key pair. * `POST /service/create` and `POST /services/(id or name)/update` now take the field `Platforms` as part of the service `Placement`, allowing to specify platforms supported by the service. +* `POST /containers/(name)/wait` now accepts a `condition` query parameter to indicate which state change condition to wait for. Also, response headers are now returned immediately to acknowledge that the server has registered a wait callback for the client. ## v1.29 API changes diff --git a/components/engine/docs/reference/commandline/wait.md b/components/engine/docs/reference/commandline/wait.md index ee8f9ab243..a4a65415de 100644 --- a/components/engine/docs/reference/commandline/wait.md +++ b/components/engine/docs/reference/commandline/wait.md @@ -21,7 +21,7 @@ Usage: docker wait CONTAINER [CONTAINER...] Block until one or more containers stop, then print their exit codes Options: - --help Print usage + --help Print usage ``` > **Note**: `docker wait` returns `0` when run against a container which had diff --git a/components/engine/hack/integration-cli-on-swarm/agent/worker/executor.go b/components/engine/hack/integration-cli-on-swarm/agent/worker/executor.go index 442428ac81..3442b09400 100644 --- a/components/engine/hack/integration-cli-on-swarm/agent/worker/executor.go +++ b/components/engine/hack/integration-cli-on-swarm/agent/worker/executor.go @@ -83,11 +83,13 @@ func privilegedTestChunkExecutor(autoRemove bool) testChunkExecutor { } var b bytes.Buffer teeContainerStream(&b, os.Stdout, os.Stderr, stream) - rc, err := cli.ContainerWait(context.Background(), id) - if err != nil { + resultC, errC := cli.ContainerWait(context.Background(), id, "") + select { + case err := <-errC: return 0, "", err + case result := <-resultC: + return result.StatusCode, b.String(), nil } - return rc, b.String(), nil } } diff --git a/components/engine/hack/integration-cli-on-swarm/host/host.go b/components/engine/hack/integration-cli-on-swarm/host/host.go index 40f7a1a57b..d881b7a262 100644 --- a/components/engine/hack/integration-cli-on-swarm/host/host.go +++ b/components/engine/hack/integration-cli-on-swarm/host/host.go @@ -188,5 +188,11 @@ func waitForContainerCompletion(cli *client.Client, stdout, stderr io.Writer, co } stdcopy.StdCopy(stdout, stderr, stream) stream.Close() - return cli.ContainerWait(context.Background(), containerID) + resultC, errC := cli.ContainerWait(context.Background(), containerID, "") + select { + case err := <-errC: + return 1, err + case result := <-resultC: + return result.StatusCode, nil + } } diff --git a/components/engine/pkg/term/proxy.go b/components/engine/pkg/term/proxy.go new file mode 100644 index 0000000000..e648eb8120 --- /dev/null +++ b/components/engine/pkg/term/proxy.go @@ -0,0 +1,74 @@ +package term + +import ( + "io" +) + +// EscapeError is special error which returned by a TTY proxy reader's Read() +// method in case its detach escape sequence is read. +type EscapeError struct{} + +func (EscapeError) Error() string { + return "read escape sequence" +} + +// escapeProxy is used only for attaches with a TTY. It is used to proxy +// stdin keypresses from the underlying reader and look for the passed in +// escape key sequence to signal a detach. +type escapeProxy struct { + escapeKeys []byte + escapeKeyPos int + r io.Reader +} + +// NewEscapeProxy returns a new TTY proxy reader which wraps the given reader +// and detects when the specified escape keys are read, in which case the Read +// method will return an error of type EscapeError. +func NewEscapeProxy(r io.Reader, escapeKeys []byte) io.Reader { + return &escapeProxy{ + escapeKeys: escapeKeys, + r: r, + } +} + +func (r *escapeProxy) Read(buf []byte) (int, error) { + nr, err := r.r.Read(buf) + + preserve := func() { + // this preserves the original key presses in the passed in buffer + nr += r.escapeKeyPos + preserve := make([]byte, 0, r.escapeKeyPos+len(buf)) + preserve = append(preserve, r.escapeKeys[:r.escapeKeyPos]...) + preserve = append(preserve, buf...) + r.escapeKeyPos = 0 + copy(buf[0:nr], preserve) + } + + if nr != 1 || err != nil { + if r.escapeKeyPos > 0 { + preserve() + } + return nr, err + } + + if buf[0] != r.escapeKeys[r.escapeKeyPos] { + if r.escapeKeyPos > 0 { + preserve() + } + return nr, nil + } + + if r.escapeKeyPos == len(r.escapeKeys)-1 { + return 0, EscapeError{} + } + + // Looks like we've got an escape key, but we need to match again on the next + // read. + // Store the current escape key we found so we can look for the next one on + // the next read. + // Since this is an escape key, make sure we don't let the caller read it + // If later on we find that this is not the escape sequence, we'll add the + // keys back + r.escapeKeyPos++ + return nr - r.escapeKeyPos, nil +}