Move container user inspect to StepConnectDocker.

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)
-		}
+	owner := c.ContainerUser
+	if owner == "" {
+		owner = "root"
+	}
+
+	chownArgs := []string{
+		"docker", "exec", "--user", "root", c.ContainerID, "/bin/sh", "-c",
+		fmt.Sprintf("chown -R %s %s", owner, destination),
+	}
+	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)
 	}
 
 	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)
-	}
-	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)
-	}
-
-	stderrP, err := localCmd.StderrPipe()
-	if err != nil {
-		return nil, fmt.Errorf("failed to open stderr pipe, %s", err)
-	}
-
-	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
-}

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": [
     {

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,

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
+}