diff --git a/internal/stacks/stackruntime/internal/stackeval/component.go b/internal/stacks/stackruntime/internal/stackeval/component.go index 3ae127749560..bfcd35a5f9ac 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component.go +++ b/internal/stacks/stackruntime/internal/stackeval/component.go @@ -239,6 +239,42 @@ func (c *Component) ResultValue(ctx context.Context, phase EvalPhase) cty.Value } } +// PlanIsComplete can be called only during the planning phase, and returns +// true only if all instances of this component have "complete" plans. +// +// A component instance plan is "incomplete" if it was either created with +// resource targets set in its planning options or if the modules runtime +// decided it needed to defer at least one action for a future round. +func (c *Component) PlanIsComplete(ctx context.Context) bool { + if !c.main.Planning() { + panic("PlanIsComplete used when not in the planning phase") + } + insts := c.Instances(ctx, PlanPhase) + if insts == nil { + // Suggests that the configuration was not even valid enough to + // decide what the instances are, so we'll return false to be + // conservative and let the error be returned by a different path. + return false + } + + for _, inst := range insts { + plan := inst.ModuleTreePlan(ctx) + if plan == nil { + // Seems that we weren't even able to create a plan for this + // one, so we'll just assume it was incomplete to be conservative, + // and assume that whatever errors caused this nil result will + // get returned by a different return path. + return false + } + if !plan.Complete { + return false + } + } + // If we get here without returning false then we can say that + // all of the instance plans are complete. + return true +} + // ExprReferenceValue implements Referenceable. func (c *Component) ExprReferenceValue(ctx context.Context, phase EvalPhase) cty.Value { return c.ResultValue(ctx, phase) diff --git a/internal/stacks/stackruntime/internal/stackeval/component_instance.go b/internal/stacks/stackruntime/internal/stackeval/component_instance.go index 6c9d1b3c92e1..3ae87192bacd 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_instance.go @@ -529,15 +529,37 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla return nil, diags } + // If any of our upstream components have incomplete plans then + // we need to force treating everything in this component as + // deferred so we can preserve the correct dependency ordering. + upstreamDeferred := false + for _, depAddr := range c.call.RequiredComponents(ctx).Elems() { + depStack := c.main.Stack(ctx, depAddr.Stack, PlanPhase) + if depStack == nil { + upstreamDeferred = true // to be conservative + break + } + depComponent := depStack.Component(ctx, depAddr.Item) + if depComponent == nil { + upstreamDeferred = true // to be conservative + break + } + if !depComponent.PlanIsComplete(ctx) { + upstreamDeferred = true + break + } + } + // NOTE: This ComponentInstance type only deals with component // instances currently declared in the configuration. See // [ComponentInstanceRemoved] for the model of a component instance // that existed in the prior state but is not currently declared // in the configuration. plan, moreDiags := tfCtx.Plan(moduleTree, prevState, &terraform.PlanOpts{ - Mode: stackPlanOpts.PlanningMode, - SetVariables: inputValues, - ExternalProviders: providerClients, + Mode: stackPlanOpts.PlanningMode, + SetVariables: inputValues, + ExternalProviders: providerClients, + ExternalDependencyDeferred: upstreamDeferred, // This is set by some tests but should not be used in main code. // (nil means to use the real time when tfCtx.Plan was called.) @@ -748,14 +770,22 @@ func (c *ComponentInstance) ApplyModuleTreePlan(ctx context.Context, plan *plans return noOpResult, diags } - // NOTE: tfCtx.Apply tends to make changes to the given plan while it - // works, and so code after this point should not make any further use - // of either "modifiedPlan" or "plan" (since they share lots of the same - // pointers to mutable objects and so both can get modified together.) - newState, moreDiags := tfCtx.Apply(&modifiedPlan, moduleTree, &terraform.ApplyOpts{ - ExternalProviders: providerClients, - }) - diags = diags.Append(moreDiags) + var newState *states.State + if modifiedPlan.Applyable { + // NOTE: tfCtx.Apply tends to make changes to the given plan while it + // works, and so code after this point should not make any further use + // of either "modifiedPlan" or "plan" (since they share lots of the same + // pointers to mutable objects and so both can get modified together.) + newState, moreDiags = tfCtx.Apply(&modifiedPlan, moduleTree, &terraform.ApplyOpts{ + ExternalProviders: providerClients, + }) + diags = diags.Append(moreDiags) + } else { + // For a non-applyable plan, we just skip trying to apply it altogether + // and just propagate the prior state (including any refreshing we + // did during the plan phase) forward. + newState = modifiedPlan.PriorState + } if newState != nil { cic := &hooks.ComponentInstanceChange{ diff --git a/internal/stacks/stackruntime/internal/stackeval/planning_test.go b/internal/stacks/stackruntime/internal/stackeval/planning_test.go index 8fe80f948b8d..123ad1ccbdfa 100644 --- a/internal/stacks/stackruntime/internal/stackeval/planning_test.go +++ b/internal/stacks/stackruntime/internal/stackeval/planning_test.go @@ -21,6 +21,7 @@ import ( "github.com/hashicorp/terraform/internal/promising" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" + "github.com/hashicorp/terraform/internal/stacks/stackplan" "github.com/hashicorp/terraform/internal/stacks/stackstate" "github.com/hashicorp/terraform/internal/stacks/stackstate/statekeys" "github.com/hashicorp/terraform/internal/stacks/tfstackdata1" @@ -581,6 +582,105 @@ func TestPlanning_RequiredComponents(t *testing.T) { }) } +func TestPlanning_DeferredChangesPropagation(t *testing.T) { + // This test arranges for one component's plan to signal deferred changes, + // and checks that a downstream component's plan also has everything + // deferred even though it could potentially have been plannable in + // isolation, since we need to respect the dependency ordering between + // components. + + cfg := testStackConfig(t, "planning", "deferred_changes_propagation") + main := NewForPlanning(cfg, stackstate.NewState(), PlanOpts{ + PlanningMode: plans.NormalMode, + InputVariableValues: map[stackaddrs.InputVariable]ExternalInputValue{ + // This causes the first component to have a module whose + // instance count isn't known yet. + {Name: "first_count"}: { + Value: cty.UnknownVal(cty.Number), + }, + }, + ProviderFactories: ProviderFactories{ + addrs.NewBuiltInProvider("test"): func() (providers.Interface, error) { + return &terraform.MockProvider{ + GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ + Provider: providers.Schema{ + Block: &configschema.Block{}, + }, + ResourceTypes: map[string]providers.Schema{ + "test": { + Block: &configschema.Block{}, + }, + }, + }, + }, nil + }, + }, + }) + // TEMP: This test currently relies on the experimental module language + // feature of allowing unknown values in a resource's "count" argument. + // We should remove this if the experiment gets stabilized. + main.AllowLanguageExperiments(true) + + componentFirstInstAddr := stackaddrs.AbsComponentInstance{ + Stack: stackaddrs.RootStackInstance, + Item: stackaddrs.ComponentInstance{ + Component: stackaddrs.Component{ + Name: "first", + }, + }, + } + componentSecondInstAddr := stackaddrs.AbsComponentInstance{ + Stack: stackaddrs.RootStackInstance, + Item: stackaddrs.ComponentInstance{ + Component: stackaddrs.Component{ + Name: "second", + }, + }, + } + + componentPlanResourceActions := func(plan *stackplan.Component) map[string]plans.Action { + ret := make(map[string]plans.Action) + for _, elem := range plan.ResourceInstancePlanned.Elems { + ret[elem.Key.String()] = elem.Value.Action + } + return ret + } + + inPromisingTask(t, func(ctx context.Context, t *testing.T) { + plan, diags := testPlan(t, main) + assertNoErrors(t, diags) + + firstPlan := plan.Components.Get(componentFirstInstAddr) + if firstPlan.PlanComplete { + t.Error("first component has a complete plan; should be incomplete because it has deferred actions") + } + secondPlan := plan.Components.Get(componentSecondInstAddr) + if secondPlan.PlanComplete { + t.Error("second component has a complete plan; should be incomplete because everything in it should've been deferred") + } + + gotFirstActions := componentPlanResourceActions(firstPlan) + wantFirstActions := map[string]plans.Action{ + // Only test.a is planned, because test.b has unknown count + // and must therefore be deferred. + "test.a": plans.Create, + } + gotSecondActions := componentPlanResourceActions(secondPlan) + wantSecondActions := map[string]plans.Action{ + // Nothing at all expected for the second, because all of its + // planned actions should've been deferred to respect the + // dependency on the first component. + } + + if diff := cmp.Diff(wantFirstActions, gotFirstActions); diff != "" { + t.Errorf("wrong actions for first component\n%s", diff) + } + if diff := cmp.Diff(wantSecondActions, gotSecondActions); diff != "" { + t.Errorf("wrong actions for second component\n%s", diff) + } + }) +} + func TestPlanning_NoWorkspaceNameRef(t *testing.T) { // This test verifies that a reference to terraform.workspace is treated // as invalid for modules used in a stacks context, because there's diff --git a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tf b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tf new file mode 100644 index 000000000000..1760edf92191 --- /dev/null +++ b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tf @@ -0,0 +1,32 @@ + +terraform { + required_providers { + test = { + source = "terraform.io/builtin/test" + } + } + + # TODO: Remove this if this experiment gets stabilized. + # If you're removing this, remember to also update the calling test so + # that it no longer enables the use of experiments, to ensure that we're + # really not depending on any experimental features. + experiments = [unknown_instances] +} + +variable "instance_count" { + type = number +} + +resource "test" "a" { + # This one has on intrinsic need to be deferred, but + # should still be deferred when an upstream component + # has a deferral. +} + +resource "test" "b" { + count = var.instance_count +} + +output "constant_one" { + value = 1 +} diff --git a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tfstack.hcl b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tfstack.hcl new file mode 100644 index 000000000000..d429a4d63e1a --- /dev/null +++ b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tfstack.hcl @@ -0,0 +1,35 @@ + +required_providers { + test = { + source = "terraform.io/builtin/test" + } +} + +provider "test" "main" { +} + +variable "first_count" { + type = number +} + +component "first" { + source = "./" + + inputs = { + instance_count = var.first_count + } + providers = { + test = provider.test.main + } +} + +component "second" { + source = "./" + + inputs = { + instance_count = component.first.constant_one + } + providers = { + test = provider.test.main + } +}