core: [refactor] pull Deposed out of Tainted list

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
+}

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

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
+	Name   string
-	Tainted      bool
+	Output **InstanceState
-	TaintedIndex int
-	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 {
+	// Get the index. If it is negative, then we get the last one
-		// Return the primary
+	idx := n.TaintedIndex
-		result = rs.Primary
+	if idx < 0 {
-	} else {
+		idx = len(rs.Tainted) - 1
-		// Get the index. If it is negative, then we get the last one
+	}
-		idx := n.TaintedIndex
+	if idx >= 0 && idx < len(rs.Tainted) {
-		if idx < 0 {
+		// Return the proper tainted resource
-			idx = len(rs.Tainted) - 1
+		result = rs.Tainted[idx]
-		}
-		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
+	// Depose
-	rs.Tainted = append(rs.Tainted, rs.Primary)
+	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 we don't have any desposed resource, then we don't have anything to do
-	if len(rs.Tainted) == 0 {
+	if rs.Deposed == nil {
 		return nil, nil
 	}
 
-	// Undepose to the tainted
+	// Undepose
-	idx := len(rs.Tainted) - 1
+	rs.Primary = rs.Deposed
-	rs.Primary = rs.Tainted[idx]
+	rs.Deposed = nil
-	rs.Tainted[idx] = nil
 
 	return nil, nil
 }

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
+	}
+}

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 {
+		steps = append(steps, &DeposedTransformer{
-			// If we're only destroying tainted resources, then we only
+			State: state,
-			// want to find tainted resources and destroy them here.
+			View:  n.Resource.Id(),
-			steps = append(steps, &TaintedTransformer{
+		})
-				State:          state,
-				View:           n.Resource.Id(),
-				Deposed:        n.Resource.Lifecycle.CreateBeforeDestroy,
-				DeposedInclude: true,
-			})
-		}
 	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,
+			State: state,
-			View:           n.Resource.Id(),
+			View:  n.Resource.Id(),
-			Deposed:        n.Resource.Lifecycle.CreateBeforeDestroy,
-			DeposedInclude: false,
 		})
 	}
 

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() {

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
+}

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{
+				&EvalIf{
-					Name:         n.stateId(),
+					If: func(ctx EvalContext) (bool, error) {
-					Output:       &state,
+						return n.Resource.Lifecycle.CreateBeforeDestroy, nil
-					Tainted:      n.Resource.Lifecycle.CreateBeforeDestroy,
+					},
-					TaintedIndex: -1,
+					Then: &EvalReadStateTainted{
+						Name:         n.stateId(),
+						Output:       &state,
+						TaintedIndex: -1,
+					},
+					Else: &EvalReadState{
+						Name:   n.stateId(),
+						Output: &state,
+					},
 				},
 				&EvalRequireState{
 					State: &state,

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,
 				},