Merge pull request #6753 from hashicorp/b-output-destroy-fix

: core: Fix destroy w/module vars used in counts

terraform/context_apply_test.go

@@ -2634,6 +2634,176 @@ module.child:
 	}
 }
 
+func TestContext2Apply_destroyWithModuleVariableAndCount(t *testing.T) {
+	m := testModule(t, "apply-destroy-mod-var-and-count")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	var state *State
+	var err error
+	{
+		ctx := testContext2(t, &ContextOpts{
+			Module: m,
+			Providers: map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		})
+
+		// First plan and apply a create operation
+		if _, err := ctx.Plan(); err != nil {
+			t.Fatalf("plan err: %s", err)
+		}
+
+		state, err = ctx.Apply()
+		if err != nil {
+			t.Fatalf("apply err: %s", err)
+		}
+	}
+
+	h := new(HookRecordApplyOrder)
+	h.Active = true
+
+	{
+		ctx := testContext2(t, &ContextOpts{
+			Destroy: true,
+			Module:  m,
+			State:   state,
+			Hooks:   []Hook{h},
+			Providers: map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		})
+
+		// First plan and apply a create operation
+		plan, err := ctx.Plan()
+		if err != nil {
+			t.Fatalf("destroy plan err: %s", err)
+		}
+
+		var buf bytes.Buffer
+		if err := WritePlan(plan, &buf); err != nil {
+			t.Fatalf("plan write err: %s", err)
+		}
+
+		planFromFile, err := ReadPlan(&buf)
+		if err != nil {
+			t.Fatalf("plan read err: %s", err)
+		}
+
+		ctx, err = planFromFile.Context(&ContextOpts{
+			Providers: map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		})
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		state, err = ctx.Apply()
+		if err != nil {
+			t.Fatalf("destroy apply err: %s", err)
+		}
+	}
+
+	//Test that things were destroyed
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(`
+<no state>
+module.child:
+  <no state>
+		`)
+	if actual != expected {
+		t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual)
+	}
+}
+
+func TestContext2Apply_destroyWithModuleVariableAndCountNested(t *testing.T) {
+	m := testModule(t, "apply-destroy-mod-var-and-count-nested")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	var state *State
+	var err error
+	{
+		ctx := testContext2(t, &ContextOpts{
+			Module: m,
+			Providers: map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		})
+
+		// First plan and apply a create operation
+		if _, err := ctx.Plan(); err != nil {
+			t.Fatalf("plan err: %s", err)
+		}
+
+		state, err = ctx.Apply()
+		if err != nil {
+			t.Fatalf("apply err: %s", err)
+		}
+	}
+
+	h := new(HookRecordApplyOrder)
+	h.Active = true
+
+	{
+		ctx := testContext2(t, &ContextOpts{
+			Destroy: true,
+			Module:  m,
+			State:   state,
+			Hooks:   []Hook{h},
+			Providers: map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		})
+
+		// First plan and apply a create operation
+		plan, err := ctx.Plan()
+		if err != nil {
+			t.Fatalf("destroy plan err: %s", err)
+		}
+
+		var buf bytes.Buffer
+		if err := WritePlan(plan, &buf); err != nil {
+			t.Fatalf("plan write err: %s", err)
+		}
+
+		planFromFile, err := ReadPlan(&buf)
+		if err != nil {
+			t.Fatalf("plan read err: %s", err)
+		}
+
+		ctx, err = planFromFile.Context(&ContextOpts{
+			Providers: map[string]ResourceProviderFactory{
+				"aws": testProviderFuncFixed(p),
+			},
+		})
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+
+		state, err = ctx.Apply()
+		if err != nil {
+			t.Fatalf("destroy apply err: %s", err)
+		}
+	}
+
+	//Test that things were destroyed
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(`
+<no state>
+module.child:
+  <no state>
+module.child.child2:
+  <no state>
+		`)
+	if actual != expected {
+		t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual)
+	}
+}
+
 func TestContext2Apply_destroyOutputs(t *testing.T) {
 	m := testModule(t, "apply-destroy-outputs")
 	h := new(HookRecordApplyOrder)

terraform/graph_config_node_variable.go

@@ -62,13 +62,16 @@ func (n *GraphNodeConfigVariable) VariableName() string {
 func (n *GraphNodeConfigVariable) DestroyEdgeInclude(v dag.Vertex) bool {
 	// Only include this variable in a destroy edge if the source vertex
 	// "v" has a count dependency on this variable.
+	log.Printf("[DEBUG] DestroyEdgeInclude: Checking: %s", dag.VertexName(v))
 	cv, ok := v.(GraphNodeCountDependent)
 	if !ok {
+		log.Printf("[DEBUG] DestroyEdgeInclude: Not GraphNodeCountDependent: %s", dag.VertexName(v))
 		return false
 	}
 
 	for _, d := range cv.CountDependentOn() {
 		for _, d2 := range n.DependableName() {
+			log.Printf("[DEBUG] DestroyEdgeInclude: d = %s : d2 = %s", d, d2)
 			if d == d2 {
 				return true
 			}
@@ -94,9 +97,15 @@ func (n *GraphNodeConfigVariable) Noop(opts *NoopOpts) bool {
 	// the flat node's implementation of Path() below.
 	modDiff := opts.Diff.ModuleByPath(n.ModulePath)
 
-	// If we're destroying, we have no need of variables.
+	// If we're destroying, we have no need of variables unless they are depended
+	// on by the count of a resource.
 	if modDiff != nil && modDiff.Destroy {
-		log.Printf("[DEBUG] Destroy diff, treating variable as a noop")
+		if n.hasDestroyEdgeInPath(opts, nil) {
+			log.Printf("[DEBUG] Variable has destroy edge from %s, not a noop",
+				dag.VertexName(opts.Vertex))
+			return false
+		}
+		log.Printf("[DEBUG] Variable has no included destroy edges: noop!")
 		return true
 	}
 
@@ -114,6 +123,31 @@ func (n *GraphNodeConfigVariable) Noop(opts *NoopOpts) bool {
 	return true
 }
 
+// hasDestroyEdgeInPath recursively walks for a destroy edge, ensuring that
+// a variable both has no immediate destroy edges or any in its full module
+// path, ensuring that links do not get severed in the middle.
+func (n *GraphNodeConfigVariable) hasDestroyEdgeInPath(opts *NoopOpts, vertex dag.Vertex) bool {
+	if vertex == nil {
+		vertex = opts.Vertex
+	}
+	log.Printf("[DEBUG] hasDestroyEdgeInPath: Looking for destroy edge: %s - %T", dag.VertexName(vertex), vertex)
+	for _, v := range opts.Graph.UpEdges(vertex).List() {
+		if len(opts.Graph.UpEdges(v).List()) > 1 {
+			if n.hasDestroyEdgeInPath(opts, v) == true {
+				return true
+			}
+		}
+		// Here we borrow the implementation of DestroyEdgeInclude, whose logic
+		// and semantics are exactly what we want here.
+		if cv, ok := vertex.(*GraphNodeConfigVariableFlat); ok {
+			if cv.DestroyEdgeInclude(v) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // GraphNodeProxy impl.
 func (n *GraphNodeConfigVariable) Proxy() bool {
 	return true

terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/child/child2/main.tf

@@ -0,0 +1,5 @@
+variable "mod_count_child2" { }
+
+resource "aws_instance" "foo" {
+  count = "${var.mod_count_child2}"
+}

terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/child/main.tf

@@ -0,0 +1,8 @@
+variable "mod_count_child" { }
+
+module "child2" {
+  source    = "./child2"
+  mod_count_child2 = "${var.mod_count_child}"
+}
+
+resource "aws_instance" "foo" { }

terraform/test-fixtures/apply-destroy-mod-var-and-count-nested/main.tf

@@ -0,0 +1,9 @@
+variable "mod_count_root" {
+  type = "string"
+  default = "3"
+}
+
+module "child" {
+  source    = "./child"
+  mod_count_child = "${var.mod_count_root}"
+}

terraform/test-fixtures/apply-destroy-mod-var-and-count/child/main.tf

@@ -0,0 +1,5 @@
+variable "mod_count" { }
+
+resource "aws_instance" "foo" {
+  count = "${var.mod_count}"
+}

terraform/test-fixtures/apply-destroy-mod-var-and-count/main.tf

@@ -0,0 +1,4 @@
+module "child" {
+  source    = "./child"
+  mod_count = "3"
+}