From 6fd82ef97e37c45fdb56cdd980048933a1e7e54d Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 4 Sep 2018 18:30:33 -0700 Subject: [PATCH] core: Split Replace changes into separate Delete/Create changes Since we do our deletes using a separate graph node from all of the other actions, and a "Replace" change implies both a delete _and_ a create, we need to pretend at apply time that a single replace change was actually two separate changes. This will also early-exit eval if a destroy node finds a non-Delete change or if an apply node finds a Delete change. These should not happen in practice because we leave these nodes out of the graph when they are not needed for the given action, but we do this here for robustness so as not to have an invisible dependency between the graph builder and the eval phase. --- plans/changes.go | 80 ++++++++++++++++++++++++++++++ terraform/eval_diff.go | 37 ++++++++++++++ terraform/node_resource_apply.go | 20 ++++++++ terraform/node_resource_destroy.go | 19 ++++--- 4 files changed, 149 insertions(+), 7 deletions(-) diff --git a/plans/changes.go b/plans/changes.go index 6b930cd8a..fac1fec74 100644 --- a/plans/changes.go +++ b/plans/changes.go @@ -121,6 +121,86 @@ func (rc *ResourceInstanceChange) Encode(ty cty.Type) (*ResourceInstanceChangeSr }, err } +// Simplify will, where possible, produce a change with a simpler action than +// the receiever given a flag indicating whether the caller is dealing with +// a normal apply or a destroy. This flag deals with the fact that Terraform +// Core uses a specialized graph node type for destroying; only that +// specialized node should set "destroying" to true. +// +// The following table shows the simplification behavior: +// +// Action Destroying? New Action +// --------+-------------+----------- +// Create true NoOp +// Delete false NoOp +// Replace true Delete +// Replace false Create +// +// For any combination not in the above table, the Simplify just returns the +// receiver as-is. +func (rc *ResourceInstanceChange) Simplify(destroying bool) *ResourceInstanceChange { + if destroying { + switch rc.Action { + case Delete: + // We'll fall out and just return rc verbatim, then. + case Replace: + return &ResourceInstanceChange{ + Addr: rc.Addr, + DeposedKey: rc.DeposedKey, + Private: rc.Private, + ProviderAddr: rc.ProviderAddr, + Change: Change{ + Action: Delete, + Before: rc.Before, + After: cty.NullVal(rc.Before.Type()), + }, + } + default: + return &ResourceInstanceChange{ + Addr: rc.Addr, + DeposedKey: rc.DeposedKey, + Private: rc.Private, + ProviderAddr: rc.ProviderAddr, + Change: Change{ + Action: NoOp, + Before: rc.Before, + After: rc.Before, + }, + } + } + } else { + switch rc.Action { + case Delete: + return &ResourceInstanceChange{ + Addr: rc.Addr, + DeposedKey: rc.DeposedKey, + Private: rc.Private, + ProviderAddr: rc.ProviderAddr, + Change: Change{ + Action: NoOp, + Before: rc.Before, + After: rc.Before, + }, + } + case Replace: + return &ResourceInstanceChange{ + Addr: rc.Addr, + DeposedKey: rc.DeposedKey, + Private: rc.Private, + ProviderAddr: rc.ProviderAddr, + Change: Change{ + Action: Create, + Before: cty.NullVal(rc.After.Type()), + After: rc.After, + }, + } + } + } + + // If we fall out here then our change is already simple enough. + return rc +} + // OutputChange describes a change to an output value. type OutputChange struct { // Change is an embedded description of the change. diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index e89d4e658..45b458e4c 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -722,6 +722,43 @@ func (n *EvalDiffDestroyModule) Eval(ctx EvalContext) (interface{}, error) { */ } +// EvalReduceDiff is an EvalNode implementation that takes a planned resource +// instance change as might be produced by EvalDiff or EvalDiffDestroy and +// "simplifies" it to a single atomic action to be performed by a specific +// graph node. +// +// Callers must specify whether they are a destroy node or a regular apply +// node. If the result is NoOp then the given change requires no action for +// the specific graph node calling this and so evaluation of the that graph +// node should exit early and take no action. +// +// The object written to OutChange may either be identical to InChange or +// a new change object derived from InChange. Because of the former case, the +// caller must not mutate the object returned in OutChange. +type EvalReduceDiff struct { + Addr addrs.ResourceInstance + InChange **plans.ResourceInstanceChange + Destroy bool + OutChange **plans.ResourceInstanceChange +} + +// TODO: test +func (n *EvalReduceDiff) Eval(ctx EvalContext) (interface{}, error) { + in := *n.InChange + out := in.Simplify(n.Destroy) + if n.OutChange != nil { + *n.OutChange = out + } + if out.Action != in.Action { + if n.Destroy { + log.Printf("[TRACE] EvalReduceDiff: %s change simplified from %s to %s for destroy node", n.Addr, in.Action, out.Action) + } else { + log.Printf("[TRACE] EvalReduceDiff: %s change simplified from %s to %s for apply node", n.Addr, in.Action, out.Action) + } + } + return nil, nil +} + // EvalReadDiff is an EvalNode implementation that retrieves the planned // change for a particular resource instance object. type EvalReadDiff struct { diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index d8d3ca881..e2f2dc2d8 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -298,6 +298,26 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe Output: &state, }, + &EvalReduceDiff{ + Addr: addr.Resource, + InChange: &diffApply, + Destroy: false, + OutChange: &diffApply, + }, + + // EvalReduceDiff may have simplified our planned change + // into a NoOp if it only requires destroying, since destroying + // is handled by NodeDestroyResourceInstance. + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + if diffApply == nil || diffApply.Action == plans.NoOp { + return true, EvalEarlyExitError{} + } + return true, nil + }, + Then: EvalNoop{}, + }, + // Call pre-apply hook &EvalApplyPre{ Addr: addr.Resource, diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index 760a081e3..fec49da44 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -189,16 +189,21 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode { Change: &changeApply, }, - // If we're not destroying, then compare diffs + &EvalReduceDiff{ + Addr: addr.Resource, + InChange: &changeApply, + Destroy: true, + OutChange: &changeApply, + }, + + // EvalReduceDiff may have simplified our planned change + // into a NoOp if it does not require destroying. &EvalIf{ If: func(ctx EvalContext) (bool, error) { - if changeApply != nil { - if changeApply.Action == plans.Delete || changeApply.Action == plans.Replace { - return true, nil - } + if changeApply == nil || changeApply.Action == plans.NoOp { + return true, EvalEarlyExitError{} } - - return true, EvalEarlyExitError{} + return true, nil }, Then: EvalNoop{}, },