diff --git a/daemon/cluster/executor/container/controller.go b/daemon/cluster/executor/container/controller.go index 50de5ddad5..61d6a34fd9 100644 --- a/daemon/cluster/executor/container/controller.go +++ b/daemon/cluster/executor/container/controller.go @@ -23,6 +23,10 @@ type controller struct { adapter *containerAdapter closed chan struct{} err error + + pulled chan struct{} // closed after pull + cancelPull func() // cancels pull context if not nil + pullErr error // pull error, only read after pulled closed } var _ exec.Controller = &controller{} @@ -84,24 +88,40 @@ func (r *controller) Prepare(ctx context.Context) error { return err } - if err := r.adapter.pullImage(ctx); err != nil { - cause := errors.Cause(err) - if cause == context.Canceled || cause == context.DeadlineExceeded { - return err - } + if r.pulled == nil { + // Fork the pull to a different context to allow pull to continue + // on re-entrant calls to Prepare. This ensures that Prepare can be + // idempotent and not incur the extra cost of pulling when + // cancelled on updates. + var pctx context.Context - // NOTE(stevvooe): We always try to pull the image to make sure we have - // the most up to date version. This will return an error, but we only - // log it. If the image truly doesn't exist, the create below will - // error out. - // - // This gives us some nice behavior where we use up to date versions of - // mutable tags, but will still run if the old image is available but a - // registry is down. - // - // If you don't want this behavior, lock down your image to an - // immutable tag or digest. - log.G(ctx).WithError(err).Error("pulling image failed") + r.pulled = make(chan struct{}) + pctx, r.cancelPull = context.WithCancel(context.Background()) // TODO(stevvooe): Bind a context to the entire controller. + + go func() { + defer close(r.pulled) + r.pullErr = r.adapter.pullImage(pctx) // protected by closing r.pulled + }() + } + + select { + case <-ctx.Done(): + return ctx.Err() + case <-r.pulled: + if r.pullErr != nil { + // NOTE(stevvooe): We always try to pull the image to make sure we have + // the most up to date version. This will return an error, but we only + // log it. If the image truly doesn't exist, the create below will + // error out. + // + // This gives us some nice behavior where we use up to date versions of + // mutable tags, but will still run if the old image is available but a + // registry is down. + // + // If you don't want this behavior, lock down your image to an + // immutable tag or digest. + log.G(ctx).WithError(r.pullErr).Error("pulling image failed") + } } if err := r.adapter.create(ctx, r.backend); err != nil { @@ -249,6 +269,10 @@ func (r *controller) Shutdown(ctx context.Context) error { return err } + if r.cancelPull != nil { + r.cancelPull() + } + if err := r.adapter.shutdown(ctx); err != nil { if isUnknownContainer(err) || isStoppedContainer(err) { return nil @@ -266,6 +290,10 @@ func (r *controller) Terminate(ctx context.Context) error { return err } + if r.cancelPull != nil { + r.cancelPull() + } + if err := r.adapter.terminate(ctx); err != nil { if isUnknownContainer(err) { return nil @@ -283,6 +311,10 @@ func (r *controller) Remove(ctx context.Context) error { return err } + if r.cancelPull != nil { + r.cancelPull() + } + // It may be necessary to shut down the task before removing it. if err := r.Shutdown(ctx); err != nil { if isUnknownContainer(err) { @@ -317,6 +349,10 @@ func (r *controller) Close() error { case <-r.closed: return r.err default: + if r.cancelPull != nil { + r.cancelPull() + } + r.err = exec.ErrControllerClosed close(r.closed) }