From 842d66183b4e5853cdc693e6fd67de2d26d8b361 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 13 May 2015 17:19:26 -0500 Subject: [PATCH] core: respect roots in CBD transform Because CBD now runs after a RootTransformer, it's now operating on a graph that _may_ have had a graphNodeRoot added to it (a noop node whose only purpose is to be a root). CBD includes a step that tells the destroy node to depend on any parents of the create node. When one of those parents was "root", this was causing the destroy node to depend on "root", making it cease to be an actual root node. Because graphNodeRoot is a singleton, the follow-up RootTransformer was not sufficient to slap another root on top - it wasn't being seen as a fresh node, so edges were just accumulating, and we ended up in a state with "no roots". refs #1903 (not sure if this will fix all the "no root found" cases, or just the one I bumped into) --- terraform/context_test.go | 37 +++++++++++++++++++ .../plan-cbd-maintain-root/main.tf | 13 +++++++ terraform/transform_destroy.go | 12 ++++-- 3 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 terraform/test-fixtures/plan-cbd-maintain-root/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index 2ae5f8ba0..31308e8d5 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -40,6 +40,43 @@ func TestContext2Plan(t *testing.T) { } } +func TestContext2Plan_createBefore_maintainRoot(t *testing.T) { + m := testModule(t, "plan-cbd-maintain-root") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + Variables: map[string]string{ + "in": "a,b,c", + }, + }) + + plan, err := ctx.Plan() + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(` +DIFF: + +CREATE: aws_instance.bar.0 +CREATE: aws_instance.bar.1 +CREATE: aws_instance.foo.0 +CREATE: aws_instance.foo.1 + +STATE: + + + `) + if actual != expected { + t.Fatalf("expected:\n%s, got:\n%s", expected, actual) + } +} + func TestContext2Plan_emptyDiff(t *testing.T) { m := testModule(t, "plan-empty") p := testProvider("aws") diff --git a/terraform/test-fixtures/plan-cbd-maintain-root/main.tf b/terraform/test-fixtures/plan-cbd-maintain-root/main.tf new file mode 100644 index 000000000..9a826475c --- /dev/null +++ b/terraform/test-fixtures/plan-cbd-maintain-root/main.tf @@ -0,0 +1,13 @@ +resource "aws_instance" "foo" { + count = "2" + lifecycle { create_before_destroy = true } +} + +resource "aws_instance" "bar" { + count = "2" + lifecycle { create_before_destroy = true } +} + +output "out" { + value = "${aws_instance.foo.0.id}" +} diff --git a/terraform/transform_destroy.go b/terraform/transform_destroy.go index 3fe585664..9fb235107 100644 --- a/terraform/transform_destroy.go +++ b/terraform/transform_destroy.go @@ -1,8 +1,6 @@ package terraform -import ( - "github.com/hashicorp/terraform/dag" -) +import "github.com/hashicorp/terraform/dag" type GraphNodeDestroyMode byte @@ -193,6 +191,14 @@ func (t *CreateBeforeDestroyTransformer) Transform(g *Graph) error { // This ensures that. for _, sourceRaw := range g.UpEdges(cn).List() { source := sourceRaw.(dag.Vertex) + + // If the graph has a "root" node (one added by a RootTransformer and not + // just a resource that happens to have no ancestors), we don't want to + // add any edges to it, because then it ceases to be a root. + if _, ok := source.(graphNodeRoot); ok { + continue + } + connect = append(connect, dag.BasicEdge(dn, source)) }