Commit Graph

5 Commits

Author SHA1 Message Date
Kristin Laemmert
c7bf43154f
Mildwonkey/eval apply (#27222)
* rename files for consistency with contents

* terraform: refactor EvalValidateSelfref

The EvalValidateSelfref eval node implementation was removed in favor of a regular function.

* terraform: refactor EvalValidateProvisioner

EvalValidateProvisioner is now a method on NodeValidatableResource.

* terraform: refactor EvalValidateResource

EvalValidateResource is now a method on NodeValidatableResource, and the
functions called by (the new) validateResource are now standalone
functions.

This particular refactor gets the prize for "most complicated test
refactoring".

* terraform: refactor EvalMaybeTainted

EvalMaybeTainted was a relatively simple operation which never returned
an error, so I've refactored it into a plain function and moved it into
the only file its called from.

* terraform: eval-related cleanup

De-exported preApplyHook, which got missed in my general cleanup sweeps.

Removed resourceHasUserVisibleApply in favor of moving the logic inline
- it was a single-line check so calling the function was (nearly) as
much code as just checking if the resource was managed.

* terraform: refactor EvalApplyProvisioners

EvalApplyProvisioners.Eval is now a method on
NodeResourceAbstractInstance. There were two "apply"ish functions, so I
named the first "evalApplyProvisioners" since it mainly determined if
provisioners should be run before passing off execution to
applyProvisioners.

* terraform: refactor EvalApply

EvalApply is now a method on NodeAbstractResourceInstance. This was one
of the trickier Eval()s to refactor, and my goal was to change as little
as possible to avoid unintended side effects.

One notable change: there was a createNew boolean that was only used in
NodeApplyableResourceInstance.managedResourceExecute, and that boolean
was populated from the change (which was available from
managedResourceExecute), so I removed it from apply entirely. Out of an
abundance of caution I assigned the value to createNew in (roughtly) the same spot,
in case I was missing some place where the change might get modified.

TODO: Destroy nodes passed nil configs into apply, and I am curious if
we can get the same functionality by checking if the planned change is a
destroy, instead of passing a config into apply. That felt too risky for
this refactor but it is something I would like to explore at a future
point.

There are also a few updates to log output in this PR, since I spent
some time staring at logs and noticed various spots I missed.
2020-12-10 08:05:53 -05:00
Kristin Laemmert
e7aaf9e39f
Eval() Refactor: Plan Edition (#27177)
* terraforn: refactor EvalRefresh

EvalRefresh.Eval(ctx) is now Refresh(evalRefreshReqest, ctx). While none
of the inner logic of the function has changed, it now returns a
states.ResourceInstanceObject instead of updating a pointer. This is a
human-centric change, meant to make the logic flow (in the calling
functions) easier to follow.

* terraform: refactor EvalReadDataPlan and Apply

This is a very minor refactor that removes the (currently) redundant
types EvalReadDataPlan and EvalReadDataApply in favor of using
EvalReadData with a Plan and Apply functions.

This is in effect an aesthetic change; since there is no longer an
Eval() abstraction we can rename functions to make their functionality
as obvious as possible.

* terraform: refactor EvalCheckPlannedChange

EvalCheckPlannedChange was only used by NodeApplyableResourceInstance
and has been refactored into a method on that type called
checkPlannedChange.

* terraform: refactor EvalDiff.Eval

EvalDiff.Eval is now a method on NodeResourceAbstracted called Plan
which takes as a parameter an EvalPlanRequest. Instead of updating
pointers it returns a new plan and state.

I removed as many redundant fields from the original EvalDiff struct as
possible.

* terraform: refactor EvalReduceDiff

EvalReduceDiff is now reducePlan, a regular function (without a method)
that returns a value.

* terraform: refactor EvalDiffDestroy

EvalDiffDestroy.Eval is now NodeAbstractResourceInstance.PlanDestroy
which takes ctx, state and optional DeposedKey and returns a change.
I've removed the state return value since it was only ever returning a
nil state.

* terraform: refactor EvalWriteDiff

EvalWriteDiff.Eval is now NodeAbstractResourceInstance.WriteChange.

* rename files to something more logical

* terrafrom: refresh refactor, continued!

I had originally made Refresh a stand-alone function since it was
(obnoxiously) called from a graphNodeImportStateSub, but after some
(greatly appreciated) prompting in the PR I instead made it a method on
the NodeAbstractResourceInstance, in keeping with the other refactored
eval nodes, and then built a NodeAbstractResourceInstance inside import.

Since I did that I could also remove my duplicated 'writeState' code
inside graphNodeImportStateSub and use n.writeResourceInstanceState, so
double thanks!

* unexport eval methods

* re-refactor Plan, it made more sense on NodeAbstractResourceInstance. Sorry

* Remove uninformative `Eval`s from EvalReadData, consolidate to a single
file, and rename file to match function names.

* manual rebase
2020-12-08 08:50:30 -05:00
Kristin Laemmert
7370a98ab7
Eval() Refactor (#27087)
* terraform: refactor EvalPreApply and EvalPostApply

EvalPreApply and EvalPostApply have been refactored as methods on
NodeAbstractResourceInstance.

* terraform: remove EvalReadState and EvalReadStateDeposed

These two functions had already been re-implemented as functions on
NodeAbstractResource, so this commit finished the process of removing
the Evals and refactoring the tests.

* terraform: remove EvalRefreshLifecycle

EvalRefreshLifecycle was only used in one node,
NodePlannableResourceInstance, so the functionality has been moved
directly inline.

* terraform: remove EvalDeposeState

EvalDeposeState was only used in one function, so it has been removed
and the logic placed in-line in
NodeApplyableResourceInstance.managedResourceExecute.

* terraform: remove EvalMaybeRestoreDeposedObject

EvalMaybeRestoreDeposedObject was only used in one place, so I've
removed it in favor of in-line code.
2020-12-04 09:16:26 -05:00
Kristin Laemmert
90588c036b
terraform: minor cleanup from EvalTree() refactor (#26429)
* Split node_resource_abstract.go into two files, putting
NodeAbstractResourceInstance methods in their own file - it was getting
large enough to be tricky for (my) human eyeballs.

* un-exported the functions that were created as part of the EvalTree()
refactor; they did not need to be public.
2020-10-01 08:12:10 -04:00
Martin Atkins
b1eec0fbcd core: NodeAbstractResourceInstance.Provider correct implied provider
When we need to select a qualified provider address based on an implied
provider name, we have a special case that the name "terraform" maps to
terraform.io/builtin/terraform instead of
registry.terraform.io/hashicorp/terraform as would be the case for other
prefixes.

However, in order for that to work properly we need to use
addrs.ImpliedProviderForUnqualifiedType instead of
addrs.NewDefaultProvider, because the latter just unconditionally always
produces a "default" provider configuration (belonging to the "hashicorp"
namespace on the public registry).
2020-08-24 11:41:28 -07:00