Add -on-error command line argument to allow preserving artifacts on builder errors #3885

orivej · 2016-09-14T00:16:24Z

This pull request resolves #409 by providing an option to delay or skip cleanup after unsuccessful packer build:

  -on-error=cleanup          When a builder fails, clean up and exit (the default)
  -on-error=abort            When a builder fails, abort without cleanup
  -on-error=ask              When a builder fails, prompt for action

Running with -on-error=ask:

==> qemu: Step "StepProvision" failed
==> qemu: [C]lean up and exit, [A]bort without cleanup, or [R]etry step (build may fail even if retry succeeds)? [car] |

rickard-von-essen · 2016-09-14T00:42:32Z

Looks interesting! This is missing some documentation. (Currently the build is busted, you can ignore Travis)

boumenot · 2016-09-14T04:24:59Z

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)


I really like how this code is simplified for every builder.

boumenot · 2016-09-14T04:33:06Z

Azure stuff LGTM. Thanks for the contribution.

…builder errors Resolves hashicorp#409

rickard-von-essen · 2016-09-17T12:31:34Z

We should fail on unknown values.

==> parallels-iso: Ignoring -on-error="ASK" argument: unknown on-error value

orivej · 2016-09-17T14:38:25Z

Done. Is the PR still missing some documentation?

rickard-von-essen

Everything works really great! I just have some comments about text and appearance. After they have been adjusted/discussed I would like to merge this, but I'd like the other maintainers to have a few days to also add their input.

rickard-von-essen · 2016-09-17T12:33:02Z

common/multistep_runner.go

+			steps[i] = askStep{step, ui}
+		}
+	default:
+		ui.Error(fmt.Sprintf("Ignoring -on-error=%q argument: unknown on-error value", config.PackerOnError))


This should instead make packer exit.

I have already deleted the default clause since it can no longer happen. Now enumflag makes packer exit with an error like invalid value "ASK" for flag -on-error: expected one of ["cleanup" "abort" "ask"]

rickard-von-essen · 2016-09-17T19:38:33Z

