From 41937499f546b27f9815053c59b4759cca0cac74 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 9 Feb 2024 18:15:50 -0800 Subject: [PATCH] stackeval: Basic support for deferred-change propagation This is the bare minimum functionality to ensure that we defer all actions in any component that depends on a component that already had deferred actions. We will also eventually need to propagate out a signal to the caller for whether the stack plan as a whole is complete or incomplete, but we'll save that for later commits, since the stack orchestration in Terraform Cloud will do the right thing regardless, aside from the cosmetic concern that it won't yet know to show a message to the user saying that there are deferred changes. --- .../internal/stackeval/component.go | 36 +++++++ .../internal/stackeval/component_instance.go | 52 +++++++-- .../internal/stackeval/planning_test.go | 100 ++++++++++++++++++ .../deferred-changes-propagation.tf | 32 ++++++ .../deferred-changes-propagation.tfstack.hcl | 35 ++++++ 5 files changed, 244 insertions(+), 11 deletions(-) create mode 100644 internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tf create mode 100644 internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/planning/deferred_changes_propagation/deferred-changes-propagation.tfstack.hcl 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 + } +}