From 1ccc013586be80ec5d1d30fef95ecd9873a92af7 Mon Sep 17 00:00:00 2001 From: Alexandr Morozov Date: Mon, 6 Oct 2014 09:41:22 -0700 Subject: [PATCH 1/2] Use logs instead of attach for builder Signed-off-by: Alexandr Morozov Upstream-commit: 6f09d064bd438ab4425d6105f40887f02bb9e97e Component: engine --- components/engine/builder/internals.go | 23 +++++++------------ .../integration-cli/docker_cli_build_test.go | 19 +++++++++++++++ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/components/engine/builder/internals.go b/components/engine/builder/internals.go index 5fd03f7745..20f3380fb8 100644 --- a/components/engine/builder/internals.go +++ b/components/engine/builder/internals.go @@ -24,7 +24,6 @@ import ( "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/log" "github.com/docker/docker/pkg/parsers" - "github.com/docker/docker/pkg/promise" "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/system" "github.com/docker/docker/pkg/tarsum" @@ -512,25 +511,19 @@ func (b *Builder) create() (*daemon.Container, error) { } func (b *Builder) run(c *daemon.Container) error { - var errCh chan error - if b.Verbose { - errCh = promise.Go(func() error { - // FIXME: call the 'attach' job so that daemon.Attach can be made private - // - // FIXME (LK4D4): Also, maybe makes sense to call "logs" job, it is like attach - // but without hijacking for stdin. Also, with attach there can be race - // condition because of some output already was printed before it. - return <-b.Daemon.Attach(&c.StreamConfig, c.Config.OpenStdin, c.Config.StdinOnce, c.Config.Tty, nil, nil, b.OutStream, b.ErrStream) - }) - } - //start the container if err := c.Start(); err != nil { return err } - if errCh != nil { - if err := <-errCh; err != nil { + if b.Verbose { + logsJob := b.Engine.Job("logs", c.ID) + logsJob.Setenv("follow", "1") + logsJob.Setenv("stdout", "1") + logsJob.Setenv("stderr", "1") + logsJob.Stdout.Add(b.OutStream) + logsJob.Stderr.Add(b.ErrStream) + if err := logsJob.Run(); err != nil { return err } } diff --git a/components/engine/integration-cli/docker_cli_build_test.go b/components/engine/integration-cli/docker_cli_build_test.go index 76105fbf6a..c9b9db6b3c 100644 --- a/components/engine/integration-cli/docker_cli_build_test.go +++ b/components/engine/integration-cli/docker_cli_build_test.go @@ -2752,3 +2752,22 @@ func TestBuildVerifySingleQuoteFails(t *testing.T) { logDone("build - verify single quotes fail") } + +func TestBuildVerboseOut(t *testing.T) { + name := "testbuildverboseout" + defer deleteImages(name) + + _, out, err := buildImageWithOut(name, + `FROM busybox +RUN echo 123`, + false) + + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "\n123\n") { + t.Fatalf("Output should contain %q: %q", "123", out) + } + + logDone("build - verbose output from commands") +} From 9bac41ed108e48d4710520d80ce5b4c56c189503 Mon Sep 17 00:00:00 2001 From: Alexandr Morozov Date: Mon, 6 Oct 2014 11:57:18 -0700 Subject: [PATCH 2/2] Make daemon.Attach private Signed-off-by: Alexandr Morozov Upstream-commit: 2db1caee4f23e81107b2647c06b4c677f6ecd7a1 Component: engine --- components/engine/daemon/attach.go | 10 ++-------- components/engine/daemon/exec.go | 2 +- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/components/engine/daemon/attach.go b/components/engine/daemon/attach.go index 7ccaadf442..e115dac2e0 100644 --- a/components/engine/daemon/attach.go +++ b/components/engine/daemon/attach.go @@ -103,7 +103,7 @@ func (daemon *Daemon) ContainerAttach(job *engine.Job) engine.Status { cStderr = job.Stderr } - <-daemon.Attach(&container.StreamConfig, container.Config.OpenStdin, container.Config.StdinOnce, container.Config.Tty, cStdin, cStdinCloser, cStdout, cStderr) + <-daemon.attach(&container.StreamConfig, container.Config.OpenStdin, container.Config.StdinOnce, container.Config.Tty, cStdin, cStdinCloser, cStdout, cStderr) // If we are in stdinonce mode, wait for the process to end // otherwise, simply return if container.Config.StdinOnce && !container.Config.Tty { @@ -113,13 +113,7 @@ func (daemon *Daemon) ContainerAttach(job *engine.Job) engine.Status { return engine.StatusOK } -// FIXME: this should be private, and every outside subsystem -// should go through the "container_attach" job. But that would require -// that job to be properly documented, as well as the relationship between -// Attach and ContainerAttach. -// -// This method is in use by builder/builder.go. -func (daemon *Daemon) Attach(streamConfig *StreamConfig, openStdin, stdinOnce, tty bool, stdin io.ReadCloser, stdinCloser io.Closer, stdout io.Writer, stderr io.Writer) chan error { +func (daemon *Daemon) attach(streamConfig *StreamConfig, openStdin, stdinOnce, tty bool, stdin io.ReadCloser, stdinCloser io.Closer, stdout io.Writer, stderr io.Writer) chan error { var ( cStdout, cStderr io.ReadCloser nJobs int diff --git a/components/engine/daemon/exec.go b/components/engine/daemon/exec.go index 0ab1c0bf5f..a6113b0fca 100644 --- a/components/engine/daemon/exec.go +++ b/components/engine/daemon/exec.go @@ -204,7 +204,7 @@ func (d *Daemon) ContainerExecStart(job *engine.Job) engine.Status { execConfig.StreamConfig.stdinPipe = ioutils.NopWriteCloser(ioutil.Discard) // Silently drop stdin } - attachErr := d.Attach(&execConfig.StreamConfig, execConfig.OpenStdin, false, execConfig.ProcessConfig.Tty, cStdin, cStdinCloser, cStdout, cStderr) + attachErr := d.attach(&execConfig.StreamConfig, execConfig.OpenStdin, false, execConfig.ProcessConfig.Tty, cStdin, cStdinCloser, cStdout, cStderr) execErr := make(chan error)