From 596e891b807aff8f44f66e1062eed688e8f1e024 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Fri, 27 Feb 2015 11:38:17 -0600 Subject: [PATCH] core: [refactor] pull Deposed out of Tainted list --- terraform/eval_error.go | 16 ++++ terraform/eval_if.go | 9 +- terraform/eval_state.go | 120 +++++++++++++++++++------ terraform/eval_state_test.go | 81 +++++++++++++++++ terraform/graph_config_node.go | 20 ++--- terraform/state.go | 26 +++++- terraform/transform_deposed.go | 155 ++++++++++++++++++++++++++++++++ terraform/transform_resource.go | 26 ++++-- terraform/transform_tainted.go | 25 +----- 9 files changed, 403 insertions(+), 75 deletions(-) create mode 100644 terraform/eval_error.go create mode 100644 terraform/transform_deposed.go diff --git a/terraform/eval_error.go b/terraform/eval_error.go new file mode 100644 index 000000000..690fe28e9 --- /dev/null +++ b/terraform/eval_error.go @@ -0,0 +1,16 @@ +package terraform + +// EvalReturnError is an EvalNode implementation that returns an +// error if it is present. +// +// This is useful for scenarios where an error has been captured by +// another EvalNode (like EvalApply) for special EvalTree-based error +// handling, and that handling has completed, so the error should be +// returned normally. +type EvalReturnError struct { + Error *error +} + +func (n *EvalReturnError) Eval(ctx EvalContext) (interface{}, error) { + return nil, *n.Error +} diff --git a/terraform/eval_if.go b/terraform/eval_if.go index c96e13229..d6b46a1f2 100644 --- a/terraform/eval_if.go +++ b/terraform/eval_if.go @@ -3,7 +3,8 @@ package terraform // EvalIf is an EvalNode that is a conditional. type EvalIf struct { If func(EvalContext) (bool, error) - Node EvalNode + Then EvalNode + Else EvalNode } // TODO: test @@ -14,7 +15,11 @@ func (n *EvalIf) Eval(ctx EvalContext) (interface{}, error) { } if yes { - return EvalRaw(n.Node, ctx) + return EvalRaw(n.Then, ctx) + } else { + if n.Else != nil { + return EvalRaw(n.Else, ctx) + } } return nil, nil diff --git a/terraform/eval_state.go b/terraform/eval_state.go index 161752eb4..bd1adb4d9 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -7,13 +7,10 @@ import ( // EvalReadState is an EvalNode implementation that reads the // InstanceState for a specific resource out of the state. type EvalReadState struct { - Name string - Tainted bool - TaintedIndex int - Output **InstanceState + Name string + Output **InstanceState } -// TODO: test func (n *EvalReadState) Eval(ctx EvalContext) (interface{}, error) { state, lock := ctx.State() @@ -33,20 +30,53 @@ func (n *EvalReadState) Eval(ctx EvalContext) (interface{}, error) { return nil, nil } + // Write the result to the output pointer + if n.Output != nil { + *n.Output = rs.Primary + } + + return rs.Primary, nil +} + +// EvalReadStateTainted is an EvalNode implementation that reads the +// InstanceState for a specific tainted resource out of the state +type EvalReadStateTainted struct { + Name string + Output **InstanceState + + // Tainted is a per-resource list, this index determines which item in the + // list we are addressing + TaintedIndex int +} + +func (n *EvalReadStateTainted) 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 + } + var result *InstanceState - if !n.Tainted { - // Return the primary - result = rs.Primary - } else { - // Get the index. If it is negative, then we get the last one - idx := n.TaintedIndex - if idx < 0 { - idx = len(rs.Tainted) - 1 - } - if idx >= 0 && idx < len(rs.Tainted) { - // Return the proper tainted resource - result = rs.Tainted[idx] - } + // Get the index. If it is negative, then we get the last one + idx := n.TaintedIndex + if idx < 0 { + idx = len(rs.Tainted) - 1 + } + if idx >= 0 && idx < len(rs.Tainted) { + // Return the proper tainted resource + result = rs.Tainted[idx] } // Write the result to the output pointer @@ -57,6 +87,40 @@ func (n *EvalReadState) Eval(ctx EvalContext) (interface{}, error) { return result, nil } +// EvalReadStateDeposed is an EvalNode implementation that reads the +// InstanceState for a specific deposed resource out of the state +type EvalReadStateDeposed struct { + Name string + Output **InstanceState +} + +func (n *EvalReadStateDeposed) 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 + } + + // Write the result to the output pointer + if n.Output != nil { + *n.Output = rs.Deposed + } + + return rs.Deposed, nil +} + // EvalRequireState is an EvalNode implementation that early exits // if the state doesn't have an ID. type EvalRequireState struct { @@ -108,6 +172,7 @@ type EvalWriteState struct { Tainted *bool TaintedIndex int TaintedClearPrimary bool + Deposed bool } // TODO: test @@ -147,6 +212,8 @@ func (n *EvalWriteState) Eval(ctx EvalContext) (interface{}, error) { if n.TaintedClearPrimary { rs.Primary = nil } + } else if n.Deposed { + rs.Deposed = *n.State } else { // Set the primary state rs.Primary = *n.State @@ -156,7 +223,7 @@ func (n *EvalWriteState) Eval(ctx EvalContext) (interface{}, error) { } // EvalDeposeState is an EvalNode implementation that takes the primary -// out of a state and makes it tainted. This is done at the beggining of +// out of a state and makes it Deposed. This is done at the beginning of // create-before-destroy calls so that the create can create while preserving // the old state of the to-be-destroyed resource. type EvalDeposeState struct { @@ -188,8 +255,8 @@ func (n *EvalDeposeState) Eval(ctx EvalContext) (interface{}, error) { return nil, nil } - // Depose to the tainted - rs.Tainted = append(rs.Tainted, rs.Primary) + // Depose + rs.Deposed = rs.Primary rs.Primary = nil return nil, nil @@ -221,15 +288,14 @@ func (n *EvalUndeposeState) Eval(ctx EvalContext) (interface{}, error) { return nil, nil } - // If we don't have any tainted, then we don't have anything to do - if len(rs.Tainted) == 0 { + // If we don't have any desposed resource, then we don't have anything to do + if rs.Deposed == nil { return nil, nil } - // Undepose to the tainted - idx := len(rs.Tainted) - 1 - rs.Primary = rs.Tainted[idx] - rs.Tainted[idx] = nil + // Undepose + rs.Primary = rs.Deposed + rs.Deposed = nil return nil, nil } diff --git a/terraform/eval_state_test.go b/terraform/eval_state_test.go index b2783da9a..fc638ee3e 100644 --- a/terraform/eval_state_test.go +++ b/terraform/eval_state_test.go @@ -66,3 +66,84 @@ func TestEvalUpdateStateHook(t *testing.T) { t.Fatalf("bad: %#v", mockHook.PostStateUpdateState) } } + +func TestEvalReadState(t *testing.T) { + var output *InstanceState + cases := map[string]struct { + Resources map[string]*ResourceState + Node EvalNode + ExpectedInstanceId string + }{ + "ReadState gets primary instance state": { + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Primary: &InstanceState{ + ID: "i-abc123", + }, + }, + }, + Node: &EvalReadState{ + Name: "aws_instance.bar", + Output: &output, + }, + ExpectedInstanceId: "i-abc123", + }, + "ReadStateTainted gets tainted instance": { + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Tainted: []*InstanceState{ + &InstanceState{ID: "i-abc123"}, + }, + }, + }, + Node: &EvalReadStateTainted{ + Name: "aws_instance.bar", + Output: &output, + TaintedIndex: 0, + }, + ExpectedInstanceId: "i-abc123", + }, + "ReadStateDeposed gets deposed instance": { + Resources: map[string]*ResourceState{ + "aws_instance.bar": &ResourceState{ + Deposed: &InstanceState{ID: "i-abc123"}, + }, + }, + Node: &EvalReadStateDeposed{ + Name: "aws_instance.bar", + Output: &output, + }, + ExpectedInstanceId: "i-abc123", + }, + } + + for k, c := range cases { + ctx := new(MockEvalContext) + ctx.StateState = &State{ + Modules: []*ModuleState{ + &ModuleState{ + Path: rootModulePath, + Resources: c.Resources, + }, + }, + } + ctx.StateLock = new(sync.RWMutex) + ctx.PathPath = rootModulePath + + result, err := c.Node.Eval(ctx) + if err != nil { + t.Fatalf("[%s] Got err: %#v", k, err) + } + + expected := c.ExpectedInstanceId + if !(result != nil && result.(*InstanceState).ID == expected) { + t.Fatalf("[%s] Expected return with ID %#v, got: %#v", k, expected, result) + } + + if !(output != nil && output.ID == expected) { + t.Fatalf("[%s] Expected output with ID %#v, got: %#v", k, expected, output) + } + + output = nil + } +} diff --git a/terraform/graph_config_node.go b/terraform/graph_config_node.go index 07e53ef09..625992f3f 100644 --- a/terraform/graph_config_node.go +++ b/terraform/graph_config_node.go @@ -293,24 +293,16 @@ func (n *GraphNodeConfigResource) DynamicExpand(ctx EvalContext) (*Graph, error) View: n.Resource.Id(), }) - if n.Resource.Lifecycle.CreateBeforeDestroy { - // 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(), - Deposed: n.Resource.Lifecycle.CreateBeforeDestroy, - DeposedInclude: true, - }) - } + steps = append(steps, &DeposedTransformer{ + 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(), - Deposed: n.Resource.Lifecycle.CreateBeforeDestroy, - DeposedInclude: false, + State: state, + View: n.Resource.Id(), }) } diff --git a/terraform/state.go b/terraform/state.go index 85492f31c..9c9dd7e24 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -548,7 +548,12 @@ func (m *ModuleState) String() string { taintStr = fmt.Sprintf(" (%d tainted)", len(rs.Tainted)) } - buf.WriteString(fmt.Sprintf("%s:%s\n", k, taintStr)) + deposedStr := "" + if rs.Deposed != nil { + deposedStr = fmt.Sprintf(" (1 deposed)") + } + + buf.WriteString(fmt.Sprintf("%s:%s%s\n", k, taintStr, deposedStr)) buf.WriteString(fmt.Sprintf(" ID = %s\n", id)) var attributes map[string]string @@ -574,6 +579,10 @@ func (m *ModuleState) String() string { buf.WriteString(fmt.Sprintf(" Tainted ID %d = %s\n", idx+1, t.ID)) } + if rs.Deposed != nil { + buf.WriteString(fmt.Sprintf(" Deposed ID = %s\n", rs.Deposed.ID)) + } + if len(rs.Dependencies) > 0 { buf.WriteString(fmt.Sprintf("\n Dependencies:\n")) for _, dep := range rs.Dependencies { @@ -644,6 +653,14 @@ type ResourceState struct { // 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 + // Deposed instance is cleaned up. If there were problems creating the + // replacement, we mark the replacement as Tainted and Undepose the former + // Primary. + Deposed *InstanceState `json:"deposed,omitempty"` } // Equal tests whether two ResourceStates are equal. @@ -744,6 +761,9 @@ func (r *ResourceState) deepcopy() *ResourceState { n.Tainted = append(n.Tainted, inst.deepcopy()) } } + if r.Deposed != nil { + n.Deposed = r.Deposed.deepcopy() + } return n } @@ -762,6 +782,10 @@ func (r *ResourceState) prune() { } r.Tainted = r.Tainted[:n] + + if r.Deposed != nil && r.Deposed.ID == "" { + r.Deposed = nil + } } func (r *ResourceState) sort() { diff --git a/terraform/transform_deposed.go b/terraform/transform_deposed.go new file mode 100644 index 000000000..d9739b96e --- /dev/null +++ b/terraform/transform_deposed.go @@ -0,0 +1,155 @@ +package terraform + +import "fmt" + +// DeposedTransformer is a GraphTransformer that adds tainted resources +// to the graph. +type DeposedTransformer 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 deposed resources. + View string +} + +func (t *DeposedTransformer) 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 rs.Deposed == nil { + continue + } + + g.Add(&graphNodeDeposedResource{ + ResourceName: k, + ResourceType: rs.Type, + }) + } + + return nil +} + +// graphNodeDeposedResource is the graph vertex representing a deposed resource. +type graphNodeDeposedResource struct { + ResourceName string + ResourceType string +} + +func (n *graphNodeDeposedResource) Name() string { + return fmt.Sprintf("%s (deposed)", n.ResourceName) +} + +func (n *graphNodeDeposedResource) ProvidedBy() []string { + return []string{resourceProvider(n.ResourceName)} +} + +// GraphNodeEvalable impl. +func (n *graphNodeDeposedResource) 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, + }, + &EvalReadStateDeposed{ + Name: n.ResourceName, + Output: &state, + }, + &EvalRefresh{ + Info: info, + Provider: &provider, + State: &state, + Output: &state, + }, + &EvalWriteState{ + Name: n.ResourceName, + ResourceType: n.ResourceType, + State: &state, + Deposed: true, + }, + }, + }, + }) + + // Apply + var diff *InstanceDiff + var err error + var emptyState *InstanceState + tainted := true + seq.Nodes = append(seq.Nodes, &EvalOpFilter{ + Ops: []walkOperation{walkApply}, + Node: &EvalSequence{ + Nodes: []EvalNode{ + &EvalGetProvider{ + Name: n.ProvidedBy()[0], + Output: &provider, + }, + &EvalReadStateDeposed{ + Name: n.ResourceName, + Output: &state, + }, + &EvalDiffDestroy{ + Info: info, + State: &state, + Output: &diff, + }, + &EvalApply{ + Info: info, + State: &state, + Diff: &diff, + Provider: &provider, + Output: &state, + Error: &err, + }, + // Always write the resource back to the state tainted... if it + // successfully destroyed it will be pruned. If it did not, it will + // remain tainted. + &EvalWriteState{ + Name: n.ResourceName, + ResourceType: n.ResourceType, + State: &state, + Tainted: &tainted, + TaintedIndex: -1, + }, + // Then clear the deposed state. + &EvalWriteState{ + Name: n.ResourceName, + ResourceType: n.ResourceType, + State: &emptyState, + Deposed: true, + }, + &EvalReturnError{ + Error: &err, + }, + &EvalUpdateStateHook{}, + }, + }, + }) + + return seq +} diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index 4b1985450..6a38ab624 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -285,7 +285,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { diffApply.Destroy = false return true, nil }, - Node: EvalNoop{}, + Then: EvalNoop{}, }, &EvalIf{ @@ -301,7 +301,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { return createBeforeDestroyEnabled, nil }, - Node: &EvalDeposeState{ + Then: &EvalDeposeState{ Name: n.stateId(), }, }, @@ -382,7 +382,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { failure := tainted || err != nil return createBeforeDestroyEnabled && failure, nil }, - Node: &EvalUndeposeState{ + Then: &EvalUndeposeState{ Name: n.stateId(), }, }, @@ -480,18 +480,26 @@ func (n *graphNodeExpandedResourceDestroy) EvalTree() EvalNode { return true, EvalEarlyExitError{} }, - Node: EvalNoop{}, + Then: EvalNoop{}, }, &EvalGetProvider{ Name: n.ProvidedBy()[0], Output: &provider, }, - &EvalReadState{ - Name: n.stateId(), - Output: &state, - Tainted: n.Resource.Lifecycle.CreateBeforeDestroy, - TaintedIndex: -1, + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + return n.Resource.Lifecycle.CreateBeforeDestroy, nil + }, + Then: &EvalReadStateTainted{ + Name: n.stateId(), + Output: &state, + TaintedIndex: -1, + }, + Else: &EvalReadState{ + Name: n.stateId(), + Output: &state, + }, }, &EvalRequireState{ State: &state, diff --git a/terraform/transform_tainted.go b/terraform/transform_tainted.go index 1e1b2cb31..7e68b87cd 100644 --- a/terraform/transform_tainted.go +++ b/terraform/transform_tainted.go @@ -4,7 +4,7 @@ import ( "fmt" ) -// TraintedTransformer is a GraphTransformer that adds tainted resources +// 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 @@ -14,12 +14,6 @@ type TaintedTransformer struct { // View, if non-empty, is the ModuleState.View used around the state // to find tainted resources. View string - - // Deposed, if set to true, assumes that the last tainted index - // represents a "deposed" resource, or a resource that was previously - // a primary but is now tainted since it is demoted. - Deposed bool - DeposedInclude bool } func (t *TaintedTransformer) Transform(g *Graph) error { @@ -43,17 +37,6 @@ func (t *TaintedTransformer) Transform(g *Graph) error { } tainted := rs.Tainted - // If we expect a deposed resource, then shuffle a bit - if t.Deposed { - if t.DeposedInclude { - // Only include the deposed resource - tainted = rs.Tainted[len(rs.Tainted)-1:] - } else { - // Exclude the deposed resource - tainted = rs.Tainted[:len(rs.Tainted)-1] - } - } - 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 @@ -105,9 +88,8 @@ func (n *graphNodeTaintedResource) EvalTree() EvalNode { Name: n.ProvidedBy()[0], Output: &provider, }, - &EvalReadState{ + &EvalReadStateTainted{ Name: n.ResourceName, - Tainted: true, TaintedIndex: n.Index, Output: &state, }, @@ -138,9 +120,8 @@ func (n *graphNodeTaintedResource) EvalTree() EvalNode { Name: n.ProvidedBy()[0], Output: &provider, }, - &EvalReadState{ + &EvalReadStateTainted{ Name: n.ResourceName, - Tainted: true, TaintedIndex: n.Index, Output: &state, },