From 77396107c4363227bb581d698a79300deaebc004 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Thu, 28 Sep 2017 12:43:18 -0400
Subject: [PATCH] don't evaluate locals during destroy

Locals don't need to be evaluated during destroy.  Rather than simply
skipping them, remove them from the state as they are encountered. Even
though they are not persisted in the state, it keeps the state up to
date as the destroy happens, and we reduce the chance of other
inconstancies later on.
---
 terraform/context_apply_test.go | 62 ++++++++++++++++++++-------------
 terraform/eval_local.go         | 28 +++++++++++++++
 terraform/node_local.go         | 44 +++++++++++++++--------
 3 files changed, 95 insertions(+), 39 deletions(-)

diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go
index 32ce76391..c5bc754b6 100644
--- a/terraform/context_apply_test.go
+++ b/terraform/context_apply_test.go
@@ -8812,46 +8812,58 @@ module.child:
 
 func TestContext2Apply_destroyWithLocals(t *testing.T) {
 	m := testModule(t, "apply-destroy-with-locals")
-	ctx := testContext2(t, &ContextOpts{
-		Module: m,
-		ProviderResolver: ResourceProviderResolverFixed(
-			map[string]ResourceProviderFactory{
-				"aws": testProviderFuncFixed(testProvider("aws")),
-			},
-		),
-		State: &State{
-			Modules: []*ModuleState{
-				&ModuleState{
-					Path: rootModulePath,
-					Outputs: map[string]*OutputState{
-						"name": &OutputState{
-							Type:  "string",
-							Value: "test-bar",
-						},
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = func(info *InstanceInfo, s *InstanceState, c *ResourceConfig) (*InstanceDiff, error) {
+		d, err := testDiffFn(info, s, c)
+		fmt.Println("DIFF:", d)
+		return d, err
+	}
+
+	s := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: rootModulePath,
+				Outputs: map[string]*OutputState{
+					"name": &OutputState{
+						Type:  "string",
+						Value: "test-bar",
 					},
-					Resources: map[string]*ResourceState{
-						"aws_instance.foo": &ResourceState{
-							Type: "aws_instance",
-							Primary: &InstanceState{
-								ID: "foo",
-							},
+				},
+				Resources: map[string]*ResourceState{
+					"aws_instance.foo": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "foo",
 						},
 					},
 				},
 			},
 		},
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		ProviderResolver: ResourceProviderResolverFixed(
+			map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		),
+		State:   s,
 		Destroy: true,
 	})
 
+	if _, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
 	state, err := ctx.Apply()
 	if err != nil {
 		t.Fatalf("error during apply: %s", err)
 	}
 
 	got := strings.TrimSpace(state.String())
-	want := strings.TrimSpace(`
-	TODO
-`)
+	want := strings.TrimSpace(`<no state>`)
 	if got != want {
 		t.Fatalf("wrong final state\ngot:\n%s\nwant:\n%s", got, want)
 	}
diff --git a/terraform/eval_local.go b/terraform/eval_local.go
index 1b63bf4f4..a4b2a5059 100644
--- a/terraform/eval_local.go
+++ b/terraform/eval_local.go
@@ -56,3 +56,31 @@ func (n *EvalLocal) Eval(ctx EvalContext) (interface{}, error) {
 
 	return nil, nil
 }
+
+// EvalDeleteLocal is an EvalNode implementation that deletes a Local value
+// from the state. Locals aren't persisted, but we don't need to evaluate them
+// during destroy.
+type EvalDeleteLocal struct {
+	Name string
+}
+
+func (n *EvalDeleteLocal) Eval(ctx EvalContext) (interface{}, error) {
+	state, lock := ctx.State()
+	if state == nil {
+		return nil, nil
+	}
+
+	// Get a write lock so we can access this instance
+	lock.Lock()
+	defer lock.Unlock()
+
+	// Look for the module state. If we don't have one, create it.
+	mod := state.ModuleByPath(ctx.Path())
+	if mod == nil {
+		return nil, nil
+	}
+
+	delete(mod.Locals, n.Name)
+
+	return nil, nil
+}
diff --git a/terraform/node_local.go b/terraform/node_local.go
index da1564e39..e58f1f987 100644
--- a/terraform/node_local.go
+++ b/terraform/node_local.go
@@ -59,20 +59,36 @@ func (n *NodeLocal) References() []string {
 
 // GraphNodeEvalable
 func (n *NodeLocal) EvalTree() EvalNode {
-	return &EvalOpFilter{
-		Ops: []walkOperation{
-			walkInput,
-			walkValidate,
-			walkRefresh,
-			walkPlan,
-			walkApply,
-			walkDestroy,
-		},
-		Node: &EvalSequence{
-			Nodes: []EvalNode{
-				&EvalLocal{
-					Name:  n.Config.Name,
-					Value: n.Config.RawConfig,
+	return &EvalSequence{
+		Nodes: []EvalNode{
+			&EvalOpFilter{
+				Ops: []walkOperation{
+					walkInput,
+					walkValidate,
+					walkRefresh,
+					walkPlan,
+					walkApply,
+				},
+				Node: &EvalSequence{
+					Nodes: []EvalNode{
+						&EvalLocal{
+							Name:  n.Config.Name,
+							Value: n.Config.RawConfig,
+						},
+					},
+				},
+			},
+			&EvalOpFilter{
+				Ops: []walkOperation{
+					walkPlanDestroy,
+					walkDestroy,
+				},
+				Node: &EvalSequence{
+					Nodes: []EvalNode{
+						&EvalDeleteLocal{
+							Name: n.Config.Name,
+						},
+					},
 				},
 			},
 		},