From 6762965696ee05789ad293628e8f2a2d2b8062f7 Mon Sep 17 00:00:00 2001 From: Orivej Desh Date: Wed, 14 Sep 2016 00:04:18 +0000 Subject: [PATCH 1/6] Add -on-error command line argument to allow preserving artifacts on builder errors Resolves #409 --- builder/amazon/chroot/builder.go | 10 +- builder/amazon/ebs/builder.go | 10 +- builder/amazon/instance/builder.go | 10 +- builder/azure/arm/builder.go | 15 +-- builder/digitalocean/builder.go | 10 +- builder/docker/builder.go | 10 +- builder/googlecompute/builder.go | 9 +- builder/null/builder.go | 10 +- builder/openstack/builder.go | 10 +- builder/parallels/iso/builder.go | 12 +-- builder/parallels/pvm/builder.go | 11 +-- builder/qemu/builder.go | 12 +-- builder/virtualbox/iso/builder.go | 12 +-- builder/virtualbox/ovf/builder.go | 11 +-- builder/vmware/iso/builder.go | 12 +-- builder/vmware/vmx/builder.go | 11 +-- command/build.go | 7 ++ common/multistep_runner.go | 148 +++++++++++++++++++++++++++++ common/packer_config.go | 1 + packer/build.go | 22 +++++ packer/build_test.go | 2 + packer/rpc/build.go | 14 ++- packer/rpc/build_test.go | 32 +++++-- 23 files changed, 231 insertions(+), 170 deletions(-) create mode 100644 common/multistep_runner.go diff --git a/builder/amazon/chroot/builder.go b/builder/amazon/chroot/builder.go index 5e6013a54..dcfd7bc41 100644 --- a/builder/amazon/chroot/builder.go +++ b/builder/amazon/chroot/builder.go @@ -262,15 +262,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe ) // Run! - if b.config.PackerDebug { - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: common.MultistepDebugFn(ui), - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } - + b.runner = common.NewRunner(steps, b.config.PackerConfig, ui) b.runner.Run(state) // If there was an error, return that diff --git a/builder/amazon/ebs/builder.go b/builder/amazon/ebs/builder.go index 7174d6b8e..9916d821d 100644 --- a/builder/amazon/ebs/builder.go +++ b/builder/amazon/ebs/builder.go @@ -179,15 +179,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } // Run! - if b.config.PackerDebug { - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: common.MultistepDebugFn(ui), - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } - + b.runner = common.NewRunner(steps, b.config.PackerConfig, ui) b.runner.Run(state) // If there was an error, return that diff --git a/builder/amazon/instance/builder.go b/builder/amazon/instance/builder.go index 96b8fa9cc..a900aef9b 100644 --- a/builder/amazon/instance/builder.go +++ b/builder/amazon/instance/builder.go @@ -262,15 +262,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } // Run! - if b.config.PackerDebug { - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: common.MultistepDebugFn(ui), - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } - + b.runner = common.NewRunner(steps, b.config.PackerConfig, ui) b.runner.Run(state) // If there was an error, return that diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index db344acb3..30029be52 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -157,7 +157,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe ui.Message(fmt.Sprintf("temp admin password: '%s'", b.config.Password)) } - b.runner = b.createRunner(&steps, ui) + b.runner = packerCommon.NewRunner(steps, b.config.PackerConfig, ui) b.runner.Run(b.stateBag) // Report any errors. @@ -198,19 +198,6 @@ func (b *Builder) Cancel() { } } -func (b *Builder) createRunner(steps *[]multistep.Step, ui packer.Ui) multistep.Runner { - if b.config.PackerDebug { - return &multistep.DebugRunner{ - Steps: *steps, - PauseFn: packerCommon.MultistepDebugFn(ui), - } - } - - return &multistep.BasicRunner{ - Steps: *steps, - } -} - func (b *Builder) getBlobEndpoint(client *AzureClient, resourceGroupName string, storageAccountName string) (string, error) { account, err := client.AccountsClient.GetProperties(resourceGroupName, storageAccountName) if err != nil { diff --git a/builder/digitalocean/builder.go b/builder/digitalocean/builder.go index 0c2cf9686..bafad7522 100644 --- a/builder/digitalocean/builder.go +++ b/builder/digitalocean/builder.go @@ -73,15 +73,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } // Run the steps - if b.config.PackerDebug { - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: common.MultistepDebugFn(ui), - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } - + b.runner = common.NewRunner(steps, b.config.PackerConfig, ui) b.runner.Run(state) // If there was an error, return that diff --git a/builder/docker/builder.go b/builder/docker/builder.go index cfc5bf423..8e7e2cf19 100644 --- a/builder/docker/builder.go +++ b/builder/docker/builder.go @@ -78,15 +78,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe state.Put("driver", driver) // Run! - if b.config.PackerDebug { - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: common.MultistepDebugFn(ui), - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } - + b.runner = common.NewRunner(steps, b.config.PackerConfig, ui) b.runner.Run(state) // If there was an error, return that diff --git a/builder/googlecompute/builder.go b/builder/googlecompute/builder.go index d1c733629..014c40a38 100644 --- a/builder/googlecompute/builder.go +++ b/builder/googlecompute/builder.go @@ -73,14 +73,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } // Run the steps. - if b.config.PackerDebug { - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: common.MultistepDebugFn(ui), - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } + b.runner = common.NewRunner(steps, b.config.PackerConfig, ui) b.runner.Run(state) // Report any errors. diff --git a/builder/null/builder.go b/builder/null/builder.go index 949c01c2b..cce9227d6 100644 --- a/builder/null/builder.go +++ b/builder/null/builder.go @@ -46,15 +46,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe state.Put("ui", ui) // Run! - if b.config.PackerDebug { - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: common.MultistepDebugFn(ui), - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } - + b.runner = common.NewRunner(steps, b.config.PackerConfig, ui) b.runner.Run(state) // If there was an error, return that diff --git a/builder/openstack/builder.go b/builder/openstack/builder.go index d83086182..2e7f00e7f 100644 --- a/builder/openstack/builder.go +++ b/builder/openstack/builder.go @@ -116,15 +116,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } // Run! - if b.config.PackerDebug { - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: common.MultistepDebugFn(ui), - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } - + b.runner = common.NewRunner(steps, b.config.PackerConfig, ui) b.runner.Run(state) // If there was an error, return that diff --git a/builder/parallels/iso/builder.go b/builder/parallels/iso/builder.go index 134f980f3..359a439b5 100644 --- a/builder/parallels/iso/builder.go +++ b/builder/parallels/iso/builder.go @@ -215,17 +215,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe state.Put("ui", ui) // Run - if b.config.PackerDebug { - pauseFn := common.MultistepDebugFn(ui) - state.Put("pauseFn", pauseFn) - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: pauseFn, - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } - + b.runner = common.NewRunnerWithPauseFn(steps, b.config.PackerConfig, ui, state) b.runner.Run(state) // If there was an error, return that diff --git a/builder/parallels/pvm/builder.go b/builder/parallels/pvm/builder.go index b62181b06..45aed55bf 100644 --- a/builder/parallels/pvm/builder.go +++ b/builder/parallels/pvm/builder.go @@ -108,16 +108,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } // Run the steps. - if b.config.PackerDebug { - pauseFn := common.MultistepDebugFn(ui) - state.Put("pauseFn", pauseFn) - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: pauseFn, - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } + b.runner = common.NewRunnerWithPauseFn(steps, b.config.PackerConfig, ui, state) b.runner.Run(state) // Report any errors. diff --git a/builder/qemu/builder.go b/builder/qemu/builder.go index 7b9630b76..82bff8d74 100644 --- a/builder/qemu/builder.go +++ b/builder/qemu/builder.go @@ -422,17 +422,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe state.Put("ui", ui) // Run - if b.config.PackerDebug { - pauseFn := common.MultistepDebugFn(ui) - state.Put("pauseFn", pauseFn) - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: pauseFn, - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } - + b.runner = common.NewRunnerWithPauseFn(steps, b.config.PackerConfig, ui, state) b.runner.Run(state) // If there was an error, return that diff --git a/builder/virtualbox/iso/builder.go b/builder/virtualbox/iso/builder.go index 0d0790ea1..a69ad83ea 100644 --- a/builder/virtualbox/iso/builder.go +++ b/builder/virtualbox/iso/builder.go @@ -275,17 +275,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe state.Put("ui", ui) // Run - if b.config.PackerDebug { - pauseFn := common.MultistepDebugFn(ui) - state.Put("pauseFn", pauseFn) - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: pauseFn, - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } - + b.runner = common.NewRunnerWithPauseFn(steps, b.config.PackerConfig, ui, state) b.runner.Run(state) // If there was an error, return that diff --git a/builder/virtualbox/ovf/builder.go b/builder/virtualbox/ovf/builder.go index 502f66734..56e349b83 100644 --- a/builder/virtualbox/ovf/builder.go +++ b/builder/virtualbox/ovf/builder.go @@ -135,16 +135,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } // Run the steps. - if b.config.PackerDebug { - pauseFn := common.MultistepDebugFn(ui) - state.Put("pauseFn", pauseFn) - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: pauseFn, - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } + b.runner = common.NewRunnerWithPauseFn(steps, b.config.PackerConfig, ui, state) b.runner.Run(state) // Report any errors. diff --git a/builder/vmware/iso/builder.go b/builder/vmware/iso/builder.go index 5a6495cb2..4d43b3685 100755 --- a/builder/vmware/iso/builder.go +++ b/builder/vmware/iso/builder.go @@ -305,17 +305,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } // Run! - if b.config.PackerDebug { - pauseFn := common.MultistepDebugFn(ui) - state.Put("pauseFn", pauseFn) - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: pauseFn, - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } - + b.runner = common.NewRunnerWithPauseFn(steps, b.config.PackerConfig, ui, state) b.runner.Run(state) // If there was an error, return that diff --git a/builder/vmware/vmx/builder.go b/builder/vmware/vmx/builder.go index d82f03f1c..cf2df99ea 100644 --- a/builder/vmware/vmx/builder.go +++ b/builder/vmware/vmx/builder.go @@ -122,16 +122,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe } // Run the steps. - if b.config.PackerDebug { - pauseFn := common.MultistepDebugFn(ui) - state.Put("pauseFn", pauseFn) - b.runner = &multistep.DebugRunner{ - Steps: steps, - PauseFn: pauseFn, - } - } else { - b.runner = &multistep.BasicRunner{Steps: steps} - } + b.runner = common.NewRunnerWithPauseFn(steps, b.config.PackerConfig, ui, state) b.runner.Run(state) // Report any errors. diff --git a/command/build.go b/command/build.go index 5d683d4b0..7cbc6bed2 100644 --- a/command/build.go +++ b/command/build.go @@ -20,11 +20,13 @@ type BuildCommand struct { func (c BuildCommand) Run(args []string) int { var cfgColor, cfgDebug, cfgForce, cfgParallel bool + var cfgOnError string flags := c.Meta.FlagSet("build", FlagSetBuildFilter|FlagSetVars) flags.Usage = func() { c.Ui.Say(c.Help()) } flags.BoolVar(&cfgColor, "color", true, "") flags.BoolVar(&cfgDebug, "debug", false, "") flags.BoolVar(&cfgForce, "force", false, "") + flags.StringVar(&cfgOnError, "on-error", "cleanup", "") flags.BoolVar(&cfgParallel, "parallel", true, "") if err := flags.Parse(args); err != nil { return 1 @@ -99,12 +101,14 @@ func (c BuildCommand) Run(args []string) int { log.Printf("Build debug mode: %v", cfgDebug) log.Printf("Force build: %v", cfgForce) + log.Printf("On error: %v", cfgOnError) // Set the debug and force mode and prepare all the builds for _, b := range builds { log.Printf("Preparing build: %s", b.Name()) b.SetDebug(cfgDebug) b.SetForce(cfgForce) + b.SetOnError(cfgOnError) warnings, err := b.Prepare() if err != nil { @@ -284,6 +288,9 @@ Options: -except=foo,bar,baz Build all builds other than these -force Force a build to continue if artifacts exist, deletes existing artifacts -machine-readable Machine-readable output + -on-error=abort When a builder fails, abort without cleanup + -on-error=ask When a builder fails, prompt for action + -on-error=cleanup When a builder fails, clean up and exit (the default) -only=foo,bar,baz Only build the given builds by name -parallel=false Disable parallelization (on by default) -var 'key=value' Variable for templates, can be used multiple times. diff --git a/common/multistep_runner.go b/common/multistep_runner.go new file mode 100644 index 000000000..7dda2bbd0 --- /dev/null +++ b/common/multistep_runner.go @@ -0,0 +1,148 @@ +package common + +import ( + "fmt" + "log" + "os" + "reflect" + "time" + + "github.com/mitchellh/multistep" + "github.com/mitchellh/packer/packer" +) + +func newRunner(steps []multistep.Step, config PackerConfig, ui packer.Ui) (multistep.Runner, interface{}) { + switch config.PackerOnError { + case "cleanup", "": + case "abort": + for i, step := range steps { + steps[i] = abortStep{step, ui} + } + case "ask": + for i, step := range steps { + steps[i] = askStep{step, ui} + } + default: + ui.Error(fmt.Sprintf("Ignoring unknown on-error value %q", config.PackerOnError)) + } + + if config.PackerDebug { + pauseFn := MultistepDebugFn(ui) + return &multistep.DebugRunner{Steps: steps, PauseFn: pauseFn}, pauseFn + } else { + return &multistep.BasicRunner{Steps: steps}, nil + } +} + +func NewRunner(steps []multistep.Step, config PackerConfig, ui packer.Ui) multistep.Runner { + runner, _ := newRunner(steps, config, ui) + return runner +} + +func NewRunnerWithPauseFn(steps []multistep.Step, config PackerConfig, ui packer.Ui, state multistep.StateBag) multistep.Runner { + runner, pauseFn := newRunner(steps, config, ui) + if pauseFn != nil { + state.Put("pauseFn", pauseFn) + } + return runner +} + +func typeName(i interface{}) string { + return reflect.Indirect(reflect.ValueOf(i)).Type().Name() +} + +type abortStep struct { + step multistep.Step + ui packer.Ui +} + +func (s abortStep) Run(state multistep.StateBag) multistep.StepAction { + return s.step.Run(state) +} + +func (s abortStep) Cleanup(state multistep.StateBag) { + if _, ok := state.GetOk(multistep.StateCancelled); ok { + s.ui.Error("Interrupted, aborting...") + os.Exit(1) + } + if _, ok := state.GetOk(multistep.StateHalted); ok { + s.ui.Error(fmt.Sprintf("Step %q failed, aborting...", typeName(s.step))) + os.Exit(1) + } + s.step.Cleanup(state) +} + +type askStep struct { + step multistep.Step + ui packer.Ui +} + +func (s askStep) Run(state multistep.StateBag) (action multistep.StepAction) { + for { + action = s.step.Run(state) + + if action != multistep.ActionHalt { + return + } + + switch ask(s.ui, typeName(s.step), state) { + case askCleanup: + return + case askAbort: + os.Exit(1) + case askRetry: + continue + } + } +} + +func (s askStep) Cleanup(state multistep.StateBag) { + s.step.Cleanup(state) +} + +type askResponse int + +const ( + askCleanup askResponse = iota + askAbort + askRetry +) + +func ask(ui packer.Ui, name string, state multistep.StateBag) askResponse { + ui.Say(fmt.Sprintf("Step %q failed", name)) + + result := make(chan askResponse) + go func() { + result <- askPrompt(ui) + }() + + for { + select { + case response := <-result: + return response + case <-time.After(100 * time.Millisecond): + if _, ok := state.GetOk(multistep.StateCancelled); ok { + return askCleanup + } + } + } +} + +func askPrompt(ui packer.Ui) askResponse { + for { + line, err := ui.Ask("[C]lean up and exit, [A]bort without cleanup, or [R]etry step (build may fail even if retry succeeds)? [car]") + if err != nil { + log.Printf("Error asking for input: %s", err) + } + + switch { + case len(line) == 0 || line[0] == 'c': + return askCleanup + case line[0] == 'a': + return askAbort + case line[0] == 'r': + return askRetry + } + ui.Say(fmt.Sprintf("Incorrect input: %#v", line)) + } +} diff --git a/common/packer_config.go b/common/packer_config.go index 2ef86e582..3a14f64b5 100644 --- a/common/packer_config.go +++ b/common/packer_config.go @@ -8,5 +8,6 @@ type PackerConfig struct { PackerBuilderType string `mapstructure:"packer_builder_type"` PackerDebug bool `mapstructure:"packer_debug"` PackerForce bool `mapstructure:"packer_force"` + PackerOnError string `mapstructure:"packer_on_error"` PackerUserVars map[string]string `mapstructure:"packer_user_variables"` } diff --git a/packer/build.go b/packer/build.go index 757ef89eb..bd3c2acbc 100644 --- a/packer/build.go +++ b/packer/build.go @@ -24,6 +24,12 @@ const ( // force build is enabled. ForceConfigKey = "packer_force" + // This key determines what to do when a normal multistep step fails + // - "cleanup" - run cleanup steps + // - "abort" - exit without cleanup + // - "ask" - ask the user + OnErrorConfigKey = "packer_on_error" + // TemplatePathKey is the path to the template that configured this build TemplatePathKey = "packer_template_path" @@ -67,6 +73,12 @@ type Build interface { // When SetForce is set to true, existing artifacts from the build are // deleted prior to the build. SetForce(bool) + + // SetOnError will determines what to do when a normal multistep step fails + // - "cleanup" - run cleanup steps + // - "abort" - exit without cleanup + // - "ask" - ask the user + SetOnError(string) } // A build struct represents a single build job, the result of which should @@ -86,6 +98,7 @@ type coreBuild struct { debug bool force bool + onError string l sync.Mutex prepareCalled bool } @@ -129,6 +142,7 @@ func (b *coreBuild) Prepare() (warn []string, err error) { BuilderTypeConfigKey: b.builderType, DebugConfigKey: b.debug, ForceConfigKey: b.force, + OnErrorConfigKey: b.onError, TemplatePathKey: b.templatePath, UserVariablesConfigKey: b.variables, } @@ -306,6 +320,14 @@ func (b *coreBuild) SetForce(val bool) { b.force = val } +func (b *coreBuild) SetOnError(val string) { + if b.prepareCalled { + panic("prepare has already been called") + } + + b.onError = val +} + // Cancels the build if it is running. func (b *coreBuild) Cancel() { b.builder.Cancel() diff --git a/packer/build_test.go b/packer/build_test.go index e29318972..804ecbaf4 100644 --- a/packer/build_test.go +++ b/packer/build_test.go @@ -23,6 +23,7 @@ func testBuild() *coreBuild { }, }, variables: make(map[string]string), + onError: "cleanup", } } @@ -32,6 +33,7 @@ func testDefaultPackerConfig() map[string]interface{} { BuilderTypeConfigKey: "foo", DebugConfigKey: false, ForceConfigKey: false, + OnErrorConfigKey: "cleanup", TemplatePathKey: "", UserVariablesConfigKey: make(map[string]string), } diff --git a/packer/rpc/build.go b/packer/rpc/build.go index 4d0b7edf4..7a99a54a4 100644 --- a/packer/rpc/build.go +++ b/packer/rpc/build.go @@ -1,8 +1,9 @@ package rpc import ( - "github.com/mitchellh/packer/packer" "net/rpc" + + "github.com/mitchellh/packer/packer" ) // An implementation of packer.Build where the build is actually executed @@ -79,6 +80,12 @@ func (b *build) SetForce(val bool) { } } +func (b *build) SetOnError(val string) { + if err := b.client.Call("Build.SetOnError", val, new(interface{})); err != nil { + panic(err) + } +} + func (b *build) Cancel() { if err := b.client.Call("Build.Cancel", new(interface{}), new(interface{})); err != nil { panic(err) @@ -134,6 +141,11 @@ func (b *BuildServer) SetForce(val *bool, reply *interface{}) error { return nil } +func (b *BuildServer) SetOnError(val *string, reply *interface{}) error { + b.build.SetOnError(*val) + return nil +} + func (b *BuildServer) Cancel(args *interface{}, reply *interface{}) error { b.build.Cancel() return nil diff --git a/packer/rpc/build_test.go b/packer/rpc/build_test.go index d2f058e7d..57a4b8dc2 100644 --- a/packer/rpc/build_test.go +++ b/packer/rpc/build_test.go @@ -2,23 +2,25 @@ package rpc import ( "errors" - "github.com/mitchellh/packer/packer" "reflect" "testing" + + "github.com/mitchellh/packer/packer" ) var testBuildArtifact = &packer.MockArtifact{} type testBuild struct { - nameCalled bool - prepareCalled bool - prepareWarnings []string - runCalled bool - runCache packer.Cache - runUi packer.Ui - setDebugCalled bool - setForceCalled bool - cancelCalled bool + nameCalled bool + prepareCalled bool + prepareWarnings []string + runCalled bool + runCache packer.Cache + runUi packer.Ui + setDebugCalled bool + setForceCalled bool + setOnErrorCalled bool + cancelCalled bool errRunResult bool } @@ -53,6 +55,10 @@ func (b *testBuild) SetForce(bool) { b.setForceCalled = true } +func (b *testBuild) SetOnError(string) { + b.setOnErrorCalled = true +} + func (b *testBuild) Cancel() { b.cancelCalled = true } @@ -116,6 +122,12 @@ func TestBuild(t *testing.T) { t.Fatal("should be called") } + // Test SetOnError + bClient.SetOnError("ask") + if !b.setOnErrorCalled { + t.Fatal("should be called") + } + // Test Cancel bClient.Cancel() if !b.cancelCalled { From e9cc28565bcc95862c44cfbe131946017f247458 Mon Sep 17 00:00:00 2001 From: Orivej Desh Date: Wed, 14 Sep 2016 01:00:58 +0000 Subject: [PATCH 2/6] Document -on-error on the "packer build" page --- command/build.go | 6 +++--- website/source/docs/command-line/build.html.md | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/command/build.go b/command/build.go index 7cbc6bed2..55f3e5b92 100644 --- a/command/build.go +++ b/command/build.go @@ -288,9 +288,9 @@ Options: -except=foo,bar,baz Build all builds other than these -force Force a build to continue if artifacts exist, deletes existing artifacts -machine-readable Machine-readable output - -on-error=abort When a builder fails, abort without cleanup - -on-error=ask When a builder fails, prompt for action - -on-error=cleanup When a builder fails, clean up and exit (the default) + -on-error=abort If the build fails, abort without cleanup + -on-error=ask If the build fails, prompt for action + -on-error=cleanup If the build fails, clean up and exit (the default) -only=foo,bar,baz Only build the given builds by name -parallel=false Disable parallelization (on by default) -var 'key=value' Variable for templates, can be used multiple times. diff --git a/website/source/docs/command-line/build.html.md b/website/source/docs/command-line/build.html.md index ba4421293..9ee339bc8 100644 --- a/website/source/docs/command-line/build.html.md +++ b/website/source/docs/command-line/build.html.md @@ -36,6 +36,13 @@ artifacts that are created will be outputted at the end of the build. remove the artifacts from the previous build. This will allow the user to repeat a build without having to manually clean these artifacts beforehand. +- `-on-error=cleanup` (default), `-on-error=abort`, `-on-error=ask` - Selects + what to do when the build fails. `cleanup` cleans up after the previous + steps, deleting temporary files and virtual machines. `abort` exits without + any cleanup, but the next build may require `-force`. `ask` presents a + prompt and waits for you to decide to clean up, abort, or retry the failed + step. + - `-only=foo,bar,baz` - Only build the builds with the given comma-separated names. Build names by default are the names of their builders, unless a specific `name` attribute is specified within From 115cb5080f313e9fb0313e0f499b9234e6696eda Mon Sep 17 00:00:00 2001 From: Orivej Desh Date: Wed, 14 Sep 2016 01:11:36 +0000 Subject: [PATCH 3/6] Document NewRunner --- common/multistep_runner.go | 8 +++++++- website/source/docs/command-line/build.html.md | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/common/multistep_runner.go b/common/multistep_runner.go index 7dda2bbd0..1b8a6d9cd 100644 --- a/common/multistep_runner.go +++ b/common/multistep_runner.go @@ -11,7 +11,7 @@ import ( "github.com/mitchellh/packer/packer" ) -func newRunner(steps []multistep.Step, config PackerConfig, ui packer.Ui) (multistep.Runner, interface{}) { +func newRunner(steps []multistep.Step, config PackerConfig, ui packer.Ui) (multistep.Runner, multistep.DebugPauseFn) { switch config.PackerOnError { case "cleanup", "": case "abort": @@ -34,11 +34,17 @@ func newRunner(steps []multistep.Step, config PackerConfig, ui packer.Ui) (multi } } +// NewRunner returns a multistep.Runner that runs steps augmented with support +// for -debug and -on-error command line arguments. func NewRunner(steps []multistep.Step, config PackerConfig, ui packer.Ui) multistep.Runner { runner, _ := newRunner(steps, config, ui) return runner } +// NewRunnerWithPauseFn returns a multistep.Runner that runs steps augmented +// with support for -debug and -on-error command line arguments. With -debug it +// puts the multistep.DebugPauseFn that will pause execution between steps into +// the state under the key "pauseFn". func NewRunnerWithPauseFn(steps []multistep.Step, config PackerConfig, ui packer.Ui, state multistep.StateBag) multistep.Runner { runner, pauseFn := newRunner(steps, config, ui) if pauseFn != nil { diff --git a/website/source/docs/command-line/build.html.md b/website/source/docs/command-line/build.html.md index 9ee339bc8..6939173e7 100644 --- a/website/source/docs/command-line/build.html.md +++ b/website/source/docs/command-line/build.html.md @@ -47,3 +47,6 @@ artifacts that are created will be outputted at the end of the build. comma-separated names. Build names by default are the names of their builders, unless a specific `name` attribute is specified within the configuration. + +- `-parallel=false` - Disable parallelization of multiple builders (on by + default). From 389603cc0f5e8c10eb19aef6990a2bbd1e880e37 Mon Sep 17 00:00:00 2001 From: Orivej Desh Date: Wed, 14 Sep 2016 09:29:20 +0000 Subject: [PATCH 4/6] Allow upper case input to -on-error=ask --- common/multistep_runner.go | 16 +++++++++------- packer/build.go | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/common/multistep_runner.go b/common/multistep_runner.go index 1b8a6d9cd..77e29dcf4 100644 --- a/common/multistep_runner.go +++ b/common/multistep_runner.go @@ -5,6 +5,7 @@ import ( "log" "os" "reflect" + "strings" "time" "github.com/mitchellh/multistep" @@ -13,7 +14,7 @@ import ( func newRunner(steps []multistep.Step, config PackerConfig, ui packer.Ui) (multistep.Runner, multistep.DebugPauseFn) { switch config.PackerOnError { - case "cleanup", "": + case "cleanup": case "abort": for i, step := range steps { steps[i] = abortStep{step, ui} @@ -23,7 +24,7 @@ func newRunner(steps []multistep.Step, config PackerConfig, ui packer.Ui) (multi steps[i] = askStep{step, ui} } default: - ui.Error(fmt.Sprintf("Ignoring unknown on-error value %q", config.PackerOnError)) + ui.Error(fmt.Sprintf("Ignoring -on-error=%q argument: unknown on-error value", config.PackerOnError)) } if config.PackerDebug { @@ -136,17 +137,18 @@ func ask(ui packer.Ui, name string, state multistep.StateBag) askResponse { func askPrompt(ui packer.Ui) askResponse { for { - line, err := ui.Ask("[C]lean up and exit, [A]bort without cleanup, or [R]etry step (build may fail even if retry succeeds)? [car]") + line, err := ui.Ask("[C]lean up and exit, [a]bort without cleanup, or [r]etry step (build may fail even if retry succeeds)?") if err != nil { log.Printf("Error asking for input: %s", err) } - switch { - case len(line) == 0 || line[0] == 'c': + input := strings.ToLower(line) + "c" + switch input[0] { + case 'c': return askCleanup - case line[0] == 'a': + case 'a': return askAbort - case line[0] == 'r': + case 'r': return askRetry } ui.Say(fmt.Sprintf("Incorrect input: %#v", line)) diff --git a/packer/build.go b/packer/build.go index bd3c2acbc..a7aa5973a 100644 --- a/packer/build.go +++ b/packer/build.go @@ -74,7 +74,7 @@ type Build interface { // deleted prior to the build. SetForce(bool) - // SetOnError will determines what to do when a normal multistep step fails + // SetOnError will determine what to do when a normal multistep step fails // - "cleanup" - run cleanup steps // - "abort" - exit without cleanup // - "ask" - ask the user From 639bf356aa5c04b41e899a05113bf8df591ba6cb Mon Sep 17 00:00:00 2001 From: Orivej Desh Date: Sat, 17 Sep 2016 14:34:37 +0000 Subject: [PATCH 5/6] Fail on unknown values of -on-error --- command/build.go | 4 +++- common/multistep_runner.go | 4 +--- helper/enumflag/flag.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 helper/enumflag/flag.go diff --git a/command/build.go b/command/build.go index 55f3e5b92..89e5101df 100644 --- a/command/build.go +++ b/command/build.go @@ -10,6 +10,7 @@ import ( "strings" "sync" + "github.com/mitchellh/packer/helper/enumflag" "github.com/mitchellh/packer/packer" "github.com/mitchellh/packer/template" ) @@ -26,7 +27,8 @@ func (c BuildCommand) Run(args []string) int { flags.BoolVar(&cfgColor, "color", true, "") flags.BoolVar(&cfgDebug, "debug", false, "") flags.BoolVar(&cfgForce, "force", false, "") - flags.StringVar(&cfgOnError, "on-error", "cleanup", "") + flagOnError := enumflag.New(&cfgOnError, "cleanup", "abort", "ask") + flags.Var(flagOnError, "on-error", "") flags.BoolVar(&cfgParallel, "parallel", true, "") if err := flags.Parse(args); err != nil { return 1 diff --git a/common/multistep_runner.go b/common/multistep_runner.go index 77e29dcf4..edacee6a0 100644 --- a/common/multistep_runner.go +++ b/common/multistep_runner.go @@ -14,7 +14,7 @@ import ( func newRunner(steps []multistep.Step, config PackerConfig, ui packer.Ui) (multistep.Runner, multistep.DebugPauseFn) { switch config.PackerOnError { - case "cleanup": + case "", "cleanup": case "abort": for i, step := range steps { steps[i] = abortStep{step, ui} @@ -23,8 +23,6 @@ func newRunner(steps []multistep.Step, config PackerConfig, ui packer.Ui) (multi for i, step := range steps { steps[i] = askStep{step, ui} } - default: - ui.Error(fmt.Sprintf("Ignoring -on-error=%q argument: unknown on-error value", config.PackerOnError)) } if config.PackerDebug { diff --git a/helper/enumflag/flag.go b/helper/enumflag/flag.go new file mode 100644 index 000000000..ab6dce29b --- /dev/null +++ b/helper/enumflag/flag.go @@ -0,0 +1,28 @@ +package enumflag + +import "fmt" + +type enumFlag struct { + target *string + options []string +} + +// New returns a flag.Value implementation for parsing flags with a one-of-a-set value +func New(target *string, options ...string) *enumFlag { + return &enumFlag{target: target, options: options} +} + +func (f *enumFlag) String() string { + return *f.target +} + +func (f *enumFlag) Set(value string) error { + for _, v := range f.options { + if v == value { + *f.target = value + return nil + } + } + + return fmt.Errorf("expected one of %q", f.options) +} From 4fe86244a501f35c60dc7f5269da21058016915c Mon Sep 17 00:00:00 2001 From: Orivej Desh Date: Sun, 18 Sep 2016 02:48:53 +0000 Subject: [PATCH 6/6] Improve -on-error descriptions --- command/build.go | 5 +---- common/multistep_runner.go | 2 +- website/source/docs/command-line/build.html.md | 6 +++--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/command/build.go b/command/build.go index 89e5101df..8f7b3d8ad 100644 --- a/command/build.go +++ b/command/build.go @@ -290,10 +290,7 @@ Options: -except=foo,bar,baz Build all builds other than these -force Force a build to continue if artifacts exist, deletes existing artifacts -machine-readable Machine-readable output - -on-error=abort If the build fails, abort without cleanup - -on-error=ask If the build fails, prompt for action - -on-error=cleanup If the build fails, clean up and exit (the default) - -only=foo,bar,baz Only build the given builds by name + -on-error=[cleanup|abort|ask] If the build fails do: clean up (default), abort, or ask -parallel=false Disable parallelization (on by default) -var 'key=value' Variable for templates, can be used multiple times. -var-file=path JSON file containing user variables. diff --git a/common/multistep_runner.go b/common/multistep_runner.go index edacee6a0..a9b155086 100644 --- a/common/multistep_runner.go +++ b/common/multistep_runner.go @@ -135,7 +135,7 @@ func ask(ui packer.Ui, name string, state multistep.StateBag) askResponse { func askPrompt(ui packer.Ui) askResponse { for { - line, err := ui.Ask("[C]lean up and exit, [a]bort without cleanup, or [r]etry step (build may fail even if retry succeeds)?") + line, err := ui.Ask("[c] Clean up and exit, [a] abort without cleanup, or [r] retry step (build may fail even if retry succeeds)?") if err != nil { log.Printf("Error asking for input: %s", err) } diff --git a/website/source/docs/command-line/build.html.md b/website/source/docs/command-line/build.html.md index 6939173e7..12a2175be 100644 --- a/website/source/docs/command-line/build.html.md +++ b/website/source/docs/command-line/build.html.md @@ -39,9 +39,9 @@ artifacts that are created will be outputted at the end of the build. - `-on-error=cleanup` (default), `-on-error=abort`, `-on-error=ask` - Selects what to do when the build fails. `cleanup` cleans up after the previous steps, deleting temporary files and virtual machines. `abort` exits without - any cleanup, but the next build may require `-force`. `ask` presents a - prompt and waits for you to decide to clean up, abort, or retry the failed - step. + any cleanup, which might require the next build to use `-force`. `ask` + presents a prompt and waits for you to decide to clean up, abort, or retry + the failed step. - `-only=foo,bar,baz` - Only build the builds with the given comma-separated names. Build names by default are the names of their