Skip to content

Commit

Permalink
fix CreateBeforeDestroy with datasources
Browse files Browse the repository at this point in the history
The graph transformation we implement around create_before_destroy
need to re-order all resources that depend on the create_before_destroy
resource. Up until now, we've requires that users mark all of these
resources as create_before_destroy. Data soruces however don't have a
lifecycle block for create_before_destroy, and could not be marked this
way.

This PR checks each DestroyNode that doesn't implement CreateBeforeDestroy
for any ancestors that do implement CreateBeforeDestroy. If there are
any, we inherit the behavior and re-order the graph as such.
  • Loading branch information
jbardin committed Nov 3, 2016
1 parent 9cc8815 commit cf3a259
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
5 changes: 3 additions & 2 deletions terraform/graph_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ func TestBuiltinGraphBuilder_cbdDepNonCbd(t *testing.T) {
}
}

// This now returns no errors due to a general fix while building the graph
func TestBuiltinGraphBuilder_cbdDepNonCbd_errorsWhenVerbose(t *testing.T) {
b := &BuiltinGraphBuilder{
Root: testModule(t, "graph-builder-cbd-non-cbd"),
Expand All @@ -181,8 +182,8 @@ func TestBuiltinGraphBuilder_cbdDepNonCbd_errorsWhenVerbose(t *testing.T) {
}

_, err := b.Build(RootModulePath)
if err == nil {
t.Fatalf("expected err, got none")
if err != nil {
t.Fatalf("err: %s", err)
}
}

Expand Down
30 changes: 26 additions & 4 deletions terraform/transform_destroy.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package terraform

import (
"github.com/hashicorp/terraform/dag"
)
import "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
Expand Down Expand Up @@ -153,7 +151,7 @@ func (t *CreateBeforeDestroyTransformer) Transform(g *Graph) error {
}

// If the node doesn't need to create before destroy, then continue
if !dn.CreateBeforeDestroy() {
if !dn.CreateBeforeDestroy() && noCreateBeforeDestroyAncestors(g, dn) {
continue
}

Expand Down Expand Up @@ -200,6 +198,30 @@ func (t *CreateBeforeDestroyTransformer) Transform(g *Graph) error {
return nil
}

// noCreateBeforeDestroyAncestors verifies that a vertex has no ancestors that
// are CreateBeforeDestroy.
// If this vertex has an ancestor with CreateBeforeDestroy, we will need to
// inherit that behavior and re-order the edges even if this node type doesn't
// directly implement CreateBeforeDestroy.
func noCreateBeforeDestroyAncestors(g *Graph, v dag.Vertex) bool {
s, _ := g.Ancestors(v)
if s == nil {
return true
}
for _, v := range s.List() {
dn, ok := v.(GraphNodeDestroy)
if !ok {
continue
}

if dn.CreateBeforeDestroy() {
// some ancestor is CreateBeforeDestroy, so we need to follow suit
return false
}
}
return true
}

// PruneDestroyTransformer is a GraphTransformer that removes the destroy
// nodes that aren't in the diff.
type PruneDestroyTransformer struct {
Expand Down

0 comments on commit cf3a259

Please sign in to comment.