Browse code

Merge pull request #8415 from LK4D4/use_logs_in_builder

Use logs instead of attach for builder

Jessie Frazelle authored on 2014/10/18 05:32:13
Showing 4 changed files
... ...
@@ -24,7 +24,6 @@ import (
24 24
 	"github.com/docker/docker/pkg/archive"
25 25
 	"github.com/docker/docker/pkg/log"
26 26
 	"github.com/docker/docker/pkg/parsers"
27
-	"github.com/docker/docker/pkg/promise"
28 27
 	"github.com/docker/docker/pkg/symlink"
29 28
 	"github.com/docker/docker/pkg/system"
30 29
 	"github.com/docker/docker/pkg/tarsum"
... ...
@@ -512,25 +511,19 @@ func (b *Builder) create() (*daemon.Container, error) {
512 512
 }
513 513
 
514 514
 func (b *Builder) run(c *daemon.Container) error {
515
-	var errCh chan error
516
-	if b.Verbose {
517
-		errCh = promise.Go(func() error {
518
-			// FIXME: call the 'attach' job so that daemon.Attach can be made private
519
-			//
520
-			// FIXME (LK4D4): Also, maybe makes sense to call "logs" job, it is like attach
521
-			// but without hijacking for stdin. Also, with attach there can be race
522
-			// condition because of some output already was printed before it.
523
-			return <-b.Daemon.Attach(&c.StreamConfig, c.Config.OpenStdin, c.Config.StdinOnce, c.Config.Tty, nil, nil, b.OutStream, b.ErrStream)
524
-		})
525
-	}
526
-
527 515
 	//start the container
528 516
 	if err := c.Start(); err != nil {
529 517
 		return err
530 518
 	}
531 519
 
532
-	if errCh != nil {
533
-		if err := <-errCh; err != nil {
520
+	if b.Verbose {
521
+		logsJob := b.Engine.Job("logs", c.ID)
522
+		logsJob.Setenv("follow", "1")
523
+		logsJob.Setenv("stdout", "1")
524
+		logsJob.Setenv("stderr", "1")
525
+		logsJob.Stdout.Add(b.OutStream)
526
+		logsJob.Stderr.Add(b.ErrStream)
527
+		if err := logsJob.Run(); err != nil {
534 528
 			return err
535 529
 		}
536 530
 	}
... ...
@@ -103,7 +103,7 @@ func (daemon *Daemon) ContainerAttach(job *engine.Job) engine.Status {
103 103
 			cStderr = job.Stderr
104 104
 		}
105 105
 
106
-		<-daemon.Attach(&container.StreamConfig, container.Config.OpenStdin, container.Config.StdinOnce, container.Config.Tty, cStdin, cStdinCloser, cStdout, cStderr)
106
+		<-daemon.attach(&container.StreamConfig, container.Config.OpenStdin, container.Config.StdinOnce, container.Config.Tty, cStdin, cStdinCloser, cStdout, cStderr)
107 107
 		// If we are in stdinonce mode, wait for the process to end
108 108
 		// otherwise, simply return
109 109
 		if container.Config.StdinOnce && !container.Config.Tty {
... ...
@@ -113,13 +113,7 @@ func (daemon *Daemon) ContainerAttach(job *engine.Job) engine.Status {
113 113
 	return engine.StatusOK
114 114
 }
115 115
 
116
-// FIXME: this should be private, and every outside subsystem
117
-// should go through the "container_attach" job. But that would require
118
-// that job to be properly documented, as well as the relationship between
119
-// Attach and ContainerAttach.
120
-//
121
-// This method is in use by builder/builder.go.
122
-func (daemon *Daemon) Attach(streamConfig *StreamConfig, openStdin, stdinOnce, tty bool, stdin io.ReadCloser, stdinCloser io.Closer, stdout io.Writer, stderr io.Writer) chan error {
116
+func (daemon *Daemon) attach(streamConfig *StreamConfig, openStdin, stdinOnce, tty bool, stdin io.ReadCloser, stdinCloser io.Closer, stdout io.Writer, stderr io.Writer) chan error {
123 117
 	var (
124 118
 		cStdout, cStderr io.ReadCloser
125 119
 		nJobs            int
... ...
@@ -204,7 +204,7 @@ func (d *Daemon) ContainerExecStart(job *engine.Job) engine.Status {
204 204
 		execConfig.StreamConfig.stdinPipe = ioutils.NopWriteCloser(ioutil.Discard) // Silently drop stdin
205 205
 	}
206 206
 
207
-	attachErr := d.Attach(&execConfig.StreamConfig, execConfig.OpenStdin, false, execConfig.ProcessConfig.Tty, cStdin, cStdinCloser, cStdout, cStderr)
207
+	attachErr := d.attach(&execConfig.StreamConfig, execConfig.OpenStdin, false, execConfig.ProcessConfig.Tty, cStdin, cStdinCloser, cStdout, cStderr)
208 208
 
209 209
 	execErr := make(chan error)
210 210
 
... ...
@@ -2833,3 +2833,22 @@ func TestBuildVerifySingleQuoteFails(t *testing.T) {
2833 2833
 
2834 2834
 	logDone("build - verify single quotes fail")
2835 2835
 }
2836
+
2837
+func TestBuildVerboseOut(t *testing.T) {
2838
+	name := "testbuildverboseout"
2839
+	defer deleteImages(name)
2840
+
2841
+	_, out, err := buildImageWithOut(name,
2842
+		`FROM busybox
2843
+RUN echo 123`,
2844
+		false)
2845
+
2846
+	if err != nil {
2847
+		t.Fatal(err)
2848
+	}
2849
+	if !strings.Contains(out, "\n123\n") {
2850
+		t.Fatalf("Output should contain %q: %q", "123", out)
2851
+	}
2852
+
2853
+	logDone("build - verbose output from commands")
2854
+}