This patch consolidates the two WaitStop and WaitWithContext methods
on the container.State type. Now there is a single method, Wait, which
takes a context and a bool specifying whether to wait for not just a
container exit but also removal.
The behavior has been changed slightly so that a wait call during a
Created state will not return immediately but instead wait for the
container to be started and then exited.
The interface has been changed to no longer block, but instead returns
a channel on which the caller can receive a *StateStatus value which
indicates the ExitCode or an error if there was one (like a context
timeout or state transition error).
These changes have been propagated through the rest of the deamon to
preserve all other existing behavior.
Docker-DCO-1.1-Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)
| ... | ... |
@@ -2,7 +2,6 @@ package container |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 | 4 |
"io" |
| 5 |
- "time" |
|
| 6 | 5 |
|
| 7 | 6 |
"golang.org/x/net/context" |
| 8 | 7 |
|
| ... | ... |
@@ -10,6 +9,7 @@ import ( |
| 10 | 10 |
"github.com/docker/docker/api/types/backend" |
| 11 | 11 |
"github.com/docker/docker/api/types/container" |
| 12 | 12 |
"github.com/docker/docker/api/types/filters" |
| 13 |
+ containerpkg "github.com/docker/docker/container" |
|
| 13 | 14 |
"github.com/docker/docker/pkg/archive" |
| 14 | 15 |
) |
| 15 | 16 |
|
| ... | ... |
@@ -44,7 +44,7 @@ type stateBackend interface {
|
| 44 | 44 |
ContainerStop(name string, seconds *int) error |
| 45 | 45 |
ContainerUnpause(name string) error |
| 46 | 46 |
ContainerUpdate(name string, hostConfig *container.HostConfig) (container.ContainerUpdateOKBody, error) |
| 47 |
- ContainerWait(name string, timeout time.Duration) (int, error) |
|
| 47 |
+ ContainerWait(ctx context.Context, name string, untilRemoved bool) (<-chan *containerpkg.StateStatus, error) |
|
| 48 | 48 |
} |
| 49 | 49 |
|
| 50 | 50 |
// monitorBackend includes functions to implement to provide containers monitoring functionality. |
| ... | ... |
@@ -7,7 +7,6 @@ import ( |
| 7 | 7 |
"net/http" |
| 8 | 8 |
"strconv" |
| 9 | 9 |
"syscall" |
| 10 |
- "time" |
|
| 11 | 10 |
|
| 12 | 11 |
"github.com/Sirupsen/logrus" |
| 13 | 12 |
"github.com/docker/docker/api" |
| ... | ... |
@@ -284,13 +283,15 @@ func (s *containerRouter) postContainersUnpause(ctx context.Context, w http.Resp |
| 284 | 284 |
} |
| 285 | 285 |
|
| 286 | 286 |
func (s *containerRouter) postContainersWait(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
|
| 287 |
- status, err := s.backend.ContainerWait(vars["name"], -1*time.Second) |
|
| 287 |
+ waitC, err := s.backend.ContainerWait(ctx, vars["name"], false) |
|
| 288 | 288 |
if err != nil {
|
| 289 | 289 |
return err |
| 290 | 290 |
} |
| 291 | 291 |
|
| 292 |
+ status := <-waitC |
|
| 293 |
+ |
|
| 292 | 294 |
return httputils.WriteJSON(w, http.StatusOK, &container.ContainerWaitOKBody{
|
| 293 |
- StatusCode: int64(status), |
|
| 295 |
+ StatusCode: int64(status.ExitCode()), |
|
| 294 | 296 |
}) |
| 295 | 297 |
} |
| 296 | 298 |
|
| ... | ... |
@@ -6,12 +6,13 @@ package builder |
| 6 | 6 |
|
| 7 | 7 |
import ( |
| 8 | 8 |
"io" |
| 9 |
- "time" |
|
| 9 |
+ |
|
| 10 |
+ "golang.org/x/net/context" |
|
| 10 | 11 |
|
| 11 | 12 |
"github.com/docker/docker/api/types" |
| 12 | 13 |
"github.com/docker/docker/api/types/backend" |
| 13 | 14 |
"github.com/docker/docker/api/types/container" |
| 14 |
- "golang.org/x/net/context" |
|
| 15 |
+ containerpkg "github.com/docker/docker/container" |
|
| 15 | 16 |
) |
| 16 | 17 |
|
| 17 | 18 |
const ( |
| ... | ... |
@@ -49,7 +50,7 @@ type Backend interface {
|
| 49 | 49 |
// ContainerStart starts a new container |
| 50 | 50 |
ContainerStart(containerID string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error |
| 51 | 51 |
// ContainerWait stops processing until the given container is stopped. |
| 52 |
- ContainerWait(containerID string, timeout time.Duration) (int, error) |
|
| 52 |
+ ContainerWait(ctx context.Context, name string, untilRemoved bool) (<-chan *containerpkg.StateStatus, error) |
|
| 53 | 53 |
// ContainerCreateWorkdir creates the workdir |
| 54 | 54 |
ContainerCreateWorkdir(containerID string) error |
| 55 | 55 |
|
| ... | ... |
@@ -4,6 +4,7 @@ package dockerfile |
| 4 | 4 |
// non-contiguous functionality. Please read the comments. |
| 5 | 5 |
|
| 6 | 6 |
import ( |
| 7 |
+ "context" |
|
| 7 | 8 |
"crypto/sha256" |
| 8 | 9 |
"encoding/hex" |
| 9 | 10 |
"fmt" |
| ... | ... |
@@ -596,16 +597,25 @@ func (b *Builder) run(cID string, cmd []string) (err error) {
|
| 596 | 596 |
return err |
| 597 | 597 |
} |
| 598 | 598 |
|
| 599 |
- if ret, _ := b.docker.ContainerWait(cID, -1); ret != 0 {
|
|
| 599 |
+ waitC, err := b.docker.ContainerWait(context.Background(), cID, false) |
|
| 600 |
+ if err != nil {
|
|
| 601 |
+ // Unable to begin waiting for container. |
|
| 602 |
+ close(finished) |
|
| 603 |
+ if cancelErr := <-cancelErrCh; cancelErr != nil {
|
|
| 604 |
+ logrus.Debugf("Build cancelled (%v) and unable to begin ContainerWait: %d", cancelErr, err)
|
|
| 605 |
+ } |
|
| 606 |
+ return err |
|
| 607 |
+ } |
|
| 608 |
+ |
|
| 609 |
+ if status := <-waitC; status.ExitCode() != 0 {
|
|
| 600 | 610 |
close(finished) |
| 601 | 611 |
if cancelErr := <-cancelErrCh; cancelErr != nil {
|
| 602 |
- logrus.Debugf("Build cancelled (%v) and got a non-zero code from ContainerWait: %d",
|
|
| 603 |
- cancelErr, ret) |
|
| 612 |
+ logrus.Debugf("Build cancelled (%v) and got a non-zero code from ContainerWait: %d", cancelErr, status.ExitCode())
|
|
| 604 | 613 |
} |
| 605 | 614 |
// TODO: change error type, because jsonmessage.JSONError assumes HTTP |
| 606 | 615 |
return &jsonmessage.JSONError{
|
| 607 |
- Message: fmt.Sprintf("The command '%s' returned a non-zero code: %d", strings.Join(cmd, " "), ret),
|
|
| 608 |
- Code: ret, |
|
| 616 |
+ Message: fmt.Sprintf("The command '%s' returned a non-zero code: %d", strings.Join(cmd, " "), status.ExitCode()),
|
|
| 617 |
+ Code: status.ExitCode(), |
|
| 609 | 618 |
} |
| 610 | 619 |
} |
| 611 | 620 |
close(finished) |
| ... | ... |
@@ -1,6 +1,7 @@ |
| 1 | 1 |
package container |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 |
+ "errors" |
|
| 4 | 5 |
"fmt" |
| 5 | 6 |
"sync" |
| 6 | 7 |
"time" |
| ... | ... |
@@ -29,23 +30,25 @@ type State struct {
|
| 29 | 29 |
ErrorMsg string `json:"Error"` // contains last known error when starting the container |
| 30 | 30 |
StartedAt time.Time |
| 31 | 31 |
FinishedAt time.Time |
| 32 |
- waitChan chan struct{}
|
|
| 33 | 32 |
Health *Health |
| 33 |
+ |
|
| 34 |
+ waitStop chan struct{}
|
|
| 35 |
+ waitRemove chan struct{}
|
|
| 34 | 36 |
} |
| 35 | 37 |
|
| 36 |
-// StateStatus is used to return an error type implementing both |
|
| 37 |
-// exec.ExitCode and error. |
|
| 38 |
+// StateStatus is used to return container wait results. |
|
| 39 |
+// Implements exec.ExitCode interface. |
|
| 38 | 40 |
// This type is needed as State include a sync.Mutex field which make |
| 39 | 41 |
// copying it unsafe. |
| 40 | 42 |
type StateStatus struct {
|
| 41 | 43 |
exitCode int |
| 42 |
- error string |
|
| 44 |
+ err error |
|
| 43 | 45 |
} |
| 44 | 46 |
|
| 45 |
-func newStateStatus(ec int, err string) *StateStatus {
|
|
| 47 |
+func newStateStatus(ec int, err error) *StateStatus {
|
|
| 46 | 48 |
return &StateStatus{
|
| 47 | 49 |
exitCode: ec, |
| 48 |
- error: err, |
|
| 50 |
+ err: err, |
|
| 49 | 51 |
} |
| 50 | 52 |
} |
| 51 | 53 |
|
| ... | ... |
@@ -54,15 +57,17 @@ func (ss *StateStatus) ExitCode() int {
|
| 54 | 54 |
return ss.exitCode |
| 55 | 55 |
} |
| 56 | 56 |
|
| 57 |
-// Error returns current error for the state. |
|
| 58 |
-func (ss *StateStatus) Error() string {
|
|
| 59 |
- return ss.error |
|
| 57 |
+// Err returns current error for the state. Returns nil if the container had |
|
| 58 |
+// exited on its own. |
|
| 59 |
+func (ss *StateStatus) Err() error {
|
|
| 60 |
+ return ss.err |
|
| 60 | 61 |
} |
| 61 | 62 |
|
| 62 | 63 |
// NewState creates a default state object with a fresh channel for state changes. |
| 63 | 64 |
func NewState() *State {
|
| 64 | 65 |
return &State{
|
| 65 |
- waitChan: make(chan struct{}),
|
|
| 66 |
+ waitStop: make(chan struct{}),
|
|
| 67 |
+ waitRemove: make(chan struct{}),
|
|
| 66 | 68 |
} |
| 67 | 69 |
} |
| 68 | 70 |
|
| ... | ... |
@@ -160,64 +165,73 @@ func IsValidStateString(s string) bool {
|
| 160 | 160 |
return true |
| 161 | 161 |
} |
| 162 | 162 |
|
| 163 |
-func wait(waitChan <-chan struct{}, timeout time.Duration) error {
|
|
| 164 |
- if timeout < 0 {
|
|
| 165 |
- <-waitChan |
|
| 166 |
- return nil |
|
| 167 |
- } |
|
| 168 |
- select {
|
|
| 169 |
- case <-time.After(timeout): |
|
| 170 |
- return fmt.Errorf("Timed out: %v", timeout)
|
|
| 171 |
- case <-waitChan: |
|
| 172 |
- return nil |
|
| 163 |
+func (s *State) isStopped() bool {
|
|
| 164 |
+ // The state is not considered "stopped" if it is either "created", |
|
| 165 |
+ // "running", or "paused". |
|
| 166 |
+ switch s.StateString() {
|
|
| 167 |
+ case "created", "running", "paused": |
|
| 168 |
+ return false |
|
| 169 |
+ default: |
|
| 170 |
+ return true |
|
| 173 | 171 |
} |
| 174 | 172 |
} |
| 175 | 173 |
|
| 176 |
-// WaitStop waits until state is stopped. If state already stopped it returns |
|
| 177 |
-// immediately. If you want wait forever you must supply negative timeout. |
|
| 178 |
-// Returns exit code, that was passed to SetStopped |
|
| 179 |
-func (s *State) WaitStop(timeout time.Duration) (int, error) {
|
|
| 180 |
- ctx := context.Background() |
|
| 181 |
- if timeout >= 0 {
|
|
| 182 |
- var cancel func() |
|
| 183 |
- ctx, cancel = context.WithTimeout(ctx, timeout) |
|
| 184 |
- defer cancel() |
|
| 174 |
+// Wait waits until the continer is in a "stopped" state. A context can be used |
|
| 175 |
+// for cancelling the request or controlling timeouts. If untilRemoved is true, |
|
| 176 |
+// Wait will block until the SetRemoved() method has been called. Wait must be |
|
| 177 |
+// called without holding the state lock. Returns a channel which can be used |
|
| 178 |
+// to receive the result. If the container exited on its own, the result's Err() method wil be nil and |
|
| 179 |
+// its ExitCode() method will return the conatiners exit code, otherwise, the |
|
| 180 |
+// results Err() method will return an error indicating why the wait operation |
|
| 181 |
+// failed. |
|
| 182 |
+func (s *State) Wait(ctx context.Context, untilRemoved bool) <-chan *StateStatus {
|
|
| 183 |
+ s.Lock() |
|
| 184 |
+ defer s.Unlock() |
|
| 185 |
+ |
|
| 186 |
+ if !untilRemoved && s.isStopped() {
|
|
| 187 |
+ // We are not waiting for removal and the container is already |
|
| 188 |
+ // in a stopped state so just return the current state. |
|
| 189 |
+ result := newStateStatus(s.ExitCode(), s.Err()) |
|
| 190 |
+ |
|
| 191 |
+ // Buffer so we don't block putting it in the channel. |
|
| 192 |
+ resultC := make(chan *StateStatus, 1) |
|
| 193 |
+ resultC <- result |
|
| 194 |
+ |
|
| 195 |
+ return resultC |
|
| 185 | 196 |
} |
| 186 |
- if err := s.WaitWithContext(ctx); err != nil {
|
|
| 187 |
- if status, ok := err.(*StateStatus); ok {
|
|
| 188 |
- return status.ExitCode(), nil |
|
| 189 |
- } |
|
| 190 |
- return -1, err |
|
| 197 |
+ |
|
| 198 |
+ // The waitStop chan will remain nil if we are waiting for removal, in |
|
| 199 |
+ // which case it would block forever. |
|
| 200 |
+ var waitStop chan struct{}
|
|
| 201 |
+ if !untilRemoved {
|
|
| 202 |
+ waitStop = s.waitStop |
|
| 191 | 203 |
} |
| 192 |
- return 0, nil |
|
| 193 |
-} |
|
| 194 | 204 |
|
| 195 |
-// WaitWithContext waits for the container to stop. Optional context can be |
|
| 196 |
-// passed for canceling the request. |
|
| 197 |
-func (s *State) WaitWithContext(ctx context.Context) error {
|
|
| 198 |
- s.Lock() |
|
| 199 |
- if !s.Running {
|
|
| 200 |
- state := newStateStatus(s.ExitCode(), s.Error()) |
|
| 201 |
- defer s.Unlock() |
|
| 202 |
- if state.ExitCode() == 0 {
|
|
| 203 |
- return nil |
|
| 205 |
+ // Always wait for removal, just in case the container gets removed |
|
| 206 |
+ // while it is still in a "created" state, in which case it is never |
|
| 207 |
+ // actually stopped. |
|
| 208 |
+ waitRemove := s.waitRemove |
|
| 209 |
+ |
|
| 210 |
+ resultC := make(chan *StateStatus) |
|
| 211 |
+ |
|
| 212 |
+ go func() {
|
|
| 213 |
+ select {
|
|
| 214 |
+ case <-ctx.Done(): |
|
| 215 |
+ // Context timeout or cancellation. |
|
| 216 |
+ resultC <- newStateStatus(-1, ctx.Err()) |
|
| 217 |
+ return |
|
| 218 |
+ case <-waitStop: |
|
| 219 |
+ case <-waitRemove: |
|
| 204 | 220 |
} |
| 205 |
- return state |
|
| 206 |
- } |
|
| 207 |
- waitChan := s.waitChan |
|
| 208 |
- s.Unlock() |
|
| 209 |
- select {
|
|
| 210 |
- case <-waitChan: |
|
| 221 |
+ |
|
| 211 | 222 |
s.Lock() |
| 212 |
- state := newStateStatus(s.ExitCode(), s.Error()) |
|
| 223 |
+ result := newStateStatus(s.ExitCode(), s.Err()) |
|
| 213 | 224 |
s.Unlock() |
| 214 |
- if state.ExitCode() == 0 {
|
|
| 215 |
- return nil |
|
| 216 |
- } |
|
| 217 |
- return state |
|
| 218 |
- case <-ctx.Done(): |
|
| 219 |
- return ctx.Err() |
|
| 220 |
- } |
|
| 225 |
+ |
|
| 226 |
+ resultC <- result |
|
| 227 |
+ }() |
|
| 228 |
+ |
|
| 229 |
+ return resultC |
|
| 221 | 230 |
} |
| 222 | 231 |
|
| 223 | 232 |
// IsRunning returns whether the running flag is set. Used by Container to check whether a container is running. |
| ... | ... |
@@ -268,8 +282,8 @@ func (s *State) SetStopped(exitStatus *ExitStatus) {
|
| 268 | 268 |
s.Pid = 0 |
| 269 | 269 |
s.FinishedAt = time.Now().UTC() |
| 270 | 270 |
s.setFromExitStatus(exitStatus) |
| 271 |
- close(s.waitChan) // fire waiters for stop |
|
| 272 |
- s.waitChan = make(chan struct{})
|
|
| 271 |
+ close(s.waitStop) // Fire waiters for stop |
|
| 272 |
+ s.waitStop = make(chan struct{})
|
|
| 273 | 273 |
} |
| 274 | 274 |
|
| 275 | 275 |
// SetRestarting sets the container state to "restarting" without locking. |
| ... | ... |
@@ -282,8 +296,8 @@ func (s *State) SetRestarting(exitStatus *ExitStatus) {
|
| 282 | 282 |
s.Pid = 0 |
| 283 | 283 |
s.FinishedAt = time.Now().UTC() |
| 284 | 284 |
s.setFromExitStatus(exitStatus) |
| 285 |
- close(s.waitChan) // fire waiters for stop |
|
| 286 |
- s.waitChan = make(chan struct{})
|
|
| 285 |
+ close(s.waitStop) // Fire waiters for stop |
|
| 286 |
+ s.waitStop = make(chan struct{})
|
|
| 287 | 287 |
} |
| 288 | 288 |
|
| 289 | 289 |
// SetError sets the container's error state. This is useful when we want to |
| ... | ... |
@@ -335,6 +349,23 @@ func (s *State) SetDead() {
|
| 335 | 335 |
s.Unlock() |
| 336 | 336 |
} |
| 337 | 337 |
|
| 338 |
+// SetRemoved assumes this container is already in the "dead" state and |
|
| 339 |
+// closes the internal waitRemove channel to unblock callers waiting for a |
|
| 340 |
+// container to be removed. |
|
| 341 |
+func (s *State) SetRemoved() {
|
|
| 342 |
+ s.Lock() |
|
| 343 |
+ close(s.waitRemove) // Unblock those waiting on remove. |
|
| 344 |
+ s.Unlock() |
|
| 345 |
+} |
|
| 346 |
+ |
|
| 347 |
+// Err returns an error if there is one. |
|
| 348 |
+func (s *State) Err() error {
|
|
| 349 |
+ if s.ErrorMsg != "" {
|
|
| 350 |
+ return errors.New(s.ErrorMsg) |
|
| 351 |
+ } |
|
| 352 |
+ return nil |
|
| 353 |
+} |
|
| 354 |
+ |
|
| 338 | 355 |
// Error returns current error for the state. |
| 339 | 356 |
func (s *State) Error() string {
|
| 340 | 357 |
return s.ErrorMsg |
| ... | ... |
@@ -1,7 +1,7 @@ |
| 1 | 1 |
package container |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 |
- "sync/atomic" |
|
| 4 |
+ "context" |
|
| 5 | 5 |
"testing" |
| 6 | 6 |
"time" |
| 7 | 7 |
|
| ... | ... |
@@ -30,31 +30,49 @@ func TestIsValidHealthString(t *testing.T) {
|
| 30 | 30 |
|
| 31 | 31 |
func TestStateRunStop(t *testing.T) {
|
| 32 | 32 |
s := NewState() |
| 33 |
- for i := 1; i < 3; i++ { // full lifecycle two times
|
|
| 33 |
+ |
|
| 34 |
+ // An initial wait (in "created" state) should block until the |
|
| 35 |
+ // container has started and exited. It shouldn't take more than 100 |
|
| 36 |
+ // milliseconds. |
|
| 37 |
+ ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) |
|
| 38 |
+ defer cancel() |
|
| 39 |
+ initialWait := s.Wait(ctx, false) |
|
| 40 |
+ |
|
| 41 |
+ // Begin another wait for the final removed state. It should complete |
|
| 42 |
+ // within 200 milliseconds. |
|
| 43 |
+ ctx, cancel = context.WithTimeout(context.Background(), 200*time.Millisecond) |
|
| 44 |
+ defer cancel() |
|
| 45 |
+ removalWait := s.Wait(ctx, true) |
|
| 46 |
+ |
|
| 47 |
+ // Full lifecycle two times. |
|
| 48 |
+ for i := 1; i <= 2; i++ {
|
|
| 49 |
+ // Set the state to "Running". |
|
| 34 | 50 |
s.Lock() |
| 35 |
- s.SetRunning(i+100, false) |
|
| 51 |
+ s.SetRunning(i, true) |
|
| 36 | 52 |
s.Unlock() |
| 37 | 53 |
|
| 54 |
+ // Assert desired state. |
|
| 38 | 55 |
if !s.IsRunning() {
|
| 39 | 56 |
t.Fatal("State not running")
|
| 40 | 57 |
} |
| 41 |
- if s.Pid != i+100 {
|
|
| 42 |
- t.Fatalf("Pid %v, expected %v", s.Pid, i+100)
|
|
| 58 |
+ if s.Pid != i {
|
|
| 59 |
+ t.Fatalf("Pid %v, expected %v", s.Pid, i)
|
|
| 43 | 60 |
} |
| 44 | 61 |
if s.ExitCode() != 0 {
|
| 45 | 62 |
t.Fatalf("ExitCode %v, expected 0", s.ExitCode())
|
| 46 | 63 |
} |
| 47 | 64 |
|
| 48 |
- stopped := make(chan struct{})
|
|
| 49 |
- var exit int64 |
|
| 50 |
- go func() {
|
|
| 51 |
- exitCode, _ := s.WaitStop(-1 * time.Second) |
|
| 52 |
- atomic.StoreInt64(&exit, int64(exitCode)) |
|
| 53 |
- close(stopped) |
|
| 54 |
- }() |
|
| 65 |
+ // Async wait up to 50 milliseconds for the exit status. |
|
| 66 |
+ ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) |
|
| 67 |
+ defer cancel() |
|
| 68 |
+ exitWait := s.Wait(ctx, false) |
|
| 69 |
+ |
|
| 70 |
+ // Set the state to "Exited". |
|
| 55 | 71 |
s.Lock() |
| 56 | 72 |
s.SetStopped(&ExitStatus{ExitCode: i})
|
| 57 | 73 |
s.Unlock() |
| 74 |
+ |
|
| 75 |
+ // Assert desired state. |
|
| 58 | 76 |
if s.IsRunning() {
|
| 59 | 77 |
t.Fatal("State is running")
|
| 60 | 78 |
} |
| ... | ... |
@@ -64,50 +82,86 @@ func TestStateRunStop(t *testing.T) {
|
| 64 | 64 |
if s.Pid != 0 {
|
| 65 | 65 |
t.Fatalf("Pid %v, expected 0", s.Pid)
|
| 66 | 66 |
} |
| 67 |
- select {
|
|
| 68 |
- case <-time.After(100 * time.Millisecond): |
|
| 69 |
- t.Fatal("Stop callback doesn't fire in 100 milliseconds")
|
|
| 70 |
- case <-stopped: |
|
| 71 |
- t.Log("Stop callback fired")
|
|
| 67 |
+ |
|
| 68 |
+ // Receive the exitWait result. |
|
| 69 |
+ status := <-exitWait |
|
| 70 |
+ if status.ExitCode() != i {
|
|
| 71 |
+ t.Fatalf("ExitCode %v, expected %v, err %q", status.ExitCode(), i, status.Err())
|
|
| 72 | 72 |
} |
| 73 |
- exitCode := int(atomic.LoadInt64(&exit)) |
|
| 74 |
- if exitCode != i {
|
|
| 75 |
- t.Fatalf("ExitCode %v, expected %v", exitCode, i)
|
|
| 73 |
+ |
|
| 74 |
+ // A repeated call to Wait() should not block at this point. |
|
| 75 |
+ ctx, cancel = context.WithTimeout(context.Background(), 10*time.Millisecond) |
|
| 76 |
+ defer cancel() |
|
| 77 |
+ exitWait = s.Wait(ctx, false) |
|
| 78 |
+ |
|
| 79 |
+ status = <-exitWait |
|
| 80 |
+ if status.ExitCode() != i {
|
|
| 81 |
+ t.Fatalf("ExitCode %v, expected %v, err %q", status.ExitCode(), i, status.Err())
|
|
| 76 | 82 |
} |
| 77 |
- if exitCode, err := s.WaitStop(-1 * time.Second); err != nil || exitCode != i {
|
|
| 78 |
- t.Fatalf("WaitStop returned exitCode: %v, err: %v, expected exitCode: %v, err: %v", exitCode, err, i, nil)
|
|
| 83 |
+ |
|
| 84 |
+ if i == 1 {
|
|
| 85 |
+ // Make sure our initial wait also succeeds. |
|
| 86 |
+ status = <-initialWait |
|
| 87 |
+ if status.ExitCode() != i {
|
|
| 88 |
+ // Should have the exit code from this first loop. |
|
| 89 |
+ t.Fatalf("Initial wait exitCode %v, expected %v, err %q", status.ExitCode(), i, status.Err())
|
|
| 90 |
+ } |
|
| 79 | 91 |
} |
| 80 | 92 |
} |
| 93 |
+ |
|
| 94 |
+ // Set the state to dead and removed. |
|
| 95 |
+ s.SetDead() |
|
| 96 |
+ s.SetRemoved() |
|
| 97 |
+ |
|
| 98 |
+ // Wait for removed status or timeout. |
|
| 99 |
+ status := <-removalWait |
|
| 100 |
+ if status.ExitCode() != 2 {
|
|
| 101 |
+ // Should have the final exit code from the loop. |
|
| 102 |
+ t.Fatalf("Removal wait exitCode %v, expected %v, err %q", status.ExitCode(), 2, status.Err())
|
|
| 103 |
+ } |
|
| 81 | 104 |
} |
| 82 | 105 |
|
| 83 | 106 |
func TestStateTimeoutWait(t *testing.T) {
|
| 84 | 107 |
s := NewState() |
| 85 |
- stopped := make(chan struct{})
|
|
| 86 |
- go func() {
|
|
| 87 |
- s.WaitStop(100 * time.Millisecond) |
|
| 88 |
- close(stopped) |
|
| 89 |
- }() |
|
| 108 |
+ |
|
| 109 |
+ // Start a wait with a timeout. |
|
| 110 |
+ ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) |
|
| 111 |
+ defer cancel() |
|
| 112 |
+ waitC := s.Wait(ctx, false) |
|
| 113 |
+ |
|
| 114 |
+ // It should timeout *before* this 200ms timer does. |
|
| 90 | 115 |
select {
|
| 91 | 116 |
case <-time.After(200 * time.Millisecond): |
| 92 | 117 |
t.Fatal("Stop callback doesn't fire in 200 milliseconds")
|
| 93 |
- case <-stopped: |
|
| 118 |
+ case status := <-waitC: |
|
| 94 | 119 |
t.Log("Stop callback fired")
|
| 120 |
+ // Should be a timeout error. |
|
| 121 |
+ if status.Err() == nil {
|
|
| 122 |
+ t.Fatal("expected timeout error, got nil")
|
|
| 123 |
+ } |
|
| 124 |
+ if status.ExitCode() != -1 {
|
|
| 125 |
+ t.Fatalf("expected exit code %v, got %v", -1, status.ExitCode())
|
|
| 126 |
+ } |
|
| 95 | 127 |
} |
| 96 | 128 |
|
| 97 | 129 |
s.Lock() |
| 98 |
- s.SetStopped(&ExitStatus{ExitCode: 1})
|
|
| 130 |
+ s.SetRunning(0, true) |
|
| 131 |
+ s.SetStopped(&ExitStatus{ExitCode: 0})
|
|
| 99 | 132 |
s.Unlock() |
| 100 | 133 |
|
| 101 |
- stopped = make(chan struct{})
|
|
| 102 |
- go func() {
|
|
| 103 |
- s.WaitStop(100 * time.Millisecond) |
|
| 104 |
- close(stopped) |
|
| 105 |
- }() |
|
| 134 |
+ // Start another wait with a timeout. This one should return |
|
| 135 |
+ // immediately. |
|
| 136 |
+ ctx, cancel = context.WithTimeout(context.Background(), 100*time.Millisecond) |
|
| 137 |
+ defer cancel() |
|
| 138 |
+ waitC = s.Wait(ctx, false) |
|
| 139 |
+ |
|
| 106 | 140 |
select {
|
| 107 | 141 |
case <-time.After(200 * time.Millisecond): |
| 108 | 142 |
t.Fatal("Stop callback doesn't fire in 200 milliseconds")
|
| 109 |
- case <-stopped: |
|
| 143 |
+ case status := <-waitC: |
|
| 110 | 144 |
t.Log("Stop callback fired")
|
| 145 |
+ if status.ExitCode() != 0 {
|
|
| 146 |
+ t.Fatalf("expected exit code %v, got %v, err %q", 0, status.ExitCode(), status.Err())
|
|
| 147 |
+ } |
|
| 111 | 148 |
} |
| 112 |
- |
|
| 113 | 149 |
} |
| ... | ... |
@@ -1,9 +1,9 @@ |
| 1 | 1 |
package daemon |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 |
+ "context" |
|
| 4 | 5 |
"fmt" |
| 5 | 6 |
"io" |
| 6 |
- "time" |
|
| 7 | 7 |
|
| 8 | 8 |
"github.com/Sirupsen/logrus" |
| 9 | 9 |
"github.com/docker/docker/api/errors" |
| ... | ... |
@@ -160,14 +160,11 @@ func (daemon *Daemon) containerAttach(c *container.Container, cfg *stream.Attach |
| 160 | 160 |
cfg.Stdin = nil |
| 161 | 161 |
} |
| 162 | 162 |
|
| 163 |
- waitChan := make(chan struct{})
|
|
| 164 | 163 |
if c.Config.StdinOnce && !c.Config.Tty {
|
| 164 |
+ // Wait for the container to stop before returning. |
|
| 165 |
+ waitChan := c.Wait(context.Background(), false) |
|
| 165 | 166 |
defer func() {
|
| 166 |
- <-waitChan |
|
| 167 |
- }() |
|
| 168 |
- go func() {
|
|
| 169 |
- c.WaitStop(-1 * time.Second) |
|
| 170 |
- close(waitChan) |
|
| 167 |
+ _ = <-waitChan // Ignore returned exit code. |
|
| 171 | 168 |
}() |
| 172 | 169 |
} |
| 173 | 170 |
|
| ... | ... |
@@ -13,6 +13,7 @@ import ( |
| 13 | 13 |
"github.com/docker/docker/api/types/filters" |
| 14 | 14 |
"github.com/docker/docker/api/types/network" |
| 15 | 15 |
swarmtypes "github.com/docker/docker/api/types/swarm" |
| 16 |
+ containerpkg "github.com/docker/docker/container" |
|
| 16 | 17 |
clustertypes "github.com/docker/docker/daemon/cluster/provider" |
| 17 | 18 |
"github.com/docker/docker/plugin" |
| 18 | 19 |
"github.com/docker/libnetwork" |
| ... | ... |
@@ -39,7 +40,7 @@ type Backend interface {
|
| 39 | 39 |
DeactivateContainerServiceBinding(containerName string) error |
| 40 | 40 |
UpdateContainerServiceConfig(containerName string, serviceConfig *clustertypes.ServiceConfig) error |
| 41 | 41 |
ContainerInspectCurrent(name string, size bool) (*types.ContainerJSON, error) |
| 42 |
- ContainerWaitWithContext(ctx context.Context, name string) error |
|
| 42 |
+ ContainerWait(ctx context.Context, name string, untilRemoved bool) (<-chan *containerpkg.StateStatus, error) |
|
| 43 | 43 |
ContainerRm(name string, config *types.ContainerRmConfig) error |
| 44 | 44 |
ContainerKill(name string, sig uint64) error |
| 45 | 45 |
SetContainerDependencyStore(name string, store exec.DependencyGetter) error |
| ... | ... |
@@ -17,6 +17,7 @@ import ( |
| 17 | 17 |
"github.com/docker/docker/api/types/backend" |
| 18 | 18 |
containertypes "github.com/docker/docker/api/types/container" |
| 19 | 19 |
"github.com/docker/docker/api/types/events" |
| 20 |
+ containerpkg "github.com/docker/docker/container" |
|
| 20 | 21 |
"github.com/docker/docker/daemon/cluster/convert" |
| 21 | 22 |
executorpkg "github.com/docker/docker/daemon/cluster/executor" |
| 22 | 23 |
"github.com/docker/libnetwork" |
| ... | ... |
@@ -337,8 +338,8 @@ func (c *containerAdapter) events(ctx context.Context) <-chan events.Message {
|
| 337 | 337 |
return eventsq |
| 338 | 338 |
} |
| 339 | 339 |
|
| 340 |
-func (c *containerAdapter) wait(ctx context.Context) error {
|
|
| 341 |
- return c.backend.ContainerWaitWithContext(ctx, c.container.nameOrID()) |
|
| 340 |
+func (c *containerAdapter) wait(ctx context.Context) (<-chan *containerpkg.StateStatus, error) {
|
|
| 341 |
+ return c.backend.ContainerWait(ctx, c.container.nameOrID(), false) |
|
| 342 | 342 |
} |
| 343 | 343 |
|
| 344 | 344 |
func (c *containerAdapter) shutdown(ctx context.Context) error {
|
| ... | ... |
@@ -279,25 +279,27 @@ func (r *controller) Wait(pctx context.Context) error {
|
| 279 | 279 |
} |
| 280 | 280 |
}() |
| 281 | 281 |
|
| 282 |
- err := r.adapter.wait(ctx) |
|
| 283 |
- if ctx.Err() != nil {
|
|
| 284 |
- return ctx.Err() |
|
| 282 |
+ waitC, err := r.adapter.wait(ctx) |
|
| 283 |
+ if err != nil {
|
|
| 284 |
+ return err |
|
| 285 | 285 |
} |
| 286 | 286 |
|
| 287 |
- if err != nil {
|
|
| 288 |
- ee := &exitError{}
|
|
| 289 |
- if ec, ok := err.(exec.ExitCoder); ok {
|
|
| 290 |
- ee.code = ec.ExitCode() |
|
| 287 |
+ if status := <-waitC; status.ExitCode() != 0 {
|
|
| 288 |
+ exitErr := &exitError{
|
|
| 289 |
+ code: status.ExitCode(), |
|
| 291 | 290 |
} |
| 291 |
+ |
|
| 292 |
+ // Set the cause if it is knowable. |
|
| 292 | 293 |
select {
|
| 293 | 294 |
case e := <-healthErr: |
| 294 |
- ee.cause = e |
|
| 295 |
+ exitErr.cause = e |
|
| 295 | 296 |
default: |
| 296 |
- if err.Error() != "" {
|
|
| 297 |
- ee.cause = err |
|
| 297 |
+ if status.Err() != nil {
|
|
| 298 |
+ exitErr.cause = status.Err() |
|
| 298 | 299 |
} |
| 299 | 300 |
} |
| 300 |
- return ee |
|
| 301 |
+ |
|
| 302 |
+ return exitErr |
|
| 301 | 303 |
} |
| 302 | 304 |
|
| 303 | 305 |
return nil |
| ... | ... |
@@ -3,6 +3,7 @@ |
| 3 | 3 |
package daemon |
| 4 | 4 |
|
| 5 | 5 |
import ( |
| 6 |
+ "context" |
|
| 6 | 7 |
"fmt" |
| 7 | 8 |
"io/ioutil" |
| 8 | 9 |
"os" |
| ... | ... |
@@ -291,7 +292,12 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) {
|
| 291 | 291 |
} |
| 292 | 292 |
|
| 293 | 293 |
func killProcessDirectly(container *container.Container) error {
|
| 294 |
- if _, err := container.WaitStop(10 * time.Second); err != nil {
|
|
| 294 |
+ ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) |
|
| 295 |
+ defer cancel() |
|
| 296 |
+ |
|
| 297 |
+ // Block until the container to stops or timeout. |
|
| 298 |
+ status := <-container.Wait(ctx, false) |
|
| 299 |
+ if status.Err() != nil {
|
|
| 295 | 300 |
// Ensure that we don't kill ourselves |
| 296 | 301 |
if pid := container.GetPID(); pid != 0 {
|
| 297 | 302 |
logrus.Infof("Container %s failed to exit within 10 seconds of kill - trying direct SIGKILL", stringid.TruncateID(container.ID))
|
| ... | ... |
@@ -6,6 +6,7 @@ |
| 6 | 6 |
package daemon |
| 7 | 7 |
|
| 8 | 8 |
import ( |
| 9 |
+ "context" |
|
| 9 | 10 |
"fmt" |
| 10 | 11 |
"io/ioutil" |
| 11 | 12 |
"net" |
| ... | ... |
@@ -773,7 +774,12 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error {
|
| 773 | 773 |
if err := daemon.containerUnpause(c); err != nil {
|
| 774 | 774 |
return fmt.Errorf("Failed to unpause container %s with error: %v", c.ID, err)
|
| 775 | 775 |
} |
| 776 |
- if _, err := c.WaitStop(time.Duration(stopTimeout) * time.Second); err != nil {
|
|
| 776 |
+ |
|
| 777 |
+ ctx, cancel := context.WithTimeout(context.Background(), time.Duration(stopTimeout)*time.Second) |
|
| 778 |
+ defer cancel() |
|
| 779 |
+ |
|
| 780 |
+ // Wait with timeout for container to exit. |
|
| 781 |
+ if status := <-c.Wait(ctx, false); status.Err() != nil {
|
|
| 777 | 782 |
logrus.Debugf("container %s failed to exit in %d second of SIGTERM, sending SIGKILL to force", c.ID, stopTimeout)
|
| 778 | 783 |
sig, ok := signal.SignalMap["KILL"] |
| 779 | 784 |
if !ok {
|
| ... | ... |
@@ -782,8 +788,10 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error {
|
| 782 | 782 |
if err := daemon.kill(c, int(sig)); err != nil {
|
| 783 | 783 |
logrus.Errorf("Failed to SIGKILL container %s", c.ID)
|
| 784 | 784 |
} |
| 785 |
- c.WaitStop(-1 * time.Second) |
|
| 786 |
- return err |
|
| 785 |
+ // Wait for exit again without a timeout. |
|
| 786 |
+ // Explicitly ignore the result. |
|
| 787 |
+ _ = <-c.Wait(context.Background(), false) |
|
| 788 |
+ return status.Err() |
|
| 787 | 789 |
} |
| 788 | 790 |
} |
| 789 | 791 |
// If container failed to exit in stopTimeout seconds of SIGTERM, then using the force |
| ... | ... |
@@ -791,7 +799,9 @@ func (daemon *Daemon) shutdownContainer(c *container.Container) error {
|
| 791 | 791 |
return fmt.Errorf("Failed to stop container %s with error: %v", c.ID, err)
|
| 792 | 792 |
} |
| 793 | 793 |
|
| 794 |
- c.WaitStop(-1 * time.Second) |
|
| 794 |
+ // Wait without timeout for the container to exit. |
|
| 795 |
+ // Ignore the result. |
|
| 796 |
+ _ = <-c.Wait(context.Background(), false) |
|
| 795 | 797 |
return nil |
| 796 | 798 |
} |
| 797 | 799 |
|
| ... | ... |
@@ -134,6 +134,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo |
| 134 | 134 |
if e := daemon.removeMountPoints(container, removeVolume); e != nil {
|
| 135 | 135 |
logrus.Error(e) |
| 136 | 136 |
} |
| 137 |
+ container.SetRemoved() |
|
| 137 | 138 |
stateCtr.del(container.ID) |
| 138 | 139 |
daemon.LogContainerEvent(container, "destroy") |
| 139 | 140 |
return nil |
| ... | ... |
@@ -1,6 +1,7 @@ |
| 1 | 1 |
package daemon |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 |
+ "context" |
|
| 4 | 5 |
"fmt" |
| 5 | 6 |
"runtime" |
| 6 | 7 |
"strings" |
| ... | ... |
@@ -131,7 +132,10 @@ func (daemon *Daemon) Kill(container *container.Container) error {
|
| 131 | 131 |
return nil |
| 132 | 132 |
} |
| 133 | 133 |
|
| 134 |
- if _, err2 := container.WaitStop(2 * time.Second); err2 != nil {
|
|
| 134 |
+ ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) |
|
| 135 |
+ defer cancel() |
|
| 136 |
+ |
|
| 137 |
+ if status := <-container.Wait(ctx, false); status.Err() != nil {
|
|
| 135 | 138 |
return err |
| 136 | 139 |
} |
| 137 | 140 |
} |
| ... | ... |
@@ -144,7 +148,10 @@ func (daemon *Daemon) Kill(container *container.Container) error {
|
| 144 | 144 |
return err |
| 145 | 145 |
} |
| 146 | 146 |
|
| 147 |
- container.WaitStop(-1 * time.Second) |
|
| 147 |
+ // Wait for exit with no timeout. |
|
| 148 |
+ // Ignore returned status. |
|
| 149 |
+ _ = <-container.Wait(context.Background(), false) |
|
| 150 |
+ |
|
| 148 | 151 |
return nil |
| 149 | 152 |
} |
| 150 | 153 |
|
| ... | ... |
@@ -1,6 +1,7 @@ |
| 1 | 1 |
package daemon |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 |
+ "context" |
|
| 4 | 5 |
"fmt" |
| 5 | 6 |
"net/http" |
| 6 | 7 |
"time" |
| ... | ... |
@@ -60,7 +61,10 @@ func (daemon *Daemon) containerStop(container *container.Container, seconds int) |
| 60 | 60 |
// So, instead we'll give it up to 2 more seconds to complete and if |
| 61 | 61 |
// by that time the container is still running, then the error |
| 62 | 62 |
// we got is probably valid and so we force kill it. |
| 63 |
- if _, err := container.WaitStop(2 * time.Second); err != nil {
|
|
| 63 |
+ ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) |
|
| 64 |
+ defer cancel() |
|
| 65 |
+ |
|
| 66 |
+ if status := <-container.Wait(ctx, false); status.Err() != nil {
|
|
| 64 | 67 |
logrus.Infof("Container failed to stop after sending signal %d to the process, force killing", stopSignal)
|
| 65 | 68 |
if err := daemon.killPossiblyDeadProcess(container, 9); err != nil {
|
| 66 | 69 |
return err |
| ... | ... |
@@ -69,11 +73,15 @@ func (daemon *Daemon) containerStop(container *container.Container, seconds int) |
| 69 | 69 |
} |
| 70 | 70 |
|
| 71 | 71 |
// 2. Wait for the process to exit on its own |
| 72 |
- if _, err := container.WaitStop(time.Duration(seconds) * time.Second); err != nil {
|
|
| 72 |
+ ctx, cancel := context.WithTimeout(context.Background(), time.Duration(seconds)*time.Second) |
|
| 73 |
+ defer cancel() |
|
| 74 |
+ |
|
| 75 |
+ if status := <-container.Wait(ctx, false); status.Err() != nil {
|
|
| 73 | 76 |
logrus.Infof("Container %v failed to exit within %d seconds of signal %d - using the force", container.ID, seconds, stopSignal)
|
| 74 | 77 |
// 3. If it doesn't, then send SIGKILL |
| 75 | 78 |
if err := daemon.Kill(container); err != nil {
|
| 76 |
- container.WaitStop(-1 * time.Second) |
|
| 79 |
+ // Wait without a timeout, ignore result. |
|
| 80 |
+ _ = <-container.Wait(context.Background(), false) |
|
| 77 | 81 |
logrus.Warn(err) // Don't return error because we only care that container is stopped, not what function stopped it |
| 78 | 82 |
} |
| 79 | 83 |
} |
| ... | ... |
@@ -1,32 +1,22 @@ |
| 1 | 1 |
package daemon |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 |
- "time" |
|
| 5 |
- |
|
| 4 |
+ "github.com/docker/docker/container" |
|
| 6 | 5 |
"golang.org/x/net/context" |
| 7 | 6 |
) |
| 8 | 7 |
|
| 9 |
-// ContainerWait stops processing until the given container is |
|
| 10 |
-// stopped. If the container is not found, an error is returned. On a |
|
| 11 |
-// successful stop, the exit code of the container is returned. On a |
|
| 12 |
-// timeout, an error is returned. If you want to wait forever, supply |
|
| 13 |
-// a negative duration for the timeout. |
|
| 14 |
-func (daemon *Daemon) ContainerWait(name string, timeout time.Duration) (int, error) {
|
|
| 15 |
- container, err := daemon.GetContainer(name) |
|
| 16 |
- if err != nil {
|
|
| 17 |
- return -1, err |
|
| 18 |
- } |
|
| 19 |
- |
|
| 20 |
- return container.WaitStop(timeout) |
|
| 21 |
-} |
|
| 22 |
- |
|
| 23 |
-// ContainerWaitWithContext returns a channel where exit code is sent |
|
| 24 |
-// when container stops. Channel can be cancelled with a context. |
|
| 25 |
-func (daemon *Daemon) ContainerWaitWithContext(ctx context.Context, name string) error {
|
|
| 8 |
+// ContainerWait stops processing until the given container is stopped or |
|
| 9 |
+// removed (if untilRemoved is true). If the container is not found, a nil |
|
| 10 |
+// channel and non-nil error is returned immediately. If the container is |
|
| 11 |
+// found, a status result will be sent on the returned channel once the wait |
|
| 12 |
+// condition is met or if an error occurs waiting for the container (such as a |
|
| 13 |
+// context timeout or cancellation). On a successful stop, the exit code of the |
|
| 14 |
+// container is returned in the status with a non-nil Err() value. |
|
| 15 |
+func (daemon *Daemon) ContainerWait(ctx context.Context, name string, untilRemoved bool) (<-chan *container.StateStatus, error) {
|
|
| 26 | 16 |
container, err := daemon.GetContainer(name) |
| 27 | 17 |
if err != nil {
|
| 28 |
- return err |
|
| 18 |
+ return nil, err |
|
| 29 | 19 |
} |
| 30 | 20 |
|
| 31 |
- return container.WaitWithContext(ctx) |
|
| 21 |
+ return container.Wait(ctx, untilRemoved), nil |
|
| 32 | 22 |
} |