Merge pull request #2775 from hashicorp/b-noop-prune

Prune unused resources from graph

terraform/context_apply_test.go

@@ -215,6 +215,49 @@ func TestContext2Apply_createBeforeDestroyUpdate(t *testing.T) {
 	}
 }
 
+func TestContext2Apply_destroyComputed(t *testing.T) {
+	m := testModule(t, "apply-destroy-computed")
+	p := testProvider("aws")
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+	state := &State{
+		Modules: []*ModuleState{
+			&ModuleState{
+				Path: rootModulePath,
+				Resources: map[string]*ResourceState{
+					"aws_instance.foo": &ResourceState{
+						Type: "aws_instance",
+						Primary: &InstanceState{
+							ID: "foo",
+							Attributes: map[string]string{
+								"output": "value",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	ctx := testContext2(t, &ContextOpts{
+		Module: m,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State:   state,
+		Destroy: true,
+	})
+
+	if p, err := ctx.Plan(); err != nil {
+		t.Fatalf("err: %s", err)
+	} else {
+		t.Logf(p.String())
+	}
+
+	if _, err := ctx.Apply(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+}
+
 func TestContext2Apply_minimal(t *testing.T) {
 	m := testModule(t, "apply-minimal")
 	p := testProvider("aws")

terraform/graph_builder.go

@@ -164,6 +164,9 @@ func (b *BuiltinGraphBuilder) Steps(path []string) []GraphTransformer {
 				Then: &PruneDestroyTransformer{Diff: b.Diff, State: b.State},
 			}),
 
+			// Remove the noop nodes
+			&PruneNoopTransformer{Diff: b.Diff, State: b.State},
+
 			// Insert nodes to close opened plugin connections
 			&CloseProviderTransformer{},
 			&CloseProvisionerTransformer{},

terraform/graph_config_node_resource.go

@@ -249,6 +249,41 @@ func (n *GraphNodeConfigResource) DestroyNode(mode GraphNodeDestroyMode) GraphNo
 	return result
 }
 
+// GraphNodeNoopPrunable
+func (n *GraphNodeConfigResource) Noop(opts *NoopOpts) bool {
+	// We don't have any noop optimizations for destroy nodes yet
+	if n.DestroyMode != DestroyNone {
+		return false
+	}
+
+	// If there is no diff, then we aren't a noop since something needs to
+	// be done (such as a plan). We only check if we're a noop in a diff.
+	if opts.Diff == nil || opts.Diff.Empty() {
+		return false
+	}
+
+	// If we have no module diff, we're certainly a noop. This is because
+	// it means there is a diff, and that the module we're in just isn't
+	// in it, meaning we're not doing anything.
+	if opts.ModDiff == nil || opts.ModDiff.Empty() {
+		return true
+	}
+
+	// Grab the ID which is the prefix (in the case count > 0 at some point)
+	prefix := n.Resource.Id()
+
+	// Go through the diff and if there are any with our name on it, keep us
+	found := false
+	for k, _ := range opts.ModDiff.Resources {
+		if strings.HasPrefix(k, prefix) {
+			found = true
+			break
+		}
+	}
+
+	return !found
+}
+
 // Same as GraphNodeConfigResource, but for flattening
 type GraphNodeConfigResourceFlat struct {
 	*GraphNodeConfigResource

terraform/graph_config_node_variable.go

@@ -75,6 +75,28 @@ func (n *GraphNodeConfigVariable) DestroyEdgeInclude(v dag.Vertex) bool {
 	return false
 }
 
+// GraphNodeNoopPrunable
+func (n *GraphNodeConfigVariable) Noop(opts *NoopOpts) bool {
+	// If we have no diff, always keep this in the graph. We have to do
+	// this primarily for validation: we want to validate that variable
+	// interpolations are valid even if there are no resources that
+	// depend on them.
+	if opts.Diff == nil || opts.Diff.Empty() {
+		return false
+	}
+
+	for _, v := range opts.Graph.UpEdges(opts.Vertex).List() {
+		// This is terrible, but I can't think of a better way to do this.
+		if dag.VertexName(v) == rootNodeName {
+			continue
+		}
+
+		return false
+	}
+
+	return true
+}
+
 // GraphNodeProxy impl.
 func (n *GraphNodeConfigVariable) Proxy() bool {
 	return true

terraform/interpolate.go

@@ -370,7 +370,7 @@ MISSING:
 	// be unknown. Instead, we return that the value is computed so
 	// that the graph can continue to refresh other nodes. It doesn't
 	// matter because the config isn't interpolated anyways.
-	if i.Operation == walkRefresh {
+	if i.Operation == walkRefresh || i.Operation == walkPlanDestroy {
 		return config.UnknownVariableValue, nil
 	}
 
@@ -446,7 +446,7 @@ func (i *Interpolater) computeResourceMultiVariable(
 		// be unknown. Instead, we return that the value is computed so
 		// that the graph can continue to refresh other nodes. It doesn't
 		// matter because the config isn't interpolated anyways.
-		if i.Operation == walkRefresh {
+		if i.Operation == walkRefresh || i.Operation == walkPlanDestroy {
 			return config.UnknownVariableValue, nil
 		}
 

terraform/test-fixtures/apply-destroy-computed/child/main.tf

@@ -0,0 +1,5 @@
+variable "value" {}
+
+resource "aws_instance" "bar" {
+    value = "${var.value}"
+}

terraform/test-fixtures/apply-destroy-computed/main.tf

@@ -0,0 +1,6 @@
+resource "aws_instance" "foo" {}
+
+module "child" {
+    source = "./child"
+    value = "${aws_instance.foo.output}"
+}

terraform/transform_noop.go

@@ -0,0 +1,104 @@
+package terraform
+
+import (
+	"github.com/hashicorp/terraform/dag"
+)
+
+// GraphNodeNoopPrunable can be implemented by nodes that can be
+// pruned if they are noops.
+type GraphNodeNoopPrunable interface {
+	Noop(*NoopOpts) bool
+}
+
+// NoopOpts are the options available to determine if your node is a noop.
+type NoopOpts struct {
+	Graph    *Graph
+	Vertex   dag.Vertex
+	Diff     *Diff
+	State    *State
+	ModDiff  *ModuleDiff
+	ModState *ModuleState
+}
+
+// PruneNoopTransformer is a graph transform that prunes nodes that
+// consider themselves no-ops. This is done to both simplify the graph
+// as well as to remove graph nodes that might otherwise cause problems
+// during the graph run. Therefore, this transformer isn't completely
+// an optimization step, and can instead be considered critical to
+// Terraform operations.
+//
+// Example of the above case: variables for modules interpolate their values.
+// Interpolation will fail on destruction (since attributes are being deleted),
+// but variables shouldn't even eval if there is nothing that will consume
+// the variable. Therefore, variables can note that they can be omitted
+// safely in this case.
+//
+// The PruneNoopTransformer will prune nodes depth first, and will automatically
+// create connect through the dependencies of pruned nodes. For example,
+// if we have a graph A => B => C (A depends on B, etc.), and B decides to
+// be removed, we'll still be left with A => C; the edge will be properly
+// connected.
+type PruneNoopTransformer struct {
+	Diff  *Diff
+	State *State
+}
+
+func (t *PruneNoopTransformer) Transform(g *Graph) error {
+	// Find the leaves.
+	leaves := make([]dag.Vertex, 0, 10)
+	for _, v := range g.Vertices() {
+		if g.DownEdges(v).Len() == 0 {
+			leaves = append(leaves, v)
+		}
+	}
+
+	// Do a depth first walk from the leaves and remove things.
+	return g.ReverseDepthFirstWalk(leaves, func(v dag.Vertex, depth int) error {
+		// We need a prunable
+		pn, ok := v.(GraphNodeNoopPrunable)
+		if !ok {
+			return nil
+		}
+
+		// Start building the noop opts
+		path := g.Path
+		if pn, ok := v.(GraphNodeSubPath); ok {
+			path = pn.Path()
+		}
+
+		var modDiff *ModuleDiff
+		var modState *ModuleState
+		if t.Diff != nil {
+			modDiff = t.Diff.ModuleByPath(path)
+		}
+		if t.State != nil {
+			modState = t.State.ModuleByPath(path)
+		}
+
+		// Determine if its a noop. If it isn't, just return
+		noop := pn.Noop(&NoopOpts{
+			Graph:    g,
+			Vertex:   v,
+			Diff:     t.Diff,
+			State:    t.State,
+			ModDiff:  modDiff,
+			ModState: modState,
+		})
+		if !noop {
+			return nil
+		}
+
+		// It is a noop! We first preserve edges.
+		up := g.UpEdges(v).List()
+		for _, downV := range g.DownEdges(v).List() {
+			for _, upV := range up {
+				g.Connect(dag.BasicEdge(upV, downV))
+			}
+		}
+
+		// Then remove it
+		g.Remove(v)
+
+		return nil
+	})
+}

terraform/transform_noop_test.go

@@ -0,0 +1,54 @@
+package terraform
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/terraform/dag"
+)
+
+func TestPruneNoopTransformer(t *testing.T) {
+	g := Graph{Path: RootModulePath}
+
+	a := &testGraphNodeNoop{NameValue: "A"}
+	b := &testGraphNodeNoop{NameValue: "B", Value: true}
+	c := &testGraphNodeNoop{NameValue: "C"}
+
+	g.Add(a)
+	g.Add(b)
+	g.Add(c)
+	g.Connect(dag.BasicEdge(a, b))
+	g.Connect(dag.BasicEdge(b, c))
+
+	{
+		tf := &PruneNoopTransformer{}
+		if err := tf.Transform(&g); err != nil {
+			t.Fatalf("err: %s", err)
+		}
+	}
+
+	actual := strings.TrimSpace(g.String())
+	expected := strings.TrimSpace(testTransformPruneNoopStr)
+	if actual != expected {
+		t.Fatalf("bad:\n\n%s", actual)
+	}
+}
+
+const testTransformPruneNoopStr = `
+A
+  C
+C
+`
+
+type testGraphNodeNoop struct {
+	NameValue string
+	Value     bool
+}
+
+func (v *testGraphNodeNoop) Name() string {
+	return v.NameValue
+}
+
+func (v *testGraphNodeNoop) Noop(*NoopOpts) bool {
+	return v.Value
+}

terraform/transform_root.go

@@ -2,6 +2,8 @@ package terraform
 
 import "github.com/hashicorp/terraform/dag"
 
+const rootNodeName = "root"
+
 // RootTransformer is a GraphTransformer that adds a root to the graph.
 type RootTransformer struct{}
 
@@ -32,7 +34,7 @@ func (t *RootTransformer) Transform(g *Graph) error {
 type graphNodeRoot struct{}
 
 func (n graphNodeRoot) Name() string {
-	return "root"
+	return rootNodeName
 }
 
 func (n graphNodeRoot) Flatten(p []string) (dag.Vertex, error) {