website/source/docs/command-line/build.html.md

 -   `-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
    the configuration.
+
+-   `-parallel=false` - Disable parallelization of multiple builders (on by
+    default).


rickard-von-essen · 2016-09-17T19:41:47Z

website/source/docs/command-line/build.html.md

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


abort exits without any cleanup, but the next build may require -force.

Maybe rephrase to:

abort exits without any cleanup, which might require the next build to use -force.

rickard-von-essen · 2016-09-17T19:45:34Z

common/multistep_runner.go

+
+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)?")


[C]lean up and exit, [a]bort without cleanup, or [r]etry step (build may fail even if retry succeeds)?

I think I ratter liked it to look like:

[c] Clean up and exit, [a] abort without cleanup, or [r] retry step (build may fail even if retry succeeds)?

Which makes it a bit more easy to read.

rickard-von-essen · 2016-09-17T19:49:14Z

command/build.go

@@ -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            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)


I would prefer if they where on the same line, maybe:

-on-error=[cleanup|ask|abort] If the build fails do: clean up (default), ask, or abort

I think it's clear enough and the details are in the docs.

rickard-von-essen · 2016-09-17T19:52:55Z

Is the PR still missing some documentation?

No except for the comments I just gave I think this is done (and great).

rickard-von-essen · 2016-09-20T10:28:02Z

LGTM

Thanks for this great improvement!

robmorgan · 2016-09-20T10:29:57Z

thanks @orivej 👍

gregorskii · 2016-10-15T20:05:37Z

Hi,

This was merged into master 25 days ago, and the last release v0.10.2 was cut 16 days ago, but this update is not in the release 0.10.2 on the site, or in homebrew. I had to build from source to get it. Is this a known issue?

rickard-von-essen · 2016-10-15T20:28:38Z

It's not an issue, it's just read the CHANGELOG.md 😉 or:

 $ git log1 --graph --decorate=full -10 v0.10.2 v0.10.1                                                                                    14s  Sat Oct 15 22:25:18 2016
* 9e8a3fa (tag: refs/tags/v0.10.2) bump gem deps and www version
* fb1c968 Cut version 0.10.2
* 1414401 Cut version 0.10.2
* bb92eef Cut version 0.10.2
* 489c75a Go\'s -X linker flag now requires only one argument (#3540)
* 7bbc29d Fix go vet casing issue
* 8e7bb5e Change version.go to 0.10.1 release
| * 4e5f651 (tag: refs/tags/v0.10.1) Cut version 0.10.1
| * 2c27461 Change version.go to 0.10.1 release
|/
* aa47c90 Udpdated test/race timeouts to 2m because AWS seems to be taking a while

v0.10.2 is a rebuild of v0.10.1.

gregorskii · 2016-10-15T20:32:55Z

Ah ok, will pay more attention to the change logs!. Will await 0.10.3. Also this feature works great. 👍

orivej · 2016-10-15T20:47:35Z

@gregorskii Thanks! Which mode do you use: abort, ask, or both?

gregorskii · 2016-10-15T20:49:54Z

Currently ask, as I am debugging some googlecompute Ansible and Chef work, was pulling my hair out last weekend debugging roles as they would fail, and delete the instance before I could SSH in and poke around my work to see if the correct files and config was in place. Having ask when building out Packer files is a life saver.

lklepner · 2017-05-02T14:25:24Z

It would be great if -on-error=ask spit out a SSH key and IP addresses along with the prompt. This would make it easy to jump into the build to do some debugging prior to selecting cleanup or abort.

petemounce · 2017-06-28T14:57:31Z

Similarly, for Windows, if the communicator is winrm and a password was generated, it would be really handy to dump that password to output so it can be used to log in to the failed build to inspect.

SwampDragons · 2017-06-28T16:19:10Z

Given that the main conversation on this branch happened last year, it's a lot easier for us to track feature requests if you open a new ticket -- or, even better, a PR :)

orivej force-pushed the on-error branch from 664a4ca to bed3ba4 Compare September 14, 2016 00:38

rickard-von-essen added enhancement need-documentation core Core components of Packer pr/needs-review labels Sep 14, 2016

orivej force-pushed the on-error branch 5 times, most recently from 6d51023 to 897463e Compare September 14, 2016 01:22

orivej mentioned this pull request Sep 14, 2016

--no-destroy-on-error like Vagrant #409

Closed

boumenot reviewed Sep 14, 2016
View reviewed changes

orivej force-pushed the on-error branch 2 times, most recently from d584c09 to 3381f96 Compare September 16, 2016 12:13

orivej added 4 commits September 16, 2016 12:15

Add -on-error command line argument to allow preserving artifacts on …

6762965

…builder errors Resolves hashicorp#409

Document -on-error on the "packer build" page

e9cc285

Document NewRunner

115cb50

Allow upper case input to -on-error=ask

389603c

orivej force-pushed the on-error branch from 3381f96 to 389603c Compare September 16, 2016 12:15

orivej force-pushed the on-error branch from dacae90 to b912c40 Compare September 17, 2016 14:37

orivej force-pushed the on-error branch from b912c40 to 7a8cc4e Compare September 17, 2016 14:40

Fail on unknown values of -on-error

639bf35

orivej force-pushed the on-error branch from 7a8cc4e to 639bf35 Compare September 17, 2016 14:42

rickard-von-essen reviewed Sep 17, 2016

View reviewed changes

rickard-von-essen self-assigned this Sep 17, 2016

rickard-von-essen removed need-documentation pr/needs-review labels Sep 17, 2016

Improve -on-error descriptions

4fe8624

orivej force-pushed the on-error branch from aec10e5 to 4fe8624 Compare September 18, 2016 03:00

rickard-von-essen added the pr/reviewed label Sep 20, 2016

rickard-von-essen merged commit 13c9db5 into hashicorp:master Sep 20, 2016

rickard-von-essen mentioned this pull request Oct 12, 2016

Packer should not delete docker container if provisioning fails. #3988

Closed

rickard-von-essen mentioned this pull request Nov 11, 2016

Make the --debug pausing after each step optional #1687

Closed

hashicorp locked and limited conversation to collaborators Jun 28, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add -on-error command line argument to allow preserving artifacts on builder errors #3885

Add -on-error command line argument to allow preserving artifacts on builder errors #3885

orivej commented Sep 14, 2016

rickard-von-essen commented Sep 14, 2016

boumenot Sep 14, 2016

boumenot commented Sep 14, 2016

rickard-von-essen commented Sep 17, 2016

orivej commented Sep 17, 2016

rickard-von-essen left a comment

rickard-von-essen Sep 17, 2016

orivej Sep 18, 2016

rickard-von-essen Sep 17, 2016

rickard-von-essen Sep 17, 2016

rickard-von-essen Sep 17, 2016

rickard-von-essen Sep 17, 2016

rickard-von-essen commented Sep 17, 2016

rickard-von-essen commented Sep 20, 2016

robmorgan commented Sep 20, 2016

gregorskii commented Oct 15, 2016 •

edited

Loading

rickard-von-essen commented Oct 15, 2016 •

edited

Loading

gregorskii commented Oct 15, 2016

orivej commented Oct 15, 2016

gregorskii commented Oct 15, 2016

lklepner commented May 2, 2017

petemounce commented Jun 28, 2017

SwampDragons commented Jun 28, 2017

Add -on-error command line argument to allow preserving artifacts on builder errors #3885

Add -on-error command line argument to allow preserving artifacts on builder errors #3885

Conversation

orivej commented Sep 14, 2016

rickard-von-essen commented Sep 14, 2016

boumenot Sep 14, 2016

Choose a reason for hiding this comment

boumenot commented Sep 14, 2016

rickard-von-essen commented Sep 17, 2016

orivej commented Sep 17, 2016

rickard-von-essen left a comment

Choose a reason for hiding this comment

rickard-von-essen Sep 17, 2016

Choose a reason for hiding this comment

orivej Sep 18, 2016

Choose a reason for hiding this comment

rickard-von-essen Sep 17, 2016

Choose a reason for hiding this comment

rickard-von-essen Sep 17, 2016

Choose a reason for hiding this comment

rickard-von-essen Sep 17, 2016

Choose a reason for hiding this comment

rickard-von-essen Sep 17, 2016

Choose a reason for hiding this comment

rickard-von-essen commented Sep 17, 2016

rickard-von-essen commented Sep 20, 2016

robmorgan commented Sep 20, 2016

gregorskii commented Oct 15, 2016 • edited Loading

rickard-von-essen commented Oct 15, 2016 • edited Loading

gregorskii commented Oct 15, 2016

orivej commented Oct 15, 2016

gregorskii commented Oct 15, 2016

lklepner commented May 2, 2017

petemounce commented Jun 28, 2017

SwampDragons commented Jun 28, 2017

gregorskii commented Oct 15, 2016 •

edited

Loading

rickard-von-essen commented Oct 15, 2016 •

edited

Loading