From d31fe5ab9dc79d69d1ab41f92f50f73724c7caea Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 29 Jan 2018 19:17:31 -0500 Subject: [PATCH] delete outputs during destroy Now that outputs are always evaluated, we still need a way to remove them from state when they are destroyed. Previously, outputs were removed during destroy from the same "Applyable" node type that evaluates them. Now that we need to possibly both evaluate and remove output during an apply, we add a new node - NodeDestroyableOutput. This new node is added to the graph by the DestroyOutputTransformer, which make the new destroy node depend on all descendants of the output node. This ensures that the output remains in the state as long as everything which may interpolate the output still exists. --- terraform/context_apply_test.go | 14 ++------- terraform/graph_builder_apply.go | 5 +++ terraform/node_output.go | 52 ++++++++++++++++++++++++++++++++ terraform/transform_output.go | 46 +++++++++++++++++++++++++--- terraform/transform_reference.go | 7 +++-- 5 files changed, 104 insertions(+), 20 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index ebd9ae4ae..e4d40fb6d 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -2590,24 +2590,14 @@ func TestContext2Apply_moduleDestroyOrder(t *testing.T) { &ModuleState{ Path: rootModulePath, Resources: map[string]*ResourceState{ - "aws_instance.b": &ResourceState{ - Type: "aws_instance", - Primary: &InstanceState{ - ID: "b", - }, - }, + "aws_instance.b": resourceState("aws_instance", "b"), }, }, &ModuleState{ Path: []string{"root", "child"}, Resources: map[string]*ResourceState{ - "aws_instance.a": &ResourceState{ - Type: "aws_instance", - Primary: &InstanceState{ - ID: "a", - }, - }, + "aws_instance.a": resourceState("aws_instance", "a"), }, Outputs: map[string]*OutputState{ "a_output": &OutputState{ diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 53c24dc3d..62dc2d275 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -126,6 +126,11 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &DestroyValueReferenceTransformer{}, ), + GraphTransformIf( + func() bool { return b.Destroy }, + &DestroyOutputTransformer{}, + ), + // Add the node to fix the state count boundaries &CountBoundaryTransformer{}, diff --git a/terraform/node_output.go b/terraform/node_output.go index 191de839c..be30911cf 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -93,3 +93,55 @@ func (n *NodeApplyableOutput) EvalTree() EvalNode { }, } } + +// NodeDestroyableOutput represents an output that is "destroybale": +// its application will remove the output from the state. +type NodeDestroyableOutput struct { + PathValue []string + Config *config.Output // Config is the output in the config +} + +func (n *NodeDestroyableOutput) Name() string { + result := fmt.Sprintf("output.%s (destroy)", n.Config.Name) + if len(n.PathValue) > 1 { + result = fmt.Sprintf("%s.%s", modulePrefixStr(n.PathValue), result) + } + + return result +} + +// GraphNodeSubPath +func (n *NodeDestroyableOutput) Path() []string { + return n.PathValue +} + +// RemovableIfNotTargeted +func (n *NodeDestroyableOutput) 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 +} + +// GraphNodeReferencer +func (n *NodeDestroyableOutput) References() []string { + var result []string + result = append(result, n.Config.DependsOn...) + result = append(result, ReferencesFromConfig(n.Config.RawConfig)...) + for _, v := range result { + split := strings.Split(v, "/") + for i, s := range split { + split[i] = s + ".destroy" + } + + result = append(result, strings.Join(split, "/")) + } + + return result +} + +// GraphNodeEvalable +func (n *NodeDestroyableOutput) EvalTree() EvalNode { + return &EvalDeleteOutput{ + Name: n.Config.Name, + } +} diff --git a/terraform/transform_output.go b/terraform/transform_output.go index b260f4caa..faa25e412 100644 --- a/terraform/transform_output.go +++ b/terraform/transform_output.go @@ -1,7 +1,10 @@ package terraform import ( + "log" + "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" ) // OutputTransformer is a GraphTransformer that adds all the outputs @@ -41,11 +44,6 @@ func (t *OutputTransformer) transform(g *Graph, m *module.Tree) error { // Add all outputs here for _, o := range os { - // Build the node. - // - // NOTE: For now this is just an "applyable" output. As we build - // new graph builders for the other operations I suspect we'll - // find a way to parameterize this, require new transforms, etc. node := &NodeApplyableOutput{ PathValue: normalizeModulePath(m.Path()), Config: o, @@ -57,3 +55,41 @@ func (t *OutputTransformer) transform(g *Graph, m *module.Tree) error { return nil } + +// DestroyOutputTransformer 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 DestroyOutputTransformer struct { +} + +func (t *DestroyOutputTransformer) Transform(g *Graph) error { + for _, v := range g.Vertices() { + output, ok := v.(*NodeApplyableOutput) + if !ok { + continue + } + + // create the destroy node for this output + node := &NodeDestroyableOutput{ + PathValue: output.PathValue, + Config: output.Config, + } + + log.Printf("[TRACE] creating %s", node.Name()) + g.Add(node) + + deps, err := g.Descendents(v) + if err != nil { + return err + } + + // the destroy node must depend on the eval node + deps.Add(v) + + for _, d := range deps.List() { + log.Printf("[TRACE] %s depends on %s", node.Name(), dag.VertexName(d)) + g.Connect(dag.BasicEdge(node, d)) + } + } + return nil +} diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index b1c10d00c..fa4f99989 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -96,11 +96,12 @@ func (t *DestroyValueReferenceTransformer) Transform(g *Graph) error { // reverse any outgoing edges so that the value is evaluated first. for _, e := range g.EdgesFrom(v) { target := e.Target() - switch target.(type) { - case *NodeApplyableOutput, *NodeLocal: - // don't reverse other values + + // only destroy nodes will be evaluated in reverse + if _, ok := target.(GraphNodeDestroyer); !ok { continue } + log.Printf("[TRACE] output dep: %s", dag.VertexName(target)) g.RemoveEdge(e)