From 165ec694ab9743ab758fa4a7e241d377ecab541a Mon Sep 17 00:00:00 2001 From: "Billie H. Cleek" Date: Wed, 27 Apr 2016 13:54:40 -0700 Subject: [PATCH] fix docker builder with ansible provisioner Refactor the docker builder so that it does not expect output from the container to be strictly line oriented or even text, because SFTP, used by Ansible, is a binary protocol. Since `docker exec` was introduced in 2014, remove support for older versions of docker that required using `docker attach`. The old notes in the docker builder referring to specific issues have all been resolved or else closed because they could not be reproduced. --- builder/docker/communicator.go | 195 ++++++++------------------------- 1 file changed, 47 insertions(+), 148 deletions(-) diff --git a/builder/docker/communicator.go b/builder/docker/communicator.go index c10327344..012a9cea8 100644 --- a/builder/docker/communicator.go +++ b/builder/docker/communicator.go @@ -2,7 +2,6 @@ package docker import ( "archive/tar" - "bytes" "fmt" "io" "io/ioutil" @@ -10,12 +9,10 @@ import ( "os" "os/exec" "path/filepath" - "strconv" + "strings" "sync" "syscall" - "time" - "github.com/ActiveState/tail" "github.com/hashicorp/go-version" "github.com/mitchellh/packer/packer" ) @@ -30,42 +27,35 @@ type Communicator struct { } func (c *Communicator) Start(remote *packer.RemoteCmd) error { - // Create a temporary file to store the output. Because of a bug in - // Docker, sometimes all the output doesn't properly show up. This - // file will capture ALL of the output, and we'll read that. - // - // https://github.com/dotcloud/docker/issues/2625 - outputFile, err := ioutil.TempFile(c.HostDir, "cmd") + var cmd *exec.Cmd + if c.Config.Pty { + cmd = exec.Command("docker", "exec", "-i", "-t", c.ContainerId, "/bin/sh", "-c", fmt.Sprintf("(%s)", remote.Command)) + } else { + cmd = exec.Command("docker", "exec", "-i", c.ContainerId, "/bin/sh", "-c", fmt.Sprintf("(%s)", remote.Command)) + } + + var ( + stdin_w io.WriteCloser + err error + ) + + stdin_w, err = cmd.StdinPipe() if err != nil { return err } - outputFile.Close() - // This file will store the exit code of the command once it is complete. - exitCodePath := outputFile.Name() + "-exit" - - var cmd *exec.Cmd - if c.canExec() { - if c.Config.Pty { - cmd = exec.Command("docker", "exec", "-i", "-t", c.ContainerId, "/bin/sh") - } else { - cmd = exec.Command("docker", "exec", "-i", c.ContainerId, "/bin/sh") - } - } else { - cmd = exec.Command("docker", "attach", c.ContainerId) + stderr_r, err := cmd.StderrPipe() + if err != nil { + return err } - stdin_w, err := cmd.StdinPipe() + stdout_r, err := cmd.StdoutPipe() if err != nil { - // We have to do some cleanup since run was never called - os.Remove(outputFile.Name()) - os.Remove(exitCodePath) - return err } // Run the actual command in a goroutine so that Start doesn't block - go c.run(cmd, remote, stdin_w, outputFile, exitCodePath) + go c.run(cmd, remote, stdin_w, stdout_r, stderr_r) return nil } @@ -237,105 +227,51 @@ func (c *Communicator) DownloadDir(src string, dst string, exclude []string) err return fmt.Errorf("DownloadDir is not implemented for docker") } -// canExec tells us whether `docker exec` is supported -func (c *Communicator) canExec() bool { - execConstraint, err := version.NewConstraint(">= 1.4.0") - if err != nil { - panic(err) - } - return execConstraint.Check(c.Version) -} - // Runs the given command and blocks until completion -func (c *Communicator) run(cmd *exec.Cmd, remote *packer.RemoteCmd, stdin_w io.WriteCloser, outputFile *os.File, exitCodePath string) { +func (c *Communicator) run(cmd *exec.Cmd, remote *packer.RemoteCmd, stdin io.WriteCloser, stdout, stderr io.ReadCloser) { // For Docker, remote communication must be serialized since it // only supports single execution. c.lock.Lock() defer c.lock.Unlock() - // Clean up after ourselves by removing our temporary files - defer os.Remove(outputFile.Name()) - defer os.Remove(exitCodePath) - - // Tail the output file and send the data to the stdout listener - tail, err := tail.TailFile(outputFile.Name(), tail.Config{ - Poll: true, - ReOpen: true, - Follow: true, - }) - if err != nil { - log.Printf("Error tailing output file: %s", err) - remote.SetExited(254) - return + wg := sync.WaitGroup{} + repeat := func(w io.Writer, r io.ReadCloser) { + io.Copy(w, r) + r.Close() + wg.Done() } - defer tail.Stop() - // Modify the remote command so that all the output of the commands - // go to a single file and so that the exit code is redirected to - // a single file. This lets us determine both when the command - // is truly complete (because the file will have data), what the - // exit status is (because Docker loses it because of the pty, not - // Docker's fault), and get the output (Docker bug). - remoteCmd := fmt.Sprintf("(%s) >%s 2>&1; echo $? >%s", - remote.Command, - filepath.Join(c.ContainerDir, filepath.Base(outputFile.Name())), - filepath.Join(c.ContainerDir, filepath.Base(exitCodePath))) + if remote.Stdout != nil { + wg.Add(1) + go repeat(remote.Stdout, stdout) + } + + if remote.Stderr != nil { + wg.Add(1) + go repeat(remote.Stderr, stderr) + } // Start the command - log.Printf("Executing in container %s: %#v", c.ContainerId, remoteCmd) + log.Printf("Executing %s:", strings.Join(cmd.Args, " ")) if err := cmd.Start(); err != nil { log.Printf("Error executing: %s", err) remote.SetExited(254) return } - go func() { - defer stdin_w.Close() - - // This sleep needs to be here because of the issue linked to below. - // Basically, without it, Docker will hang on reading stdin forever, - // and won't see what we write, for some reason. - // - // https://github.com/dotcloud/docker/issues/2628 - time.Sleep(2 * time.Second) - - stdin_w.Write([]byte(remoteCmd + "\n")) - }() - - // Start a goroutine to read all the lines out of the logs. These channels - // allow us to stop the go-routine and wait for it to be stopped. - stopTailCh := make(chan struct{}) - doneCh := make(chan struct{}) - go func() { - defer close(doneCh) - - for { - select { - case <-tail.Dead(): - return - case line := <-tail.Lines: - if remote.Stdout != nil { - remote.Stdout.Write([]byte(line.Text + "\n")) - } else { - log.Printf("Command stdout: %#v", line.Text) - } - case <-time.After(2 * time.Second): - // If we're done, then return. Otherwise, keep grabbing - // data. This gives us a chance to flush all the lines - // out of the tailed file. - select { - case <-stopTailCh: - return - default: - } - } - } - }() - - var exitRaw []byte var exitStatus int - var exitStatusRaw int64 - err = cmd.Wait() + + if remote.Stdin != nil { + go func() { + io.Copy(stdin, remote.Stdin) + // close stdin to support commands that wait for stdin to be closed before exiting. + stdin.Close() + }() + } + + wg.Wait() + err := cmd.Wait() + if exitErr, ok := err.(*exec.ExitError); ok { exitStatus = 1 @@ -344,45 +280,8 @@ func (c *Communicator) run(cmd *exec.Cmd, remote *packer.RemoteCmd, stdin_w io.W if status, ok := exitErr.Sys().(syscall.WaitStatus); ok { exitStatus = status.ExitStatus() } - - // Say that we ended, since if Docker itself failed, then - // the command must've not run, or so we assume - goto REMOTE_EXIT } - // Wait for the exit code to appear in our file... - log.Println("Waiting for exit code to appear for remote command...") - for { - fi, err := os.Stat(exitCodePath) - if err == nil && fi.Size() > 0 { - break - } - - time.Sleep(1 * time.Second) - } - - // Read the exit code - exitRaw, err = ioutil.ReadFile(exitCodePath) - if err != nil { - log.Printf("Error executing: %s", err) - exitStatus = 254 - goto REMOTE_EXIT - } - - exitStatusRaw, err = strconv.ParseInt(string(bytes.TrimSpace(exitRaw)), 10, 0) - if err != nil { - log.Printf("Error executing: %s", err) - exitStatus = 254 - goto REMOTE_EXIT - } - exitStatus = int(exitStatusRaw) - log.Printf("Executed command exit status: %d", exitStatus) - -REMOTE_EXIT: - // Wait for the tail to finish - close(stopTailCh) - <-doneCh - // Set the exit status which triggers waiters remote.SetExited(exitStatus) }