From f3a62c694d2b0dabe0b261f867b5b8237f4a2579 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 2 Dec 2016 09:44:55 -0500 Subject: [PATCH] terraform: when promoting non-CBD to CBD, mark the config as such Fixes #10439 When a CBD resource depends on a non-CBD resource, the non-CBD resource is auto-promoted to CBD. This was done in cf3a259. This PR makes it so that we also set the config CBD to true. This causes the proper runtime execution behavior to occur where we depose state and so on. So in addition to simple graph edge tricks we also treat the non-CBD resources as CBD resources. --- terraform/context_apply_test.go | 67 +++++++++++++++++++ .../apply-cbd-depends-non-cbd/main.tf | 12 ++++ terraform/transform_destroy.go | 29 +++++++- 3 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 terraform/test-fixtures/apply-cbd-depends-non-cbd/main.tf diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index f933301bc..c2a5674bf 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -881,6 +881,73 @@ func TestContext2Apply_createBeforeDestroyUpdate(t *testing.T) { } } +// This tests that when a CBD resource depends on a non-CBD resource, +// we can still properly apply changes that require new for both. +func TestContext2Apply_createBeforeDestroy_dependsNonCBD(t *testing.T) { + m := testModule(t, "apply-cbd-depends-non-cbd") + p := testProvider("aws") + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + state := &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "require_new": "abc", + }, + }, + }, + + "aws_instance.foo": &ResourceState{ + Type: "aws_instance", + Primary: &InstanceState{ + ID: "foo", + Attributes: map[string]string{ + "require_new": "abc", + }, + }, + }, + }, + }, + }, + } + ctx := testContext2(t, &ContextOpts{ + Module: m, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: state, + }) + + if p, err := ctx.Plan(); err != nil { + t.Fatalf("err: %s", err) + } else { + t.Logf(p.String()) + } + + state, err := ctx.Apply() + if err != nil { + t.Fatalf("err: %s", err) + } + + checkStateString(t, state, ` +aws_instance.bar: + ID = foo + require_new = yes + type = aws_instance + value = foo +aws_instance.foo: + ID = foo + require_new = yes + type = aws_instance + `) +} + func TestContext2Apply_createBeforeDestroy_hook(t *testing.T) { h := new(MockHook) m := testModule(t, "apply-good-create-before") diff --git a/terraform/test-fixtures/apply-cbd-depends-non-cbd/main.tf b/terraform/test-fixtures/apply-cbd-depends-non-cbd/main.tf new file mode 100644 index 000000000..6ba1b983f --- /dev/null +++ b/terraform/test-fixtures/apply-cbd-depends-non-cbd/main.tf @@ -0,0 +1,12 @@ +resource "aws_instance" "foo" { + require_new = "yes" +} + +resource "aws_instance" "bar" { + require_new = "yes" + value = "${aws_instance.foo.id}" + + lifecycle { + create_before_destroy = true + } +} diff --git a/terraform/transform_destroy.go b/terraform/transform_destroy.go index 47c42c0da..64e295805 100644 --- a/terraform/transform_destroy.go +++ b/terraform/transform_destroy.go @@ -1,6 +1,9 @@ package terraform -import "github.com/hashicorp/terraform/dag" +import ( + "github.com/hashicorp/terraform/config" + "github.com/hashicorp/terraform/dag" +) // GraphNodeDestroyable is the interface that nodes that can be destroyed // must implement. This is used to automatically handle the creation of @@ -151,8 +154,28 @@ func (t *CreateBeforeDestroyTransformer) Transform(g *Graph) error { } // If the node doesn't need to create before destroy, then continue - if !dn.CreateBeforeDestroy() && noCreateBeforeDestroyAncestors(g, dn) { - continue + if !dn.CreateBeforeDestroy() { + if noCreateBeforeDestroyAncestors(g, dn) { + continue + } + + // PURPOSELY HACKY FIX SINCE THIS TRANSFORM IS DEPRECATED. + // This is a hacky way to fix GH-10439. For a detailed description + // of the fix, see CBDEdgeTransformer, which is the equivalent + // transform used by the new graphs. + // + // This transform is deprecated because it is only used by the + // old graphs which are going to be removed. + var update *config.Resource + if dn, ok := v.(*graphNodeResourceDestroy); ok { + update = dn.Original.Resource + } + if dn, ok := v.(*graphNodeResourceDestroyFlat); ok { + update = dn.Original.Resource + } + if update != nil { + update.Lifecycle.CreateBeforeDestroy = true + } } // Get the creation side of this node