From 3b87f2a5191d3fcfefb762521869384fc6a6f969 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 27 Mar 2019 14:51:50 -0700 Subject: [PATCH] stop container before committing if windows --- builder/docker/config.go | 2 +- builder/docker/driver.go | 5 +- builder/docker/driver_docker.go | 7 +++ builder/docker/step_commit.go | 10 ++++ builder/docker/step_connect_docker.go | 30 +++++++--- builder/docker/step_run.go | 2 +- .../docker/windows_container_communicator.go | 57 ------------------- 7 files changed, 44 insertions(+), 69 deletions(-) diff --git a/builder/docker/config.go b/builder/docker/config.go index d7b2cd50c..684e88bd9 100644 --- a/builder/docker/config.go +++ b/builder/docker/config.go @@ -38,7 +38,7 @@ type Config struct { RunCommand []string `mapstructure:"run_command"` Volumes map[string]string FixUploadOwner bool `mapstructure:"fix_upload_owner"` - WindowsContainer bool `windows_container` + WindowsContainer bool `mapstructure:"windows_container"` // This is used to login to dockerhub to pull a private base container. For // pushing to dockerhub, see the docker post-processors diff --git a/builder/docker/driver.go b/builder/docker/driver.go index 7359a667d..73bf5e1a8 100644 --- a/builder/docker/driver.go +++ b/builder/docker/driver.go @@ -46,7 +46,10 @@ type Driver interface { // along with a potential error. StartContainer(*ContainerConfig) (string, error) - // StopContainer forcibly stops a container. + // KillContainer forcibly stops a container. + KillContainer(id string) error + + // StopContainer gently stops a container. StopContainer(id string) error // TagImage tags the image with the given ID diff --git a/builder/docker/driver_docker.go b/builder/docker/driver_docker.go index 9b0a4556b..6b946e954 100644 --- a/builder/docker/driver_docker.go +++ b/builder/docker/driver_docker.go @@ -314,6 +314,13 @@ func (d *DockerDriver) StartContainer(config *ContainerConfig) (string, error) { } func (d *DockerDriver) StopContainer(id string) error { + if err := exec.Command("docker", "stop", id).Run(); err != nil { + return err + } + return nil +} + +func (d *DockerDriver) KillContainer(id string) error { if err := exec.Command("docker", "kill", id).Run(); err != nil { return err } diff --git a/builder/docker/step_commit.go b/builder/docker/step_commit.go index d248ee64c..c75ab649c 100644 --- a/builder/docker/step_commit.go +++ b/builder/docker/step_commit.go @@ -19,6 +19,16 @@ func (s *StepCommit) Run(_ context.Context, state multistep.StateBag) multistep. config := state.Get("config").(*Config) ui := state.Get("ui").(packer.Ui) + if config.WindowsContainer { + // docker can't commit a running Windows container + err := driver.StopContainer(containerId) + if err != nil { + state.Put("error", err) + ui.Error(fmt.Sprintf("Error halting windows container for commit: %s", + err.Error())) + return multistep.ActionHalt + } + } ui.Say("Committing the container") imageId, err := driver.Commit(containerId, config.Author, config.Changes, config.Message) if err != nil { diff --git a/builder/docker/step_connect_docker.go b/builder/docker/step_connect_docker.go index ef0222a91..3cd4bc173 100644 --- a/builder/docker/step_connect_docker.go +++ b/builder/docker/step_connect_docker.go @@ -32,16 +32,28 @@ func (s *StepConnectDocker) Run(_ context.Context, state multistep.StateBag) mul // 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, - ContainerUser: containerUser, - } + if config.WindowsContainer { + comm := &WindowsContainerCommunicator{ + ContainerID: containerId, + HostDir: tempDir, + ContainerDir: config.ContainerDir, + Version: version, + Config: config, + ContainerUser: containerUser, + } + state.Put("communicator", comm) - state.Put("communicator", comm) + } else { + comm := &Communicator{ + ContainerID: containerId, + HostDir: tempDir, + ContainerDir: config.ContainerDir, + Version: version, + Config: config, + ContainerUser: containerUser, + } + state.Put("communicator", comm) + } return multistep.ActionContinue } diff --git a/builder/docker/step_run.go b/builder/docker/step_run.go index 1d2ba6862..25c5dee0c 100644 --- a/builder/docker/step_run.go +++ b/builder/docker/step_run.go @@ -58,7 +58,7 @@ func (s *StepRun) Cleanup(state multistep.StateBag) { // just mean that the container doesn't exist anymore, which isn't a // big deal. ui.Say(fmt.Sprintf("Killing the container: %s", s.containerId)) - driver.StopContainer(s.containerId) + driver.KillContainer(s.containerId) // Reset the container ID so that we're idempotent s.containerId = "" diff --git a/builder/docker/windows_container_communicator.go b/builder/docker/windows_container_communicator.go index 88054429c..cc93975a4 100644 --- a/builder/docker/windows_container_communicator.go +++ b/builder/docker/windows_container_communicator.go @@ -260,63 +260,6 @@ func (c *WindowsContainerCommunicator) uploadFileOld(dst string, src io.Reader, return nil } -func (c *WindowsContainerCommunicator) UploadDir(dst string, src string, exclude []string) error { - /* - from https://docs.docker.com/engine/reference/commandline/cp/#extended-description - SRC_PATH specifies a directory - DEST_PATH does not exist - DEST_PATH is created as a directory and the contents of the source directory are copied into this directory - DEST_PATH exists and is a file - Error condition: cannot copy a directory to a file - DEST_PATH exists and is a directory - SRC_PATH does not end with /. (that is: slash followed by dot) - the source directory is copied into this directory - SRC_PATH does end with /. (that is: slash followed by dot) - the content of the source directory is copied into this directory - - translating that in to our semantics: - - if source ends in / - docker cp src. dest - otherwise, cp source dest - - */ - - var dockerSource string - - if src[len(src)-1] == '/' { - dockerSource = fmt.Sprintf("%s.", src) - } else { - dockerSource = fmt.Sprintf("%s", src) - } - - // Make the directory, then copy into it - localCmd := exec.Command("docker", "cp", dockerSource, fmt.Sprintf("%s:%s", c.ContainerID, dst)) - - stderrP, err := localCmd.StderrPipe() - if err != nil { - return fmt.Errorf("Failed to open pipe: %s", err) - } - if err := localCmd.Start(); err != nil { - return fmt.Errorf("Failed to copy: %s", err) - } - stderrOut, err := ioutil.ReadAll(stderrP) - if err != nil { - return err - } - - // Wait for the copy to complete - if err := localCmd.Wait(); err != nil { - return fmt.Errorf("Failed to upload to '%s' in container: %s. %s.", dst, stderrOut, err) - } - - if err := c.fixDestinationOwner(dst); err != nil { - return err - } - - return nil -} - // Download pulls a file out of a container using `docker cp`. We have a source // path and want to write to an io.Writer, not a file. We use - to make docker // cp to write to stdout, and then copy the stream to our destination io.Writer.