From 8560f50cbc5c6de09e019a3c9e49b6d74495a619 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Thu, 21 Apr 2016 21:55:29 +0200 Subject: [PATCH] Change taint behaviour to act as a normal resource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This means it’s shown correctly in a plan and takes into account any actions that are dependant on the tainted resource and, vice verse, any actions that the tainted resource depends on. So this changes the behaviour from saying this resource is tainted so just forget about it and make sure it gets deleted in the background, to saying I want that resource to be recreated (taking into account the existing resource and it’s place in the graph). --- command/format_plan.go | 9 +- command/format_state.go | 2 +- command/untaint.go | 15 +-- helper/schema/resource_data.go | 4 + helper/schema/schema.go | 8 ++ terraform/diff.go | 11 +- terraform/eval_diff.go | 37 +----- terraform/eval_state.go | 56 +-------- terraform/graph_config_node_resource.go | 128 ++++++-------------- terraform/state.go | 121 +++---------------- terraform/transform_destroy.go | 48 ++------ terraform/transform_orphan.go | 12 +- terraform/transform_resource.go | 41 +------ terraform/transform_tainted.go | 154 ------------------------ terraform/transform_tainted_test.go | 71 ----------- 15 files changed, 106 insertions(+), 611 deletions(-) delete mode 100644 terraform/transform_tainted.go delete mode 100644 terraform/transform_tainted_test.go diff --git a/command/format_plan.go b/command/format_plan.go index 7846f0e8c..9ef62b4bf 100644 --- a/command/format_plan.go +++ b/command/format_plan.go @@ -112,9 +112,14 @@ func formatPlanModuleExpand( symbol = "-" } + taintStr := "" + if rdiff.DestroyTainted { + taintStr = " (tainted)" + } + buf.WriteString(opts.Color.Color(fmt.Sprintf( - "[%s]%s %s\n", - color, symbol, name))) + "[%s]%s %s%s\n", + color, symbol, name, taintStr))) // Get all the attributes that are changing, and sort them. Also // determine the longest key so that we can align them all. diff --git a/command/format_state.go b/command/format_state.go index 765fb9d7a..ccfd6573d 100644 --- a/command/format_state.go +++ b/command/format_state.go @@ -101,7 +101,7 @@ func formatStateModuleExpand( } taintStr := "" - if len(rs.Tainted) > 0 { + if rs.Primary.Tainted { taintStr = " (tainted)" } diff --git a/command/untaint.go b/command/untaint.go index b34b9a2fd..099c94859 100644 --- a/command/untaint.go +++ b/command/untaint.go @@ -17,14 +17,12 @@ func (c *UntaintCommand) Run(args []string) int { var allowMissing bool var module string - var index int cmdFlags := c.Meta.flagSet("untaint") cmdFlags.BoolVar(&allowMissing, "allow-missing", false, "module") cmdFlags.StringVar(&module, "module", "", "module") cmdFlags.StringVar(&c.Meta.statePath, "state", DefaultStateFilename, "path") cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path") cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path") - cmdFlags.IntVar(&index, "index", -1, "index") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { return 1 @@ -108,11 +106,7 @@ func (c *UntaintCommand) Run(args []string) int { } // Untaint the resource - if err := rs.Untaint(index); err != nil { - c.Ui.Error(fmt.Sprintf("Error untainting %s: %s", name, err)) - c.Ui.Error("You can use `terraform show` to inspect the current state.") - return 1 - } + rs.Untaint() log.Printf("[INFO] Writing state output to: %s", c.Meta.StateOutPath()) if err := c.Meta.PersistState(s); err != nil { @@ -148,13 +142,6 @@ Options: modifying. Defaults to the "-state-out" path with ".backup" extension. Set to "-" to disable backup. - -index=n Selects a single tainted instance when there are more - than one tainted instances present in the state for a - given resource. This flag is required when multiple - tainted instances are present. The vast majority of the - time, there is a maxiumum of one tainted instance per - resource, so this flag can be safely omitted. - -module=path The module path where the resource lives. By default this will be root. Child modules can be specified by names. Ex. "consul" or "consul.vpc" (nested modules). diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index 36599aebb..500de0a3d 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -290,6 +290,10 @@ func (d *ResourceData) State() *terraform.InstanceState { result.Attributes["id"] = d.Id() } + if d.state != nil { + result.Tainted = d.state.Tainted + } + return &result } diff --git a/helper/schema/schema.go b/helper/schema/schema.go index deed582c1..a113273f3 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -308,6 +308,11 @@ func (m schemaMap) Diff( result := new(terraform.InstanceDiff) result.Attributes = make(map[string]*terraform.ResourceAttrDiff) + // Make sure to mark if the resource is tainted + if s != nil { + result.DestroyTainted = s.Tainted + } + d := &ResourceData{ schema: m, state: s, @@ -330,6 +335,9 @@ func (m schemaMap) Diff( result2 := new(terraform.InstanceDiff) result2.Attributes = make(map[string]*terraform.ResourceAttrDiff) + // Preserve the DestroyTainted flag + result2.DestroyTainted = result.DestroyTainted + // Reset the data to not contain state. We have to call init() // again in order to reset the FieldReaders. d.state = nil diff --git a/terraform/diff.go b/terraform/diff.go index c44c06c18..73d56f823 100644 --- a/terraform/diff.go +++ b/terraform/diff.go @@ -215,11 +215,12 @@ func (d *ModuleDiff) String() string { rdiff := d.Resources[name] crud := "UPDATE" - if rdiff.RequiresNew() && (rdiff.Destroy || rdiff.DestroyTainted) { + switch { + case rdiff.RequiresNew() && (rdiff.Destroy || rdiff.DestroyTainted): crud = "DESTROY/CREATE" - } else if rdiff.Destroy { + case rdiff.Destroy: crud = "DESTROY" - } else if rdiff.RequiresNew() { + case rdiff.RequiresNew(): crud = "CREATE" } @@ -356,6 +357,10 @@ func (d *InstanceDiff) RequiresNew() bool { return false } + if d.DestroyTainted { + return true + } + for _, rd := range d.Attributes { if rd != nil && rd.RequiresNew { return true diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index ea6b124c3..10deacb26 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -104,7 +104,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { diff = new(InstanceDiff) } - // Require a destroy if there is no ID and it requires new. + // Require a destroy if there is an ID and it requires new. if diff.RequiresNew() && state != nil && state.ID != "" { diff.Destroy = true } @@ -216,41 +216,6 @@ func (n *EvalDiffDestroyModule) Eval(ctx EvalContext) (interface{}, error) { return nil, nil } -// EvalDiffTainted is an EvalNode implementation that writes the diff to -// the full diff. -type EvalDiffTainted struct { - Name string - Diff **InstanceDiff -} - -// TODO: test -func (n *EvalDiffTainted) Eval(ctx EvalContext) (interface{}, error) { - state, lock := ctx.State() - - // Get a read lock so we can access this instance - lock.RLock() - defer lock.RUnlock() - - // Look for the module state. If we don't have one, then it doesn't matter. - mod := state.ModuleByPath(ctx.Path()) - if mod == nil { - return nil, nil - } - - // Look for the resource state. If we don't have one, then it is okay. - rs := mod.Resources[n.Name] - if rs == nil { - return nil, nil - } - - // If we have tainted, then mark it on the diff - if len(rs.Tainted) > 0 { - (*n.Diff).DestroyTainted = true - } - - return nil, nil -} - // EvalFilterDiff is an EvalNode implementation that filters the diff // according to some filter. type EvalFilterDiff struct { diff --git a/terraform/eval_state.go b/terraform/eval_state.go index 3a3e9efd6..aa1d1e28e 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -1,8 +1,6 @@ package terraform -import ( - "fmt" -) +import "fmt" // EvalReadState is an EvalNode implementation that reads the // primary InstanceState for a specific resource out of the state. @@ -17,31 +15,6 @@ func (n *EvalReadState) Eval(ctx EvalContext) (interface{}, error) { }) } -// EvalReadStateTainted is an EvalNode implementation that reads a -// tainted InstanceState for a specific resource out of the state -type EvalReadStateTainted struct { - Name string - Output **InstanceState - // Index indicates which instance in the Tainted list to target, or -1 for - // the last item. - Index int -} - -func (n *EvalReadStateTainted) Eval(ctx EvalContext) (interface{}, error) { - return readInstanceFromState(ctx, n.Name, n.Output, func(rs *ResourceState) (*InstanceState, error) { - // Get the index. If it is negative, then we get the last one - idx := n.Index - if idx < 0 { - idx = len(rs.Tainted) - 1 - } - if idx >= 0 && idx < len(rs.Tainted) { - return rs.Tainted[idx], nil - } else { - return nil, fmt.Errorf("bad tainted index: %d, for resource: %#v", idx, rs) - } - }) -} - // EvalReadStateDeposed is an EvalNode implementation that reads the // deposed InstanceState for a specific resource out of the state type EvalReadStateDeposed struct { @@ -168,33 +141,6 @@ func (n *EvalWriteState) Eval(ctx EvalContext) (interface{}, error) { ) } -// EvalWriteStateTainted is an EvalNode implementation that writes -// an InstanceState out to the Tainted list of a resource in the state. -type EvalWriteStateTainted struct { - Name string - ResourceType string - Provider string - Dependencies []string - State **InstanceState - // Index indicates which instance in the Tainted list to target, or -1 to append. - Index int -} - -// EvalWriteStateTainted is an EvalNode implementation that writes the -// one of the tainted InstanceStates for a specific resource out of the state. -func (n *EvalWriteStateTainted) Eval(ctx EvalContext) (interface{}, error) { - return writeInstanceToState(ctx, n.Name, n.ResourceType, n.Provider, n.Dependencies, - func(rs *ResourceState) error { - if n.Index == -1 { - rs.Tainted = append(rs.Tainted, *n.State) - } else { - rs.Tainted[n.Index] = *n.State - } - return nil - }, - ) -} - // EvalWriteStateDeposed is an EvalNode implementation that writes // an InstanceState out to the Deposed list of a resource in the state. type EvalWriteStateDeposed struct { diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index a2c688da4..dbe5a0ecd 100644 --- a/terraform/graph_config_node_resource.go +++ b/terraform/graph_config_node_resource.go @@ -20,9 +20,9 @@ type GraphNodeCountDependent interface { type GraphNodeConfigResource struct { Resource *config.Resource - // If this is set to anything other than destroyModeNone, then this - // resource represents a resource that will be destroyed in some way. - DestroyMode GraphNodeDestroyMode + // If set to true, this resource represents a resource + // that will be destroyed in some way. + Destroy bool // Used during DynamicExpand to target indexes Targets []ResourceAddress @@ -32,10 +32,10 @@ type GraphNodeConfigResource struct { func (n *GraphNodeConfigResource) Copy() *GraphNodeConfigResource { ncr := &GraphNodeConfigResource{ - Resource: n.Resource.Copy(), - DestroyMode: n.DestroyMode, - Targets: make([]ResourceAddress, 0, len(n.Targets)), - Path: make([]string, 0, len(n.Path)), + Resource: n.Resource.Copy(), + Destroy: n.Destroy, + Targets: make([]ResourceAddress, 0, len(n.Targets)), + Path: make([]string, 0, len(n.Path)), } for _, t := range n.Targets { ncr.Targets = append(ncr.Targets, *t.Copy()) @@ -120,23 +120,12 @@ func (n *GraphNodeConfigResource) VarWalk(fn func(config.InterpolatedVariable)) } func (n *GraphNodeConfigResource) Name() string { - result := n.Resource.Id() - switch n.DestroyMode { - case DestroyNone: - case DestroyPrimary: - result += " (destroy)" - case DestroyTainted: - result += " (destroy tainted)" - default: - result += " (unknown destroy type)" - } - - return result + return n.Resource.Id() + " (destroy)" } // GraphNodeDotter impl. func (n *GraphNodeConfigResource) DotNode(name string, opts *GraphDotOpts) *dot.Node { - if n.DestroyMode != DestroyNone && !opts.Verbose { + if n.Destroy && !opts.Verbose { return nil } return dot.NewNode(name, map[string]string{ @@ -162,23 +151,18 @@ func (n *GraphNodeConfigResource) DynamicExpand(ctx EvalContext) (*Graph, error) // Start creating the steps steps := make([]GraphTransformer, 0, 5) - // Primary and non-destroy modes are responsible for creating/destroying - // all the nodes, expanding counts. - switch n.DestroyMode { - case DestroyNone, DestroyPrimary: - steps = append(steps, &ResourceCountTransformer{ - Resource: n.Resource, - Destroy: n.DestroyMode != DestroyNone, - Targets: n.Targets, - }) - } + // Expand counts. + steps = append(steps, &ResourceCountTransformer{ + Resource: n.Resource, + Destroy: n.Destroy, + Targets: n.Targets, + }) // Additional destroy modifications. - switch n.DestroyMode { - case DestroyPrimary: - // If we're destroying the primary instance, then we want to + if n.Destroy { + // If we're destroying a primary or tainted resource, we want to // expand orphans, which have all the same semantics in a destroy - // as a primary. + // as a primary or tainted resource. steps = append(steps, &OrphanTransformer{ State: state, View: n.Resource.Id(), @@ -188,19 +172,12 @@ func (n *GraphNodeConfigResource) DynamicExpand(ctx EvalContext) (*Graph, error) State: state, View: n.Resource.Id(), }) - case DestroyTainted: - // If we're only destroying tainted resources, then we only - // want to find tainted resources and destroy them here. - steps = append(steps, &TaintedTransformer{ - State: state, - View: n.Resource.Id(), - }) } // We always want to apply targeting steps = append(steps, &TargetsTransformer{ ParsedTargets: n.Targets, - Destroy: n.DestroyMode != DestroyNone, + Destroy: n.Destroy, }) // Always end with the root being added @@ -258,9 +235,9 @@ func (n *GraphNodeConfigResource) ProvisionedBy() []string { } // GraphNodeDestroyable -func (n *GraphNodeConfigResource) DestroyNode(mode GraphNodeDestroyMode) GraphNodeDestroy { +func (n *GraphNodeConfigResource) DestroyNode() GraphNodeDestroy { // If we're already a destroy node, then don't do anything - if n.DestroyMode != DestroyNone { + if n.Destroy { return nil } @@ -268,7 +245,8 @@ func (n *GraphNodeConfigResource) DestroyNode(mode GraphNodeDestroyMode) GraphNo GraphNodeConfigResource: *n.Copy(), Original: n, } - result.DestroyMode = mode + result.Destroy = true + return result } @@ -276,7 +254,7 @@ func (n *GraphNodeConfigResource) DestroyNode(mode GraphNodeDestroyMode) GraphNo func (n *GraphNodeConfigResource) Noop(opts *NoopOpts) bool { log.Printf("[DEBUG] Checking resource noop: %s", n.Name()) // We don't have any noop optimizations for destroy nodes yet - if n.DestroyMode != DestroyNone { + if n.Destroy { log.Printf("[DEBUG] Destroy node, not a noop") return false } @@ -365,9 +343,9 @@ func (n *GraphNodeConfigResourceFlat) ProvisionedBy() []string { } // GraphNodeDestroyable impl. -func (n *GraphNodeConfigResourceFlat) DestroyNode(mode GraphNodeDestroyMode) GraphNodeDestroy { +func (n *GraphNodeConfigResourceFlat) DestroyNode() GraphNodeDestroy { // Get our parent destroy node. If we don't have any, just return - raw := n.GraphNodeConfigResource.DestroyNode(mode) + raw := n.GraphNodeConfigResource.DestroyNode() if raw == nil { return nil } @@ -423,13 +401,8 @@ type graphNodeResourceDestroy struct { } func (n *graphNodeResourceDestroy) CreateBeforeDestroy() bool { - // CBD is enabled if the resource enables it in addition to us - // being responsible for destroying the primary state. The primary - // state destroy node is the only destroy node that needs to be - // "shuffled" according to the CBD rules, since tainted resources - // don't have the same inverse dependencies. - return n.Original.Resource.Lifecycle.CreateBeforeDestroy && - n.DestroyMode == DestroyPrimary + // CBD is enabled if the resource enables it + return n.Original.Resource.Lifecycle.CreateBeforeDestroy && n.Destroy } func (n *graphNodeResourceDestroy) CreateNode() dag.Vertex { @@ -437,43 +410,14 @@ func (n *graphNodeResourceDestroy) CreateNode() dag.Vertex { } func (n *graphNodeResourceDestroy) DestroyInclude(d *ModuleDiff, s *ModuleState) bool { - switch n.DestroyMode { - case DestroyPrimary: - return n.destroyIncludePrimary(d, s) - case DestroyTainted: - return n.destroyIncludeTainted(d, s) - default: - return true + if n.Destroy { + return n.destroyInclude(d, s) } + + return true } -func (n *graphNodeResourceDestroy) destroyIncludeTainted( - d *ModuleDiff, s *ModuleState) bool { - // If there is no state, there can't by any tainted. - if s == nil { - return false - } - - // Grab the ID which is the prefix (in the case count > 0 at some point) - prefix := n.Original.Resource.Id() - - // Go through the resources and find any with our prefix. If there - // are any tainted, we need to keep it. - for k, v := range s.Resources { - if !strings.HasPrefix(k, prefix) { - continue - } - - if len(v.Tainted) > 0 { - return true - } - } - - // We didn't find any tainted nodes, return - return false -} - -func (n *graphNodeResourceDestroy) destroyIncludePrimary( +func (n *graphNodeResourceDestroy) destroyInclude( d *ModuleDiff, s *ModuleState) bool { // Get the count, and specifically the raw value of the count // (with interpolations and all). If the count is NOT a static "1", @@ -516,7 +460,7 @@ func (n *graphNodeResourceDestroy) destroyIncludePrimary( // If the count is set to ANYTHING other than a static "1" (variable, // computed attribute, static number greater than 1), then we keep the // destroy, since it is required for dynamic graph expansion to find - // orphan/tainted count objects. + // orphan count objects. // // This isn't ideal logic, but its strictly better without introducing // new impossibilities. It breaks the cycle in practical cases, and the @@ -535,9 +479,9 @@ func (n *graphNodeResourceDestroy) destroyIncludePrimary( // only for resources in the diff that match our resource or a count-index // of our resource that are marked for destroy. if d != nil { - for k, d := range d.Resources { + for k, v := range d.Resources { match := k == prefix || strings.HasPrefix(k, prefix+".") - if match && d.Destroy { + if match && v.Destroy { return true } } diff --git a/terraform/state.go b/terraform/state.go index 12fb901e4..19b781b55 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -780,7 +780,7 @@ func (m *ModuleState) prune() { for k, v := range m.Resources { v.prune() - if (v.Primary == nil || v.Primary.ID == "") && len(v.Tainted) == 0 && len(v.Deposed) == 0 { + if (v.Primary == nil || v.Primary.ID == "") && len(v.Deposed) == 0 { delete(m.Resources, k) } } @@ -826,8 +826,8 @@ func (m *ModuleState) String() string { } taintStr := "" - if len(rs.Tainted) > 0 { - taintStr = fmt.Sprintf(" (%d tainted)", len(rs.Tainted)) + if rs.Primary.Tainted { + taintStr = " (tainted)" } deposedStr := "" @@ -860,10 +860,6 @@ func (m *ModuleState) String() string { buf.WriteString(fmt.Sprintf(" %s = %s\n", ak, av)) } - for idx, t := range rs.Tainted { - buf.WriteString(fmt.Sprintf(" Tainted ID %d = %s\n", idx+1, t.ID)) - } - for idx, t := range rs.Deposed { buf.WriteString(fmt.Sprintf(" Deposed ID %d = %s\n", idx+1, t.ID)) } @@ -1031,14 +1027,6 @@ type ResourceState struct { // This is the instances on which providers will act. Primary *InstanceState `json:"primary"` - // Tainted is used to track any underlying instances that - // have been created but are in a bad or unknown state and - // need to be cleaned up subsequently. In the - // standard case, there is only at most a single instance. - // However, in pathological cases, it is possible for the number - // of instances to accumulate. - Tainted []*InstanceState `json:"tainted,omitempty"` - // Deposed is used in the mechanics of CreateBeforeDestroy: the existing // Primary is Deposed to get it out of the way for the replacement Primary to // be created by Apply. If the replacement Primary creates successfully, the @@ -1084,80 +1072,21 @@ func (s *ResourceState) Equal(other *ResourceState) bool { } // Tainted - taints := make(map[string]*InstanceState) - for _, t := range other.Tainted { - if t == nil { - continue - } - - taints[t.ID] = t - } - for _, t := range s.Tainted { - if t == nil { - continue - } - - otherT, ok := taints[t.ID] - if !ok { - return false - } - delete(taints, t.ID) - - if !t.Equal(otherT) { - return false - } - } - - // This means that we have stuff in other tainted that we don't - // have, so it is not equal. - if len(taints) > 0 { + if s.Primary.Tainted != other.Primary.Tainted { return false } return true } -// Taint takes the primary state and marks it as tainted. If there is no -// primary state, this does nothing. +// Taint marks a resource as tainted. func (r *ResourceState) Taint() { - // If there is no primary, nothing to do - if r.Primary == nil { - return - } - - // Shuffle to the end of the taint list and set primary to nil - r.Tainted = append(r.Tainted, r.Primary) - r.Primary = nil + r.Primary.Tainted = true } -// Untaint takes a tainted InstanceState and marks it as primary. -// The index argument is used to select a single InstanceState from the -// array of Tainted when there are more than one. If index is -1, the -// first Tainted InstanceState will be untainted iff there is only one -// Tainted InstanceState. Index must be >= 0 to specify an InstanceState -// when Tainted has more than one member. -func (r *ResourceState) Untaint(index int) error { - if len(r.Tainted) == 0 { - return fmt.Errorf("Nothing to untaint.") - } - if r.Primary != nil { - return fmt.Errorf("Resource has a primary instance in the state that would be overwritten by untainting. If you want to restore a tainted resource to primary, taint the existing primary instance first.") - } - if index == -1 && len(r.Tainted) > 1 { - return fmt.Errorf("There are %d tainted instances for this resource, please specify an index to select which one to untaint.", len(r.Tainted)) - } - if index == -1 { - index = 0 - } - if index >= len(r.Tainted) { - return fmt.Errorf("There are %d tainted instances for this resource, the index specified (%d) is out of range.", len(r.Tainted), index) - } - - // Perform the untaint - r.Primary = r.Tainted[index] - r.Tainted = append(r.Tainted[:index], r.Tainted[index+1:]...) - - return nil +// Untaint unmarks a resource as tainted. +func (r *ResourceState) Untaint() { + r.Primary.Tainted = false } func (r *ResourceState) init() { @@ -1176,19 +1105,12 @@ func (r *ResourceState) deepcopy() *ResourceState { Type: r.Type, Dependencies: nil, Primary: r.Primary.DeepCopy(), - Tainted: nil, Provider: r.Provider, } if r.Dependencies != nil { n.Dependencies = make([]string, len(r.Dependencies)) copy(n.Dependencies, r.Dependencies) } - if r.Tainted != nil { - n.Tainted = make([]*InstanceState, 0, len(r.Tainted)) - for _, inst := range r.Tainted { - n.Tainted = append(n.Tainted, inst.DeepCopy()) - } - } if r.Deposed != nil { n.Deposed = make([]*InstanceState, 0, len(r.Deposed)) for _, inst := range r.Deposed { @@ -1201,20 +1123,7 @@ func (r *ResourceState) deepcopy() *ResourceState { // prune is used to remove any instances that are no longer required func (r *ResourceState) prune() { - n := len(r.Tainted) - for i := 0; i < n; i++ { - inst := r.Tainted[i] - if inst == nil || inst.ID == "" { - copy(r.Tainted[i:], r.Tainted[i+1:]) - r.Tainted[n-1] = nil - n-- - i-- - } - } - - r.Tainted = r.Tainted[:n] - - n = len(r.Deposed) + n := len(r.Deposed) for i := 0; i < n; i++ { inst := r.Deposed[i] if inst == nil || inst.ID == "" { @@ -1263,6 +1172,9 @@ type InstanceState struct { // ignored by Terraform core. It's meant to be used for accounting by // external client code. Meta map[string]string `json:"meta,omitempty"` + + // Tainted is used to mark a resource for recreation. + Tainted bool `json:"tainted,omitempty"` } func (i *InstanceState) init() { @@ -1282,6 +1194,7 @@ func (i *InstanceState) DeepCopy() *InstanceState { n := &InstanceState{ ID: i.ID, Ephemeral: *i.Ephemeral.DeepCopy(), + Tainted: i.Tainted, } if i.Attributes != nil { n.Attributes = make(map[string]string, len(i.Attributes)) @@ -1343,6 +1256,10 @@ func (s *InstanceState) Equal(other *InstanceState) bool { } } + if s.Tainted != other.Tainted { + return false + } + return true } @@ -1413,6 +1330,8 @@ func (i *InstanceState) String() string { buf.WriteString(fmt.Sprintf("%s = %s\n", ak, av)) } + buf.WriteString(fmt.Sprintf("Tainted = %t\n", i.Tainted)) + return buf.String() } diff --git a/terraform/transform_destroy.go b/terraform/transform_destroy.go index 8d5aeb4c9..af8ccc4ab 100644 --- a/terraform/transform_destroy.go +++ b/terraform/transform_destroy.go @@ -4,14 +4,6 @@ import ( "github.com/hashicorp/terraform/dag" ) -type GraphNodeDestroyMode byte - -const ( - DestroyNone GraphNodeDestroyMode = 0 - DestroyPrimary GraphNodeDestroyMode = 1 << iota - DestroyTainted -) - // GraphNodeDestroyable is the interface that nodes that can be destroyed // must implement. This is used to automatically handle the creation of // destroy nodes in the graph and the dependency ordering of those destroys. @@ -19,7 +11,7 @@ type GraphNodeDestroyable interface { // DestroyNode returns the node used for the destroy with the given // mode. If this returns nil, then a destroy node for that mode // will not be added. - DestroyNode(GraphNodeDestroyMode) GraphNodeDestroy + DestroyNode() GraphNodeDestroy } // GraphNodeDestroy is the interface that must implemented by @@ -60,32 +52,6 @@ type DestroyTransformer struct { func (t *DestroyTransformer) Transform(g *Graph) error { var connect, remove []dag.Edge - - modes := []GraphNodeDestroyMode{DestroyPrimary, DestroyTainted} - for _, m := range modes { - connectMode, removeMode, err := t.transform(g, m) - if err != nil { - return err - } - - connect = append(connect, connectMode...) - remove = append(remove, removeMode...) - } - - // Atomatically add/remove the edges - for _, e := range connect { - g.Connect(e) - } - for _, e := range remove { - g.RemoveEdge(e) - } - - return nil -} - -func (t *DestroyTransformer) transform( - g *Graph, mode GraphNodeDestroyMode) ([]dag.Edge, []dag.Edge, error) { - var connect, remove []dag.Edge nodeToCn := make(map[dag.Vertex]dag.Vertex, len(g.Vertices())) nodeToDn := make(map[dag.Vertex]dag.Vertex, len(g.Vertices())) for _, v := range g.Vertices() { @@ -96,7 +62,7 @@ func (t *DestroyTransformer) transform( } // Grab the destroy side of the node and connect it through - n := cn.DestroyNode(mode) + n := cn.DestroyNode() if n == nil { continue } @@ -155,7 +121,15 @@ func (t *DestroyTransformer) transform( } } - return connect, remove, nil + // Atomatically add/remove the edges + for _, e := range connect { + g.Connect(e) + } + for _, e := range remove { + g.RemoveEdge(e) + } + + return nil } // CreateBeforeDestroyTransformer is a GraphTransformer that modifies diff --git a/terraform/transform_orphan.go b/terraform/transform_orphan.go index 540bd2cc6..f47f51681 100644 --- a/terraform/transform_orphan.go +++ b/terraform/transform_orphan.go @@ -368,11 +368,7 @@ func (n *graphNodeOrphanResource) dependableName() string { } // GraphNodeDestroyable impl. -func (n *graphNodeOrphanResource) DestroyNode(mode GraphNodeDestroyMode) GraphNodeDestroy { - if mode != DestroyPrimary { - return nil - } - +func (n *graphNodeOrphanResource) DestroyNode() GraphNodeDestroy { return n } @@ -402,11 +398,7 @@ func (n *graphNodeOrphanResourceFlat) Path() []string { } // GraphNodeDestroyable impl. -func (n *graphNodeOrphanResourceFlat) DestroyNode(mode GraphNodeDestroyMode) GraphNodeDestroy { - if mode != DestroyPrimary { - return nil - } - +func (n *graphNodeOrphanResourceFlat) DestroyNode() GraphNodeDestroy { return n } diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index dccc3cd94..07da19b77 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -355,10 +355,6 @@ func (n *graphNodeExpandedResource) managedResourceEvalNodes(resource *Resource, Dependencies: n.StateDependencies(), State: &state, }, - &EvalDiffTainted{ - Diff: &diff, - Name: n.stateId(), - }, &EvalWriteDiff{ Name: n.stateId(), Diff: &diff, @@ -540,37 +536,12 @@ func (n *graphNodeExpandedResource) managedResourceEvalNodes(resource *Resource, Diff: nil, }, - &EvalIf{ - If: func(ctx EvalContext) (bool, error) { - return tainted, nil - }, - Then: &EvalSequence{ - Nodes: []EvalNode{ - &EvalWriteStateTainted{ - Name: n.stateId(), - ResourceType: n.Resource.Type, - Provider: n.Resource.Provider, - Dependencies: n.StateDependencies(), - State: &state, - Index: -1, - }, - &EvalIf{ - If: func(ctx EvalContext) (bool, error) { - return !n.Resource.Lifecycle.CreateBeforeDestroy, nil - }, - Then: &EvalClearPrimaryState{ - Name: n.stateId(), - }, - }, - }, - }, - Else: &EvalWriteState{ - Name: n.stateId(), - ResourceType: n.Resource.Type, - Provider: n.Resource.Provider, - Dependencies: n.StateDependencies(), - State: &state, - }, + &EvalWriteState{ + Name: n.stateId(), + ResourceType: n.Resource.Type, + Provider: n.Resource.Provider, + Dependencies: n.StateDependencies(), + State: &state, }, &EvalApplyPost{ Info: info, diff --git a/terraform/transform_tainted.go b/terraform/transform_tainted.go deleted file mode 100644 index 37e25df32..000000000 --- a/terraform/transform_tainted.go +++ /dev/null @@ -1,154 +0,0 @@ -package terraform - -import ( - "fmt" -) - -// TaintedTransformer is a GraphTransformer that adds tainted resources -// to the graph. -type TaintedTransformer struct { - // State is the global state. We'll automatically find the correct - // ModuleState based on the Graph.Path that is being transformed. - State *State - - // View, if non-empty, is the ModuleState.View used around the state - // to find tainted resources. - View string -} - -func (t *TaintedTransformer) Transform(g *Graph) error { - state := t.State.ModuleByPath(g.Path) - if state == nil { - // If there is no state for our module there can't be any tainted - // resources, since they live in the state. - return nil - } - - // If we have a view, apply it now - if t.View != "" { - state = state.View(t.View) - } - - // Go through all the resources in our state to look for tainted resources - for k, rs := range state.Resources { - // If we have no tainted resources, then move on - if len(rs.Tainted) == 0 { - continue - } - tainted := rs.Tainted - - for i, _ := range tainted { - // Add the graph node and make the connection from any untainted - // resources with this name to the tainted resource, so that - // the tainted resource gets destroyed first. - g.Add(&graphNodeTaintedResource{ - Index: i, - ResourceName: k, - ResourceType: rs.Type, - Provider: rs.Provider, - }) - } - } - - return nil -} - -// graphNodeTaintedResource is the graph vertex representing a tainted resource. -type graphNodeTaintedResource struct { - Index int - ResourceName string - ResourceType string - Provider string -} - -func (n *graphNodeTaintedResource) Name() string { - return fmt.Sprintf("%s (tainted #%d)", n.ResourceName, n.Index+1) -} - -func (n *graphNodeTaintedResource) ProvidedBy() []string { - return []string{resourceProvider(n.ResourceName, n.Provider)} -} - -// GraphNodeEvalable impl. -func (n *graphNodeTaintedResource) EvalTree() EvalNode { - var provider ResourceProvider - var state *InstanceState - - seq := &EvalSequence{Nodes: make([]EvalNode, 0, 5)} - - // Build instance info - info := &InstanceInfo{Id: n.ResourceName, Type: n.ResourceType} - seq.Nodes = append(seq.Nodes, &EvalInstanceInfo{Info: info}) - - // Refresh the resource - seq.Nodes = append(seq.Nodes, &EvalOpFilter{ - Ops: []walkOperation{walkRefresh}, - Node: &EvalSequence{ - Nodes: []EvalNode{ - &EvalGetProvider{ - Name: n.ProvidedBy()[0], - Output: &provider, - }, - &EvalReadStateTainted{ - Name: n.ResourceName, - Index: n.Index, - Output: &state, - }, - &EvalRefresh{ - Info: info, - Provider: &provider, - State: &state, - Output: &state, - }, - &EvalWriteStateTainted{ - Name: n.ResourceName, - ResourceType: n.ResourceType, - Provider: n.Provider, - State: &state, - Index: n.Index, - }, - }, - }, - }) - - // Apply - var diff *InstanceDiff - seq.Nodes = append(seq.Nodes, &EvalOpFilter{ - Ops: []walkOperation{walkApply, walkDestroy}, - Node: &EvalSequence{ - Nodes: []EvalNode{ - &EvalGetProvider{ - Name: n.ProvidedBy()[0], - Output: &provider, - }, - &EvalReadStateTainted{ - Name: n.ResourceName, - Index: n.Index, - Output: &state, - }, - &EvalDiffDestroy{ - Info: info, - State: &state, - Output: &diff, - }, - &EvalApply{ - Info: info, - State: &state, - Diff: &diff, - Provider: &provider, - Output: &state, - }, - &EvalWriteStateTainted{ - Name: n.ResourceName, - ResourceType: n.ResourceType, - Provider: n.Provider, - State: &state, - Index: n.Index, - }, - &EvalUpdateStateHook{}, - }, - }, - }) - - return seq -} diff --git a/terraform/transform_tainted_test.go b/terraform/transform_tainted_test.go deleted file mode 100644 index 208165ea7..000000000 --- a/terraform/transform_tainted_test.go +++ /dev/null @@ -1,71 +0,0 @@ -package terraform - -import ( - "strings" - "testing" - - "github.com/hashicorp/terraform/dag" -) - -func TestTaintedTransformer(t *testing.T) { - mod := testModule(t, "transform-tainted-basic") - state := &State{ - Modules: []*ModuleState{ - &ModuleState{ - Path: RootModulePath, - Resources: map[string]*ResourceState{ - "aws_instance.web": &ResourceState{ - Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ID: "foo"}, - }, - }, - }, - }, - }, - } - - g := Graph{Path: RootModulePath} - { - tf := &ConfigTransformer{Module: mod} - if err := tf.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - } - - transform := &TaintedTransformer{State: state} - if err := transform.Transform(&g); err != nil { - t.Fatalf("err: %s", err) - } - - actual := strings.TrimSpace(g.String()) - expected := strings.TrimSpace(testTransformTaintedBasicStr) - if actual != expected { - t.Fatalf("bad:\n\n%s", actual) - } -} - -func TestGraphNodeTaintedResource_impl(t *testing.T) { - var _ dag.Vertex = new(graphNodeTaintedResource) - var _ dag.NamedVertex = new(graphNodeTaintedResource) - var _ GraphNodeProviderConsumer = new(graphNodeTaintedResource) -} - -func TestGraphNodeTaintedResource_ProvidedBy(t *testing.T) { - n := &graphNodeTaintedResource{ResourceName: "aws_instance.foo"} - if v := n.ProvidedBy(); v[0] != "aws" { - t.Fatalf("bad: %#v", v) - } -} - -func TestGraphNodeTaintedResource_ProvidedBy_alias(t *testing.T) { - n := &graphNodeTaintedResource{ResourceName: "aws_instance.foo", Provider: "aws.bar"} - if v := n.ProvidedBy(); v[0] != "aws.bar" { - t.Fatalf("bad: %#v", v) - } -} - -const testTransformTaintedBasicStr = ` -aws_instance.web -aws_instance.web (tainted #1) -`