diff --git a/daemon/errors.go b/daemon/errors.go index 7282e4f462..803c070f12 100644 --- a/daemon/errors.go +++ b/daemon/errors.go @@ -59,19 +59,6 @@ func (e nameConflictError) Error() string { func (nameConflictError) Conflict() {} -type containerNotModifiedError struct { - running bool -} - -func (e containerNotModifiedError) Error() string { - if e.running { - return "Container is already started" - } - return "Container is already stopped" -} - -func (e containerNotModifiedError) NotModified() {} - type invalidIdentifier string func (e invalidIdentifier) Error() string { diff --git a/daemon/start.go b/daemon/start.go index 87bb8fba55..110d9c222d 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -14,6 +14,30 @@ import ( "github.com/pkg/errors" ) +// validateState verifies if the container is in a non-conflicting state. +func validateState(ctr *container.Container) error { + ctr.Lock() + defer ctr.Unlock() + + // Intentionally checking paused first, because a container can be + // BOTH running AND paused. To start a paused (but running) container, + // it must be thawed ("un-paused"). + if ctr.Paused { + return errdefs.Conflict(errors.New("cannot start a paused container, try unpause instead")) + } else if ctr.Running { + // This is not an actual error, but produces a 304 "not modified" + // when returned through the API to indicates the container is + // already in the desired state. It's implemented as an error + // to make the code calling this function terminate early (as + // no further processing is needed). + return errdefs.NotModified(errors.New("container is already running")) + } + if ctr.RemovalInProgress || ctr.Dead { + return errdefs.Conflict(errors.New("container is marked for removal and cannot be started")) + } + return nil +} + // ContainerStart starts a container. func (daemon *Daemon) ContainerStart(ctx context.Context, name string, hostConfig *containertypes.HostConfig, checkpoint string, checkpointDir string) error { daemonCfg := daemon.config() @@ -25,26 +49,7 @@ func (daemon *Daemon) ContainerStart(ctx context.Context, name string, hostConfi if err != nil { return err } - - validateState := func() error { - ctr.Lock() - defer ctr.Unlock() - - if ctr.Paused { - return errdefs.Conflict(errors.New("cannot start a paused container, try unpause instead")) - } - - if ctr.Running { - return containerNotModifiedError{running: true} - } - - if ctr.RemovalInProgress || ctr.Dead { - return errdefs.Conflict(errors.New("container is marked for removal and cannot be started")) - } - return nil - } - - if err := validateState(); err != nil { + if err := validateState(ctr); err != nil { return err } diff --git a/daemon/stop.go b/daemon/stop.go index b5b1506d73..4d22a81945 100644 --- a/daemon/stop.go +++ b/daemon/stop.go @@ -26,7 +26,12 @@ func (daemon *Daemon) ContainerStop(ctx context.Context, name string, options co return err } if !ctr.IsRunning() { - return containerNotModifiedError{} + // This is not an actual error, but produces a 304 "not modified" + // when returned through the API to indicates the container is + // already in the desired state. It's implemented as an error + // to make the code calling this function terminate early (as + // no further processing is needed). + return errdefs.NotModified(errors.New("container is already stopped")) } err = daemon.containerStop(ctx, ctr, options) if err != nil {