Browse code

builder: replace cancelled channel with net/context

Also stop execution of run immediately if request was cancelled.

Signed-off-by: Alexander Morozov <lk4d4@docker.com>

Alexander Morozov authored on 2016/03/23 02:49:03
Showing 4 changed files
... ...
@@ -16,5 +16,5 @@ type Backend interface {
16 16
 	// by the caller.
17 17
 	//
18 18
 	// TODO: make this return a reference instead of string
19
-	Build(clientCtx context.Context, config *types.ImageBuildOptions, context builder.Context, stdout io.Writer, stderr io.Writer, out io.Writer, clientGone <-chan bool) (string, error)
19
+	Build(clientCtx context.Context, config *types.ImageBuildOptions, context builder.Context, stdout io.Writer, stderr io.Writer, out io.Writer) (string, error)
20 20
 }
... ...
@@ -161,17 +161,12 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r *
161 161
 		return progress.NewProgressReader(in, progressOutput, r.ContentLength, "Downloading context", remoteURL)
162 162
 	}
163 163
 
164
-	var (
165
-		context        builder.ModifiableContext
166
-		dockerfileName string
167
-		out            io.Writer
168
-	)
169
-	context, dockerfileName, err = builder.DetectContextFromRemoteURL(r.Body, remoteURL, createProgressReader)
164
+	buildContext, dockerfileName, err := builder.DetectContextFromRemoteURL(r.Body, remoteURL, createProgressReader)
170 165
 	if err != nil {
171 166
 		return errf(err)
172 167
 	}
