From a7657408279e5f30629883d0f0486e365f0ed55b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 7 Feb 2017 11:49:50 -0800 Subject: [PATCH] terraform: CBD destroy nodes should not advertise themselves as normal --- terraform/graph_builder_apply_test.go | 62 +++++++++++++++++++ terraform/node_resource_destroy.go | 14 ++++- .../graph-builder-apply-dep-cbd/main.tf | 7 +++ 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 terraform/test-fixtures/graph-builder-apply-dep-cbd/main.tf diff --git a/terraform/graph_builder_apply_test.go b/terraform/graph_builder_apply_test.go index 659f8e141..272e99722 100644 --- a/terraform/graph_builder_apply_test.go +++ b/terraform/graph_builder_apply_test.go @@ -88,6 +88,68 @@ func TestApplyGraphBuilder(t *testing.T) { } } +// This tests the ordering of two resources where a non-CBD depends +// on a CBD. GH-11349. +func TestApplyGraphBuilder_depCbd(t *testing.T) { + diff := &Diff{ + Modules: []*ModuleDiff{ + &ModuleDiff{ + Path: []string{"root"}, + Resources: map[string]*InstanceDiff{ + "aws_instance.A": &InstanceDiff{ + Destroy: true, + Attributes: map[string]*ResourceAttrDiff{ + "name": &ResourceAttrDiff{ + Old: "", + New: "foo", + RequiresNew: true, + }, + }, + }, + + "aws_instance.B": &InstanceDiff{ + Attributes: map[string]*ResourceAttrDiff{ + "name": &ResourceAttrDiff{ + Old: "", + New: "foo", + }, + }, + }, + }, + }, + }, + } + + b := &ApplyGraphBuilder{ + Module: testModule(t, "graph-builder-apply-dep-cbd"), + Diff: diff, + Providers: []string{"aws"}, + Provisioners: []string{"exec"}, + DisableReduce: true, + } + + g, err := b.Build(RootModulePath) + if err != nil { + t.Fatalf("err: %s", err) + } + t.Logf("Graph: %s", g.String()) + + if !reflect.DeepEqual(g.Path, RootModulePath) { + t.Fatalf("bad: %#v", g.Path) + } + + // Create A, Modify B, Destroy A + + testGraphHappensBefore( + t, g, + "aws_instance.A", + "aws_instance.A (destroy)") + testGraphHappensBefore( + t, g, + "aws_instance.B", + "aws_instance.A (destroy)") +} + // This tests the ordering of two resources that are both CBD that // require destroy/create. func TestApplyGraphBuilder_doubleCBD(t *testing.T) { diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 0bf424a39..9a0bc8c2d 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -46,9 +46,21 @@ func (n *NodeDestroyResource) ModifyCreateBeforeDestroy(v bool) error { // GraphNodeReferenceable, overriding NodeAbstractResource func (n *NodeDestroyResource) ReferenceableName() []string { + // We modify our referenceable name to have the suffix of ".destroy" + // since depending on the creation side doesn't necessarilly mean + // depending on destruction. + suffix := ".destroy" + + // If we're CBD, we also append "-cbd". This is because CBD will setup + // its own edges (in CBDEdgeTransformer). Depending on the "destroy" + // side generally doesn't mean depending on CBD as well. See GH-11349 + if n.CreateBeforeDestroy() { + suffix += "-cbd" + } + result := n.NodeAbstractResource.ReferenceableName() for i, v := range result { - result[i] = v + ".destroy" + result[i] = v + suffix } return result diff --git a/terraform/test-fixtures/graph-builder-apply-dep-cbd/main.tf b/terraform/test-fixtures/graph-builder-apply-dep-cbd/main.tf new file mode 100644 index 000000000..0a25da772 --- /dev/null +++ b/terraform/test-fixtures/graph-builder-apply-dep-cbd/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "A" { + lifecycle { create_before_destroy = true } +} + +resource "aws_instance" "B" { + value = ["${aws_instance.A.*.id}"] +}