make the root node a nodeCloseModule for root

Replace the graphNodeRoot for the main graph with a nodeCloseModule for
the root module. USe a new transformer as well, so as to not change any
behavior of DynamicExpand graphs.

Closing out the root module like we do with sub modules means we no
longer need the OrphanResourceTransformer, or the NodeDestroyResource.
The old resource destroy logic has mostly moved into the instance nodes,
and the remaining resource node was just for cleanup, which need to be
done again by the module since there isn't always a NodeDestroyResource
to be evaluated.

The more-correct state caused a few tests to fail, which need to be
cleaned up to match the state without empty resource husks.

terraform/context_apply_test.go

@@ -220,7 +220,8 @@ func TestContext2Apply_resourceCountZeroList(t *testing.T) {
 	}
 
 	got := strings.TrimSpace(state.String())
-	want := strings.TrimSpace(`Outputs:
+	want := strings.TrimSpace(`<no state>
+Outputs:
 
 test = []`)
 	if got != want {
@@ -2830,17 +2831,11 @@ func TestContext2Apply_orphanResource(t *testing.T) {
 			Provider: addrs.NewLegacyProvider("test"),
 			Module:   addrs.RootModule,
 		}
-		zeroAddr := addrs.Resource{
-			Mode: addrs.ManagedResourceMode,
-			Type: "test_thing",
-			Name: "zero",
-		}.Absolute(addrs.RootModuleInstance)
 		oneAddr := addrs.Resource{
 			Mode: addrs.ManagedResourceMode,
 			Type: "test_thing",
 			Name: "one",
 		}.Absolute(addrs.RootModuleInstance)
-		s.SetResourceMeta(zeroAddr, states.EachList, providerAddr)
 		s.SetResourceMeta(oneAddr, states.EachList, providerAddr)
 		s.SetResourceInstanceCurrent(oneAddr.Instance(addrs.IntKey(0)), &states.ResourceInstanceObjectSrc{
 			Status:    states.ObjectReady,
@@ -6565,7 +6560,7 @@ func TestContext2Apply_destroyWithModuleVariableAndCount(t *testing.T) {
 	expected := strings.TrimSpace(`
 <no state>
 module.child:
-		`)
+  <no state>`)
 	if actual != expected {
 		t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual)
 	}