173 168
 	defer func() {
174
-		if err := context.Close(); err != nil {
169
+		if err := buildContext.Close(); err != nil {
175 170
 			logrus.Debugf("[BUILDER] failed to remove temporary context: %v", err)
176 171
 		}
177 172
 	}()
... ...
@@ -181,7 +176,7 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r *
181 181
 
182 182
 	buildOptions.AuthConfigs = authConfigs
183 183
 
184
-	out = output
184
+	var out io.Writer = output
185 185
 	if buildOptions.SuppressOutput {
186 186
 		out = notVerboseBuffer
187 187
 	}
... ...
@@ -189,15 +184,24 @@ func (br *buildRouter) postBuild(ctx context.Context, w http.ResponseWriter, r *
189 189
 	stdout := &streamformatter.StdoutFormatter{Writer: out, StreamFormatter: sf}
190 190
 	stderr := &streamformatter.StderrFormatter{Writer: out, StreamFormatter: sf}
191 191
 
192
-	closeNotifier := make(<-chan bool)
192
+	finished := make(chan struct{})
193
+	defer close(finished)
193 194
 	if notifier, ok := w.(http.CloseNotifier); ok {
194
-		closeNotifier = notifier.CloseNotify()
195
+		notifyContext, cancel := context.WithCancel(ctx)
196
+		closeNotifier := notifier.CloseNotify()
197
+		go func() {
198
+			select {
199
+			case <-closeNotifier:
200
+				cancel()
201
+			case <-finished:
202
+			}
203
+		}()
204
+		ctx = notifyContext
195 205
 	}
196 206
 
197 207
 	imgID, err := br.backend.Build(ctx, buildOptions,
198
-		builder.DockerIgnoreContext{ModifiableContext: context},
199
-		stdout, stderr, out,
200
-		closeNotifier)
208
+		builder.DockerIgnoreContext{ModifiableContext: buildContext},
209
+		stdout, stderr, out)
201 210
 	if err != nil {
202 211
 		return errf(err)
203 212
 	}
... ...
@@ -8,7 +8,6 @@ import (
8 8
 	"io/ioutil"
9 9
 	"os"
10 10
 	"strings"
11
-	"sync"
12 11
 
13 12
 	"github.com/Sirupsen/logrus"
14 13
 	"github.com/docker/docker/builder"
... ...
@@ -56,6 +55,7 @@ type Builder struct {
56 56
 	docker    builder.Backend
57 57
 	context   builder.Context
58 58
 	clientCtx context.Context
59
+	cancel    context.CancelFunc
59 60
 
60 61
 	dockerfile       *parser.Node
61 62
 	runConfig        *container.Config // runconfig for cmd, run, entrypoint etc.
... ...
@@ -67,8 +67,6 @@ type Builder struct {
67 67
 	cmdSet           bool
68 68
 	disableCommit    bool
69 69
 	cacheBusted      bool
70
-	cancelled        chan struct{}
71
-	cancelOnce       sync.Once
72 70
 	allowedBuildArgs map[string]bool // list of build-time args that are allowed for expansion/substitution and passing to commands in 'run'.
73 71
 
74 72
 	// TODO: remove once docker.Commit can receive a tag
... ...
@@ -88,23 +86,24 @@ func NewBuildManager(b builder.Backend) (bm *BuildManager) {
88 88
 // NewBuilder creates a new Dockerfile builder from an optional dockerfile and a Config.
89 89
 // If dockerfile is nil, the Dockerfile specified by Config.DockerfileName,
90 90
 // will be read from the Context passed to Build().
91
-func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, backend builder.Backend, context builder.Context, dockerfile io.ReadCloser) (b *Builder, err error) {
91
+func NewBuilder(clientCtx context.Context, config *types.ImageBuildOptions, backend builder.Backend, buildContext builder.Context, dockerfile io.ReadCloser) (b *Builder, err error) {
92 92
 	if config == nil {
93 93
 		config = new(types.ImageBuildOptions)
94 94
 	}
95 95
 	if config.BuildArgs == nil {
96 96
 		config.BuildArgs = make(map[string]string)
97 97
 	}
98
+	ctx, cancel := context.WithCancel(clientCtx)
98 99
 	b = &Builder{
99
-		clientCtx:        clientCtx,
100
+		clientCtx:        ctx,
101
+		cancel:           cancel,
100 102
 		options:          config,
101 103
 		Stdout:           os.Stdout,
102 104
 		Stderr:           os.Stderr,
103 105
 		docker:           backend,
104
-		context:          context,
106
+		context:          buildContext,
105 107
 		runConfig:        new(container.Config),
106 108
 		tmpContainers:    map[string]struct{}{},
107
-		cancelled:        make(chan struct{}),
108 109
 		id:               stringid.GenerateNonCryptoID(),
109 110
 		allowedBuildArgs: make(map[string]bool),
110 111
 	}
... ...
@@ -161,12 +160,12 @@ func sanitizeRepoAndTags(names []string) ([]reference.Named, error) {
161 161
 }
162 162
 
163 163
 // Build creates a NewBuilder, which builds the image.
164
-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) {
164
+func (bm *BuildManager) Build(clientCtx context.Context, config *types.ImageBuildOptions, context builder.Context, stdout io.Writer, stderr io.Writer, out io.Writer) (string, error) {
165 165
 	b, err := NewBuilder(clientCtx, config, bm.backend, context, nil)
166 166
 	if err != nil {
167 167
 		return "", err
168 168
 	}
169
-	img, err := b.build(config, context, stdout, stderr, out, clientGone)
169
+	img, err := b.build(config, context, stdout, stderr, out)
170 170
 	return img, err
171 171
 
172 172
 }
... ...
@@ -184,7 +183,7 @@ func (bm *BuildManager) Build(clientCtx context.Context, config *types.ImageBuil
184 184
 // * Tag image, if applicable.
185 185
 // * Print a happy message and return the image ID.
186 186
 //
187
-func (b *Builder) build(config *types.ImageBuildOptions, context builder.Context, stdout io.Writer, stderr io.Writer, out io.Writer, clientGone <-chan bool) (string, error) {
187
+func (b *Builder) build(config *types.ImageBuildOptions, context builder.Context, stdout io.Writer, stderr io.Writer, out io.Writer) (string, error) {
188 188
 	b.options = config
189 189
 	b.context = context
190 190
 	b.Stdout = stdout
... ...
@@ -198,19 +197,6 @@ func (b *Builder) build(config *types.ImageBuildOptions, context builder.Context
198 198
 		}
199 199
 	}
200 200
 
201
-	finished := make(chan struct{})
202
-	defer close(finished)
203
-	go func() {
204
-		select {
205
-		case <-finished:
206
-		case <-clientGone:
207
-			b.cancelOnce.Do(func() {
208
-				close(b.cancelled)
209
-			})
210
-		}
211
-
212
-	}()
213
-
214 201
 	repoAndTags, err := sanitizeRepoAndTags(config.Tags)
215 202
 	if err != nil {
216 203
 		return "", err
... ...
@@ -223,7 +209,7 @@ func (b *Builder) build(config *types.ImageBuildOptions, context builder.Context
223 223
 			b.addLabels()
224 224
 		}
225 225
 		select {
226
-		case <-b.cancelled:
226
+		case <-b.clientCtx.Done():
227 227
 			logrus.Debug("Builder: build cancelled!")
228 228
 			fmt.Fprintf(b.Stdout, "Build cancelled")
229 229
 			return "", fmt.Errorf("Build cancelled")
... ...
@@ -271,9 +257,7 @@ func (b *Builder) build(config *types.ImageBuildOptions, context builder.Context
271 271
 
272 272
 // Cancel cancels an ongoing Dockerfile build.
273 273
 func (b *Builder) Cancel() {
274
-	b.cancelOnce.Do(func() {
275
-		close(b.cancelled)
276
-	})
274
+	b.cancel()
277 275
 }
278 276
 
279 277
 // BuildFromConfig builds directly from `changes`, treating it as if it were the contents of a Dockerfile
... ...
@@ -6,6 +6,7 @@ package dockerfile
6 6
 import (
7 7
 	"crypto/sha256"
8 8
 	"encoding/hex"
9
+	"errors"
9 10
 	"fmt"
10 11
 	"io"
11 12
 	"io/ioutil"
... ...
@@ -16,6 +17,7 @@ import (
16 16
 	"runtime"
17 17
 	"sort"
18 18
 	"strings"
19
+	"sync"
19 20
 	"time"
20 21
 
21 22
 	"github.com/Sirupsen/logrus"
... ...
@@ -553,6 +555,8 @@ func (b *Builder) create() (string, error) {
553 553
 	return c.ID, nil
554 554
 }
555 555
 
556
+var errCancelled = errors.New("build cancelled")
557
+
556 558
 func (b *Builder) run(cID string) (err error) {
557 559
 	errCh := make(chan error)
558 560
 	go func() {
... ...
@@ -560,14 +564,19 @@ func (b *Builder) run(cID string) (err error) {
560 560
 	}()
561 561
 
562 562
 	finished := make(chan struct{})
563
-	defer close(finished)
563
+	var once sync.Once
564
+	finish := func() { close(finished) }
565
+	cancelErrCh := make(chan error, 1)
566
+	defer once.Do(finish)
564 567
 	go func() {
565 568
 		select {
566
-		case <-b.cancelled:
569
+		case <-b.clientCtx.Done():
567 570
 			logrus.Debugln("Build cancelled, killing and removing container:", cID)
568 571
 			b.docker.ContainerKill(cID, 0)
569 572
 			b.removeContainer(cID)
573
+			cancelErrCh <- errCancelled
570 574
 		case <-finished:
575
+			cancelErrCh <- nil
571 576
 		}
572 577
 	}()
573 578
 
... ...
@@ -587,8 +596,8 @@ func (b *Builder) run(cID string) (err error) {
587 587
 			Code:    ret,
588 588
 		}
589 589
 	}
590
-
591
-	return nil
590
+	once.Do(finish)
591
+	return <-cancelErrCh
592 592
 }
593 593
 
594 594
 func (b *Builder) removeContainer(c string) error {