diff --git a/dag/walk.go b/dag/walk.go index 8fb60f23a..d7b202d71 100644 --- a/dag/walk.go +++ b/dag/walk.go @@ -148,7 +148,6 @@ func (w *Walker) Wait() tfdiags.Diagnostics { // Multiple Updates can be called in parallel. Update can be called at any // time during a walk. func (w *Walker) Update(g *AcyclicGraph) { - log.Print("[TRACE] dag/walk: updating graph") w.init() v := make(Set) e := make(Set) @@ -181,7 +180,6 @@ func (w *Walker) Update(g *AcyclicGraph) { w.wait.Add(1) // Add to our own set so we know about it already - log.Printf("[TRACE] dag/walk: added new vertex: %q", VertexName(v)) w.vertices.Add(raw) // Initialize the vertex info @@ -212,8 +210,6 @@ func (w *Walker) Update(g *AcyclicGraph) { // Delete it out of the map delete(w.vertexMap, v) - - log.Printf("[TRACE] dag/walk: removed vertex: %q", VertexName(v)) w.vertices.Delete(raw) } @@ -242,10 +238,6 @@ func (w *Walker) Update(g *AcyclicGraph) { // Record that the deps changed for this waiter changedDeps.Add(waiter) - - log.Printf( - "[TRACE] dag/walk: added edge: %q waiting on %q", - VertexName(waiter), VertexName(dep)) w.edges.Add(raw) } @@ -266,10 +258,6 @@ func (w *Walker) Update(g *AcyclicGraph) { // Record that the deps changed for this waiter changedDeps.Add(waiter) - - log.Printf( - "[TRACE] dag/walk: removed edge: %q waiting on %q", - VertexName(waiter), VertexName(dep)) w.edges.Delete(raw) } @@ -310,10 +298,6 @@ func (w *Walker) Update(g *AcyclicGraph) { } info.depsCancelCh = cancelCh - log.Printf( - "[TRACE] dag/walk: dependencies changed for %q, sending new deps", - VertexName(v)) - // Start the waiter go w.waitDeps(v, deps, doneCh, cancelCh) } diff --git a/terraform/node_module_expand.go b/terraform/node_module_expand.go index f77989890..a0669b7d4 100644 --- a/terraform/node_module_expand.go +++ b/terraform/node_module_expand.go @@ -97,6 +97,27 @@ 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) +) + +func (n *nodeCloseModule) ModulePath() addrs.Module { + mod, _ := n.Addr.Call() + return mod +} + +func (n *nodeCloseModule) ReferenceableAddrs() []addrs.Referenceable { + _, call := n.Addr.Call() + return []addrs.Referenceable{ + call, + } } func (n *nodeCloseModule) Name() string { @@ -123,7 +144,8 @@ func (n *nodeCloseModule) EvalTree() EvalNode { &EvalOpFilter{ Ops: []walkOperation{walkApply, walkDestroy}, Node: &evalCloseModule{ - Addr: n.Addr, + Addr: n.Addr, + orphaned: n.orphaned, }, }, }, @@ -131,7 +153,8 @@ func (n *nodeCloseModule) EvalTree() EvalNode { } type evalCloseModule struct { - Addr addrs.Module + Addr addrs.Module + orphaned bool } func (n *evalCloseModule) Eval(ctx EvalContext) (interface{}, error) { @@ -141,7 +164,11 @@ func (n *evalCloseModule) Eval(ctx EvalContext) (interface{}, error) { defer ctx.State().Unlock() expander := ctx.InstanceExpander() - currentModuleInstances := expander.ExpandModule(n.Addr) + var currentModuleInstances []addrs.ModuleInstance + // we can't expand if we're just removing + if !n.orphaned { + currentModuleInstances = expander.ExpandModule(n.Addr) + } for modKey, mod := range state.Modules { if !n.Addr.Equal(mod.Addr.Module()) { @@ -155,13 +182,18 @@ func (n *evalCloseModule) Eval(ctx EvalContext) (interface{}, error) { } } - // if this instance is not in the current expansion, remove it from the - // state found := false - for _, current := range currentModuleInstances { - if current.Equal(mod.Addr) { - found = true - break + 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 + } } } diff --git a/terraform/node_module_removed.go b/terraform/node_module_removed.go deleted file mode 100644 index b6ea00bff..000000000 --- a/terraform/node_module_removed.go +++ /dev/null @@ -1,97 +0,0 @@ -package terraform - -import ( - "fmt" - - "github.com/hashicorp/terraform/addrs" -) - -// NodeModuleRemoved represents a module that is no longer in the -// config. -type NodeModuleRemoved struct { - Addr addrs.ModuleInstance -} - -var ( - _ GraphNodeModuleInstance = (*NodeModuleRemoved)(nil) - _ RemovableIfNotTargeted = (*NodeModuleRemoved)(nil) - _ GraphNodeEvalable = (*NodeModuleRemoved)(nil) - _ GraphNodeReferencer = (*NodeModuleRemoved)(nil) - _ GraphNodeReferenceOutside = (*NodeModuleRemoved)(nil) -) - -func (n *NodeModuleRemoved) Name() string { - return fmt.Sprintf("%s (removed)", n.Addr.String()) -} - -// GraphNodeModuleInstance -func (n *NodeModuleRemoved) Path() addrs.ModuleInstance { - return n.Addr -} - -// GraphNodeModulePath implementation -func (n *NodeModuleRemoved) ModulePath() addrs.Module { - // This node represents the module call within a module, - // so return the CallerAddr as the path, as the module - // call may expand into multiple child instances - return n.Addr.Module() -} - -// GraphNodeEvalable -func (n *NodeModuleRemoved) EvalTree() EvalNode { - return &EvalOpFilter{ - Ops: []walkOperation{walkRefresh, walkApply, walkDestroy}, - Node: &EvalCheckModuleRemoved{ - Addr: n.Addr, - }, - } -} - -func (n *NodeModuleRemoved) ReferenceOutside() (selfPath, referencePath addrs.Module) { - // Our "References" implementation indicates that this node depends on - // the call to the module it represents, which implicitly depends on - // everything inside the module. That reference must therefore be - // interpreted in terms of our parent module. - return n.Addr.Module(), n.Addr.Parent().Module() -} - -func (n *NodeModuleRemoved) References() []*addrs.Reference { - // We depend on the call to the module we represent, because that - // implicitly then depends on everything inside that module. - // Our ReferenceOutside implementation causes this to be interpreted - // within the parent module. - - _, call := n.Addr.CallInstance() - return []*addrs.Reference{ - { - Subject: call, - - // No source range here, because there's nothing reasonable for - // us to return. - }, - } -} - -// RemovableIfNotTargeted -func (n *NodeModuleRemoved) RemoveIfNotTargeted() bool { - // We need to add this so that this node will be removed if - // it isn't targeted or a dependency of a target. - return true -} - -// EvalCheckModuleRemoved is an EvalNode implementation that verifies that -// a module has been removed from the state as expected. -type EvalCheckModuleRemoved struct { - Addr addrs.ModuleInstance -} - -func (n *EvalCheckModuleRemoved) Eval(ctx EvalContext) (interface{}, error) { - mod := ctx.State().Module(n.Addr) - if mod != nil { - // If we get here then that indicates a bug either in the states - // module or in an earlier step of the graph walk, since we should've - // pruned out the module when the last resource was removed from it. - return nil, fmt.Errorf("leftover module %s in state that should have been removed; this is a bug in Terraform and should be reported", n.Addr) - } - return nil, nil -} diff --git a/terraform/transform_module_expansion.go b/terraform/transform_module_expansion.go index 4e688718f..f26bc8385 100644 --- a/terraform/transform_module_expansion.go +++ b/terraform/transform_module_expansion.go @@ -21,9 +21,12 @@ type ModuleExpansionTransformer struct { // Concrete allows injection of a wrapped module node by the graph builder // to alter the evaluation behavior. Concrete ConcreteModuleNodeFunc + + closers map[string]*nodeCloseModule } func (t *ModuleExpansionTransformer) Transform(g *Graph) error { + t.closers = make(map[string]*nodeCloseModule) // The root module is always a singleton and so does not need expansion // processing, but any descendent modules do. We'll process them // recursively using t.transform. @@ -33,6 +36,26 @@ func (t *ModuleExpansionTransformer) Transform(g *Graph) error { return err } } + + // Now go through and connect all nodes to their respective module closers. + // This is done all at once here, because orphaned modules were already + // handled by the RemovedModuleTransformer, and those module closers are in + // the graph already, and need to be connected to their parent closers. + for _, v := range g.Vertices() { + // any node that executes within the scope of a module should be a + // GraphNodeModulePath + pather, ok := v.(GraphNodeModulePath) + if !ok { + continue + } + if closer, ok := t.closers[pather.ModulePath().String()]; ok { + // The module root depends on each child resource instance, since + // during apply the module expansion will complete before the + // individual instances are applied. + g.Connect(dag.BasicEdge(closer, v)) + } + } + return nil } @@ -58,16 +81,15 @@ func (t *ModuleExpansionTransformer) transform(g *Graph, c *configs.Config, pare g.Connect(dag.BasicEdge(v, parentNode)) } - // Add the root module node to provide a single exit point for the expanded - // module. - moduleRoot := &nodeCloseModule{ + // Add the closer (which acts as the root module node) to provide a + // single exit point for the expanded module. + closer := &nodeCloseModule{ Addr: c.Path, } - g.Add(moduleRoot) - g.Connect(dag.BasicEdge(moduleRoot, v)) + g.Add(closer) + g.Connect(dag.BasicEdge(closer, v)) + t.closers[c.Path.String()] = closer - // Connect any node that reports this module as its Path to ensure that - // the module expansion will be handled before that node. for _, childV := range g.Vertices() { pather, ok := childV.(GraphNodeModulePath) if !ok { @@ -76,11 +98,6 @@ func (t *ModuleExpansionTransformer) transform(g *Graph, c *configs.Config, pare if pather.ModulePath().Equal(c.Path) { log.Printf("[TRACE] ModuleExpansionTransformer: %s must wait for expansion of %s", dag.VertexName(childV), c.Path) g.Connect(dag.BasicEdge(childV, v)) - - // The module root also depends on each child instance, since - // during apply the module expansion will complete before the - // individual instances are applied. - g.Connect(dag.BasicEdge(moduleRoot, childV)) } } diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index d92783a32..74df6628a 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -365,17 +365,6 @@ func NewReferenceMap(vs []dag.Vertex) *ReferenceMap { key := m.mapKey(path, addr) vertices[key] = append(vertices[key], v) } - - // Any node can be referenced by the address of the module it belongs - // to or any of that module's ancestors. - for _, addr := range path.Ancestors()[1:] { - // Can be referenced either as the specific call instance (with - // an instance key) or as the bare module call itself (the "module" - // block in the parent module that created the instance). - callPath, call := addr.Call() - callKey := m.mapKey(callPath, call) - vertices[callKey] = append(vertices[callKey], v) - } } m.vertices = vertices diff --git a/terraform/transform_removed_modules.go b/terraform/transform_removed_modules.go index ee71387e2..7b2464790 100644 --- a/terraform/transform_removed_modules.go +++ b/terraform/transform_removed_modules.go @@ -3,6 +3,7 @@ package terraform import ( "log" + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/states" ) @@ -20,14 +21,25 @@ func (t *RemovedModuleTransformer) Transform(g *Graph) error { return nil } + removed := map[string]addrs.Module{} + for _, m := range t.State.Modules { cc := t.Config.DescendentForInstance(m.Addr) if cc != nil { continue } - + removed[m.Addr.Module().String()] = m.Addr.Module() log.Printf("[DEBUG] %s is no longer in configuration\n", m.Addr) - g.Add(&NodeModuleRemoved{Addr: m.Addr}) } + + // add closers to collect any module instances we're removing + for _, modAddr := range removed { + closer := &nodeCloseModule{ + Addr: modAddr, + orphaned: true, + } + g.Add(closer) + } + return nil }