diff --git a/internal/backend/local/test.go b/internal/backend/local/test.go index d39713c4aa..21e18f70b8 100644 --- a/internal/backend/local/test.go +++ b/internal/backend/local/test.go @@ -58,7 +58,8 @@ type TestSuiteRunner struct { // Verbose tells the runner to print out plan files during each test run. Verbose bool - Concurrency int + Concurrency int + DeferralAllowed bool } func (runner *TestSuiteRunner) Stop() { @@ -121,6 +122,7 @@ func (runner *TestSuiteRunner) Test() (moduletest.Status, tfdiags.Diagnostics) { Render: runner.View, UnparsedVariables: currentGlobalVariables, Concurrency: runner.Concurrency, + DeferralAllowed: runner.DeferralAllowed, }) fileRunner := &TestFileRunner{ diff --git a/internal/command/arguments/test.go b/internal/command/arguments/test.go index ecf100506a..08b652db1f 100644 --- a/internal/command/arguments/test.go +++ b/internal/command/arguments/test.go @@ -47,6 +47,10 @@ type Test struct { // human-readable format or JSON for each run step depending on the // ViewType. Verbose bool + + // DeferralAllowed enables deferrals during test operations. This matches + // the same-named flag in the Operation struct. + DeferralAllowed bool } func ParseTest(args []string) (*Test, tfdiags.Diagnostics) { @@ -65,6 +69,7 @@ func ParseTest(args []string) (*Test, tfdiags.Diagnostics) { cmdFlags.BoolVar(&test.Verbose, "verbose", false, "verbose") cmdFlags.IntVar(&test.OperationParallelism, "parallelism", DefaultParallelism, "parallelism") cmdFlags.IntVar(&test.RunParallelism, "run-parallelism", DefaultParallelism, "run-parallelism") + cmdFlags.BoolVar(&test.DeferralAllowed, "allow-deferral", false, "allow-deferral") // TODO: Finalise the name of this flag. cmdFlags.StringVar(&test.CloudRunSource, "cloud-run", "", "cloud-run") diff --git a/internal/command/test.go b/internal/command/test.go index 5466560514..68538aa308 100644 --- a/internal/command/test.go +++ b/internal/command/test.go @@ -111,6 +111,17 @@ func (c *TestCommand) Run(rawArgs []string) int { view := views.NewTest(args.ViewType, c.View) + // EXPERIMENTAL: maybe enable deferred actions + if !c.AllowExperimentalFeatures && args.DeferralAllowed { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Failed to parse command-line flags", + "The -allow-deferral flag is only valid in experimental builds of Terraform.", + )) + view.Diagnostics(nil, nil, diags) + return 1 + } + // The specified testing directory must be a relative path, and it must // point to a directory that is a descendant of the configuration directory. if !filepath.IsLocal(args.TestDirectory) { @@ -228,6 +239,7 @@ func (c *TestCommand) Run(rawArgs []string) int { Filter: args.Filter, Verbose: args.Verbose, Concurrency: args.RunParallelism, + DeferralAllowed: args.DeferralAllowed, } // JUnit output is only compatible with local test execution diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 9970547ff2..5fea43fa92 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -397,6 +397,11 @@ func TestTest_Runs(t *testing.T) { expectedOut: []string{"1 passed, 0 failed."}, code: 0, }, + "deferred_changes": { + args: []string{"-allow-deferral"}, + expectedOut: []string{"3 passed, 0 failed."}, + code: 0, + }, } for name, tc := range tcs { t.Run(name, func(t *testing.T) { @@ -442,10 +447,11 @@ func TestTest_Runs(t *testing.T) { }, }, }, - Ui: ui, - View: view, - Streams: streams, - ProviderSource: providerSource, + Ui: ui, + View: view, + Streams: streams, + ProviderSource: providerSource, + AllowExperimentalFeatures: true, } init := &InitCommand{ diff --git a/internal/command/testdata/test/deferred_changes/create.tftest.hcl b/internal/command/testdata/test/deferred_changes/create.tftest.hcl new file mode 100644 index 0000000000..5d01bff793 --- /dev/null +++ b/internal/command/testdata/test/deferred_changes/create.tftest.hcl @@ -0,0 +1,11 @@ + +run "create" { + variables { + defer = true + } + + assert { + condition = test_resource.resource.defer + error_message = "deferred resource attribute should be true" + } +} diff --git a/internal/command/testdata/test/deferred_changes/main.tf b/internal/command/testdata/test/deferred_changes/main.tf new file mode 100644 index 0000000000..edd621e442 --- /dev/null +++ b/internal/command/testdata/test/deferred_changes/main.tf @@ -0,0 +1,7 @@ +variable "defer" { + type = bool +} + +resource "test_resource" "resource" { + defer = var.defer +} diff --git a/internal/command/testdata/test/deferred_changes/update.tftest.hcl b/internal/command/testdata/test/deferred_changes/update.tftest.hcl new file mode 100644 index 0000000000..f428ce75a1 --- /dev/null +++ b/internal/command/testdata/test/deferred_changes/update.tftest.hcl @@ -0,0 +1,22 @@ + +run "create" { + variables { + defer = false + } + + assert { + condition = !test_resource.resource.defer + error_message = "deferred resource attribute should be false" + } +} + +run "update" { + variables { + defer = true + } + + assert { + condition = test_resource.resource.defer + error_message = "deferred resource attribute should be true" + } +} diff --git a/internal/command/testing/test_provider.go b/internal/command/testing/test_provider.go index f559126d9a..d355366dd0 100644 --- a/internal/command/testing/test_provider.go +++ b/internal/command/testing/test_provider.go @@ -40,6 +40,7 @@ var ( "create_wait_seconds": {Type: cty.Number, Optional: true}, "destroy_wait_seconds": {Type: cty.Number, Optional: true}, "write_only": {Type: cty.String, Optional: true, WriteOnly: true}, + "defer": {Type: cty.Bool, Optional: true}, }, }, }, @@ -61,6 +62,7 @@ var ( "destroy_fail": {Type: cty.Bool, Computed: true}, "create_wait_seconds": {Type: cty.Number, Computed: true}, "destroy_wait_seconds": {Type: cty.Number, Computed: true}, + "defer": {Type: cty.Bool, Computed: true}, }, }, }, @@ -227,9 +229,18 @@ func (provider *TestProvider) ConfigureProvider(request providers.ConfigureProvi func (provider *TestProvider) PlanResourceChange(request providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { if request.ProposedNewState.IsNull() { + + var deferred *providers.Deferred + if shouldBeDeferred := request.PriorState.GetAttr("defer"); !shouldBeDeferred.IsNull() && shouldBeDeferred.True() { + deferred = &providers.Deferred{ + Reason: providers.DeferredReasonResourceConfigUnknown, + } + } + // Then this is a delete operation. return providers.PlanResourceChangeResponse{ PlannedState: request.ProposedNewState, + Deferred: deferred, } } @@ -252,8 +263,16 @@ func (provider *TestProvider) PlanResourceChange(request providers.PlanResourceC resource = cty.ObjectVal(vals) } + var deferred *providers.Deferred + if shouldBeDeferred := resource.GetAttr("defer"); !shouldBeDeferred.IsKnown() || (!shouldBeDeferred.IsNull() && shouldBeDeferred.True()) { + deferred = &providers.Deferred{ + Reason: providers.DeferredReasonResourceConfigUnknown, + } + } + return providers.PlanResourceChangeResponse{ PlannedState: resource, + Deferred: deferred, } } diff --git a/internal/moduletest/graph/eval_context.go b/internal/moduletest/graph/eval_context.go index d6b1ceb7f0..3096ac9e9b 100644 --- a/internal/moduletest/graph/eval_context.go +++ b/internal/moduletest/graph/eval_context.go @@ -80,7 +80,8 @@ type EvalContext struct { renderer views.Test verbose bool - evalSem terraform.Semaphore + deferralAllowed bool + evalSem terraform.Semaphore } type EvalContextOpts struct { @@ -91,6 +92,7 @@ type EvalContextOpts struct { UnparsedVariables map[string]backendrun.UnparsedVariableValue Config *configs.Config Concurrency int + DeferralAllowed bool } // NewEvalContext constructs a new graph evaluation context for use in @@ -119,6 +121,7 @@ func NewEvalContext(opts EvalContextOpts) *EvalContext { verbose: opts.Verbose, renderer: opts.Render, config: opts.Config, + deferralAllowed: opts.DeferralAllowed, evalSem: terraform.NewSemaphore(opts.Concurrency), } } diff --git a/internal/moduletest/graph/node_provider.go b/internal/moduletest/graph/node_provider.go index ae4e9cbc9d..d3bba441bc 100644 --- a/internal/moduletest/graph/node_provider.go +++ b/internal/moduletest/graph/node_provider.go @@ -90,7 +90,7 @@ func (n *NodeProviderConfigure) Execute(ctx *EvalContext) { TerraformVersion: version.SemVer.String(), Config: unmarkedBody, ClientCapabilities: providers.ClientCapabilities{ - DeferralAllowed: false, // TODO: Enable deferrals in test framework. + DeferralAllowed: ctx.deferralAllowed, WriteOnlyAttributesAllowed: true, }, }) diff --git a/internal/moduletest/graph/node_state_cleanup.go b/internal/moduletest/graph/node_state_cleanup.go index 15df73af60..68201e8a43 100644 --- a/internal/moduletest/graph/node_state_cleanup.go +++ b/internal/moduletest/graph/node_state_cleanup.go @@ -136,6 +136,7 @@ func (n *NodeStateCleanup) destroy(ctx *EvalContext, runNode *NodeTestRun, waite ExternalProviders: providers, SkipRefresh: true, OverridePreventDestroy: true, + DeferralAllowed: ctx.deferralAllowed, } tfCtx, _ := terraform.NewContext(n.opts.ContextOpts) @@ -144,10 +145,18 @@ func (n *NodeStateCleanup) destroy(ctx *EvalContext, runNode *NodeTestRun, waite waiter.update(tfCtx, moduletest.TearDown, nil) plan, planDiags := tfCtx.Plan(run.ModuleConfig, state, planOpts) diags = diags.Append(planDiags) - if diags.HasErrors() { + if diags.HasErrors() || plan.Errored { return state, diags } + if !plan.Complete { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Warning, + "Incomplete destroy plan", + fmt.Sprintf("The destroy plan for %s/%s was reported as incomplete."+ + " This means some of the cleanup operations were deferred due to unknown values, please check the rest of the output to see which resources could not be destroyed.", file.Name, run.Name))) + } + _, updated, applyDiags := runNode.apply(tfCtx, plan, moduletest.TearDown, variables, providers, waiter) diags = diags.Append(applyDiags) return updated, diags diff --git a/internal/moduletest/graph/plan.go b/internal/moduletest/graph/plan.go index 8386f9083c..6ef0125277 100644 --- a/internal/moduletest/graph/plan.go +++ b/internal/moduletest/graph/plan.go @@ -118,6 +118,7 @@ func (n *NodeTestRun) plan(ctx *EvalContext, tfCtx *terraform.Context, variables ExternalReferences: n.References(), ExternalProviders: providers, Overrides: mocking.PackageOverrides(run.Config, file.Config, mocks), + DeferralAllowed: ctx.deferralAllowed, } waiter.update(tfCtx, moduletest.Running, nil) diff --git a/internal/terraform/evaluate.go b/internal/terraform/evaluate.go index 87ca973026..19ab9d4188 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -594,9 +594,97 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc // continue with the rest of the function } + // Now, we're going to build up a value that represents the resource + // or resources that are in the state. + instances := map[addrs.InstanceKey]cty.Value{} + + // First, we're going to load any instances that we have written into the + // deferrals system. A deferred resource overrides anything that might be + // in the state for the resource, so we do this first. + for key, value := range d.Evaluator.Deferrals.GetDeferredResourceInstances(addr.Absolute(d.ModulePath)) { + instances[key] = value + } + + // Proactively read out all the resource changes before iteration. Not only + // does GetResourceInstanceChange have to iterate over all instances + // internally causing an n^2 lookup, but Changes is also a major point of + // lock contention. + resChanges := d.Evaluator.Changes.GetChangesForConfigResource(addr.InModule(moduleConfig.Path)) + instChanges := addrs.MakeMap[addrs.AbsResourceInstance, *plans.ResourceInstanceChange]() + for _, ch := range resChanges { + instChanges.Put(ch.Addr, ch) + } + rs := d.Evaluator.State.Resource(addr.Absolute(d.ModulePath)) + // Decode all instances in the current state + pendingDestroy := d.Operation == walkDestroy + if rs != nil { + for key, is := range rs.Instances { + if _, ok := instances[key]; ok { + // Then we've already loaded this instance from the deferrals so + // we'll just ignore it being in state. + continue + } + // Otherwise, we'll load the instance from state. + + if is == nil || is.Current == nil { + // Assume we're dealing with an instance that hasn't been created yet. + instances[key] = cty.UnknownVal(ty) + continue + } + + instAddr := addr.Instance(key).Absolute(d.ModulePath) + change := instChanges.Get(instAddr) + if change != nil { + // Don't take any resources that are yet to be deleted into account. + // If the referenced resource is CreateBeforeDestroy, then orphaned + // instances will be in the state, as they are not destroyed until + // after their dependants are updated. + if change.Action == plans.Delete { + if !pendingDestroy { + continue + } + } + } + + // Planned resources are temporarily stored in state with empty values, + // and need to be replaced by the planned value here. + if is.Current.Status == states.ObjectPlanned { + if change == nil { + // FIXME: This is usually an unfortunate case where we need to + // lookup an individual instance referenced via "self" for + // postconditions which we know exists, but because evaluation + // must always get the resource in aggregate some instance + // changes may not yet be registered. + instances[key] = cty.DynamicVal + // log the problem for debugging, since it may be a legitimate error we can't catch + log.Printf("[WARN] instance %s is marked as having a change pending but that change is not recorded in the plan", instAddr) + continue + } + instances[key] = change.After + continue + } + + ios, err := is.Current.Decode(schema) + if err != nil { + // This shouldn't happen, since by the time we get here we + // should have upgraded the state data already. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid resource instance data in state", + Detail: fmt.Sprintf("Instance %s data could not be decoded from the state: %s.", instAddr, err), + Subject: &config.DeclRange, + }) + continue + } - if rs == nil { + val := ios.Value + + instances[key] = val + } + } + + if len(instances) == 0 { switch d.Operation { case walkPlan, walkApply: // During plan and apply as we evaluate each removed instance they @@ -650,93 +738,6 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc } } - // Now, we're going to build up a value that represents the resource - // or resources that are in the state. - instances := map[addrs.InstanceKey]cty.Value{} - - // First, we're going to load any instances that we have written into the - // deferrals system. A deferred resource overrides anything that might be - // in the state for the resource, so we do this first. - for key, value := range d.Evaluator.Deferrals.GetDeferredResourceInstances(addr.Absolute(d.ModulePath)) { - instances[key] = value - } - - // Proactively read out all the resource changes before iteration. Not only - // does GetResourceInstanceChange have to iterate over all instances - // internally causing an n^2 lookup, but Changes is also a major point of - // lock contention. - resChanges := d.Evaluator.Changes.GetChangesForConfigResource(addr.InModule(moduleConfig.Path)) - instChanges := addrs.MakeMap[addrs.AbsResourceInstance, *plans.ResourceInstanceChange]() - for _, ch := range resChanges { - instChanges.Put(ch.Addr, ch) - } - - // Decode all instances in the current state - pendingDestroy := d.Operation == walkDestroy - for key, is := range rs.Instances { - if _, ok := instances[key]; ok { - // Then we've already loaded this instance from the deferrals so - // we'll just ignore it being in state. - continue - } - // Otherwise, we'll load the instance from state. - - if is == nil || is.Current == nil { - // Assume we're dealing with an instance that hasn't been created yet. - instances[key] = cty.UnknownVal(ty) - continue - } - - instAddr := addr.Instance(key).Absolute(d.ModulePath) - change := instChanges.Get(instAddr) - if change != nil { - // Don't take any resources that are yet to be deleted into account. - // If the referenced resource is CreateBeforeDestroy, then orphaned - // instances will be in the state, as they are not destroyed until - // after their dependants are updated. - if change.Action == plans.Delete { - if !pendingDestroy { - continue - } - } - } - - // Planned resources are temporarily stored in state with empty values, - // and need to be replaced by the planned value here. - if is.Current.Status == states.ObjectPlanned { - if change == nil { - // FIXME: This is usually an unfortunate case where we need to - // lookup an individual instance referenced via "self" for - // postconditions which we know exists, but because evaluation - // must always get the resource in aggregate some instance - // changes may not yet be registered. - instances[key] = cty.DynamicVal - // log the problem for debugging, since it may be a legitimate error we can't catch - log.Printf("[WARN] instance %s is marked as having a change pending but that change is not recorded in the plan", instAddr) - continue - } - instances[key] = change.After - continue - } - - ios, err := is.Current.Decode(schema) - if err != nil { - // This shouldn't happen, since by the time we get here we - // should have upgraded the state data already. - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid resource instance data in state", - Detail: fmt.Sprintf("Instance %s data could not be decoded from the state: %s.", instAddr, err), - Subject: &config.DeclRange, - }) - continue - } - - val := ios.Value - - instances[key] = val - } - // ret should be populated with a valid value in all cases below var ret cty.Value