From fdd992cfa73e3946f2e7688aeeca4bd8ffa8ec3e Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Thu, 5 Feb 2026 13:32:01 -0800 Subject: [PATCH] planning: Build manage resource instance execgraph from planned change The temporary placeholder code was relying only on the DesiredResourceInstance object, which was good enough for proving that this could work at all and had the advantage of being a new-style model with new-style representations of the provider instance address and the other resource instances that the desired object depends on. But that isn't enough information to plan anything other than "create" changes, so now we'll switch to using plans.ResourceInstanceChange as the main input to the execgraph building logic, even though for now that means we need to carry a few other values alongside it to compensate for the shortcomings of that old model designed for the old language runtime. So far this doesn't actually change what we do in response to the change so it still only supports "create" changes. In future commits we'll make the execGraphBuilder method construct different shapes of graph depending on which change action was planned. Signed-off-by: Martin Atkins --- internal/engine/planning/execgraph_managed.go | 17 ++++++---- internal/engine/planning/plan_managed.go | 34 ++++++++++++++++--- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/internal/engine/planning/execgraph_managed.go b/internal/engine/planning/execgraph_managed.go index 567abe1484..a31cee1c20 100644 --- a/internal/engine/planning/execgraph_managed.go +++ b/internal/engine/planning/execgraph_managed.go @@ -6,11 +6,10 @@ package planning import ( - "github.com/zclconf/go-cty/cty" - + "github.com/opentofu/opentofu/internal/addrs" "github.com/opentofu/opentofu/internal/engine/internal/exec" "github.com/opentofu/opentofu/internal/engine/internal/execgraph" - "github.com/opentofu/opentofu/internal/lang/eval" + "github.com/opentofu/opentofu/internal/plans" ) //////////////////////////////////////////////////////////////////////////////// @@ -29,7 +28,11 @@ import ( // originally written inline in [planGlue.planDesiredManagedResourceInstance] // just to preserve the existing functionality for now until we design a more // complete approach in later work. -func (b *execGraphBuilder) ManagedResourceInstanceSubgraph(desired *eval.DesiredResourceInstance, plannedValue cty.Value, providerClientRef execgraph.ResultRef[*exec.ProviderClient]) execgraph.ResourceInstanceResultRef { +func (b *execGraphBuilder) ManagedResourceInstanceSubgraph( + plannedChange *plans.ResourceInstanceChange, + providerClientRef execgraph.ResultRef[*exec.ProviderClient], + requiredResourceInstances addrs.Set[addrs.AbsResourceInstance], +) execgraph.ResourceInstanceResultRef { b.mu.Lock() defer b.mu.Unlock() @@ -38,13 +41,13 @@ func (b *execGraphBuilder) ManagedResourceInstanceSubgraph(desired *eval.Desired // from the data flow because these results are intermediated through the // evaluator, which indirectly incorporates the results into the // desiredInstRef result we'll build below. - dependencyWaiter, closeDependencyAfter := b.waiterForResourceInstances(desired.RequiredResourceInstances.All()) + dependencyWaiter, closeDependencyAfter := b.waiterForResourceInstances(requiredResourceInstances.All()) // FIXME: If this is one of the "replace" actions then we need to generate // a more complex graph that has two pairs of "final plan" and "apply". - instAddrRef := b.lower.ConstantResourceInstAddr(desired.Addr) + instAddrRef := b.lower.ConstantResourceInstAddr(plannedChange.Addr) priorStateRef := b.lower.ResourceInstancePrior(instAddrRef) - plannedValRef := b.lower.ConstantValue(plannedValue) + plannedValRef := b.lower.ConstantValue(plannedChange.After) desiredInstRef := b.lower.ResourceInstanceDesired(instAddrRef, dependencyWaiter) finalPlanRef := b.lower.ManagedFinalPlan( desiredInstRef, diff --git a/internal/engine/planning/plan_managed.go b/internal/engine/planning/plan_managed.go index 29c3516efd..70487e33ec 100644 --- a/internal/engine/planning/plan_managed.go +++ b/internal/engine/planning/plan_managed.go @@ -259,7 +259,13 @@ func (p *planGlue) planDesiredManagedResourceInstance(ctx context.Context, inst // and reasonable. In particular, these subgraph-building methods should // be easily unit-testable due to not depending on anything other than // their input. - finalResultRef := p.managedResourceInstanceExecSubgraph(ctx, inst, planResp.PlannedState, egb) + finalResultRef := p.managedResourceInstanceExecSubgraph( + ctx, + plannedChange, + inst.ProviderInstance, + inst.RequiredResourceInstances, + egb, + ) // Our result value for ongoing downstream planning is the planned new state. return planResp.PlannedState, finalResultRef, diags @@ -284,9 +290,29 @@ func (p *planGlue) planDeposedManagedResourceInstanceObject(ctx context.Context, // which implicitly adds execgraph items needed for the resource instance's // provider instance, which requires some information that an [execGraphBuilder] // instance cannot access directly itself. -func (p *planGlue) managedResourceInstanceExecSubgraph(ctx context.Context, inst *eval.DesiredResourceInstance, plannedValue cty.Value, egb *execGraphBuilder) execgraph.ResourceInstanceResultRef { - providerClientRef, registerProviderCloseBlocker := p.ensureProviderInstanceExecgraph(ctx, inst.ProviderInstance, egb) - finalResultRef := egb.ManagedResourceInstanceSubgraph(inst, plannedValue, providerClientRef) +// +// Note that a nil pointer for providerInstAddr is currently how we represent +// that we don't know which provider instance address to use. We'll hopefully +// do something clearer than that in future; refer to related FIXME comments in +// [planGlue.ensureProviderInstanceExecgraph] for more information. +// TODO: remove this paragraph once we've switched to a better representation. +// +// FIXME: Because we're currently still using our old model for describing +// planned changes, we need to bring a little extra baggage alongside the +// change object in our arguments here. As we start to design a new model +// for describing planned changes in future we should aspire for this function +// to take only (ctx, plannedChange, egb) as arguments and have the +// plannedChange object be a self-contained representation of everything needed +// to make the graph-building decisions. +func (p *planGlue) managedResourceInstanceExecSubgraph( + ctx context.Context, + plannedChange *plans.ResourceInstanceChange, + providerInstAddr *addrs.AbsProviderInstanceCorrect, + requiredResourceInstances addrs.Set[addrs.AbsResourceInstance], + egb *execGraphBuilder, +) execgraph.ResourceInstanceResultRef { + providerClientRef, registerProviderCloseBlocker := p.ensureProviderInstanceExecgraph(ctx, providerInstAddr, egb) + finalResultRef := egb.ManagedResourceInstanceSubgraph(plannedChange, providerClientRef, requiredResourceInstances) registerProviderCloseBlocker(finalResultRef) return finalResultRef }