@@ -6731,7 +6726,7 @@ func TestContext2Apply_destroyWithModuleVariableAndCountNested(t *testing.T) {
 	expected := strings.TrimSpace(`
 <no state>
 module.child.child2:
-		`)
+  <no state>`)
 	if actual != expected {
 		t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual)
 	}

terraform/graph_builder_apply.go

@@ -99,18 +99,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 			Changes:  b.Changes,
 		},
 
-		// Creates extra cleanup nodes for any entire resources that are
-		// no longer present in config, so we can make sure we clean up the
-		// leftover empty resource states after the instances have been
-		// destroyed.
-		// (We don't track this particular type of change in the plan because
-		// it's just cleanup of our own state object, and so doesn't effect
-		// any real remote objects or consumable outputs.)
-		&OrphanResourceTransformer{
-			Config: b.Config,
-			State:  b.State,
-		},
-
 		// Create orphan output nodes
 		&OrphanOutputTransformer{Config: b.Config, State: b.State},
 
@@ -189,8 +177,8 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		&CloseProviderTransformer{},
 		&CloseProvisionerTransformer{},
 
-		// Single root
+		// close the root module
-		&RootTransformer{},
+		&CloseRootModuleTransformer{},
 	}
 
 	if !b.DisableReduce {

terraform/graph_builder_destroy_plan.go

@@ -92,8 +92,8 @@ func (b *DestroyPlanGraphBuilder) Steps() []GraphTransformer {
 		// created proper destroy ordering.
 		&TargetsTransformer{Targets: b.Targets},
 
-		// Single root
+		// Close the root module
-		&RootTransformer{},
+		&CloseRootModuleTransformer{},
 	}
 
 	return steps

terraform/graph_builder_eval.go

@@ -97,8 +97,8 @@ func (b *EvalGraphBuilder) Steps() []GraphTransformer {
 		// to get their schemas, and so we must shut them down again here.
 		&CloseProviderTransformer{},
 
-		// Single root
+		// Close root module
-		&RootTransformer{},
+		&CloseRootModuleTransformer{},
 
 		// Remove redundant edges to simplify the graph.
 		&TransitiveReductionTransformer{},

terraform/graph_builder_import.go

@@ -89,8 +89,8 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer {
 		// Close opened plugin connections
 		&CloseProviderTransformer{},
 
-		// Single root
+		// Close root module
-		&RootTransformer{},
+		&CloseRootModuleTransformer{},
 
 		// Optimize
 		&TransitiveReductionTransformer{},

terraform/graph_builder_plan.go

@@ -170,8 +170,8 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 		&CloseProviderTransformer{},
 		&CloseProvisionerTransformer{},
 
-		// Single root
+		// Close the root module
-		&RootTransformer{},
+		&CloseRootModuleTransformer{},
 	}
 
 	if !b.DisableReduce {

terraform/graph_builder_refresh.go

@@ -186,8 +186,8 @@ func (b *RefreshGraphBuilder) Steps() []GraphTransformer {
 		// Close opened plugin connections
 		&CloseProviderTransformer{},
 
-		// Single root
+		// Close root module
-		&RootTransformer{},
+		&CloseRootModuleTransformer{},
 	}
 
 	if !b.DisableReduce {

terraform/node_module_expand.go

@@ -95,11 +95,12 @@ func (n *nodeExpandModule) EvalTree() EvalNode {
 // empty resources and modules from the state.
 type nodeCloseModule struct {
 	Addr addrs.Module
-	Config     *configs.Module
-	ModuleCall *configs.ModuleCall
 }
 
 func (n *nodeCloseModule) Name() string {
+	if len(n.Addr) == 0 {
+		return "root"
+	}
 	return n.Addr.String() + " (close)"
 }
 
@@ -121,8 +122,6 @@ func (n *nodeCloseModule) EvalTree() EvalNode {
 				Ops: []walkOperation{walkApply, walkDestroy},
 				Node: &evalModuleRoot{
 					Addr: n.Addr,
-					Config:     n.Config,
-					ModuleCall: n.ModuleCall,
 				},
 			},
 		},
@@ -131,12 +130,9 @@ func (n *nodeCloseModule) EvalTree() EvalNode {
 
 type evalModuleRoot struct {
 	Addr addrs.Module
-	Config     *configs.Module
-	ModuleCall *configs.ModuleCall
 }
 
 func (n *evalModuleRoot) Eval(ctx EvalContext) (interface{}, error) {
-	return nil, nil
 	// We need the full, locked state, because SyncState does not provide a way to
 	// transact over multiple module instances at the moment.
 	state := ctx.State().Lock()

terraform/node_resource_apply.go

@@ -68,17 +68,6 @@ func (n *nodeExpandApplyableResource) DynamicExpand(ctx EvalContext) (*Graph, er
 		}
 	}
 
-	// FIXME: this isn't used if there are only destroy nodes, so we need to do
-	// this somewhere else
-	for _, res := range orphans {
-		// add the resource cleanup node to remove the resource from state
-		cleanupNode := &NodeDestroyResource{
-			Addr: res.Addr,
-		}
-
-		g.Add(cleanupNode)
-	}
-
 	return &g, nil
 }
 

terraform/node_resource_destroy.go

@@ -278,69 +278,3 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode {
 		},
 	}
 }
-
-// NodeDestroyResource represents a resource that is to be destroyed.
-//
-// Destroying a resource is a state-only operation: it is the individual
-// instances being destroyed that affects remote objects. During graph
-// construction, NodeDestroyResource should always depend on any other node
-// related to the given resource, since it's just a final cleanup to avoid
-// leaving skeleton resource objects in state after their instances have
-// all been destroyed.
-type NodeDestroyResource struct {
-	Addr addrs.AbsResource
-}
-
-var (
-	_ GraphNodeModuleInstance = (*NodeDestroyResource)(nil)
-	_ GraphNodeConfigResource = (*NodeDestroyResource)(nil)
-	_ GraphNodeReferenceable  = (*NodeDestroyResource)(nil)
-	_ GraphNodeReferencer     = (*NodeDestroyResource)(nil)
-	_ GraphNodeEvalable       = (*NodeDestroyResource)(nil)
-)
-
-func (n *NodeDestroyResource) Path() addrs.ModuleInstance {
-	return n.Addr.Module
-}
-
-func (n *NodeDestroyResource) ModulePath() addrs.Module {
-	return n.Addr.Module.Module()
-}
-
-func (n *NodeDestroyResource) Name() string {
-	return n.Addr.String() + " (clean up state)"
-}
-
-// GraphNodeReferenceable, overriding NodeAbstractResource
-func (n *NodeDestroyResource) ReferenceableAddrs() []addrs.Referenceable {
-	// NodeDestroyResource doesn't participate in references: the graph
-	// builder that created it should ensure directly that it already depends
-	// on every other node related to its resource, without relying on
-	// references.
-	return nil
-}
-
-// GraphNodeReferencer, overriding NodeAbstractResource
-func (n *NodeDestroyResource) References() []*addrs.Reference {
-	// NodeDestroyResource doesn't participate in references: the graph
-	// builder that created it should ensure directly that it already depends
-	// on every other node related to its resource, without relying on
-	// references.
-	return nil
-}
-
-// GraphNodeEvalable
-func (n *NodeDestroyResource) EvalTree() EvalNode {
-	// This EvalNode will produce an error if the resource isn't already
-	// empty by the time it is called, since it should just be pruning the
-	// leftover husk of a resource in state after all of the child instances
-	// and their objects were destroyed.
-	return &EvalForgetResourceState{
-		Addr: n.Addr.Resource,
-	}
-}
-
-// GraphNodeResource
-func (n *NodeDestroyResource) ResourceAddr() addrs.ConfigResource {
-	return n.Addr.Config()
-}

terraform/transform_module_expansion.go

@@ -53,8 +53,6 @@ func (t *ModuleExpansionTransformer) transform(g *Graph, c *configs.Config, pare
 	// module.
 	moduleRoot := &nodeCloseModule{
 		Addr: c.Path,
-		Config:     c.Module,
-		ModuleCall: modCall,
 	}
 	g.Add(moduleRoot)
 	g.Connect(dag.BasicEdge(moduleRoot, v))

terraform/transform_orphan_resource.go

@@ -93,85 +93,3 @@ func (t *OrphanResourceInstanceTransformer) transform(g *Graph, ms *states.Modul
 
 	return nil
 }
-
-// OrphanResourceTransformer is a GraphTransformer that adds orphaned
-// resources to the graph. An "orphan" is a resource that is present in
-// the state but no longer present in the config.
-//
-// This is separate to OrphanResourceInstanceTransformer in that it deals with
-// whole resources, rather than individual instances of resources. Orphan
-// resource nodes are only used during apply to clean up leftover empty
-// resource state skeletons, after all of the instances inside have been
-// removed.
-//
-// This transformer will also create edges in the graph to any pre-existing
-// node that creates or destroys the entire orphaned resource or any of its
-// instances, to ensure that the "orphan-ness" of a resource is always dealt
-// with after all other aspects of it.
-type OrphanResourceTransformer struct {
-	Concrete ConcreteResourceNodeFunc
-
-	// State is the global state.
-	State *states.State
-
-	// Config is the root node in the configuration tree.
-	Config *configs.Config
-}
-
-func (t *OrphanResourceTransformer) Transform(g *Graph) error {
-	if t.State == nil {
-		// If the entire state is nil, there can't be any orphans
-		return nil
-	}
-	if t.Config == nil {
-		// Should never happen: we can't be doing any Terraform operations
-		// without at least an empty configuration.
-		panic("OrphanResourceTransformer used without setting Config")
-	}
-
-	// We'll first collect up the existing nodes for each resource so we can
-	// create dependency edges for any new nodes we create.
-	deps := map[string][]dag.Vertex{}
-	for _, v := range g.Vertices() {
-		switch tv := v.(type) {
-		case GraphNodeResourceInstance:
-			k := tv.ResourceInstanceAddr().ContainingResource().String()
-			deps[k] = append(deps[k], v)
-		case GraphNodeConfigResource:
-			k := tv.ResourceAddr().String()
-			deps[k] = append(deps[k], v)
-		case GraphNodeDestroyer:
-			k := tv.DestroyAddr().ContainingResource().String()
-			deps[k] = append(deps[k], v)
-		}
-	}
-
-	for _, ms := range t.State.Modules {
-		moduleAddr := ms.Addr
-
-		mc := t.Config.DescendentForInstance(moduleAddr) // might be nil if whole module has been removed
-
-		for _, rs := range ms.Resources {
-			if mc != nil {
-				if r := mc.Module.ResourceByAddr(rs.Addr.Resource); r != nil {
-					// It's in the config, so nothing to do for this one.
-					continue
-				}
-			}
-
-			node := &NodeDestroyResource{
-				Addr: rs.Addr,
-			}
-
-			log.Printf("[TRACE] OrphanResourceTransformer: adding whole-resource orphan node for %s", rs.Addr)
-			g.Add(node)
-			for _, dn := range deps[rs.Addr.String()] {
-				log.Printf("[TRACE] OrphanResourceTransformer: node %q depends on %q", dag.VertexName(node), dag.VertexName(dn))
-				g.Connect(dag.BasicEdge(node, dn))
-			}
-		}
-	}
-
-	return nil
-
-}

terraform/transform_root.go

@@ -1,6 +1,8 @@
 package terraform
 
-import "github.com/hashicorp/terraform/dag"
+import (
+	"github.com/hashicorp/terraform/dag"
+)
 
 const rootNodeName = "root"
 
@@ -36,3 +38,28 @@ type graphNodeRoot struct{}
 func (n graphNodeRoot) Name() string {
 	return rootNodeName
 }
+
+// CloseRootModuleTransformer is a GraphTransformer that adds a root to the graph.
+type CloseRootModuleTransformer struct{}
+
+func (t *CloseRootModuleTransformer) Transform(g *Graph) error {
+	// close the root module
+	closeRoot := &nodeCloseModule{}
+	g.Add(closeRoot)
+
+	// since this is closing the root module, make it depend on everything in
+	// the root module.
+	for _, v := range g.Vertices() {
+		if v == closeRoot {
+			continue
+		}
+
+		// since this is closing the root module,  and must be last, we can
+		// connect to anything that doesn't have any up edges.
+		if g.UpEdges(v).Len() == 0 {
+			g.Connect(dag.BasicEdge(closeRoot, v))
+		}
+	}
+
+	return nil
+}