From df7623d9d8c1530f0083c755690d6e81f53c2e5b Mon Sep 17 00:00:00 2001 From: Donald Guy Date: Fri, 20 Mar 2015 12:04:38 -0400 Subject: [PATCH 1/6] builder/docker: Run scripts /w `exec` if -v > 1.4 --- builder/docker/communicator.go | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/builder/docker/communicator.go b/builder/docker/communicator.go index bad2c1ff6..c6e3829f0 100644 --- a/builder/docker/communicator.go +++ b/builder/docker/communicator.go @@ -9,12 +9,14 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strconv" "sync" "syscall" "time" "github.com/ActiveState/tail" + "github.com/hashicorp/go-version" "github.com/mitchellh/packer/packer" ) @@ -26,6 +28,27 @@ type Communicator struct { lock sync.Mutex } +var dockerVersion version.Version +var useDockerExec bool + +func init() { + execConstraint, _ := version.NewConstraint(">= 1.4.0") + + versionExtractor := regexp.MustCompile(version.VersionRegexpRaw) + dockerVersionOutput, _ := exec.Command("docker", "-v").Output() + dockerVersionString := string(versionExtractor.FindSubmatch(dockerVersionOutput)[0]) + + dockerVersion, err := version.NewVersion(dockerVersionString) + if err != nil { + log.Printf("Docker returned malformed version string: %e", err) + log.Printf("Assuming no `exec` capability, using `attatch`") + useDockerExec = false + } else { + log.Printf("Docker version detected as %s", dockerVersion) + useDockerExec = execConstraint.Check(dockerVersion) + } +} + 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 @@ -41,7 +64,13 @@ func (c *Communicator) Start(remote *packer.RemoteCmd) error { // This file will store the exit code of the command once it is complete. exitCodePath := outputFile.Name() + "-exit" - cmd := exec.Command("docker", "attach", c.ContainerId) + var cmd *exec.Cmd + if useDockerExec { + cmd = exec.Command("docker", "exec", "-i", c.ContainerId, "/bin/sh") + } else { + cmd = exec.Command("docker", "attach", c.ContainerId) + } + stdin_w, err := cmd.StdinPipe() if err != nil { // We have to do some cleanup since run was never called From a7206aebd79738993ad1105faf6bc6be6161c90f Mon Sep 17 00:00:00 2001 From: Donald Guy Date: Fri, 20 Mar 2015 12:50:03 -0400 Subject: [PATCH 2/6] builder/docker: attempt to satisfy travis for #1993 --- builder/docker/communicator.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/builder/docker/communicator.go b/builder/docker/communicator.go index c6e3829f0..bd1c87240 100644 --- a/builder/docker/communicator.go +++ b/builder/docker/communicator.go @@ -28,19 +28,23 @@ type Communicator struct { lock sync.Mutex } -var dockerVersion version.Version +var dockerVersion *version.Version var useDockerExec bool func init() { execConstraint, _ := version.NewConstraint(">= 1.4.0") versionExtractor := regexp.MustCompile(version.VersionRegexpRaw) - dockerVersionOutput, _ := exec.Command("docker", "-v").Output() - dockerVersionString := string(versionExtractor.FindSubmatch(dockerVersionOutput)[0]) + dockerVersionOutput, err := exec.Command("docker", "-v").Output() + extractedVersion := versionExtractor.FindSubmatch(dockerVersionOutput) - dockerVersion, err := version.NewVersion(dockerVersionString) - if err != nil { - log.Printf("Docker returned malformed version string: %e", err) + if extractedVersion != nil { + dockerVersionString := string(extractedVersion[0]) + dockerVersion, err = version.NewVersion(dockerVersionString) + } + + if dockerVersion == nil { + log.Printf("Could not determine docker version: %v", err) log.Printf("Assuming no `exec` capability, using `attatch`") useDockerExec = false } else { From 9c7b4b63c5c585bdc34ccd5c27e2bca14a8dd49e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 29 May 2015 09:19:20 -0700 Subject: [PATCH 3/6] builder/docker: fix config parsing --- builder/docker/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builder/docker/config.go b/builder/docker/config.go index e261068df..4fc6f762a 100644 --- a/builder/docker/config.go +++ b/builder/docker/config.go @@ -30,7 +30,7 @@ type Config struct { } func NewConfig(raws ...interface{}) (*Config, []string, error) { - c := new(Config) + var c Config var md mapstructure.Metadata err := config.Decode(&c, &config.DecodeOpts{ @@ -83,5 +83,5 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { return nil, nil, errs } - return c, nil, nil + return &c, nil, nil } From 6570b53c4a5ed8ac79705c7bae6d404c77420208 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 29 May 2015 09:29:59 -0700 Subject: [PATCH 4/6] builder/docker: use exec for v1.4+ --- builder/docker/builder.go | 9 +++++++- builder/docker/communicator.go | 37 +++++++++----------------------- builder/docker/driver.go | 5 +++++ builder/docker/driver_docker.go | 16 ++++++++++++++ builder/docker/driver_mock.go | 10 +++++++++ builder/docker/step_provision.go | 9 ++++++++ 6 files changed, 58 insertions(+), 28 deletions(-) diff --git a/builder/docker/builder.go b/builder/docker/builder.go index 2dddbf94e..18ff73357 100644 --- a/builder/docker/builder.go +++ b/builder/docker/builder.go @@ -1,10 +1,11 @@ package docker import ( + "log" + "github.com/mitchellh/multistep" "github.com/mitchellh/packer/common" "github.com/mitchellh/packer/packer" - "log" ) const BuilderId = "packer.docker" @@ -31,6 +32,12 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe return nil, err } + version, err := driver.Version() + if err != nil { + return nil, err + } + log.Printf("[DEBUG] Docker version: %s", version.String()) + steps := []multistep.Step{ &StepTempDir{}, &StepPull{}, diff --git a/builder/docker/communicator.go b/builder/docker/communicator.go index bd1c87240..548c1b4d8 100644 --- a/builder/docker/communicator.go +++ b/builder/docker/communicator.go @@ -9,7 +9,6 @@ import ( "os" "os/exec" "path/filepath" - "regexp" "strconv" "sync" "syscall" @@ -24,36 +23,20 @@ type Communicator struct { ContainerId string HostDir string ContainerDir string + Version *version.Version lock sync.Mutex } -var dockerVersion *version.Version -var useDockerExec bool - -func init() { - execConstraint, _ := version.NewConstraint(">= 1.4.0") - - versionExtractor := regexp.MustCompile(version.VersionRegexpRaw) - dockerVersionOutput, err := exec.Command("docker", "-v").Output() - extractedVersion := versionExtractor.FindSubmatch(dockerVersionOutput) - - if extractedVersion != nil { - dockerVersionString := string(extractedVersion[0]) - dockerVersion, err = version.NewVersion(dockerVersionString) - } - - if dockerVersion == nil { - log.Printf("Could not determine docker version: %v", err) - log.Printf("Assuming no `exec` capability, using `attatch`") - useDockerExec = false - } else { - log.Printf("Docker version detected as %s", dockerVersion) - useDockerExec = execConstraint.Check(dockerVersion) +func (c *Communicator) Start(remote *packer.RemoteCmd) error { + // Determine if we're using docker exec or not + useExec := false + execConstraint, err := version.NewConstraint(">= 1.4.0") + if err != nil { + return err } -} + useExec = execConstraint.Check(c.Version) -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. @@ -69,7 +52,7 @@ func (c *Communicator) Start(remote *packer.RemoteCmd) error { exitCodePath := outputFile.Name() + "-exit" var cmd *exec.Cmd - if useDockerExec { + if useExec { cmd = exec.Command("docker", "exec", "-i", c.ContainerId, "/bin/sh") } else { cmd = exec.Command("docker", "attach", c.ContainerId) @@ -150,7 +133,7 @@ func (c *Communicator) UploadDir(dst string, src string, exclude []string) error return os.MkdirAll(hostpath, info.Mode()) } - if info.Mode() & os.ModeSymlink == os.ModeSymlink { + if info.Mode()&os.ModeSymlink == os.ModeSymlink { dest, err := os.Readlink(path) if err != nil { diff --git a/builder/docker/driver.go b/builder/docker/driver.go index 85b87b1d0..d9cb94f9b 100644 --- a/builder/docker/driver.go +++ b/builder/docker/driver.go @@ -2,6 +2,8 @@ package docker import ( "io" + + "github.com/hashicorp/go-version" ) // Driver is the interface that has to be implemented to communicate with @@ -48,6 +50,9 @@ type Driver interface { // Verify verifies that the driver can run Verify() error + + // Version reads the Docker version + Version() (*version.Version, error) } // ContainerConfig is the configuration used to start a container. diff --git a/builder/docker/driver_docker.go b/builder/docker/driver_docker.go index 038aa046e..017c33ee0 100644 --- a/builder/docker/driver_docker.go +++ b/builder/docker/driver_docker.go @@ -7,9 +7,11 @@ import ( "log" "os" "os/exec" + "regexp" "strings" "sync" + "github.com/hashicorp/go-version" "github.com/mitchellh/packer/packer" "github.com/mitchellh/packer/template/interpolate" ) @@ -263,3 +265,17 @@ func (d *DockerDriver) Verify() error { return nil } + +func (d *DockerDriver) Version() (*version.Version, error) { + output, err := exec.Command("docker", "-v").Output() + if err != nil { + return nil, err + } + + match := regexp.MustCompile(version.VersionRegexpRaw).FindSubmatch(output) + if match == nil { + return nil, fmt.Errorf("unknown version: %s", output) + } + + return version.NewVersion(string(match[0])) +} diff --git a/builder/docker/driver_mock.go b/builder/docker/driver_mock.go index 549e79611..390a7e308 100644 --- a/builder/docker/driver_mock.go +++ b/builder/docker/driver_mock.go @@ -2,6 +2,8 @@ package docker import ( "io" + + "github.com/hashicorp/go-version" ) // MockDriver is a driver implementation that can be used for tests. @@ -63,6 +65,9 @@ type MockDriver struct { StopCalled bool StopID string VerifyCalled bool + + VersionCalled bool + VersionVersion string } func (d *MockDriver) Commit(id string) (string, error) { @@ -162,3 +167,8 @@ func (d *MockDriver) Verify() error { d.VerifyCalled = true return d.VerifyError } + +func (d *MockDriver) Version() (*version.Version, error) { + d.VersionCalled = true + return version.NewVersion(d.VersionVersion) +} diff --git a/builder/docker/step_provision.go b/builder/docker/step_provision.go index 726653763..d9852ae2b 100644 --- a/builder/docker/step_provision.go +++ b/builder/docker/step_provision.go @@ -9,14 +9,23 @@ type StepProvision struct{} func (s *StepProvision) Run(state multistep.StateBag) multistep.StepAction { containerId := state.Get("container_id").(string) + driver := state.Get("driver").(Driver) tempDir := state.Get("temp_dir").(string) + // Get the version so we can pass it to the communicator + version, err := driver.Version() + if err != nil { + state.Put("error", err) + return multistep.ActionHalt + } + // Create the communicator that talks to Docker via various // os/exec tricks. comm := &Communicator{ ContainerId: containerId, HostDir: tempDir, ContainerDir: "/packer-files", + Version: version, } prov := common.StepProvision{Comm: comm} From ce275969e464145e674bd64e478c8c23fe398a62 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 29 May 2015 09:37:27 -0700 Subject: [PATCH 5/6] builder/docker: don't attempt to read artifact if cancelled --- builder/docker/builder.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/builder/docker/builder.go b/builder/docker/builder.go index 18ff73357..96a79b02d 100644 --- a/builder/docker/builder.go +++ b/builder/docker/builder.go @@ -77,8 +77,13 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe return nil, rawErr.(error) } - var artifact packer.Artifact + // If it was cancelled, then just return + if _, ok := state.GetOk(multistep.StateCancelled); ok { + return nil, nil + } + // No errors, must've worked + var artifact packer.Artifact if b.config.Commit { artifact = &ImportArtifact{ IdValue: state.Get("image_id").(string), @@ -88,6 +93,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } else { artifact = &ExportArtifact{path: b.config.ExportPath} } + return artifact, nil } From 31ac2652d64ddd364f0bd3ae2652559fc02caad6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 29 May 2015 11:08:41 -0700 Subject: [PATCH 6/6] bulder/docker: canExec as sep function --- builder/docker/communicator.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/builder/docker/communicator.go b/builder/docker/communicator.go index 548c1b4d8..6fedf2769 100644 --- a/builder/docker/communicator.go +++ b/builder/docker/communicator.go @@ -29,14 +29,6 @@ type Communicator struct { } func (c *Communicator) Start(remote *packer.RemoteCmd) error { - // Determine if we're using docker exec or not - useExec := false - execConstraint, err := version.NewConstraint(">= 1.4.0") - if err != nil { - return err - } - useExec = execConstraint.Check(c.Version) - // 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. @@ -52,7 +44,7 @@ func (c *Communicator) Start(remote *packer.RemoteCmd) error { exitCodePath := outputFile.Name() + "-exit" var cmd *exec.Cmd - if useExec { + if c.canExec() { cmd = exec.Command("docker", "exec", "-i", c.ContainerId, "/bin/sh") } else { cmd = exec.Command("docker", "attach", c.ContainerId) @@ -202,6 +194,15 @@ func (c *Communicator) Download(src string, dst io.Writer) error { panic("not implemented") } +// 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) { // For Docker, remote communication must be serialized since it