diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 23652200d..6b4a48b6b 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -30,6 +30,20 @@ func (n *NodeDestroyResource) CreateBeforeDestroy() bool { return n.Config.Lifecycle.CreateBeforeDestroy } +// GraphNodeDestroyerCBD +func (n *NodeDestroyResource) ModifyCreateBeforeDestroy(v bool) error { + // If we have no config, do nothing since it won't affect the + // create step anyways. + if n.Config == nil { + return nil + } + + // Set CBD to true + n.Config.Lifecycle.CreateBeforeDestroy = true + + return nil +} + // GraphNodeReferenceable, overriding NodeAbstractResource func (n *NodeDestroyResource) ReferenceableName() []string { result := n.NodeAbstractResource.ReferenceableName() diff --git a/terraform/transform_destroy_cbd.go b/terraform/transform_destroy_cbd.go index 4b3f45b1f..e1a525b88 100644 --- a/terraform/transform_destroy_cbd.go +++ b/terraform/transform_destroy_cbd.go @@ -1,6 +1,7 @@ package terraform import ( + "fmt" "log" "github.com/hashicorp/terraform/config/module" @@ -15,6 +16,11 @@ type GraphNodeDestroyerCBD interface { // CreateBeforeDestroy returns true if this node represents a node // that is doing a CBD. CreateBeforeDestroy() bool + + // ModifyCreateBeforeDestroy is called when the CBD state of a node + // is changed dynamically. This can return an error if this isn't + // allowed. + ModifyCreateBeforeDestroy(bool) error } // CBDEdgeTransformer modifies the edges of CBD nodes that went through @@ -48,7 +54,23 @@ func (t *CBDEdgeTransformer) Transform(g *Graph) error { } if !dn.CreateBeforeDestroy() { - continue + // If there are no CBD ancestors (dependent nodes), then we + // do nothing here. + if !t.hasCBDAncestor(g, v) { + continue + } + + // If this isn't naturally a CBD node, this means that an ancestor is + // and we need to auto-upgrade this node to CBD. We do this because + // a CBD node depending on non-CBD will result in cycles. To avoid this, + // we always attempt to upgrade it. + if err := dn.ModifyCreateBeforeDestroy(true); err != nil { + return fmt.Errorf( + "%s: must have create before destroy enabled because "+ + "a dependent resource has CBD enabled. However, when "+ + "attempting to automatically do this, an error occurred: %s", + dag.VertexName(v), err) + } } // Find the destroy edge. There should only be one. @@ -189,3 +211,26 @@ func (t *CBDEdgeTransformer) depMap( return depMap, nil } + +// hasCBDAncestor returns true if any ancestor (node that depends on this) +// has CBD set. +func (t *CBDEdgeTransformer) hasCBDAncestor(g *Graph, v dag.Vertex) bool { + s, _ := g.Ancestors(v) + if s == nil { + return true + } + + for _, v := range s.List() { + dn, ok := v.(GraphNodeDestroyerCBD) + if !ok { + continue + } + + if dn.CreateBeforeDestroy() { + // some ancestor is CreateBeforeDestroy, so we need to follow suit + return true + } + } + + return false +} diff --git a/terraform/transform_destroy_cbd_test.go b/terraform/transform_destroy_cbd_test.go index dad1d27bb..bb8fc24f8 100644 --- a/terraform/transform_destroy_cbd_test.go +++ b/terraform/transform_destroy_cbd_test.go @@ -36,6 +36,38 @@ func TestCBDEdgeTransformer(t *testing.T) { } } +func TestCBDEdgeTransformer_depNonCBD(t *testing.T) { + g := Graph{Path: RootModulePath} + g.Add(&graphNodeCreatorTest{AddrString: "test.A"}) + g.Add(&graphNodeCreatorTest{AddrString: "test.B"}) + g.Add(&graphNodeDestroyerTest{AddrString: "test.A"}) + g.Add(&graphNodeDestroyerTest{AddrString: "test.B", CBD: true}) + + module := testModule(t, "transform-destroy-edge-basic") + + { + tf := &DestroyEdgeTransformer{ + Module: module, + } + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &CBDEdgeTransformer{Module: module} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformCBDEdgeDepNonCBDStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + const testTransformCBDEdgeBasicStr = ` test.A test.A (destroy) @@ -43,3 +75,14 @@ test.A (destroy) test.B test.B ` + +const testTransformCBDEdgeDepNonCBDStr = ` +test.A +test.A (destroy) (modified) + test.A + test.B + test.B (destroy) +test.B +test.B (destroy) + test.B +` diff --git a/terraform/transform_destroy_edge_test.go b/terraform/transform_destroy_edge_test.go index 873b01eac..cf8e4c704 100644 --- a/terraform/transform_destroy_edge_test.go +++ b/terraform/transform_destroy_edge_test.go @@ -113,10 +113,25 @@ func (n *graphNodeCreatorTest) CreateAddr() *ResourceAddress { type graphNodeDestroyerTest struct { AddrString string CBD bool + Modified bool +} + +func (n *graphNodeDestroyerTest) Name() string { + result := n.DestroyAddr().String() + " (destroy)" + if n.Modified { + result += " (modified)" + } + + return result } -func (n *graphNodeDestroyerTest) Name() string { return n.DestroyAddr().String() + " (destroy)" } func (n *graphNodeDestroyerTest) CreateBeforeDestroy() bool { return n.CBD } + +func (n *graphNodeDestroyerTest) ModifyCreateBeforeDestroy(v bool) error { + n.Modified = true + return nil +} + func (n *graphNodeDestroyerTest) DestroyAddr() *ResourceAddress { addr, err := ParseResourceAddress(n.AddrString) if err != nil {