Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prune unused resources from graph #2775

Merged
merged 4 commits into from
Jul 20, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions terraform/context_apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 3 additions & 0 deletions terraform/graph_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down
35 changes: 35 additions & 0 deletions terraform/graph_config_node_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions terraform/graph_config_node_variable.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions terraform/interpolate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 5 additions & 0 deletions terraform/test-fixtures/apply-destroy-computed/child/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
variable "value" {}

resource "aws_instance" "bar" {
value = "${var.value}"
}
6 changes: 6 additions & 0 deletions terraform/test-fixtures/apply-destroy-computed/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
resource "aws_instance" "foo" {}

module "child" {
source = "./child"
value = "${aws_instance.foo.output}"
}
104 changes: 104 additions & 0 deletions terraform/transform_noop.go
Original file line number Diff line number Diff line change
@@ -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
})
}
54 changes: 54 additions & 0 deletions terraform/transform_noop_test.go
Original file line number Diff line number Diff line change
@@ -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
}
4 changes: 3 additions & 1 deletion terraform/transform_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}

Expand Down Expand Up @@ -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) {
Expand Down