From 7cba68326a5ec469b9a9e28be821676334495f04 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 9 Feb 2018 18:10:52 -0500 Subject: [PATCH 1/4] always wait for a RunningOperation to return If the user wishes to interrupt the running operation, only the first interrupt was communicated to the operation by canceling the provided context. A second interrupt would start the shutdown process, but not communicate this to the running operation. This order of event could cause partial writes of state. What would happen is that once the command returns, the plugin system would stop the provider processes. Once the provider processes dies, all pending Eval operations would return return with an error, and quickly cause the operation to complete. Since the backend code didn't know that the process was shutting down imminently, it would continue by attempting to write out the last known state. Under the right conditions, the process would exit part way through the writing of the state file. Add Stop and Cancel CancelFuncs to the RunningOperation, to allow it to easily differentiate between the two signals. The backend will then be able to detect a shutdown and abort more gracefully. In order to ensure that the backend is not in the process of writing the state out, the command will always attempt to wait for the process to complete after cancellation. --- backend/backend.go | 12 ++++++++-- backend/local/backend.go | 25 ++++++++++++++++---- backend/local/backend_apply.go | 21 +++++++++++++---- backend/local/backend_plan.go | 21 +++++++++++++---- backend/local/backend_refresh.go | 21 +++++++++++++---- command/apply.go | 22 +++++++++++++----- command/plan.go | 19 ++++++++++++---- command/refresh.go | 39 ++++++++++++++++++++++++++++---- 8 files changed, 143 insertions(+), 37 deletions(-) diff --git a/backend/backend.go b/backend/backend.go index efa42d4b6..f67d60ab6 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -145,8 +145,6 @@ type Operation struct { // RunningOperation is the result of starting an operation. type RunningOperation struct { - // Context should be used to track Done and Err for errors. - // // For implementers of a backend, this context should not wrap the // passed in context. Otherwise, canceling the parent context will // immediately mark this context as "done" but those aren't the semantics @@ -154,6 +152,16 @@ type RunningOperation struct { // is fully done. context.Context + // Stop requests the operation to complete early, by calling Stop on all + // the plugins. If the process needs to terminate immediately, call Cancel. + Stop context.CancelFunc + + // Cancel is the context.CancelFunc associated with the embedded context, + // and can be called to terminate the operation early. + // Once Cancel is called, the operation should return as soon as possible + // to avoid running operations during process exit. + Cancel context.CancelFunc + // Err is the error of the operation. This is populated after // the operation has completed. Err error diff --git a/backend/local/backend.go b/backend/local/backend.go index 6b119c297..bbebe3e18 100644 --- a/backend/local/backend.go +++ b/backend/local/backend.go @@ -224,7 +224,7 @@ func (b *Local) State(name string) (state.State, error) { // name conflicts, assume that the field is overwritten if set. func (b *Local) Operation(ctx context.Context, op *backend.Operation) (*backend.RunningOperation, error) { // Determine the function to call for our operation - var f func(context.Context, *backend.Operation, *backend.RunningOperation) + var f func(context.Context, context.Context, *backend.Operation, *backend.RunningOperation) switch op.Type { case backend.OperationTypeRefresh: f = b.opRefresh @@ -244,14 +244,29 @@ func (b *Local) Operation(ctx context.Context, op *backend.Operation) (*backend. b.opLock.Lock() // Build our running operation - runningCtx, runningCtxCancel := context.WithCancel(context.Background()) - runningOp := &backend.RunningOperation{Context: runningCtx} + // the runninCtx is only used to block until the operation returns. + runningCtx, done := context.WithCancel(context.Background()) + runningOp := &backend.RunningOperation{ + Context: runningCtx, + } + + // stopCtx wraps the context passed in, and is used to signal a graceful Stop. + stopCtx, stop := context.WithCancel(ctx) + runningOp.Stop = stop + + // cancelCtx is used to cancel the operation immediately, usually + // indicating that the process is exiting. + cancelCtx, cancel := context.WithCancel(context.Background()) + runningOp.Cancel = cancel // Do it go func() { + defer done() + defer stop() + defer cancel() + defer b.opLock.Unlock() - defer runningCtxCancel() - f(ctx, op, runningOp) + f(stopCtx, cancelCtx, op, runningOp) }() // Return diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index 9789e0b7c..f4351af6f 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -19,7 +19,8 @@ import ( ) func (b *Local) opApply( - ctx context.Context, + stopCtx context.Context, + cancelCtx context.Context, op *backend.Operation, runningOp *backend.RunningOperation) { log.Printf("[INFO] backend/local: starting Apply operation") @@ -55,7 +56,7 @@ func (b *Local) opApply( } if op.LockState { - lockCtx, cancel := context.WithTimeout(ctx, op.StateLockTimeout) + lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout) defer cancel() lockInfo := state.NewLockInfo() @@ -157,7 +158,7 @@ func (b *Local) opApply( // we can handle it properly. err = nil select { - case <-ctx.Done(): + case <-stopCtx.Done(): if b.CLI != nil { b.CLI.Output("stopping apply operation...") } @@ -176,8 +177,18 @@ func (b *Local) opApply( // Stop execution go tfCtx.Stop() - // Wait for completion still - <-doneCh + select { + case <-cancelCtx.Done(): + log.Println("[WARN] running operation canceled") + // if the operation was canceled, we need to return immediately + return + case <-doneCh: + } + case <-cancelCtx.Done(): + // this should not be called without first attempting to stop the + // operation + log.Println("[ERROR] running operation canceled without Stop") + return case <-doneCh: } diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index 380ce1742..d8ae0fe90 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -19,7 +19,8 @@ import ( ) func (b *Local) opPlan( - ctx context.Context, + stopCtx context.Context, + cancelCtx context.Context, op *backend.Operation, runningOp *backend.RunningOperation) { log.Printf("[INFO] backend/local: starting Plan operation") @@ -62,7 +63,7 @@ func (b *Local) opPlan( } if op.LockState { - lockCtx, cancel := context.WithTimeout(ctx, op.StateLockTimeout) + lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout) defer cancel() lockInfo := state.NewLockInfo() @@ -112,7 +113,7 @@ func (b *Local) opPlan( }() select { - case <-ctx.Done(): + case <-stopCtx.Done(): if b.CLI != nil { b.CLI.Output("stopping plan operation...") } @@ -120,8 +121,18 @@ func (b *Local) opPlan( // Stop execution go tfCtx.Stop() - // Wait for completion still - <-doneCh + select { + case <-cancelCtx.Done(): + log.Println("[WARN] running operation canceled") + // if the operation was canceled, we need to return immediately + return + case <-doneCh: + } + case <-cancelCtx.Done(): + // this should not be called without first attempting to stop the + // operation + log.Println("[ERROR] running operation canceled without Stop") + return case <-doneCh: } diff --git a/backend/local/backend_refresh.go b/backend/local/backend_refresh.go index 213334c4e..216147b3e 100644 --- a/backend/local/backend_refresh.go +++ b/backend/local/backend_refresh.go @@ -17,7 +17,8 @@ import ( ) func (b *Local) opRefresh( - ctx context.Context, + stopCtx context.Context, + cancelCtx context.Context, op *backend.Operation, runningOp *backend.RunningOperation) { // Check if our state exists if we're performing a refresh operation. We @@ -53,7 +54,7 @@ func (b *Local) opRefresh( } if op.LockState { - lockCtx, cancel := context.WithTimeout(ctx, op.StateLockTimeout) + lockCtx, cancel := context.WithTimeout(stopCtx, op.StateLockTimeout) defer cancel() lockInfo := state.NewLockInfo() @@ -91,7 +92,7 @@ func (b *Local) opRefresh( }() select { - case <-ctx.Done(): + case <-stopCtx.Done(): if b.CLI != nil { b.CLI.Output("stopping refresh operation...") } @@ -99,8 +100,18 @@ func (b *Local) opRefresh( // Stop execution go tfCtx.Stop() - // Wait for completion still - <-doneCh + select { + case <-cancelCtx.Done(): + log.Println("[WARN] running operation canceled") + // if the operation was canceled, we need to return immediately + return + case <-doneCh: + } + case <-cancelCtx.Done(): + // this should not be called without first attempting to stop the + // operation + log.Println("[ERROR] running operation canceled without Stop") + return case <-doneCh: } diff --git a/command/apply.go b/command/apply.go index c65b2df51..dd86ab68d 100644 --- a/command/apply.go +++ b/command/apply.go @@ -7,6 +7,7 @@ import ( "os" "sort" "strings" + "time" "github.com/hashicorp/terraform/tfdiags" @@ -159,10 +160,7 @@ func (c *ApplyCommand) Run(args []string) int { opReq.DestroyForce = destroyForce // Perform the operation - ctx, ctxCancel := context.WithCancel(context.Background()) - defer ctxCancel() - - op, err := b.Operation(ctx, opReq) + op, err := b.Operation(context.Background(), opReq) if err != nil { c.Ui.Error(fmt.Sprintf("Error starting operation: %s", err)) return 1 @@ -171,8 +169,8 @@ func (c *ApplyCommand) Run(args []string) int { // Wait for the operation to complete or an interrupt to occur select { case <-c.ShutdownCh: - // Cancel our context so we can start gracefully exiting - ctxCancel() + // gracefully stop the operation + op.Stop() // Notify the user c.Ui.Output(outputInterrupt) @@ -183,7 +181,19 @@ func (c *ApplyCommand) Run(args []string) int { c.Ui.Error( "Two interrupts received. Exiting immediately. Note that data\n" + "loss may have occurred.") + + // cancel the operation completely + op.Cancel() + + // the operation should return asap + // but timeout just in case + select { + case <-op.Done(): + case <-time.After(5 * time.Second): + } + return 1 + case <-op.Done(): } case <-op.Done(): diff --git a/command/plan.go b/command/plan.go index ec882b639..37bc90460 100644 --- a/command/plan.go +++ b/command/plan.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "strings" + "time" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/config" @@ -107,10 +108,7 @@ func (c *PlanCommand) Run(args []string) int { opReq.Type = backend.OperationTypePlan // Perform the operation - ctx, ctxCancel := context.WithCancel(context.Background()) - defer ctxCancel() - - op, err := b.Operation(ctx, opReq) + op, err := b.Operation(context.Background(), opReq) if err != nil { c.Ui.Error(fmt.Sprintf("Error starting operation: %s", err)) return 1 @@ -119,7 +117,7 @@ func (c *PlanCommand) Run(args []string) int { select { case <-c.ShutdownCh: // Cancel our context so we can start gracefully exiting - ctxCancel() + op.Stop() // Notify the user c.Ui.Output(outputInterrupt) @@ -129,6 +127,17 @@ func (c *PlanCommand) Run(args []string) int { case <-c.ShutdownCh: c.Ui.Error( "Two interrupts received. Exiting immediately") + + // cancel the operation completely + op.Cancel() + + // the operation should return asap + // but timeout just in case + select { + case <-op.Done(): + case <-time.After(5 * time.Second): + } + return 1 case <-op.Done(): } diff --git a/command/refresh.go b/command/refresh.go index eec74281c..1329a190c 100644 --- a/command/refresh.go +++ b/command/refresh.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "strings" + "time" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/config" @@ -82,10 +83,40 @@ func (c *RefreshCommand) Run(args []string) int { return 1 } - // Wait for the operation to complete - <-op.Done() - if err := op.Err; err != nil { - diags = diags.Append(err) + // Wait for the operation to complete or an interrupt to occur + select { + case <-c.ShutdownCh: + // gracefully stop the operation + op.Stop() + + // Notify the user + c.Ui.Output(outputInterrupt) + + // Still get the result, since there is still one + select { + case <-c.ShutdownCh: + c.Ui.Error( + "Two interrupts received. Exiting immediately. Note that data\n" + + "loss may have occurred.") + + // cancel the operation completely + op.Cancel() + + // the operation should return asap + // but timeout just in case + select { + case <-op.Done(): + case <-time.After(5 * time.Second): + } + + return 1 + + case <-op.Done(): + } + case <-op.Done(): + if err := op.Err; err != nil { + diags = diags.Append(err) + } } c.showDiagnostics(diags) From 67a615209181ef369a306f42fe749706d58a1996 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 9 Feb 2018 18:51:29 -0500 Subject: [PATCH 2/4] move backend operation cancellation into meta Create a single command method for running and operation with cancellation. --- command/apply.go | 44 ++--------------------------------------- command/meta.go | 49 ++++++++++++++++++++++++++++++++++++++++++++++ command/plan.go | 40 ++----------------------------------- command/refresh.go | 44 ++--------------------------------------- 4 files changed, 55 insertions(+), 122 deletions(-) diff --git a/command/apply.go b/command/apply.go index dd86ab68d..9ecb4ef66 100644 --- a/command/apply.go +++ b/command/apply.go @@ -2,12 +2,10 @@ package command import ( "bytes" - "context" "fmt" "os" "sort" "strings" - "time" "github.com/hashicorp/terraform/tfdiags" @@ -159,47 +157,9 @@ func (c *ApplyCommand) Run(args []string) int { opReq.AutoApprove = autoApprove opReq.DestroyForce = destroyForce - // Perform the operation - op, err := b.Operation(context.Background(), opReq) + op, err := c.RunOperation(b, opReq) if err != nil { - c.Ui.Error(fmt.Sprintf("Error starting operation: %s", err)) - return 1 - } - - // Wait for the operation to complete or an interrupt to occur - select { - case <-c.ShutdownCh: - // gracefully stop the operation - op.Stop() - - // Notify the user - c.Ui.Output(outputInterrupt) - - // Still get the result, since there is still one - select { - case <-c.ShutdownCh: - c.Ui.Error( - "Two interrupts received. Exiting immediately. Note that data\n" + - "loss may have occurred.") - - // cancel the operation completely - op.Cancel() - - // the operation should return asap - // but timeout just in case - select { - case <-op.Done(): - case <-time.After(5 * time.Second): - } - - return 1 - - case <-op.Done(): - } - case <-op.Done(): - if err := op.Err; err != nil { - diags = diags.Append(err) - } + diags = diags.Append(err) } c.showDiagnostics(diags) diff --git a/command/meta.go b/command/meta.go index dbcfb194b..91f1008fe 100644 --- a/command/meta.go +++ b/command/meta.go @@ -3,6 +3,7 @@ package command import ( "bufio" "bytes" + "context" "errors" "flag" "fmt" @@ -259,6 +260,54 @@ func (m *Meta) StdinPiped() bool { return fi.Mode()&os.ModeNamedPipe != 0 } +func (m *Meta) RunOperation(b backend.Enhanced, opReq *backend.Operation) (*backend.RunningOperation, error) { + op, err := b.Operation(context.Background(), opReq) + if err != nil { + return nil, fmt.Errorf("error starting operation: %s", err) + } + + // Wait for the operation to complete or an interrupt to occur + select { + case <-m.ShutdownCh: + // gracefully stop the operation + op.Stop() + + // Notify the user + m.Ui.Output(outputInterrupt) + + // Still get the result, since there is still one + select { + case <-m.ShutdownCh: + m.Ui.Error( + "Two interrupts received. Exiting immediately. Note that data\n" + + "loss may have occurred.") + + // cancel the operation completely + op.Cancel() + + // the operation should return asap + // but timeout just in case + select { + case <-op.Done(): + case <-time.After(5 * time.Second): + } + + return nil, errors.New("operation canceled") + + case <-op.Done(): + // operation completed after Stop + } + case <-op.Done(): + // operation completed normally + } + + if op.Err != nil { + return op, op.Err + } + + return op, nil +} + const ( ProviderSkipVerifyEnvVar = "TF_SKIP_PROVIDER_VERIFY" ) diff --git a/command/plan.go b/command/plan.go index 37bc90460..28bcbcb33 100644 --- a/command/plan.go +++ b/command/plan.go @@ -1,10 +1,8 @@ package command import ( - "context" "fmt" "strings" - "time" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/config" @@ -108,43 +106,9 @@ func (c *PlanCommand) Run(args []string) int { opReq.Type = backend.OperationTypePlan // Perform the operation - op, err := b.Operation(context.Background(), opReq) + op, err := c.RunOperation(b, opReq) if err != nil { - c.Ui.Error(fmt.Sprintf("Error starting operation: %s", err)) - return 1 - } - - select { - case <-c.ShutdownCh: - // Cancel our context so we can start gracefully exiting - op.Stop() - - // Notify the user - c.Ui.Output(outputInterrupt) - - // Still get the result, since there is still one - select { - case <-c.ShutdownCh: - c.Ui.Error( - "Two interrupts received. Exiting immediately") - - // cancel the operation completely - op.Cancel() - - // the operation should return asap - // but timeout just in case - select { - case <-op.Done(): - case <-time.After(5 * time.Second): - } - - return 1 - case <-op.Done(): - } - case <-op.Done(): - if err := op.Err; err != nil { - diags = diags.Append(err) - } + diags = diags.Append(err) } c.showDiagnostics(diags) diff --git a/command/refresh.go b/command/refresh.go index 1329a190c..ce61c4233 100644 --- a/command/refresh.go +++ b/command/refresh.go @@ -1,10 +1,8 @@ package command import ( - "context" "fmt" "strings" - "time" "github.com/hashicorp/terraform/backend" "github.com/hashicorp/terraform/config" @@ -76,47 +74,9 @@ func (c *RefreshCommand) Run(args []string) int { opReq.Type = backend.OperationTypeRefresh opReq.Module = mod - // Perform the operation - op, err := b.Operation(context.Background(), opReq) + op, err := c.RunOperation(b, opReq) if err != nil { - c.Ui.Error(fmt.Sprintf("Error starting operation: %s", err)) - return 1 - } - - // Wait for the operation to complete or an interrupt to occur - select { - case <-c.ShutdownCh: - // gracefully stop the operation - op.Stop() - - // Notify the user - c.Ui.Output(outputInterrupt) - - // Still get the result, since there is still one - select { - case <-c.ShutdownCh: - c.Ui.Error( - "Two interrupts received. Exiting immediately. Note that data\n" + - "loss may have occurred.") - - // cancel the operation completely - op.Cancel() - - // the operation should return asap - // but timeout just in case - select { - case <-op.Done(): - case <-time.After(5 * time.Second): - } - - return 1 - - case <-op.Done(): - } - case <-op.Done(): - if err := op.Err; err != nil { - diags = diags.Append(err) - } + diags = diags.Append(err) } c.showDiagnostics(diags) From ecd9ef0f7732f1fe7019ccad03f8a9e53bc00a00 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 9 Feb 2018 19:13:34 -0500 Subject: [PATCH 3/4] ignore error in plan shutdown test The error was being silently dropped before. There is an interpolation error, because the plan is canceled before some of the resources can be evaluated. There might be a better way to handle this in the walk cancellation, but the behavior has not changed. Make the plan and apply shutdown match implementation-wise --- command/apply_test.go | 29 ++++++++++++++++++++--------- command/plan_test.go | 8 +++++--- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/command/apply_test.go b/command/apply_test.go index 968f8595f..6eda227f0 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -824,12 +824,11 @@ func TestApply_refresh(t *testing.T) { } func TestApply_shutdown(t *testing.T) { - cancelled := false - stopped := make(chan struct{}) + cancelled := make(chan struct{}) + shutdownCh := make(chan struct{}) statePath := testTempFile(t) p := testProvider() - shutdownCh := make(chan struct{}) ui := new(cli.MockUi) c := &ApplyCommand{ @@ -841,8 +840,7 @@ func TestApply_shutdown(t *testing.T) { } p.StopFn = func() error { - close(stopped) - cancelled = true + close(cancelled) return nil } @@ -858,15 +856,26 @@ func TestApply_shutdown(t *testing.T) { }, }, nil } + + var once sync.Once p.ApplyFn = func( *terraform.InstanceInfo, *terraform.InstanceState, *terraform.InstanceDiff) (*terraform.InstanceState, error) { + // only cancel once - if !cancelled { + once.Do(func() { shutdownCh <- struct{}{} - <-stopped - } + }) + + // Because of the internal lock in the MockProvider, we can't + // coordiante directly with the calling of Stop, and making the + // MockProvider concurrent is disruptive to a lot of existing tests. + // Wait here a moment to help make sure the main goroutine gets to the + // Stop call before we exit, or the plan may finish before it can be + // canceled. + time.Sleep(200 * time.Millisecond) + return &terraform.InstanceState{ ID: "foo", Attributes: map[string]string{ @@ -888,7 +897,9 @@ func TestApply_shutdown(t *testing.T) { t.Fatalf("err: %s", err) } - if !cancelled { + select { + case <-cancelled: + default: t.Fatal("command not cancelled") } diff --git a/command/plan_test.go b/command/plan_test.go index e9cfd6dbb..b78de9b8e 100644 --- a/command/plan_test.go +++ b/command/plan_test.go @@ -835,8 +835,8 @@ func TestPlan_detailedExitcode_emptyDiff(t *testing.T) { func TestPlan_shutdown(t *testing.T) { cancelled := make(chan struct{}) - shutdownCh := make(chan struct{}) + p := testProvider() ui := new(cli.MockUi) c := &PlanCommand{ @@ -880,8 +880,10 @@ func TestPlan_shutdown(t *testing.T) { }, nil } - if code := c.Run([]string{testFixturePath("apply-shutdown")}); code != 0 { - t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + if code := c.Run([]string{testFixturePath("apply-shutdown")}); code != 1 { + // FIXME: we should be able to avoid the error during evaluation + // the early exit isn't caught before the interpolation is evaluated + t.Fatal(ui.OutputWriter.String()) } select { From ef8ed1e275b61ddf6da2a8a72a8e57fc1d42852b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 12 Feb 2018 11:52:21 -0500 Subject: [PATCH 4/4] coalesce the backened interrupt code Moves the nested select statements for backend operations into a single function. The only difference in this part was that apply called PersistState, which should be harmless regardless of the type of operation being run. --- backend/local/backend.go | 48 ++++++++++++++++++++++++++++++++ backend/local/backend_apply.go | 36 +----------------------- backend/local/backend_plan.go | 22 +-------------- backend/local/backend_refresh.go | 22 +-------------- 4 files changed, 51 insertions(+), 77 deletions(-) diff --git a/backend/local/backend.go b/backend/local/backend.go index bbebe3e18..b42aedb17 100644 --- a/backend/local/backend.go +++ b/backend/local/backend.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io/ioutil" + "log" "os" "path/filepath" "sort" @@ -273,6 +274,53 @@ func (b *Local) Operation(ctx context.Context, op *backend.Operation) (*backend. return runningOp, nil } +// opWait wats for the operation to complete, and a stop signal or a +// cancelation signal. +func (b *Local) opWait( + doneCh <-chan struct{}, + stopCtx context.Context, + cancelCtx context.Context, + tfCtx *terraform.Context, + opState state.State) (canceled bool) { + // Wait for the operation to finish or for us to be interrupted so + // we can handle it properly. + select { + case <-stopCtx.Done(): + if b.CLI != nil { + b.CLI.Output("stopping operation...") + } + + // try to force a PersistState just in case the process is terminated + // before we can complete. + if err := opState.PersistState(); err != nil { + // We can't error out from here, but warn the user if there was an error. + // If this isn't transient, we will catch it again below, and + // attempt to save the state another way. + if b.CLI != nil { + b.CLI.Error(fmt.Sprintf(earlyStateWriteErrorFmt, err)) + } + } + + // Stop execution + go tfCtx.Stop() + + select { + case <-cancelCtx.Done(): + log.Println("[WARN] running operation canceled") + // if the operation was canceled, we need to return immediately + canceled = true + case <-doneCh: + } + case <-cancelCtx.Done(): + // this should not be called without first attempting to stop the + // operation + log.Println("[ERROR] running operation canceled without Stop") + canceled = true + case <-doneCh: + } + return +} + // Colorize returns the Colorize structure that can be used for colorizing // output. This is gauranteed to always return a non-nil value and so is useful // as a helper to wrap any potentially colored strings. diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index f4351af6f..da8c684bc 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -154,42 +154,8 @@ func (b *Local) opApply( applyState = tfCtx.State() }() - // Wait for the apply to finish or for us to be interrupted so - // we can handle it properly. - err = nil - select { - case <-stopCtx.Done(): - if b.CLI != nil { - b.CLI.Output("stopping apply operation...") - } - - // try to force a PersistState just in case the process is terminated - // before we can complete. - if err := opState.PersistState(); err != nil { - // We can't error out from here, but warn the user if there was an error. - // If this isn't transient, we will catch it again below, and - // attempt to save the state another way. - if b.CLI != nil { - b.CLI.Error(fmt.Sprintf(earlyStateWriteErrorFmt, err)) - } - } - - // Stop execution - go tfCtx.Stop() - - select { - case <-cancelCtx.Done(): - log.Println("[WARN] running operation canceled") - // if the operation was canceled, we need to return immediately - return - case <-doneCh: - } - case <-cancelCtx.Done(): - // this should not be called without first attempting to stop the - // operation - log.Println("[ERROR] running operation canceled without Stop") + if b.opWait(doneCh, stopCtx, cancelCtx, tfCtx, opState) { return - case <-doneCh: } // Store the final state diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index d8ae0fe90..ebf053192 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -112,28 +112,8 @@ func (b *Local) opPlan( plan, planErr = tfCtx.Plan() }() - select { - case <-stopCtx.Done(): - if b.CLI != nil { - b.CLI.Output("stopping plan operation...") - } - - // Stop execution - go tfCtx.Stop() - - select { - case <-cancelCtx.Done(): - log.Println("[WARN] running operation canceled") - // if the operation was canceled, we need to return immediately - return - case <-doneCh: - } - case <-cancelCtx.Done(): - // this should not be called without first attempting to stop the - // operation - log.Println("[ERROR] running operation canceled without Stop") + if b.opWait(doneCh, stopCtx, cancelCtx, tfCtx, opState) { return - case <-doneCh: } if planErr != nil { diff --git a/backend/local/backend_refresh.go b/backend/local/backend_refresh.go index 216147b3e..959f969a3 100644 --- a/backend/local/backend_refresh.go +++ b/backend/local/backend_refresh.go @@ -91,28 +91,8 @@ func (b *Local) opRefresh( log.Printf("[INFO] backend/local: refresh calling Refresh") }() - select { - case <-stopCtx.Done(): - if b.CLI != nil { - b.CLI.Output("stopping refresh operation...") - } - - // Stop execution - go tfCtx.Stop() - - select { - case <-cancelCtx.Done(): - log.Println("[WARN] running operation canceled") - // if the operation was canceled, we need to return immediately - return - case <-doneCh: - } - case <-cancelCtx.Done(): - // this should not be called without first attempting to stop the - // operation - log.Println("[ERROR] running operation canceled without Stop") + if b.opWait(doneCh, stopCtx, cancelCtx, tfCtx, opState) { return - case <-doneCh: } // write the resulting state to the running op