From 85295e5c23b1ab79c25a12825ee589f2d62409d1 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 2 Dec 2017 17:31:28 -0500 Subject: [PATCH 1/3] watch for cancellation in plan and refresh Cancellation in the local backend was only implemented for apply. --- backend/local/backend_apply.go | 8 -------- backend/local/backend_plan.go | 32 +++++++++++++++++++++++++------ backend/local/backend_refresh.go | 33 ++++++++++++++++++++++++++++---- 3 files changed, 55 insertions(+), 18 deletions(-) diff --git a/backend/local/backend_apply.go b/backend/local/backend_apply.go index 446343181..9789e0b7c 100644 --- a/backend/local/backend_apply.go +++ b/backend/local/backend_apply.go @@ -151,14 +151,6 @@ func (b *Local) opApply( _, applyErr = tfCtx.Apply() // we always want the state, even if apply failed applyState = tfCtx.State() - - /* - // Record any shadow errors for later - if err := ctx.ShadowError(); err != nil { - shadowErr = multierror.Append(shadowErr, multierror.Prefix( - err, "apply operation:")) - } - */ }() // Wait for the apply to finish or for us to be interrupted so diff --git a/backend/local/backend_plan.go b/backend/local/backend_plan.go index a4e92c1c7..380ce1742 100644 --- a/backend/local/backend_plan.go +++ b/backend/local/backend_plan.go @@ -101,14 +101,34 @@ func (b *Local) opPlan( } } - // Perform the plan - log.Printf("[INFO] backend/local: plan calling Plan") - plan, err := tfCtx.Plan() - if err != nil { - runningOp.Err = errwrap.Wrapf("Error running plan: {{err}}", err) - return + // Perform the plan in a goroutine so we can be interrupted + var plan *terraform.Plan + var planErr error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + log.Printf("[INFO] backend/local: plan calling Plan") + plan, planErr = tfCtx.Plan() + }() + + select { + case <-ctx.Done(): + if b.CLI != nil { + b.CLI.Output("stopping plan operation...") + } + + // Stop execution + go tfCtx.Stop() + + // Wait for completion still + <-doneCh + case <-doneCh: } + if planErr != nil { + runningOp.Err = errwrap.Wrapf("Error running plan: {{err}}", planErr) + return + } // Record state runningOp.PlanEmpty = plan.Diff.Empty() diff --git a/backend/local/backend_refresh.go b/backend/local/backend_refresh.go index 282e63045..0cf50b759 100644 --- a/backend/local/backend_refresh.go +++ b/backend/local/backend_refresh.go @@ -3,6 +3,7 @@ package local import ( "context" "fmt" + "log" "os" "strings" @@ -12,6 +13,7 @@ import ( "github.com/hashicorp/terraform/command/clistate" "github.com/hashicorp/terraform/config/module" "github.com/hashicorp/terraform/state" + "github.com/hashicorp/terraform/terraform" ) func (b *Local) opRefresh( @@ -78,11 +80,34 @@ func (b *Local) opRefresh( } } - // Perform operation and write the resulting state to the running op - newState, err := tfCtx.Refresh() + // Perform the refresh in a goroutine so we can be interrupted + var newState *terraform.State + var refreshErr error + doneCh := make(chan struct{}) + go func() { + defer close(doneCh) + newState, err = tfCtx.Refresh() + log.Printf("[INFO] backend/local: plan calling Plan") + }() + + select { + case <-ctx.Done(): + if b.CLI != nil { + b.CLI.Output("stopping refresh operation...") + } + + // Stop execution + go tfCtx.Stop() + + // Wait for completion still + <-doneCh + case <-doneCh: + } + + // write the resulting state to the running op runningOp.State = newState - if err != nil { - runningOp.Err = errwrap.Wrapf("Error refreshing state: {{err}}", err) + if refreshErr != nil { + runningOp.Err = errwrap.Wrapf("Error refreshing state: {{err}}", refreshErr) return } From df38c2e3ead3c01517d2357815f72083db445837 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 2 Dec 2017 22:33:53 -0500 Subject: [PATCH 2/3] make the mock provider stoppable The mock provider couldn't be stopped during diff, because the single mutex was held through the oepration. Release the mutex so Stop can be called. --- terraform/resource_provider_mock.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/terraform/resource_provider_mock.go b/terraform/resource_provider_mock.go index 73cde0ccb..9131f0f5f 100644 --- a/terraform/resource_provider_mock.go +++ b/terraform/resource_provider_mock.go @@ -196,13 +196,14 @@ func (p *MockResourceProvider) Diff( info *InstanceInfo, state *InstanceState, desired *ResourceConfig) (*InstanceDiff, error) { - p.Lock() - defer p.Unlock() + p.Lock() p.DiffCalled = true p.DiffInfo = info p.DiffState = state p.DiffDesired = desired + p.Unlock() + if p.DiffFn != nil { return p.DiffFn(info, state, desired) } From 2941ed464ca09f804c05b22149a6f74c152bcd54 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sat, 2 Dec 2017 22:36:43 -0500 Subject: [PATCH 3/3] replace the testShutdownHook with a check for Stop Now that the local backend can be cancelled during plan and refresh, we don't really need the testShutdownHook. Simplify the tests by just checking for Stop being called on the provider. --- command/apply.go | 6 +----- command/apply_test.go | 23 +++++++++-------------- command/meta.go | 4 ---- command/plan.go | 5 ----- command/plan_test.go | 17 ++++++++--------- 5 files changed, 18 insertions(+), 37 deletions(-) diff --git a/command/apply.go b/command/apply.go index 3c3d2a189..84b16ce17 100644 --- a/command/apply.go +++ b/command/apply.go @@ -171,6 +171,7 @@ func (c *ApplyCommand) Run(args []string) int { // Perform the operation ctx, ctxCancel := context.WithCancel(context.Background()) defer ctxCancel() + op, err := b.Operation(ctx, opReq) if err != nil { c.Ui.Error(fmt.Sprintf("Error starting operation: %s", err)) @@ -183,11 +184,6 @@ func (c *ApplyCommand) Run(args []string) int { // Cancel our context so we can start gracefully exiting ctxCancel() - // notify tests that the command context was canceled - if testShutdownHook != nil { - testShutdownHook() - } - // Notify the user c.Ui.Output(outputInterrupt) diff --git a/command/apply_test.go b/command/apply_test.go index 2707f293c..3ae4d7adb 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -825,14 +825,7 @@ func TestApply_refresh(t *testing.T) { func TestApply_shutdown(t *testing.T) { cancelled := false - cancelDone := make(chan struct{}) - testShutdownHook = func() { - cancelled = true - close(cancelDone) - } - defer func() { - testShutdownHook = nil - }() + stopped := make(chan struct{}) statePath := testTempFile(t) p := testProvider() @@ -847,6 +840,12 @@ func TestApply_shutdown(t *testing.T) { }, } + p.StopFn = func() error { + close(stopped) + cancelled = true + return nil + } + p.DiffFn = func( *terraform.InstanceInfo, *terraform.InstanceState, @@ -864,11 +863,11 @@ func TestApply_shutdown(t *testing.T) { *terraform.InstanceState, *terraform.InstanceDiff) (*terraform.InstanceState, error) { + // only cancel once if !cancelled { shutdownCh <- struct{}{} - <-cancelDone + <-stopped } - return &terraform.InstanceState{ ID: "foo", Attributes: map[string]string{ @@ -898,10 +897,6 @@ func TestApply_shutdown(t *testing.T) { if state == nil { t.Fatal("state should not be nil") } - - if len(state.RootModule().Resources) != 1 { - t.Fatalf("bad: %d", len(state.RootModule().Resources)) - } } func TestApply_state(t *testing.T) { diff --git a/command/meta.go b/command/meta.go index 729020b2b..27f7765f9 100644 --- a/command/meta.go +++ b/command/meta.go @@ -641,7 +641,3 @@ func isAutoVarFile(path string) bool { return strings.HasSuffix(path, ".auto.tfvars") || strings.HasSuffix(path, ".auto.tfvars.json") } - -// testShutdownHook is used by tests to verify that a command context has been -// canceled -var testShutdownHook func() diff --git a/command/plan.go b/command/plan.go index f6f4f8f2f..bef831e16 100644 --- a/command/plan.go +++ b/command/plan.go @@ -118,11 +118,6 @@ func (c *PlanCommand) Run(args []string) int { // Cancel our context so we can start gracefully exiting ctxCancel() - // notify tests that the command context was canceled - if testShutdownHook != nil { - testShutdownHook() - } - // Notify the user c.Ui.Output(outputInterrupt) diff --git a/command/plan_test.go b/command/plan_test.go index 152486c7f..04390ab5b 100644 --- a/command/plan_test.go +++ b/command/plan_test.go @@ -833,14 +833,7 @@ func TestPlan_detailedExitcode_emptyDiff(t *testing.T) { func TestPlan_shutdown(t *testing.T) { cancelled := false - cancelDone := make(chan struct{}) - testShutdownHook = func() { - cancelled = true - close(cancelDone) - } - defer func() { - testShutdownHook = nil - }() + stopped := make(chan struct{}) shutdownCh := make(chan struct{}) p := testProvider() @@ -853,6 +846,12 @@ func TestPlan_shutdown(t *testing.T) { }, } + p.StopFn = func() error { + close(stopped) + cancelled = true + return nil + } + p.DiffFn = func( *terraform.InstanceInfo, *terraform.InstanceState, @@ -860,7 +859,7 @@ func TestPlan_shutdown(t *testing.T) { if !cancelled { shutdownCh <- struct{}{} - <-cancelDone + <-stopped } return &terraform.InstanceDiff{