From 54034689ef8fc37f6fc05b7a5b4918152284ea1c Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 25 Jul 2019 16:32:16 -0700 Subject: [PATCH 1/3] On abort, return gracefully rather than exiting so that the subprocess doesn't unexpectedly disconnect from the parent and cause a confusing EOF error in the logs --- common/multistep_runner.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/common/multistep_runner.go b/common/multistep_runner.go index b34ee3a04..0ccc4d349 100644 --- a/common/multistep_runner.go +++ b/common/multistep_runner.go @@ -71,17 +71,29 @@ func (s abortStep) Run(ctx context.Context, state multistep.StateBag) multistep. } func (s abortStep) Cleanup(state multistep.StateBag) { + _, alreadyLogged := state.GetOk("abort_step_logged") err, ok := state.GetOk("error") - if ok { + if ok && !alreadyLogged { s.ui.Error(fmt.Sprintf("%s", err)) + state.Put("abort_step_logged", true) } if _, ok := state.GetOk(multistep.StateCancelled); ok { - s.ui.Error("Interrupted, aborting...") - os.Exit(1) + if !alreadyLogged { + s.ui.Error("Interrupted, aborting...") + state.Put("abort_step_logged", true) + } else { + s.ui.Error(fmt.Sprintf("aborted: skipping cleanup of step %q", typeName(s.step))) + } + return } if _, ok := state.GetOk(multistep.StateHalted); ok { - s.ui.Error(fmt.Sprintf("Step %q failed, aborting...", typeName(s.step))) - os.Exit(1) + if !alreadyLogged { + s.ui.Error(fmt.Sprintf("Step %q failed, aborting...", typeName(s.step))) + state.Put("abort_step_logged", true) + } else { + s.ui.Error(fmt.Sprintf("aborted: skipping cleanup of step %q", typeName(s.step))) + } + return } s.step.Cleanup(state) } From 9912e569e1d6614c80aa7f4ef11956c897ba34d0 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 25 Jul 2019 16:33:02 -0700 Subject: [PATCH 2/3] deduplicate loglines that stream both to ui ERROR call and to streaming logs when PACKER_LOG=1 --- log.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/log.go b/log.go index e5c07c206..1a6722235 100644 --- a/log.go +++ b/log.go @@ -35,6 +35,9 @@ func logOutput() (logOutput io.Writer, err error) { if strings.Contains(scanner.Text(), "ui:") { continue } + if strings.Contains(scanner.Text(), "ui error:") { + continue + } os.Stderr.WriteString(fmt.Sprint(scanner.Text() + "\n")) } }(scanner) From 90c5da40f2a323d649ec54232f5d48a96a09f47b Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 26 Jul 2019 12:24:03 -0700 Subject: [PATCH 3/3] implement abort logic and printing for the askstep implementation as well --- common/multistep_runner.go | 64 ++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/common/multistep_runner.go b/common/multistep_runner.go index 0ccc4d349..b93159fc2 100644 --- a/common/multistep_runner.go +++ b/common/multistep_runner.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "log" - "os" "reflect" "strings" "time" @@ -71,28 +70,8 @@ func (s abortStep) Run(ctx context.Context, state multistep.StateBag) multistep. } func (s abortStep) Cleanup(state multistep.StateBag) { - _, alreadyLogged := state.GetOk("abort_step_logged") - err, ok := state.GetOk("error") - if ok && !alreadyLogged { - s.ui.Error(fmt.Sprintf("%s", err)) - state.Put("abort_step_logged", true) - } - if _, ok := state.GetOk(multistep.StateCancelled); ok { - if !alreadyLogged { - s.ui.Error("Interrupted, aborting...") - state.Put("abort_step_logged", true) - } else { - s.ui.Error(fmt.Sprintf("aborted: skipping cleanup of step %q", typeName(s.step))) - } - return - } - if _, ok := state.GetOk(multistep.StateHalted); ok { - if !alreadyLogged { - s.ui.Error(fmt.Sprintf("Step %q failed, aborting...", typeName(s.step))) - state.Put("abort_step_logged", true) - } else { - s.ui.Error(fmt.Sprintf("aborted: skipping cleanup of step %q", typeName(s.step))) - } + shouldCleanup := handleAbortsAndInterupts(state, s.ui, typeName(s.step)) + if !shouldCleanup { return } s.step.Cleanup(state) @@ -124,7 +103,8 @@ func (s askStep) Run(ctx context.Context, state multistep.StateBag) (action mult case askCleanup: return case askAbort: - os.Exit(1) + state.Put("aborted", true) + return case askRetry: continue } @@ -132,6 +112,12 @@ func (s askStep) Run(ctx context.Context, state multistep.StateBag) (action mult } func (s askStep) Cleanup(state multistep.StateBag) { + if _, ok := state.GetOk("aborted"); ok { + shouldCleanup := handleAbortsAndInterupts(state, s.ui, typeName(s.step)) + if !shouldCleanup { + return + } + } s.step.Cleanup(state) } @@ -182,3 +168,33 @@ func askPrompt(ui packer.Ui) askResponse { ui.Say(fmt.Sprintf("Incorrect input: %#v", line)) } } + +func handleAbortsAndInterupts(state multistep.StateBag, ui packer.Ui, stepName string) bool { + // if returns false, don't run cleanup. If true, do run cleanup. + _, alreadyLogged := state.GetOk("abort_step_logged") + + err, ok := state.GetOk("error") + if ok && !alreadyLogged { + ui.Error(fmt.Sprintf("%s", err)) + state.Put("abort_step_logged", true) + } + if _, ok := state.GetOk(multistep.StateCancelled); ok { + if !alreadyLogged { + ui.Error("Interrupted, aborting...") + state.Put("abort_step_logged", true) + } else { + ui.Error(fmt.Sprintf("aborted: skipping cleanup of step %q", stepName)) + } + return false + } + if _, ok := state.GetOk(multistep.StateHalted); ok { + if !alreadyLogged { + ui.Error(fmt.Sprintf("Step %q failed, aborting...", stepName)) + state.Put("abort_step_logged", true) + } else { + ui.Error(fmt.Sprintf("aborted: skipping cleanup of step %q", stepName)) + } + return false + } + return true +}