From 1ed37f0b7f44d1461bd3f3bc153a1f7b7d7fdd75 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 16 Feb 2015 12:44:55 -0800 Subject: [PATCH] terraform: change DiffId to DiffInclude to allow smarter logic This lets us check for orphans properly. --- terraform/graph_config_node.go | 40 +++++++++++++++++++++++-- terraform/transform_destroy.go | 53 ++++++---------------------------- 2 files changed, 46 insertions(+), 47 deletions(-) diff --git a/terraform/graph_config_node.go b/terraform/graph_config_node.go index 54da8e1e48..48664ee9f1 100644 --- a/terraform/graph_config_node.go +++ b/terraform/graph_config_node.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "strings" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/config/module" @@ -359,7 +360,12 @@ func (n *graphNodeResourceDestroy) CreateNode() dag.Vertex { return n.Original } -func (n *graphNodeResourceDestroy) DiffId() string { +func (n *graphNodeResourceDestroy) DestroyInclude(d *ModuleDiff, s *ModuleState) bool { + // Always include anything other than the primary destroy + if n.DestroyMode != DestroyPrimary { + return true + } + // Get the count, and specifically the raw value of the count // (with interpolations and all). If the count is NOT a static "1", // then we keep the destroy node no matter what. @@ -409,10 +415,38 @@ func (n *graphNodeResourceDestroy) DiffId() string { // as the cycle would already exist without this anyways. count := n.Original.Resource.RawCount if raw := count.Raw[count.Key]; raw != "1" { - return "" + return true + } + + // Okay, we're dealing with a static count. There are a few ways + // to include this resource. + prefix := n.Original.Resource.Id() + + // If we're present in the diff proper, then keep it. + if d != nil { + for k, _ := range d.Resources { + if strings.HasPrefix(k, prefix) { + return true + } + } + } + + // If we're in the state as a primary in any form, then keep it. + // This does a prefix check so it will also catch orphans on count + // decreases to "1". + if s != nil { + for k, v := range s.Resources { + if !strings.HasPrefix(k, prefix) { + continue + } + + if v.Primary != nil { + return true + } + } } - return n.Original.Resource.Id() + return false } // graphNodeModuleExpanded represents a module where the graph has diff --git a/terraform/transform_destroy.go b/terraform/transform_destroy.go index 747372dc61..7d4293ef38 100644 --- a/terraform/transform_destroy.go +++ b/terraform/transform_destroy.go @@ -1,8 +1,6 @@ package terraform import ( - "strings" - "github.com/hashicorp/terraform/dag" ) @@ -39,12 +37,12 @@ type GraphNodeDestroy interface { CreateNode() dag.Vertex } -// GraphNodeDiffPrunable is the interface that can be implemented to -// signal that this node can be pruned depending on what is in the diff. -type GraphNodeDiffPrunable interface { - // DiffId is used to return the ID that should be checked for - // pruning this resource. If this is empty, pruning won't be done. - DiffId() string +// GraphNodeDestroyPrunable is the interface that can be implemented to +// signal that this node can be pruned depending on state. +type GraphNodeDestroyPrunable interface { + // DestroyInclude is called to check if this node should be included + // with the given state. The state and diff must NOT be modified. + DestroyInclude(*ModuleDiff, *ModuleState) bool } // DestroyTransformer is a GraphTransformer that creates the destruction @@ -217,46 +215,13 @@ func (t *PruneDestroyTransformer) Transform(g *Graph) error { for _, v := range g.Vertices() { // If it is not a destroyer, we don't care - dn, ok := v.(GraphNodeDiffPrunable) + dn, ok := v.(GraphNodeDestroyPrunable) if !ok { continue } - // Grab the name to destroy - prefix := dn.DiffId() - if prefix == "" { - continue - } - - // Assume we're removing it - remove := true - - // We don't remove it if we find it in the diff - if modDiff != nil { - for k, _ := range modDiff.Resources { - if strings.HasPrefix(k, prefix) { - remove = false - break - } - } - } - - // We don't remove it if we find a tainted resource - if modState != nil { - for k, v := range modState.Resources { - if !strings.HasPrefix(k, prefix) { - continue - } - - if len(v.Tainted) > 0 { - remove = false - break - } - } - } - - // Remove the node if we have to - if remove { + // Remove it if we should + if !dn.DestroyInclude(modDiff, modState) { g.Remove(v) } }