diff --git a/builder/docker/communicator.go b/builder/docker/communicator.go index bbfdb551f..d15143172 100644 --- a/builder/docker/communicator.go +++ b/builder/docker/communicator.go @@ -23,7 +23,7 @@ type Communicator struct { ContainerDir string Version *version.Version Config *Config - containerUser *string + ContainerUser string lock sync.Mutex } @@ -322,67 +322,22 @@ func (c *Communicator) run(cmd *exec.Cmd, remote *packer.RemoteCmd, stdin io.Wri // TODO Workaround for #5307. Remove once #5409 is fixed. func (c *Communicator) fixDestinationOwner(destination string) error { - if c.containerUser == nil { - containerUser, err := c.discoverContainerUser() - if err != nil { - return err - } - c.containerUser = &containerUser + if !c.Config.FixUploadOwner { + return nil } - if *c.containerUser != "" { - chownArgs := []string{ - "docker", "exec", "--user", "root", c.ContainerID, "/bin/sh", "-c", - fmt.Sprintf("chown -R %s %s", *c.containerUser, destination), - } - if _, err := c.runLocalCommand(chownArgs[0], chownArgs[1:]...); err != nil { - return fmt.Errorf("Failed to set owner of the uploaded file: %s", err) - } - } - - return nil -} - -func (c *Communicator) discoverContainerUser() (string, error) { - var err error - var stdout []byte - inspectArgs := []string{"docker", "inspect", "--format", "{{.Config.User}}", c.ContainerID} - if stdout, err = c.runLocalCommand(inspectArgs[0], inspectArgs[1:]...); err != nil { - return "", fmt.Errorf("Failed to inspect the container: %s", err) + owner := c.ContainerUser + if owner == "" { + owner = "root" } - return strings.TrimSpace(string(stdout)), nil -} -func (c *Communicator) runLocalCommand(name string, arg ...string) (stdout []byte, err error) { - localCmd := exec.Command(name, arg...) - - stdoutP, err := localCmd.StdoutPipe() - if err != nil { - return nil, fmt.Errorf("failed to open stdout pipe, %s", err) + chownArgs := []string{ + "docker", "exec", "--user", "root", c.ContainerID, "/bin/sh", "-c", + fmt.Sprintf("chown -R %s %s", owner, destination), } - - stderrP, err := localCmd.StderrPipe() - if err != nil { - return nil, fmt.Errorf("failed to open stderr pipe, %s", err) + if output, err := exec.Command(chownArgs[0], chownArgs[1:]...).CombinedOutput(); err != nil { + return fmt.Errorf("Failed to set owner of the uploaded file: %s, %s", err, output) } - if err = localCmd.Start(); err != nil { - return nil, fmt.Errorf("failed to start command, %s", err) - } - - stdout, err = ioutil.ReadAll(stdoutP) - if err != nil { - return nil, fmt.Errorf("failed to read from stdout pipe, %s", err) - } - - stderr, err := ioutil.ReadAll(stderrP) - if err != nil { - return nil, fmt.Errorf("failed to read from stderr pipe, %s", err) - } - - if err := localCmd.Wait(); err != nil { - return nil, fmt.Errorf("%s, %s", stderr, err) - } - - return stdout, nil + return nil } diff --git a/builder/docker/communicator_test.go b/builder/docker/communicator_test.go index b5b5b1583..bdfaef996 100644 --- a/builder/docker/communicator_test.go +++ b/builder/docker/communicator_test.go @@ -209,12 +209,12 @@ func TestLargeDownload(t *testing.T) { } -// TestUploadOwner verifies that owner of uploaded files is the user the container is running as. -func TestUploadOwner(t *testing.T) { +// TestFixUploadOwner verifies that owner of uploaded files is the user the container is running as. +func TestFixUploadOwner(t *testing.T) { ui := packer.TestUi(t) cache := &packer.FileCache{CacheDir: os.TempDir()} - tpl, err := template.Parse(strings.NewReader(testUploadOwnerTemplate)) + tpl, err := template.Parse(strings.NewReader(testFixUploadOwnerTemplate)) if err != nil { t.Fatalf("Unable to parse config: %s", err) } @@ -346,7 +346,7 @@ const dockerLargeBuilderConfig = ` } ` -const testUploadOwnerTemplate = ` +const testFixUploadOwnerTemplate = ` { "builders": [ { diff --git a/builder/docker/config.go b/builder/docker/config.go index 89d28c290..fee08929d 100644 --- a/builder/docker/config.go +++ b/builder/docker/config.go @@ -23,20 +23,21 @@ type Config struct { common.PackerConfig `mapstructure:",squash"` Comm communicator.Config `mapstructure:",squash"` - Author string - Changes []string - Commit bool - ContainerDir string `mapstructure:"container_dir"` - Discard bool - ExecUser string `mapstructure:"exec_user"` - ExportPath string `mapstructure:"export_path"` - Image string - Message string - Privileged bool `mapstructure:"privileged"` - Pty bool - Pull bool - RunCommand []string `mapstructure:"run_command"` - Volumes map[string]string + Author string + Changes []string + Commit bool + ContainerDir string `mapstructure:"container_dir"` + Discard bool + ExecUser string `mapstructure:"exec_user"` + ExportPath string `mapstructure:"export_path"` + Image string + Message string + Privileged bool `mapstructure:"privileged"` + Pty bool + Pull bool + RunCommand []string `mapstructure:"run_command"` + Volumes map[string]string + FixUploadOwner bool `mapstructure:"fix_upload_owner"` // This is used to login to dockerhub to pull a private base container. For // pushing to dockerhub, see the docker post-processors @@ -54,6 +55,8 @@ type Config struct { func NewConfig(raws ...interface{}) (*Config, []string, error) { c := new(Config) + c.FixUploadOwner = true + var md mapstructure.Metadata err := config.Decode(c, &config.DecodeOpts{ Metadata: &md, diff --git a/builder/docker/step_connect_docker.go b/builder/docker/step_connect_docker.go index 7a947d500..45cf2d25d 100644 --- a/builder/docker/step_connect_docker.go +++ b/builder/docker/step_connect_docker.go @@ -1,7 +1,10 @@ package docker import ( + "fmt" "github.com/mitchellh/multistep" + "os/exec" + "strings" ) type StepConnectDocker struct{} @@ -19,14 +22,21 @@ func (s *StepConnectDocker) Run(state multistep.StateBag) multistep.StepAction { return multistep.ActionHalt } + containerUser, err := getContainerUser(containerId) + 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: config.ContainerDir, - Version: version, - Config: config, + ContainerID: containerId, + HostDir: tempDir, + ContainerDir: config.ContainerDir, + Version: version, + Config: config, + ContainerUser: containerUser, } state.Put("communicator", comm) @@ -34,3 +44,16 @@ func (s *StepConnectDocker) Run(state multistep.StateBag) multistep.StepAction { } func (s *StepConnectDocker) Cleanup(state multistep.StateBag) {} + +func getContainerUser(containerId string) (string, error) { + inspectArgs := []string{"docker", "inspect", "--format", "{{.Config.User}}", containerId} + stdout, err := exec.Command(inspectArgs[0], inspectArgs[1:]...).Output() + if err != nil { + errStr := fmt.Sprintf("Failed to inspect the container: %s", err) + if ee, ok := err.(*exec.ExitError); ok { + errStr = fmt.Sprintf("%s, %s", errStr, ee.Stderr) + } + return "", fmt.Errorf(errStr) + } + return strings.TrimSpace(string(stdout)), nil +}