diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 29f4625483..5fc2635167 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -150,17 +150,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { Schemas: b.Schemas, }, - // Create a destroy node for root outputs to remove them from the - // state. This does nothing unless invoked via the destroy command - // directly. A destroy is identical to a normal apply, except for the - // fact that we also have configuration to evaluate. While the rest of - // the unused nodes can be programmatically pruned (via - // pruneUnusedNodesTransformer), root module outputs always have an - // implied dependency on remote state. This means that if they exist in - // the configuration, the only signal to remove them is via the destroy - // command itself. - &destroyRootOutputTransformer{Destroy: b.Destroy}, - // 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. diff --git a/terraform/graph_builder_plan_test.go b/terraform/graph_builder_plan_test.go index 956b147ea9..84fec87403 100644 --- a/terraform/graph_builder_plan_test.go +++ b/terraform/graph_builder_plan_test.go @@ -288,10 +288,10 @@ local.instance_id (expand) aws_instance.web (expand) meta.count-boundary (EachMode fixup) aws_load_balancer.weblb (expand) - output.instance_id (expand) + output.instance_id openstack_floating_ip.random (expand) provider["registry.terraform.io/hashicorp/openstack"] -output.instance_id (expand) +output.instance_id local.instance_id (expand) provider["registry.terraform.io/hashicorp/aws"] openstack_floating_ip.random (expand) diff --git a/terraform/node_output.go b/terraform/node_output.go index 07d8ff3354..a9a667bd7f 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -15,8 +15,8 @@ import ( "github.com/zclconf/go-cty/cty" ) -// nodeExpandOutput is the placeholder for an output that has not yet had -// its module path expanded. +// nodeExpandOutput is the placeholder for a non-root module output that has +// not yet had its module path expanded. type nodeExpandOutput struct { Addr addrs.OutputValue Module addrs.Module @@ -37,17 +37,21 @@ var ( func (n *nodeExpandOutput) expandsInstances() {} func (n *nodeExpandOutput) temporaryValue() bool { - // this must always be evaluated if it is a root module output + // non root outputs are temporary return !n.Module.IsRoot() } func (n *nodeExpandOutput) DynamicExpand(ctx EvalContext) (*Graph, error) { if n.Destroy { - return n.planDestroyOutputs(ctx) + // if we're planning a destroy, we only need to handle the root outputs. + // The destroy plan doesn't evaluate any other config, so we can skip + // the rest of the outputs. + return n.planDestroyRootOutput(ctx) } - var g Graph expander := ctx.InstanceExpander() + + var g Graph for _, module := range expander.ExpandModule(n.Module) { absAddr := n.Addr.Absolute(module) @@ -71,32 +75,23 @@ func (n *nodeExpandOutput) DynamicExpand(ctx EvalContext) (*Graph, error) { return &g, nil } -// if we're planing a destroy operation, add destroy nodes for all root outputs -// in the state. -func (n *nodeExpandOutput) planDestroyOutputs(ctx EvalContext) (*Graph, error) { - // we only need to plan destroying root outputs - // Other module outputs may be used during destruction by providers that - // need to interpolate values. +// if we're planing a destroy operation, add a destroy node for any root output +func (n *nodeExpandOutput) planDestroyRootOutput(ctx EvalContext) (*Graph, error) { if !n.Module.IsRoot() { return nil, nil } - state := ctx.State() if state == nil { return nil, nil } - ms := state.Module(addrs.RootModuleInstance) - var g Graph - for _, output := range ms.OutputValues { - o := &NodeDestroyableOutput{ - Addr: output.Addr, - Config: n.Config, - } - log.Printf("[TRACE] Expanding output: adding %s as %T", o.Addr.String(), o) - g.Add(o) + o := &NodeDestroyableOutput{ + Addr: n.Addr.Absolute(addrs.RootModuleInstance), + Config: n.Config, } + log.Printf("[TRACE] Expanding output: adding %s as %T", o.Addr.String(), o) + g.Add(o) return &g, nil } @@ -149,6 +144,8 @@ func (n *nodeExpandOutput) ReferenceOutside() (selfPath, referencePath addrs.Mod // GraphNodeReferencer func (n *nodeExpandOutput) References() []*addrs.Reference { + // root outputs might be destroyable, and may not reference anything in + // that case return referencesForOutput(n.Config) } @@ -364,8 +361,6 @@ func (n *NodeDestroyableOutput) Execute(ctx EvalContext, op walkOperation) error change := &plans.OutputChange{ Addr: n.Addr, Change: plans.Change{ - // This is just a weird placeholder delete action since - // we don't have an actual prior value to indicate. // FIXME: Generate real planned changes for output values // that include the old values. Action: plans.Delete, @@ -379,7 +374,7 @@ func (n *NodeDestroyableOutput) Execute(ctx EvalContext, op walkOperation) error // Should never happen, since we just constructed this right above panic(fmt.Sprintf("planned change for %s could not be encoded: %s", n.Addr, err)) } - log.Printf("[TRACE] planDestroyOutput: Saving %s change for %s in changeset", change.Action, n.Addr) + log.Printf("[TRACE] NodeDestroyableOutput: Saving %s change for %s in changeset", change.Action, n.Addr) changes.RemoveOutputChange(n.Addr) // remove any existing planned change, if present changes.AppendOutputChange(cs) // add the new planned change } diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index 1994563fa8..ff09ba6145 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -26,7 +26,8 @@ var ( _ GraphNodeTargetable = (*nodeExpandApplyableResource)(nil) ) -func (n *nodeExpandApplyableResource) expandsInstances() {} +func (n *nodeExpandApplyableResource) expandsInstances() { +} func (n *nodeExpandApplyableResource) References() []*addrs.Reference { return (&NodeApplyableResource{NodeAbstractResource: n.NodeAbstractResource}).References() diff --git a/terraform/transform_output.go b/terraform/transform_output.go index f0f33aa2c8..7685666ce3 100644 --- a/terraform/transform_output.go +++ b/terraform/transform_output.go @@ -43,73 +43,67 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error { } } - // Add plannable outputs to the graph, which will be dynamically expanded + // Add outputs to the graph, which will be dynamically expanded // into NodeApplyableOutputs to reflect possible expansion // through the presence of "count" or "for_each" on the modules. + // if this is a root output, we add the apply or destroy node directly, as + // the root modules does not expand var changes []*plans.OutputChangeSrc if t.Changes != nil { changes = t.Changes.Outputs } for _, o := range c.Module.Outputs { - node := &nodeExpandOutput{ - Addr: addrs.OutputValue{Name: o.Name}, - Module: c.Path, - Config: o, - Changes: changes, - Destroy: t.Destroy, + addr := addrs.OutputValue{Name: o.Name} + + var rootChange *plans.OutputChangeSrc + for _, c := range changes { + if c.Addr.Module.IsRoot() && c.Addr.OutputValue.Name == o.Name { + rootChange = c + } } + + var node dag.Vertex + switch { + case c.Path.IsRoot() && t.Destroy: + node = &NodeDestroyableOutput{ + Addr: addr.Absolute(addrs.RootModuleInstance), + Config: o, + } + case c.Path.IsRoot(): + destroy := t.Destroy + if rootChange != nil { + destroy = rootChange.Action == plans.Delete + } + + switch { + case destroy: + node = &NodeDestroyableOutput{ + Addr: addr.Absolute(addrs.RootModuleInstance), + Config: o, + } + default: + node = &NodeApplyableOutput{ + Addr: addr.Absolute(addrs.RootModuleInstance), + Config: o, + Change: rootChange, + } + } + + default: + node = &nodeExpandOutput{ + Addr: addr, + Module: c.Path, + Config: o, + Changes: changes, + Destroy: t.Destroy, + } + } + log.Printf("[TRACE] OutputTransformer: adding %s as %T", o.Name, node) g.Add(node) } return nil } - -// destroyRootOutputTransformer is a GraphTransformer that adds nodes to delete -// outputs during destroy. We need to do this to ensure that no stale outputs -// are ever left in the state. -type destroyRootOutputTransformer struct { - Destroy bool -} - -func (t *destroyRootOutputTransformer) Transform(g *Graph) error { - // Only clean root outputs on a full destroy - if !t.Destroy { - return nil - } - - for _, v := range g.Vertices() { - output, ok := v.(*nodeExpandOutput) - if !ok { - continue - } - - // We only destroy root outputs - if !output.Module.Equal(addrs.RootModule) { - continue - } - - // create the destroy node for this output - node := &NodeDestroyableOutput{ - Addr: output.Addr.Absolute(addrs.RootModuleInstance), - Config: output.Config, - } - - log.Printf("[TRACE] creating %s", node.Name()) - g.Add(node) - - deps := g.UpEdges(v) - - for _, d := range deps { - log.Printf("[TRACE] %s depends on %s", node.Name(), dag.VertexName(d)) - g.Connect(dag.BasicEdge(node, d)) - } - - // We no longer need the expand node, since we intend to remove this - // output from the state. - g.Remove(v) - } - return nil -} diff --git a/terraform/transform_targets_test.go b/terraform/transform_targets_test.go index b38be835c7..9339b913ec 100644 --- a/terraform/transform_targets_test.go +++ b/terraform/transform_targets_test.go @@ -122,7 +122,7 @@ module.child.module.grandchild.output.id (expand) module.child.module.grandchild.aws_instance.foo module.child.output.grandchild_id (expand) module.child.module.grandchild.output.id (expand) -output.grandchild_id (expand) +output.grandchild_id module.child.output.grandchild_id (expand) `) if actual != expected { @@ -193,7 +193,7 @@ module.child.module.grandchild.output.id (expand) module.child.module.grandchild.aws_instance.foo module.child.output.grandchild_id (expand) module.child.module.grandchild.output.id (expand) -output.grandchild_id (expand) +output.grandchild_id module.child.output.grandchild_id (expand) `) if actual != expected {