Merge pull request #1523 from hashicorp/b-tainted-cycle

terraform: prune tainted destroys if no tainted in state [GH-1475]

terraform/context_test.go

@@ -115,6 +115,30 @@ func TestContext2Plan_modules(t *testing.T) {
 	}
 }
 
+// GH-1475
+func TestContext2Plan_moduleCycle(t *testing.T) {
+	m := testModule(t, "plan-module-cycle")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+	})
+
+	plan, err := ctx.Plan()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(plan.String())
+	expected := strings.TrimSpace(testTerraformPlanModuleCycleStr)
+	if actual != expected {
+		t.Fatalf("bad:\n%s", actual)
+	}
+}
+
 func TestContext2Plan_moduleInput(t *testing.T) {
 	m := testModule(t, "plan-module-input")
 	p := testProvider("aws")

terraform/graph_builder_test.go

@@ -124,13 +124,9 @@ const testBasicGraphBuilderStr = `
 
 const testBuiltinGraphBuilderBasicStr = `
 aws_instance.db
-  aws_instance.db (destroy tainted)
-aws_instance.db (destroy tainted)
-  aws_instance.web (destroy tainted)
+  provider.aws
 aws_instance.web
   aws_instance.db
-aws_instance.web (destroy tainted)
-  provider.aws
 provider.aws
 `
 

terraform/graph_config_node.go

@@ -447,11 +447,44 @@ func (n *graphNodeResourceDestroy) CreateNode() dag.Vertex {
 }
 
 func (n *graphNodeResourceDestroy) DestroyInclude(d *ModuleDiff, s *ModuleState) bool {
-	// Always include anything other than the primary destroy
-	if n.DestroyMode != DestroyPrimary {
+	switch n.DestroyMode {
+	case DestroyPrimary:
+		return n.destroyIncludePrimary(d, s)
+	case DestroyTainted:
+		return n.destroyIncludeTainted(d, s)
+	default:
 		return true
 	}
+}
 
+func (n *graphNodeResourceDestroy) destroyIncludeTainted(
+	d *ModuleDiff, s *ModuleState) bool {
+	// If there is no state, there can't by any tainted.
+	if s == nil {
+		return false
+	}
+
+	// Grab the ID which is the prefix (in the case count > 0 at some point)
+	prefix := n.Original.Resource.Id()
+
+	// Go through the resources and find any with our prefix. If there
+	// are any tainted, we need to keep it.
+	for k, v := range s.Resources {
+		if !strings.HasPrefix(k, prefix) {
+			continue
+		}
+
+		if len(v.Tainted) > 0 {
+			return true
+		}
+	}
+
+	// We didn't find any tainted nodes, return
+	return false
+}
+
+func (n *graphNodeResourceDestroy) destroyIncludePrimary(
+	d *ModuleDiff, s *ModuleState) bool {
 	// Get the count, and specifically the raw value of the count
 	// (with interpolations and all). If the count is NOT a static "1",
 	// then we keep the destroy node no matter what.

terraform/terraform_test.go

@@ -924,6 +924,19 @@ STATE:
 <no state>
 `
 
+const testTerraformPlanModuleCycleStr = `
+DIFF:
+
+CREATE: aws_instance.b
+CREATE: aws_instance.c
+  some_input: "" => "<computed>"
+  type:       "" => "aws_instance"
+
+STATE:
+
+<no state>
+`
+
 const testTerraformPlanModuleDestroyStr = `
 DIFF:
 

terraform/test-fixtures/plan-module-cycle/child/main.tf

@@ -0,0 +1,5 @@
+variable "in" {}
+
+output "out" {
+    value = "${var.in}"
+}

terraform/test-fixtures/plan-module-cycle/main.tf

@@ -0,0 +1,12 @@
+module "a" {
+    source = "./child"
+    in = "${aws_instance.b.id}"
+}
+
+resource "aws_instance" "b" {}
+
+resource "aws_instance" "c" {
+    some_input = "${module.a.out}"
+
+    depends_on = ["aws_instance.b"]
+}

terraform/transform_destroy_test.go

@@ -299,6 +299,54 @@ func TestPruneDestroyTransformer_countState(t *testing.T) {
 	}
 }
 
+func TestPruneDestroyTransformer_tainted(t *testing.T) {
+	mod := testModule(t, "transform-destroy-basic")
+
+	diff := &Diff{}
+	state := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: RootModulePath,
+				Resources: map[string]*ResourceState{
+					"aws_instance.bar": &ResourceState{
+						Tainted: []*InstanceState{
+							&InstanceState{ID: "foo"},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	g := Graph{Path: RootModulePath}
+	{
+		tf := &ConfigTransformer{Module: mod}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		tf := &DestroyTransformer{}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	{
+		tf := &PruneDestroyTransformer{Diff: diff, State: state}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testTransformPruneDestroyTaintedStr)
+	if actual != expected {
+		t.Fatalf("bad:\n\n%s", actual)
+	}
+}
+
 const testTransformDestroyBasicStr = `
 aws_instance.bar
   aws_instance.bar (destroy tainted)
@@ -317,63 +365,46 @@ aws_instance.foo (destroy)
 
 const testTransformPruneDestroyBasicStr = `
 aws_instance.bar
-  aws_instance.bar (destroy tainted)
   aws_instance.foo
-aws_instance.bar (destroy tainted)
 aws_instance.foo
-  aws_instance.foo (destroy tainted)
-aws_instance.foo (destroy tainted)
-  aws_instance.bar (destroy tainted)
 `
 
 const testTransformPruneDestroyBasicDiffStr = `
 aws_instance.bar
-  aws_instance.bar (destroy tainted)
   aws_instance.bar (destroy)
   aws_instance.foo
-aws_instance.bar (destroy tainted)
 aws_instance.bar (destroy)
 aws_instance.foo
-  aws_instance.foo (destroy tainted)
-aws_instance.foo (destroy tainted)
-  aws_instance.bar (destroy tainted)
 `
 
 const testTransformPruneDestroyCountStr = `
 aws_instance.bar
-  aws_instance.bar (destroy tainted)
   aws_instance.bar (destroy)
   aws_instance.foo
-aws_instance.bar (destroy tainted)
 aws_instance.bar (destroy)
 aws_instance.foo
-  aws_instance.foo (destroy tainted)
-aws_instance.foo (destroy tainted)
-  aws_instance.bar (destroy tainted)
 `
 
 const testTransformPruneDestroyCountDecStr = `
 aws_instance.bar
-  aws_instance.bar (destroy tainted)
   aws_instance.bar (destroy)
   aws_instance.foo
-aws_instance.bar (destroy tainted)
 aws_instance.bar (destroy)
 aws_instance.foo
-  aws_instance.foo (destroy tainted)
-aws_instance.foo (destroy tainted)
-  aws_instance.bar (destroy tainted)
 `
 
 const testTransformPruneDestroyCountStateStr = `
+aws_instance.bar
+  aws_instance.foo
+aws_instance.foo
+`
+
+const testTransformPruneDestroyTaintedStr = `
 aws_instance.bar
   aws_instance.bar (destroy tainted)
   aws_instance.foo
 aws_instance.bar (destroy tainted)
 aws_instance.foo
-  aws_instance.foo (destroy tainted)
-aws_instance.foo (destroy tainted)
-  aws_instance.bar (destroy tainted)
 `
 
 const testTransformCreateBeforeDestroyBasicStr = `