From 32978a5109ae348de759b6dfab055567b69921b1 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 18 Aug 2015 13:48:18 -0700 Subject: [PATCH 1/6] Add an explicit error message when there is no output file specified --- builder/docker/step_export.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/builder/docker/step_export.go b/builder/docker/step_export.go index aa949b610..6fbdcb63d 100644 --- a/builder/docker/step_export.go +++ b/builder/docker/step_export.go @@ -2,9 +2,10 @@ package docker import ( "fmt" + "os" + "github.com/mitchellh/multistep" "github.com/mitchellh/packer/packer" - "os" ) // StepExport exports the container to a flat tar file. @@ -17,6 +18,14 @@ func (s *StepExport) Run(state multistep.StateBag) multistep.StepAction { containerId := state.Get("container_id").(string) ui := state.Get("ui").(packer.Ui) + // We should catch this in validation, but guard anyway + if config.ExportPath == "" { + err := fmt.Errorf("No output file specified, we can't export anything") + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + // Open the file that we're going to write to f, err := os.Create(config.ExportPath) if err != nil { From 750a9c61de5133f627f6ea5051479f1eecf4452f Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 18 Aug 2015 14:38:32 -0700 Subject: [PATCH 2/6] Added discard option for docker builder, also reorganized some error messages --- builder/docker/builder.go | 16 ++++++++++++---- builder/docker/config.go | 28 ++++++++++++++++++++-------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/builder/docker/builder.go b/builder/docker/builder.go index 89880aacc..702d530ce 100644 --- a/builder/docker/builder.go +++ b/builder/docker/builder.go @@ -9,8 +9,10 @@ import ( "github.com/mitchellh/packer/packer" ) -const BuilderId = "packer.docker" -const BuilderIdImport = "packer.post-processor.docker-import" +const ( + BuilderId = "packer.docker" + BuilderIdImport = "packer.post-processor.docker-import" +) type Builder struct { config *Config @@ -54,10 +56,16 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe &common.StepProvision{}, } - if b.config.Commit { + if b.config.Discard { + log.Print("[DEBUG] Container will be discarded") + } else if b.config.Commit { + log.Print("[DEBUG] Container will be committed") steps = append(steps, new(StepCommit)) - } else { + } else if b.config.ExportPath != "" { + log.Printf("[DEBUG] Container will be exported to %s", b.config.ExportPath) steps = append(steps, new(StepExport)) + } else { + return nil, ErrArtifactNotUsed } // Setup the state bag and initial state for the steps diff --git a/builder/docker/config.go b/builder/docker/config.go index 36322080c..dc205eaf0 100644 --- a/builder/docker/config.go +++ b/builder/docker/config.go @@ -12,23 +12,33 @@ import ( "github.com/mitchellh/packer/template/interpolate" ) +var ( + ErrArtifactNotUsed = fmt.Errorf("No instructions given for handling the artifact; expected commit, discard, or export_path") + ErrArtifactUseConflict = fmt.Errorf("Cannot specify more than one of commit, discard, and export_path") + ErrExportPathNotFile = fmt.Errorf("export_path must be a file, not a directory") + ErrImageNotSpecified = fmt.Errorf("Image must be specified") +) + type Config struct { common.PackerConfig `mapstructure:",squash"` Comm communicator.Config `mapstructure:",squash"` Commit bool + Discard bool ExportPath string `mapstructure:"export_path"` Image string + Pty bool Pull bool RunCommand []string `mapstructure:"run_command"` Volumes map[string]string + // This is used to login to dockerhub to pull a private base container + // For pushing to dockerhub, see the docker post-processors Login bool LoginEmail string `mapstructure:"login_email"` - LoginUsername string `mapstructure:"login_username"` LoginPassword string `mapstructure:"login_password"` LoginServer string `mapstructure:"login_server"` - Pty bool + LoginUsername string `mapstructure:"login_username"` ctx interpolate.Context } @@ -84,18 +94,20 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { } if c.Image == "" { errs = packer.MultiErrorAppend(errs, - fmt.Errorf("image must be specified")) + ErrImageNotSpecified) } - if c.ExportPath != "" && c.Commit { - errs = packer.MultiErrorAppend(errs, - fmt.Errorf("both commit and export_path cannot be set")) + if (c.ExportPath != "" && c.Commit) || (c.ExportPath != "" && c.Discard) || (c.Commit && c.Discard) { + errs = packer.MultiErrorAppend(errs, ErrArtifactUseConflict) + } + + if c.ExportPath == "" && !c.Commit && !c.Discard { + errs = packer.MultiErrorAppend(errs, ErrArtifactNotUsed) } if c.ExportPath != "" { if fi, err := os.Stat(c.ExportPath); err == nil && fi.IsDir() { - errs = packer.MultiErrorAppend(errs, fmt.Errorf( - "export_path must be a file, not a directory")) + errs = packer.MultiErrorAppend(errs, ErrExportPathNotFile) } } From 1b1bd19c20e8e6daa4fd3eb2a2802a44efe15a25 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 18 Aug 2015 16:47:12 -0700 Subject: [PATCH 3/6] Reformat code so we can grep for this more easily --- builder/docker/config.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/builder/docker/config.go b/builder/docker/config.go index dc205eaf0..7c3565008 100644 --- a/builder/docker/config.go +++ b/builder/docker/config.go @@ -63,11 +63,7 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { // Defaults if len(c.RunCommand) == 0 { - c.RunCommand = []string{ - "-d", "-i", "-t", - "{{.Image}}", - "/bin/bash", - } + c.RunCommand = []string{"-d", "-i", "-t", "{{.Image}}", "/bin/bash"} } // Default Pull if it wasn't set From ffef8efaf4fa20f5d183e9be7cf2cc68846bc2e4 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 18 Aug 2015 16:47:58 -0700 Subject: [PATCH 4/6] Added docs for discard; clarify mutual exclusivity between commit, discard, and export_path --- website/source/docs/builders/docker.html.markdown | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/website/source/docs/builders/docker.html.markdown b/website/source/docs/builders/docker.html.markdown index 76b1d4057..48489380b 100644 --- a/website/source/docs/builders/docker.html.markdown +++ b/website/source/docs/builders/docker.html.markdown @@ -68,11 +68,17 @@ builder. ### Required: +You must specify (only) one of `commit`, `discard`, or `export_path`. + - `commit` (boolean) - If true, the container will be committed to an image - rather than exported. This cannot be set if `export_path` is set. + rather than exported. + +- `discard` (boolean) - Throw away the container when the build is complete. + This is useful for the [artifice + post-processor](https://packer.io/docs/post-processors/artifice.html). - `export_path` (string) - The path where the final container will be exported - as a tar file. This cannot be set if `commit` is set to true. + as a tar file. - `image` (string) - The base image for the Docker container that will be started. This image will be pulled from the Docker registry if it doesn't From 746b9a839198ea18690fbccd0e289305bbc2a85a Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 18 Aug 2015 16:55:29 -0700 Subject: [PATCH 5/6] Formatting tweaks --- builder/docker/config.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/builder/docker/config.go b/builder/docker/config.go index 7c3565008..c45f6baf5 100644 --- a/builder/docker/config.go +++ b/builder/docker/config.go @@ -32,8 +32,8 @@ type Config struct { RunCommand []string `mapstructure:"run_command"` Volumes map[string]string - // This is used to login to dockerhub to pull a private base container - // For pushing to dockerhub, see the docker post-processors + // This is used to login to dockerhub to pull a private base container. For + // pushing to dockerhub, see the docker post-processors Login bool LoginEmail string `mapstructure:"login_email"` LoginPassword string `mapstructure:"login_password"` @@ -89,8 +89,7 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { errs = packer.MultiErrorAppend(errs, es...) } if c.Image == "" { - errs = packer.MultiErrorAppend(errs, - ErrImageNotSpecified) + errs = packer.MultiErrorAppend(errs, ErrImageNotSpecified) } if (c.ExportPath != "" && c.Commit) || (c.ExportPath != "" && c.Discard) || (c.Commit && c.Discard) { From 5503b7f496c3e323bf0d8b9ebcc435a0072ec136 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 19 Aug 2015 13:12:16 -0700 Subject: [PATCH 6/6] Don't export errors --- builder/docker/builder.go | 2 +- builder/docker/config.go | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/builder/docker/builder.go b/builder/docker/builder.go index 702d530ce..cfc5bf423 100644 --- a/builder/docker/builder.go +++ b/builder/docker/builder.go @@ -65,7 +65,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe log.Printf("[DEBUG] Container will be exported to %s", b.config.ExportPath) steps = append(steps, new(StepExport)) } else { - return nil, ErrArtifactNotUsed + return nil, errArtifactNotUsed } // Setup the state bag and initial state for the steps diff --git a/builder/docker/config.go b/builder/docker/config.go index c45f6baf5..ad8a5634b 100644 --- a/builder/docker/config.go +++ b/builder/docker/config.go @@ -13,10 +13,10 @@ import ( ) var ( - ErrArtifactNotUsed = fmt.Errorf("No instructions given for handling the artifact; expected commit, discard, or export_path") - ErrArtifactUseConflict = fmt.Errorf("Cannot specify more than one of commit, discard, and export_path") - ErrExportPathNotFile = fmt.Errorf("export_path must be a file, not a directory") - ErrImageNotSpecified = fmt.Errorf("Image must be specified") + errArtifactNotUsed = fmt.Errorf("No instructions given for handling the artifact; expected commit, discard, or export_path") + errArtifactUseConflict = fmt.Errorf("Cannot specify more than one of commit, discard, and export_path") + errExportPathNotFile = fmt.Errorf("export_path must be a file, not a directory") + errImageNotSpecified = fmt.Errorf("Image must be specified") ) type Config struct { @@ -89,20 +89,20 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { errs = packer.MultiErrorAppend(errs, es...) } if c.Image == "" { - errs = packer.MultiErrorAppend(errs, ErrImageNotSpecified) + errs = packer.MultiErrorAppend(errs, errImageNotSpecified) } if (c.ExportPath != "" && c.Commit) || (c.ExportPath != "" && c.Discard) || (c.Commit && c.Discard) { - errs = packer.MultiErrorAppend(errs, ErrArtifactUseConflict) + errs = packer.MultiErrorAppend(errs, errArtifactUseConflict) } if c.ExportPath == "" && !c.Commit && !c.Discard { - errs = packer.MultiErrorAppend(errs, ErrArtifactNotUsed) + errs = packer.MultiErrorAppend(errs, errArtifactNotUsed) } if c.ExportPath != "" { if fi, err := os.Stat(c.ExportPath); err == nil && fi.IsDir() { - errs = packer.MultiErrorAppend(errs, ErrExportPathNotFile) + errs = packer.MultiErrorAppend(errs, errExportPathNotFile) } }