Skip to content

Commit

Permalink
Merge pull request #1523 from hashicorp/b-tainted-cycle
Browse files Browse the repository at this point in the history
terraform: prune tainted destroys if no tainted in state [GH-1475]
  • Loading branch information
mitchellh committed Apr 14, 2015
2 parents 9d9b419 + b64fd84 commit 4014710
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 30 deletions.
24 changes: 24 additions & 0 deletions terraform/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 1 addition & 5 deletions terraform/graph_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
`

Expand Down
37 changes: 35 additions & 2 deletions terraform/graph_config_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 13 additions & 0 deletions terraform/terraform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions terraform/test-fixtures/plan-module-cycle/child/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
variable "in" {}

output "out" {
value = "${var.in}"
}
12 changes: 12 additions & 0 deletions terraform/test-fixtures/plan-module-cycle/main.tf
Original file line number Diff line number Diff line change
@@ -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"]
}
77 changes: 54 additions & 23 deletions terraform/transform_destroy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 = `
Expand Down

0 comments on commit 4014710

Please sign in to comment.