diff --git a/api/server/router/build/backend.go b/api/server/router/build/backend.go index 839f316088..46b8a59ff0 100644 --- a/api/server/router/build/backend.go +++ b/api/server/router/build/backend.go @@ -16,5 +16,5 @@ type Backend interface { // by the caller. // // TODO: make this return a reference instead of string - Build(clientCtx context.Context, config *types.ImageBuildOptions, context builder.Context, stdout io.Writer, stderr io.Writer, out io.Writer, clientGone <-chan bool) (string, error) + Build(clientCtx context.Context, config *types.ImageBuildOptions, context builder.Context, stdout io.Writer, stderr io.Writer, out io.Writer) (string, error) } diff --git a/api/server/router/build/build_routes.go b/api/server/router/build/build_routes.go index a6b787d56a..b709fdc577 100644 --- a/api/server/router/build/build_routes.go +++ b/api/server/router/build/build_routes.go @@ -161,17 +161,12 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * return progress.NewProgressReader(in, progressOutput, r.ContentLength, "Downloading context", remoteURL) } - var ( - context builder.ModifiableContext - dockerfileName string - out io.Writer - ) - context, dockerfileName, err = builder.DetectContextFromRemoteURL(r.Body, remoteURL, createProgressReader) + buildContext, dockerfileName, err := builder.DetectContextFromRemoteURL(r.Body, remoteURL, createProgressReader) if err != nil { return errf(err) } defer func() { - if err := context.Close(); err != nil { + if err := buildContext.Close(); err != nil { logrus.Debugf("[BUILDER] failed to remove temporary context: %v", err) } }() @@ -181,7 +176,7 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * buildOptions.AuthConfigs = authConfigs - out = output + var out io.Writer = output if buildOptions.SuppressOutput { out = notVerboseBuffer } @@ -189,15 +184,24 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r * stdout := &streamformatter.StdoutFormatter{Writer: out, StreamFormatter: sf} stderr := &streamformatter.StderrFormatter{Writer: out, StreamFormatter: sf} - closeNotifier := make(<-chan bool) + finished := make(chan struct{}) + defer close(finished) if notifier, ok := w.(http.CloseNotifier); ok { - closeNotifier = notifier.CloseNotify() + notifyContext, cancel := context.WithCancel(ctx) + closeNotifier := notifier.CloseNotify() + go func() { + select { + case <-closeNotifier: + cancel() + case <-finished: + } + }() + ctx = notifyContext } imgID, err := br.backend.Build(ctx, buildOptions, - builder.DockerIgnoreContext{ModifiableContext: context}, - stdout, stderr, out, - closeNotifier) + builder.DockerIgnoreContext{ModifiableContext: buildContext}, + stdout, stderr, out) if err != nil { return errf(err) } diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go index 0f35f54c0e..7c8c1624e0 100644 --- a/builder/dockerfile/builder.go +++ b/builder/dockerfile/builder.go @@ -8,7 +8,6 @@ import ( "io/ioutil" "os" "strings" - "sync" "github.com/Sirupsen/logrus" "github.com/docker/docker/builder" @@ -56,6 +55,7 @@ type Builder struct { docker builder.Backend context builder.Context clientCtx context.Context + cancel context.CancelFunc dockerfile *parser.Node runConfig *container.Config // runconfig for cmd, run, entrypoint etc. @@ -67,8 +67,6 @@ type Builder struct { cmdSet bool disableCommit bool cacheBusted bool - cancelled chan struct{} - cancelOnce sync.Once allowedBuildArgs map[string]bool // list of build-time args that are allowed for expansion/substitution and passing to commands in 'run'. // TODO: remove once docker.Commit can receive a tag @@ -88,23 +86,24 @@ func NewBuildManager(b builder.Backend) (bm *BuildManager) { // NewBuilder creates a new Dockerfile builder from an optional dockerfile and a Config. // If dockerfile is nil, the Dockerfile specified by Config.DockerfileName, // will be read from the Context passed to Build(). -func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, backend builder.Backend, context builder.Context, dockerfile io.ReadCloser) (b *Builder, err error) { +func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, backend builder.Backend, buildContext builder.Context, dockerfile io.ReadCloser) (b *Builder, err error) { if config == nil { config = new(types.ImageBuildOptions) } if config.BuildArgs == nil { config.BuildArgs = make(map[string]string) } + ctx, cancel := context.WithCancel(clientCtx) b = &Builder{ - clientCtx: clientCtx, + clientCtx: ctx, + cancel: cancel, options: config, Stdout: os.Stdout, Stderr: os.Stderr, docker: backend, - context: context, + context: buildContext, runConfig: new(container.Config), tmpContainers: map[string]struct{}{}, - cancelled: make(chan struct{}), id: stringid.GenerateNonCryptoID(), allowedBuildArgs: make(map[string]bool), } @@ -161,12 +160,12 @@ func sanitizeRepoAndTags(names []string) ([]reference.Named, error) { } // Build creates a NewBuilder, which builds the image. -func (bm *BuildManager) Build(clientCtx context.Context, config *types.ImageBuildOptions, context builder.Context, stdout io.Writer, stderr io.Writer, out io.Writer, clientGone <-chan bool) (string, error) { +func (bm *BuildManager) Build(clientCtx context.Context, config *types.ImageBuildOptions, context builder.Context, stdout io.Writer, stderr io.Writer, out io.Writer) (string, error) { b, err := NewBuilder(clientCtx, config, bm.backend, context, nil) if err != nil { return "", err } - img, err := b.build(config, context, stdout, stderr, out, clientGone) + img, err := b.build(config, context, stdout, stderr, out) return img, err } @@ -184,7 +183,7 @@ func (bm *BuildManager) Build(clientCtx context.Context, config *types.ImageBuil // * Tag image, if applicable. // * Print a happy message and return the image ID. // -func (b *Builder) build(config *types.ImageBuildOptions, context builder.Context, stdout io.Writer, stderr io.Writer, out io.Writer, clientGone <-chan bool) (string, error) { +func (b *Builder) build(config *types.ImageBuildOptions, context builder.Context, stdout io.Writer, stderr io.Writer, out io.Writer) (string, error) { b.options = config b.context = context b.Stdout = stdout @@ -198,19 +197,6 @@ func (b *Builder) build(config *types.ImageBuildOptions, context builder.Context } } - finished := make(chan struct{}) - defer close(finished) - go func() { - select { - case <-finished: - case <-clientGone: - b.cancelOnce.Do(func() { - close(b.cancelled) - }) - } - - }() - repoAndTags, err := sanitizeRepoAndTags(config.Tags) if err != nil { return "", err @@ -223,7 +209,7 @@ func (b *Builder) build(config *types.ImageBuildOptions, context builder.Context b.addLabels() } select { - case <-b.cancelled: + case <-b.clientCtx.Done(): logrus.Debug("Builder: build cancelled!") fmt.Fprintf(b.Stdout, "Build cancelled") return "", fmt.Errorf("Build cancelled") @@ -271,9 +257,7 @@ func (b *Builder) build(config *types.ImageBuildOptions, context builder.Context // Cancel cancels an ongoing Dockerfile build. func (b *Builder) Cancel() { - b.cancelOnce.Do(func() { - close(b.cancelled) - }) + b.cancel() } // BuildFromConfig builds directly from `changes`, treating it as if it were the contents of a Dockerfile diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 5d9127331c..62752ae261 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -6,6 +6,7 @@ package dockerfile import ( "crypto/sha256" "encoding/hex" + "errors" "fmt" "io" "io/ioutil" @@ -16,6 +17,7 @@ import ( "runtime" "sort" "strings" + "sync" "time" "github.com/Sirupsen/logrus" @@ -553,6 +555,8 @@ func (b *Builder) create() (string, error) { return c.ID, nil } +var errCancelled = errors.New("build cancelled") + func (b *Builder) run(cID string) (err error) { errCh := make(chan error) go func() { @@ -560,14 +564,19 @@ func (b *Builder) run(cID string) (err error) { }() finished := make(chan struct{}) - defer close(finished) + var once sync.Once + finish := func() { close(finished) } + cancelErrCh := make(chan error, 1) + defer once.Do(finish) go func() { select { - case <-b.cancelled: + case <-b.clientCtx.Done(): logrus.Debugln("Build cancelled, killing and removing container:", cID) b.docker.ContainerKill(cID, 0) b.removeContainer(cID) + cancelErrCh <- errCancelled case <-finished: + cancelErrCh <- nil } }() @@ -587,8 +596,8 @@ func (b *Builder) run(cID string) (err error) { Code: ret, } } - - return nil + once.Do(finish) + return <-cancelErrCh } func (b *Builder) removeContainer(c string) error {