diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go index 95bbe0e533..106a7087cd 100644 --- a/api/server/router/container/container_routes.go +++ b/api/server/router/container/container_routes.go @@ -280,11 +280,12 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons // 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") + legacyBehaviorPre130 := versions.LessThan(version, "1.30") + legacyRemovalWaitPre134 := false // The wait condition defaults to "not-running". waitCondition := containerpkg.WaitConditionNotRunning - if !legacyBehavior { + if !legacyBehaviorPre130 { if err := httputils.ParseForm(r); err != nil { return err } @@ -293,6 +294,7 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons waitCondition = containerpkg.WaitConditionNextExit case container.WaitConditionRemoved: waitCondition = containerpkg.WaitConditionRemoved + legacyRemovalWaitPre134 = versions.LessThan(version, "1.34") } } @@ -306,7 +308,7 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons w.Header().Set("Content-Type", "application/json") - if !legacyBehavior { + if !legacyBehaviorPre130 { // Write response header immediately. w.WriteHeader(http.StatusOK) if flusher, ok := w.(http.Flusher); ok { @@ -317,8 +319,22 @@ func (s *containerRouter) postContainersWait(ctx context.Context, w http.Respons // Block on the result of the wait operation. status := <-waitC + // With API < 1.34, wait on WaitConditionRemoved did not return + // in case container removal failed. The only way to report an + // error back to the client is to not write anything (i.e. send + // an empty response which will be treated as an error). + if legacyRemovalWaitPre134 && status.Err() != nil { + return nil + } + + var waitError *container.ContainerWaitOKBodyError + if status.Err() != nil { + waitError = &container.ContainerWaitOKBodyError{Message: status.Err().Error()} + } + return json.NewEncoder(w).Encode(&container.ContainerWaitOKBody{ StatusCode: int64(status.ExitCode()), + Error: waitError, }) } diff --git a/api/swagger.yaml b/api/swagger.yaml index c3b9c29244..b0c0575fc0 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -5723,6 +5723,13 @@ paths: description: "Exit code of the container" type: "integer" x-nullable: false + Error: + description: "container waiting error, if any" + type: "object" + properties: + Message: + description: "Details of an error" + type: "string" 404: description: "no such container" schema: diff --git a/api/types/container/container_wait.go b/api/types/container/container_wait.go index 77ecdbaf7a..47fb17578a 100644 --- a/api/types/container/container_wait.go +++ b/api/types/container/container_wait.go @@ -7,10 +7,22 @@ package container // See hack/generate-swagger-api.sh // ---------------------------------------------------------------------------- +// ContainerWaitOKBodyError container waiting error, if any +// swagger:model ContainerWaitOKBodyError +type ContainerWaitOKBodyError struct { + + // Details of an error + Message string `json:"Message,omitempty"` +} + // ContainerWaitOKBody container wait o k body // swagger:model ContainerWaitOKBody type ContainerWaitOKBody struct { + // error + // Required: true + Error *ContainerWaitOKBodyError `json:"Error"` + // Exit code of the container // Required: true StatusCode int64 `json:"StatusCode"` diff --git a/container/state.go b/container/state.go index 1a4c45cbab..3af4015ed9 100644 --- a/container/state.go +++ b/container/state.go @@ -29,7 +29,7 @@ type State struct { Dead bool Pid int ExitCodeValue int `json:"ExitCode"` - ErrorMsg string `json:"Error"` // contains last known error when starting the container + ErrorMsg string `json:"Error"` // contains last known error during container start or remove StartedAt time.Time FinishedAt time.Time Health *Health @@ -326,7 +326,10 @@ func (s *State) SetRestarting(exitStatus *ExitStatus) { // know the error that occurred when container transits to another state // when inspecting it func (s *State) SetError(err error) { - s.ErrorMsg = err.Error() + s.ErrorMsg = "" + if err != nil { + s.ErrorMsg = err.Error() + } } // IsPaused returns whether the container is paused or not. @@ -392,8 +395,18 @@ func (s *State) IsDead() bool { // closes the internal waitRemove channel to unblock callers waiting for a // container to be removed. func (s *State) SetRemoved() { + s.SetRemovalError(nil) +} + +// SetRemovalError is to be called in case a container remove failed. +// It sets an error and closes the internal waitRemove channel to unblock +// callers waiting for the container to be removed. +func (s *State) SetRemovalError(err error) { + s.SetError(err) s.Lock() close(s.waitRemove) // Unblock those waiting on remove. + // Recreate the channel so next ContainerWait will work + s.waitRemove = make(chan struct{}) s.Unlock() } diff --git a/daemon/delete.go b/daemon/delete.go index 6db08f38cd..4d56d14529 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -120,12 +120,16 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo metadata, err := daemon.stores[container.OS].layerStore.ReleaseRWLayer(container.RWLayer) layer.LogReleaseMetadata(metadata) if err != nil && err != layer.ErrMountDoesNotExist && !os.IsNotExist(errors.Cause(err)) { - return errors.Wrapf(err, "driver %q failed to remove root filesystem for %s", daemon.GraphDriverName(container.OS), container.ID) + e := errors.Wrapf(err, "driver %q failed to remove root filesystem for %s", daemon.GraphDriverName(container.OS), container.ID) + container.SetRemovalError(e) + return e } } if err := system.EnsureRemoveAll(container.Root); err != nil { - return errors.Wrapf(err, "unable to remove filesystem for %s", container.ID) + e := errors.Wrapf(err, "unable to remove filesystem for %s", container.ID) + container.SetRemovalError(e) + return e } linkNames := daemon.linkIndex.delete(container) diff --git a/docs/api/version-history.md b/docs/api/version-history.md index 9ac31dcaaf..77b8545bcc 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -17,12 +17,19 @@ keywords: "API, Docker, rcli, REST, documentation" [Docker Engine API v1.34](https://docs.docker.com/engine/api/v1.34/) documentation +* `POST /containers/(name)/wait?condition=removed` now also also returns + in case of container removal failure. A pointer to a structure named + `Error` added to the response JSON in order to indicate a failure. + If `Error` is `null`, container removal has succeeded, otherwise + the test of an error message indicating why container removal has failed + is available from `Error.Message` field. + ## v1.33 API changes [Docker Engine API v1.33](https://docs.docker.com/engine/api/v1.33/) documentation * `GET /events` now supports filtering 4 more kinds of events: `config`, `node`, -`secret` and `service`. +`secret` and `service`. ## v1.32 API changes