diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 3736fd3d7..f40827859 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -163,9 +163,10 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { // Create a destroy node for outputs to remove them from the state. &DestroyOutputTransformer{Destroy: b.Destroy}, - // Prune unreferenced values, which may have interpolations that can't - // be resolved. - &pruneUnusedExpanderTransformer{}, + // We need to remove configuration nodes that are not used at all, as + // they may not be able to evaluate, especially during destroy. + // These include variables, locals, and instance expanders. + &pruneUnusedNodesTransformer{}, // Add the node to fix the state count boundaries &CountBoundaryTransformer{ diff --git a/terraform/node_module_expand.go b/terraform/node_module_expand.go index 7f9f3c3b9..5168508ee 100644 --- a/terraform/node_module_expand.go +++ b/terraform/node_module_expand.go @@ -1,7 +1,6 @@ package terraform import ( - "fmt" "log" "github.com/hashicorp/terraform/addrs" @@ -10,12 +9,6 @@ import ( "github.com/hashicorp/terraform/lang" ) -// graphNodeModuleCloser is an interface implemented by nodes that finalize the -// evaluation of modules. -type graphNodeModuleCloser interface { - CloseModule() addrs.Module -} - type ConcreteModuleNodeFunc func(n *nodeExpandModule) dag.Vertex // nodeExpandModule represents a module call in the configuration that @@ -126,14 +119,9 @@ func (n *nodeExpandModule) EvalTree() EvalNode { // empty resources and modules from the state. type nodeCloseModule struct { Addr addrs.Module - - // orphaned indicates that this module has no expansion, because it no - // longer exists in the configuration - orphaned bool } var ( - _ graphNodeModuleCloser = (*nodeCloseModule)(nil) _ GraphNodeReferenceable = (*nodeCloseModule)(nil) _ GraphNodeReferenceOutside = (*nodeCloseModule)(nil) ) @@ -160,10 +148,6 @@ func (n *nodeCloseModule) Name() string { return n.Addr.String() + " (close)" } -func (n *nodeCloseModule) CloseModule() addrs.Module { - return n.Addr -} - // RemovableIfNotTargeted implementation func (n *nodeCloseModule) RemoveIfNotTargeted() bool { // We need to add this so that this node will be removed if @@ -177,8 +161,7 @@ func (n *nodeCloseModule) EvalTree() EvalNode { &EvalOpFilter{ Ops: []walkOperation{walkApply, walkDestroy}, Node: &evalCloseModule{ - Addr: n.Addr, - orphaned: n.orphaned, + Addr: n.Addr, }, }, }, @@ -186,8 +169,7 @@ func (n *nodeCloseModule) EvalTree() EvalNode { } type evalCloseModule struct { - Addr addrs.Module - orphaned bool + Addr addrs.Module } func (n *evalCloseModule) Eval(ctx EvalContext) (interface{}, error) { @@ -196,20 +178,6 @@ func (n *evalCloseModule) Eval(ctx EvalContext) (interface{}, error) { state := ctx.State().Lock() defer ctx.State().Unlock() - expander := ctx.InstanceExpander() - var currentModuleInstances []addrs.ModuleInstance - // we can't expand if we're just removing - if !n.orphaned { - func() { - // FIXME: we need to "turn off" closers if their expander has been removed - defer func() { - recover() - n.orphaned = true - }() - currentModuleInstances = expander.ExpandModule(n.Addr) - }() - } - for modKey, mod := range state.Modules { if !n.Addr.Equal(mod.Addr.Module()) { continue @@ -222,27 +190,8 @@ func (n *evalCloseModule) Eval(ctx EvalContext) (interface{}, error) { } } - found := false - if n.orphaned { - // we're removing the entire module, so all instances must go - found = true - } else { - // if this instance is not in the current expansion, remove it from - // the state - for _, current := range currentModuleInstances { - if current.Equal(mod.Addr) { - found = true - break - } - } - } - - if !found { - if len(mod.Resources) > 0 { - // FIXME: add more info to this error - return nil, fmt.Errorf("module %q still contains resources in state", mod.Addr) - } - + // empty child modules are always removed + if len(mod.Resources) == 0 && !mod.Addr.IsRoot() { delete(state.Modules, modKey) } } diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index 7f54f0c3e..0d9a06ddf 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -167,74 +167,87 @@ func (t *DestroyEdgeTransformer) Transform(g *Graph) error { return nil } -// Nodes that register instances in the instances.Expander are not needed -// during apply if there are no instances that will lookup the expansion. This -// is the case when a module tree is removed or during a full destroy, and we -// may not be able to evaluate the expansion expression. -type pruneUnusedExpanderTransformer struct { +// Remove any nodes that aren't needed when destroying modules. +// Variables, outputs, locals, and expanders may not be able to evaluate +// correctly, so we can remove these if nothing depends on them. The module +// closers also need to disable their use of expansion if the module itself is +// no longer present. +type pruneUnusedNodesTransformer struct { } -func (t *pruneUnusedExpanderTransformer) Transform(g *Graph) error { - // We need a reverse depth first walk of modules, but it needs to be - // recursive so that we can process the lead modules first. +func (t *pruneUnusedNodesTransformer) Transform(g *Graph) error { + // We need a reverse depth first walk of modules, processing them in order + // from the leaf modules to the root. This allows us to remove unneeded + // dependencies from child modules, freeing up nodes in the parent module + // to also be removed. - // collect all nodes into their containing module - type mod struct { - addr addrs.Module - nodes []dag.Vertex - } - - // first collect the nodes into their respective modules - moduleMap := make(map[string]*mod) + // First collect the nodes into their respective modules based on + // configuration path. + moduleMap := make(map[string]pruneUnusedNodesMod) for _, v := range g.Vertices() { var path addrs.Module switch v := v.(type) { case instanceExpander: path = v.expandsInstances() - case graphNodeModuleCloser: - // module closers are connected like module calls, and report - // their parent module address - path = v.CloseModule() - case GraphNodeModulePath: path = v.ModulePath() + default: + continue } - m, ok := moduleMap[path.String()] - if !ok { - m = &mod{} - moduleMap[path.String()] = m - } - + m := moduleMap[path.String()] m.addr = path m.nodes = append(m.nodes, v) + + // We need to keep track of the closers, to make sure they don't look + // for an expansion if there's nothing being expanded. + if c, ok := v.(*nodeCloseModule); ok { + m.closer = c + } + moduleMap[path.String()] = m } // now we need to restructure the modules so we can sort them - var modules []*mod + var modules []pruneUnusedNodesMod for _, mod := range moduleMap { modules = append(modules, mod) } - // Sort them by path length, longest first, so that we process the deepest - // modules first. The order of modules at the same tree level doesn't - // matter, we just need to ensure that child modules are processed before - // parent modules. + // Sort them by path length, longest first, so that start with the deepest + // modules. The order of modules at the same tree level doesn't matter, we + // just need to ensure that child modules are processed before parent + // modules. sort.Slice(modules, func(i, j int) bool { return len(modules[i].addr) > len(modules[j].addr) }) - for _, module := range modules { - t.removeUnused(module.nodes, g) + for _, mod := range modules { + mod.removeUnused(g) } return nil } -func (t *pruneUnusedExpanderTransformer) removeUnused(nodes []dag.Vertex, g *Graph) { +// pruneUnusedNodesMod is a container to hold the nodes that belong to a +// particular configuration module for the pruneUnusedNodesTransformer +type pruneUnusedNodesMod struct { + addr addrs.Module + nodes []dag.Vertex + closer *nodeCloseModule +} + +// Remove any unused locals, variables, outputs and expanders. Since module +// closers can also lookup expansion info to detect orphaned instances, disable +// them if their associated expander is removed. +func (m *pruneUnusedNodesMod) removeUnused(g *Graph) { + // We modify the nodes slice during processing here. + // Make a copy so no one is surprised by this changing in the future. + nodes := make([]dag.Vertex, len(m.nodes)) + copy(nodes, m.nodes) + // since we have no defined structure within the module, just cycle through - // the nodes until there are no more removals + // the nodes in each module until there are no more removals removed := true for { if !removed { @@ -242,51 +255,51 @@ func (t *pruneUnusedExpanderTransformer) removeUnused(nodes []dag.Vertex, g *Gra } removed = false - last := len(nodes) - 1 - - NEXT: for i := 0; i < len(nodes); i++ { - n := nodes[i] - switch n.(type) { - case graphNodeTemporaryValue: - if n, ok := n.(GraphNodeModulePath); ok { - // root outputs always have a dependency on remote state - if n.ModulePath().IsRoot() { - continue NEXT + // run this in a closure, so we can return early rather than + // dealing with complex looping and labels + func() { + n := nodes[i] + switch n.(type) { + case graphNodeTemporaryValue: + // temporary value, which consist of variables, locals, and + // outputs, must be kept if anything refers to them. + if n, ok := n.(GraphNodeModulePath); ok { + // root outputs always have an implicit dependency on + // remote state. + if n.ModulePath().IsRoot() { + return + } } - } - for _, vv := range g.UpEdges(n) { - if _, ok := vv.(GraphNodeReferencer); ok { - continue NEXT + for _, vv := range g.UpEdges(n) { + // keep any value which is connected through a + // reference + if _, ok := vv.(GraphNodeReferencer); ok { + return + } } + + case instanceExpander: + // Any nodes that expand instances are kept when their + // instances may need to be evaluated. + for _, vv := range g.UpEdges(n) { + if _, ok := vv.(requiresInstanceExpansion); ok { + return + } + } + + default: + return } - case instanceExpander: - for _, vv := range g.UpEdges(n) { - if _, ok := vv.(requiresInstanceExpansion); ok { - continue NEXT - } - } + g.Remove(n) + removed = true - default: - continue NEXT - } - - removed = true - - //// connect through edges - //for _, d := range g.DownEdges(n) { - // for _, u := range g.UpEdges(n) { - // g.Connect(dag.BasicEdge(u, d)) - // } - //} - - g.Remove(n) - - // remove the node from our iteration as well - nodes[i], nodes[last] = nodes[last], nodes[i] - nodes = nodes[:last] - last-- + // remove the node from our iteration as well + last := len(nodes) - 1 + nodes[i], nodes[last] = nodes[last], nodes[i] + nodes = nodes[:last] + }() } } } diff --git a/terraform/transform_module_expansion.go b/terraform/transform_module_expansion.go index 8d41465b7..e6c961c32 100644 --- a/terraform/transform_module_expansion.go +++ b/terraform/transform_module_expansion.go @@ -43,7 +43,7 @@ func (t *ModuleExpansionTransformer) Transform(g *Graph) error { // the graph already, and need to be connected to their parent closers. for _, v := range g.Vertices() { // skip closers so they don't attach to themselves - if _, ok := v.(graphNodeModuleCloser); ok { + if _, ok := v.(*nodeCloseModule); ok { continue } // any node that executes within the scope of a module should be a diff --git a/terraform/transform_removed_modules.go b/terraform/transform_removed_modules.go index 7b2464790..7c354cdfe 100644 --- a/terraform/transform_removed_modules.go +++ b/terraform/transform_removed_modules.go @@ -35,8 +35,7 @@ func (t *RemovedModuleTransformer) Transform(g *Graph) error { // add closers to collect any module instances we're removing for _, modAddr := range removed { closer := &nodeCloseModule{ - Addr: modAddr, - orphaned: true, + Addr: modAddr, } g.Add(closer) }