Browse code

Fix premature close of build output on pull

The build job will sometimes trigger a pull job when the base image
does not exist. Now that engine jobs properly close their output by default
the pull job would also close the build job's stdout in a cascading close
upon completion of the pull.

This patch corrects this by wrapping the `pull` job's stdout with a
nopCloseWriter which will not close the stdout of the `build` job.

Docker-DCO-1.1-Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)

Josh Hawn authored on 2015/01/27 13:56:34
Showing 2 changed files
... ...
@@ -25,6 +25,7 @@ import (
25 25
 	imagepkg "github.com/docker/docker/image"
26 26
 	"github.com/docker/docker/pkg/archive"
27 27
 	"github.com/docker/docker/pkg/chrootarchive"
28
+	"github.com/docker/docker/pkg/ioutils"
28 29
 	"github.com/docker/docker/pkg/parsers"
29 30
 	"github.com/docker/docker/pkg/symlink"
30 31
 	"github.com/docker/docker/pkg/system"
... ...
@@ -433,7 +434,7 @@ func (b *Builder) pullImage(name string) (*imagepkg.Image, error) {
433 433
 	job.SetenvBool("json", b.StreamFormatter.Json())
434 434
 	job.SetenvBool("parallel", true)
435 435
 	job.SetenvJson("authConfig", pullRegistryAuth)
436
-	job.Stdout.Add(b.OutOld)
436
+	job.Stdout.Add(ioutils.NopWriteCloser(b.OutOld))
437 437
 	if err := job.Run(); err != nil {
438 438
 		return nil, err
439 439
 	}
... ...
@@ -4,6 +4,8 @@ import (
4 4
 	"bytes"
5 5
 	"strings"
6 6
 	"testing"
7
+
8
+	"github.com/docker/docker/pkg/ioutils"
7 9
 )
8 10
 
9 11
 func TestRegister(t *testing.T) {
... ...
@@ -150,3 +152,85 @@ func TestCatchallEmptyName(t *testing.T) {
150 150
 		t.Fatalf("Engine.Job(\"\").Run() should return an error")
151 151
 	}
152 152
 }
153
+
154
+// Ensure that a job within a job both using the same underlying standard
155
+// output writer does not close the output of the outer job when the inner
156
+// job's stdout is wrapped with a NopCloser. When not wrapped, it should
157
+// close the outer job's output.
158
+func TestNestedJobSharedOutput(t *testing.T) {
159
+	var (
160
+		outerHandler Handler
161
+		innerHandler Handler
162
+		wrapOutput   bool
163
+	)
164
+
165
+	outerHandler = func(job *Job) Status {
166
+		job.Stdout.Write([]byte("outer1"))
167
+
168
+		innerJob := job.Eng.Job("innerJob")
169
+
170
+		if wrapOutput {
171
+			innerJob.Stdout.Add(ioutils.NopWriteCloser(job.Stdout))
172
+		} else {
173
+			innerJob.Stdout.Add(job.Stdout)
174
+		}
175
+
176
+		if err := innerJob.Run(); err != nil {
177
+			t.Fatal(err)
178
+		}
179
+
180
+		// If wrapOutput was *false* this write will do nothing.
181
+		// FIXME (jlhawn): It should cause an error to write to
182
+		// closed output.
183
+		job.Stdout.Write([]byte(" outer2"))
184
+
185
+		return StatusOK
186
+	}
187
+
188
+	innerHandler = func(job *Job) Status {
189
+		job.Stdout.Write([]byte(" inner"))
190
+
191
+		return StatusOK
192
+	}
193
+
194
+	eng := New()
195
+	eng.Register("outerJob", outerHandler)
196
+	eng.Register("innerJob", innerHandler)
197
+
198
+	// wrapOutput starts *false* so the expected
199
+	// output of running the outer job will be:
200
+	//
201
+	//     "outer1 inner"
202
+	//
203
+	outBuf := new(bytes.Buffer)
204
+	outerJob := eng.Job("outerJob")
205
+	outerJob.Stdout.Add(outBuf)
206
+
207
+	if err := outerJob.Run(); err != nil {
208
+		t.Fatal(err)
209
+	}
210
+
211
+	expectedOutput := "outer1 inner"
212
+	if outBuf.String() != expectedOutput {
213
+		t.Fatalf("expected job output to be %q, got %q", expectedOutput, outBuf.String())
214
+	}
215
+
216
+	// Set wrapOutput to true so that the expected
217
+	// output of running the outer job will be:
218
+	//
219
+	//     "outer1 inner outer2"
220
+	//
221
+	wrapOutput = true
222
+	outBuf.Reset()
223
+	outerJob = eng.Job("outerJob")
224
+	outerJob.Stdout.Add(outBuf)
225
+
226
+	if err := outerJob.Run(); err != nil {
227
+		t.Fatal(err)
228
+	}
229
+
230
+	expectedOutput = "outer1 inner outer2"
231
+	if outBuf.String() != expectedOutput {
232
+		t.Fatalf("expected job output to be %q, got %q", expectedOutput, outBuf.String())
233
+	}
234
+}