From 3dccfa0cc9cd2bd642295cd27d9fb45ff741fe2a Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Fri, 12 Aug 2016 17:20:09 -0500 Subject: [PATCH] terraform: diffs with only tainted set are non-empty Fixes issue where a resource marked as tainted with no other attribute diffs would never show up in the plan or apply as needing to be replaced. One unrelated test needed updating due to a quirk in the testDiffFn logic - it adds a "type" field diff if the diff is non-Empty. NBD --- helper/schema/schema_test.go | 20 ++++++++++++++++++++ terraform/context_plan_test.go | 3 ++- terraform/diff.go | 2 +- terraform/diff_test.go | 13 +++++++++++++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index d9d023d6a..5700aba2a 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -2400,6 +2400,26 @@ func TestSchemaMap_Diff(t *testing.T) { Err: false, }, + + "tainted in state w/ no attr changes is still a replacement": { + Schema: map[string]*Schema{}, + + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "id": "someid", + }, + Tainted: true, + }, + + Config: map[string]interface{}{}, + + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{}, + DestroyTainted: true, + }, + + Err: false, + }, } for tn, tc := range cases { diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 65ba03566..96a40119a 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -1949,6 +1949,7 @@ func TestContext2Plan_taintDestroyInterpolatedCountRace(t *testing.T) { DIFF: DESTROY/CREATE: aws_instance.foo.0 + type: "" => "aws_instance" STATE: @@ -1960,7 +1961,7 @@ aws_instance.foo.2: ID = bar `) if actual != expected { - t.Fatalf("bad:\n%s", actual) + t.Fatalf("[%d] bad:\n%s\nexpected:\n%s\n", i, actual, expected) } } } diff --git a/terraform/diff.go b/terraform/diff.go index f3e7a092f..5de15a24a 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -361,7 +361,7 @@ func (d *InstanceDiff) Empty() bool { d.mu.Lock() defer d.mu.Unlock() - return !d.Destroy && len(d.Attributes) == 0 + return !d.Destroy && !d.DestroyTainted && len(d.Attributes) == 0 } func (d *InstanceDiff) GoString() string { diff --git a/terraform/diff_test.go b/terraform/diff_test.go index bf1c1c9f2..faffcbbcc 100644 --- a/terraform/diff_test.go +++ b/terraform/diff_test.go @@ -27,6 +27,19 @@ func TestDiffEmpty(t *testing.T) { } } +func TestDiffEmpty_taintedIsNotEmpty(t *testing.T) { + diff := new(Diff) + + mod := diff.AddModule(rootModulePath) + mod.Resources["nodeA"] = &InstanceDiff{ + DestroyTainted: true, + } + + if diff.Empty() { + t.Fatal("should not be empty, since DestroyTainted was set") + } +} + func TestModuleDiff_ChangeType(t *testing.T) { cases := []struct { Diff *ModuleDiff