From 3d86dc51e0ef2dddc71df6392962a545ec384946 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 27 Aug 2018 16:45:07 -0700 Subject: [PATCH] core: Re-implement EvalReadDiff for the new plan types This also includes passing in the provider schema to a few more EvalNodes that were expecting it but not getting it, in order to be able to successfully test the implementation of EvalReadDiff here. --- plans/changes_sync.go | 28 +++++++++++++ terraform/eval_diff.go | 57 ++++++++++++++++---------- terraform/node_resource_apply.go | 65 +++++++++++++++++------------- terraform/node_resource_destroy.go | 32 ++++++++------- terraform/transform_deposed.go | 16 ++++---- 5 files changed, 127 insertions(+), 71 deletions(-) diff --git a/plans/changes_sync.go b/plans/changes_sync.go index 32716b3d0..f5bfb1afb 100644 --- a/plans/changes_sync.go +++ b/plans/changes_sync.go @@ -1,7 +1,11 @@ package plans import ( + "fmt" "sync" + + "github.com/hashicorp/terraform/addrs" + "github.com/hashicorp/terraform/states" ) // ChangesSync is a wrapper around a Changes that provides a concurrency-safe @@ -33,3 +37,27 @@ func (cs *ChangesSync) AppendResourceInstanceChange(changeSrc *ResourceInstanceC s := changeSrc.DeepCopy() cs.changes.Resources = append(cs.changes.Resources, s) } + +// GetResourceInstanceChange searches the set of resource instance changes for +// one matching the given address and generation, returning it if it exists. +// +// If no such change exists, nil is returned. +// +// The returned object is a deep copy of the change recorded in the plan, so +// callers may mutate it although it's generally better (less confusing) to +// treat planned changes as immutable after they've been initially constructed. +func (cs *ChangesSync) GetResourceInstanceChange(addr addrs.AbsResourceInstance, gen states.Generation) *ResourceInstanceChangeSrc { + if cs == nil { + panic("GetResourceInstanceChange on nil ChangesSync") + } + cs.lock.Lock() + defer cs.lock.Unlock() + + if gen == states.CurrentGen { + return cs.changes.ResourceInstance(addr) + } + if dk, ok := gen.(states.DeposedKey); ok { + return cs.changes.ResourceInstanceDeposed(addr, dk) + } + panic(fmt.Sprintf("unsupported generation value %#v", gen)) +} diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index 80b194faf..87d2d54be 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -703,30 +703,44 @@ func (n *EvalDiffDestroyModule) Eval(ctx EvalContext) (interface{}, error) { // EvalReadDiff is an EvalNode implementation that retrieves the planned // change for a particular resource instance object. type EvalReadDiff struct { - Addr addrs.ResourceInstance - DeposedKey states.DeposedKey - Change **plans.ResourceInstanceChange + Addr addrs.ResourceInstance + DeposedKey states.DeposedKey + ProviderSchema **ProviderSchema + Change **plans.ResourceInstanceChange } func (n *EvalReadDiff) Eval(ctx EvalContext) (interface{}, error) { - return nil, fmt.Errorf("EvalReadDiff not yet updated for new plan types") - /* - diff, lock := ctx.Diff() + providerSchema := *n.ProviderSchema + changes := ctx.Changes() + addr := n.Addr.Absolute(ctx.Path()) - // Acquire the lock so that we can do this safely concurrently - lock.Lock() - defer lock.Unlock() - - // Write the diff - modDiff := diff.ModuleByPath(ctx.Path()) - if modDiff == nil { - return nil, nil - } - - *n.Diff = modDiff.Resources[n.Name] + schema := providerSchema.ResourceTypes[n.Addr.Resource.Type] + if schema == nil { + // Should be caught during validation, so we don't bother with a pretty error here + return nil, fmt.Errorf("provider does not support resource type %q", n.Addr.Resource.Type) + } + gen := states.CurrentGen + if n.DeposedKey != states.NotDeposed { + gen = n.DeposedKey + } + csrc := changes.GetResourceInstanceChange(addr, gen) + if csrc == nil { + log.Printf("[TRACE] EvalReadDiff: No planned change recorded for %s", addr) return nil, nil - */ + } + + change, err := csrc.Decode(schema.ImpliedType()) + if err != nil { + return nil, fmt.Errorf("failed to decode planned changes for %s: %s", addr, err) + } + if n.Change != nil { + *n.Change = change + } + + log.Printf("[TRACE] EvalReadDiff: Read %s change from plan for %s", change.Action, addr) + + return nil, nil } // EvalWriteDiff is an EvalNode implementation that saves a planned change @@ -742,6 +756,7 @@ type EvalWriteDiff struct { func (n *EvalWriteDiff) Eval(ctx EvalContext) (interface{}, error) { providerSchema := *n.ProviderSchema change := *n.Change + addr := n.Addr.Absolute(ctx.Path()) if change.Addr.String() != n.Addr.String() || change.DeposedKey != n.DeposedKey { // Should never happen, and indicates a bug in the caller. @@ -756,15 +771,15 @@ func (n *EvalWriteDiff) Eval(ctx EvalContext) (interface{}, error) { csrc, err := change.Encode(schema.ImpliedType()) if err != nil { - return nil, fmt.Errorf("failed to encode planned changes for %s: %s", n.Addr, err) + return nil, fmt.Errorf("failed to encode planned changes for %s: %s", addr, err) } changes := ctx.Changes() changes.AppendResourceInstanceChange(csrc) if n.DeposedKey == states.NotDeposed { - log.Printf("[TRACE] EvalWriteDiff: recorded %s change for %s", change.Action, n.Addr) + log.Printf("[TRACE] EvalWriteDiff: recorded %s change for %s", change.Action, addr) } else { - log.Printf("[TRACE] EvalWriteDiff: recorded %s change for %s deposed object %s", change.Action, n.Addr, n.DeposedKey) + log.Printf("[TRACE] EvalWriteDiff: recorded %s change for %s deposed object %s", change.Action, addr, n.DeposedKey) } return nil, nil diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index 4ca75ae0b..4b97be52d 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -129,10 +129,17 @@ func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResou return &EvalSequence{ Nodes: []EvalNode{ + &EvalGetProvider{ + Addr: n.ResolvedProvider, + Output: &provider, + Schema: &providerSchema, + }, + // Get the saved diff for apply &EvalReadDiff{ - Addr: addr.Resource, - Change: &change, + Addr: addr.Resource, + ProviderSchema: &providerSchema, + Change: &change, }, // Stop early if we don't actually have a diff @@ -146,12 +153,6 @@ func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResou Then: EvalNoop{}, }, - &EvalGetProvider{ - Addr: n.ResolvedProvider, - Output: &provider, - Schema: &providerSchema, - }, - // Make a new diff, in case we've learned new values in the state // during apply which we can now incorporate. &EvalReadDataDiff{ @@ -206,10 +207,17 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe return &EvalSequence{ Nodes: []EvalNode{ + &EvalGetProvider{ + Addr: n.ResolvedProvider, + Output: &provider, + Schema: &providerSchema, + }, + // Get the saved diff for apply &EvalReadDiff{ - Addr: addr.Resource, - Change: &diffApply, + Addr: addr.Resource, + ProviderSchema: &providerSchema, + Change: &diffApply, }, // We don't want to do any destroys @@ -244,11 +252,6 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe }, }, - &EvalGetProvider{ - Addr: n.ResolvedProvider, - Output: &provider, - Schema: &providerSchema, - }, &EvalReadState{ Addr: addr.Resource, Provider: &provider, @@ -273,16 +276,18 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe // Get the saved diff &EvalReadDiff{ - Addr: addr.Resource, - Change: &diff, + Addr: addr.Resource, + ProviderSchema: &providerSchema, + Change: &diff, }, // Compare the diffs &EvalCheckPlannedChange{ - Addr: addr.Resource, - ProviderAddr: n.ResolvedProvider, - Planned: &diff, - Actual: &diffApply, + Addr: addr.Resource, + ProviderAddr: n.ResolvedProvider, + ProviderSchema: &providerSchema, + Planned: &diff, + Actual: &diffApply, }, &EvalGetProvider{ @@ -305,14 +310,16 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe Change: &diffApply, }, &EvalApply{ - Addr: addr.Resource, - Config: n.Config, - State: &state, - Change: &diffApply, - Provider: &provider, - Output: &state, - Error: &err, - CreateNew: &createNew, + Addr: addr.Resource, + Config: n.Config, + State: &state, + Change: &diffApply, + Provider: &provider, + ProviderAddr: n.ResolvedProvider, + ProviderSchema: &providerSchema, + Output: &state, + Error: &err, + CreateNew: &createNew, }, &EvalWriteState{ Addr: addr.Resource, diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 05733889f..e6d333487 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -176,10 +176,17 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode { Ops: []walkOperation{walkApply, walkDestroy}, Node: &EvalSequence{ Nodes: []EvalNode{ + &EvalGetProvider{ + Addr: n.ResolvedProvider, + Output: &provider, + Schema: &providerSchema, + }, + // Get the saved diff for apply &EvalReadDiff{ - Addr: addr.Resource, - Change: &changeApply, + Addr: addr.Resource, + ProviderSchema: &providerSchema, + Change: &changeApply, }, // If we're not destroying, then compare diffs @@ -196,11 +203,6 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode { Then: EvalNoop{}, }, - &EvalGetProvider{ - Addr: n.ResolvedProvider, - Output: &provider, - Schema: &providerSchema, - }, &EvalReadState{ Addr: addr.Resource, Output: &state, @@ -264,13 +266,15 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode { Output: &state, }, Else: &EvalApply{ - Addr: addr.Resource, - Config: nil, // No configuration because we are destroying - State: &state, - Change: &changeApply, - Provider: &provider, - Output: &state, - Error: &err, + Addr: addr.Resource, + Config: nil, // No configuration because we are destroying + State: &state, + Change: &changeApply, + Provider: &provider, + ProviderAddr: n.ResolvedProvider, + ProviderSchema: &providerSchema, + Output: &state, + Error: &err, }, }, &EvalWriteState{ diff --git a/terraform/transform_deposed.go b/terraform/transform_deposed.go index 599384949..24163c2b7 100644 --- a/terraform/transform_deposed.go +++ b/terraform/transform_deposed.go @@ -151,13 +151,15 @@ func (n *graphNodeDeposedResource) EvalTree() EvalNode { Change: &change, }, &EvalApply{ - Addr: addr.Resource, - Config: nil, // No configuration because we are destroying - State: &state, - Change: &change, - Provider: &provider, - Output: &state, - Error: &err, + Addr: addr.Resource, + Config: nil, // No configuration because we are destroying + State: &state, + Change: &change, + Provider: &provider, + ProviderAddr: n.ResolvedProvider, + ProviderSchema: &providerSchema, + Output: &state, + Error: &err, }, // Always write the resource back to the state deposed... if it // was successfully destroyed it will be pruned. If it was not, it will