Merge pull request #2628 from mitchellh/b-docker-export-validation

Improve artifact validation for docker builder

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

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
 }
@@ -53,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
@@ -83,19 +89,20 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 		errs = packer.MultiErrorAppend(errs, es...)
 	}
 	if c.Image == "" {
-		errs = packer.MultiErrorAppend(errs,
-			fmt.Errorf("image must be specified"))
+		errs = packer.MultiErrorAppend(errs, 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)
 		}
 	}
 

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 {

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