From 2559151d0a82c6342aa92ced702565a766d820fa Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 10 Oct 2023 16:27:38 -0700 Subject: [PATCH] stackruntime: Count actions successfully applied Callers expect to be given an updated set of counts for how many of each action were completed, which we'll achieve by having the terraform.Hook implementation keep track of those and then announce them based on what it collected. We can't use the plan for this directly because terraform.Context.Plan seems to modify the plan it's given during its work, and on success the plan ends up being totally empty. --- .../stackruntime/hooks/component_instance.go | 17 +++++ .../internal/stackeval/component_instance.go | 69 +++++++++++-------- .../internal/stackeval/terraform_hook.go | 62 ++++++++++++++++- 3 files changed, 117 insertions(+), 31 deletions(-) diff --git a/internal/stacks/stackruntime/hooks/component_instance.go b/internal/stacks/stackruntime/hooks/component_instance.go index 950f270e00..05e5593dfc 100644 --- a/internal/stacks/stackruntime/hooks/component_instance.go +++ b/internal/stacks/stackruntime/hooks/component_instance.go @@ -4,6 +4,7 @@ package hooks import ( + "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/rpcapi/terraform1" "github.com/hashicorp/terraform/internal/stacks/stackaddrs" ) @@ -63,3 +64,19 @@ type ComponentInstanceChange struct { func (cic ComponentInstanceChange) Total() int { return cic.Add + cic.Change + cic.Import + cic.Remove } + +// CountNewAction increments zero or more of the count fields based on the +// given action. +func (cic *ComponentInstanceChange) CountNewAction(action plans.Action) { + switch action { + case plans.Create: + cic.Add++ + case plans.Delete: + cic.Remove++ + case plans.Update: + cic.Change++ + case plans.CreateThenDelete, plans.DeleteThenCreate: + cic.Add++ + cic.Remove++ + } +} diff --git a/internal/stacks/stackruntime/internal/stackeval/component_instance.go b/internal/stacks/stackruntime/internal/stackeval/component_instance.go index 8f558c392d..6bff45774f 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_instance.go @@ -515,8 +515,6 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla return nil, diags } - // TODO: Should pass in the previous run state once we have a - // previous stack state to take it from. // NOTE: This ComponentInstance type only deals with component // instances currently declared in the configuration. See // [ComponentInstanceRemoved] for the model of a component instance @@ -533,10 +531,11 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla }) diags = diags.Append(moreDiags) - cic := &hooks.ComponentInstanceChange{ - Addr: addr, - } if plan != nil { + cic := &hooks.ComponentInstanceChange{ + Addr: addr, + } + for _, rsrcChange := range plan.DriftedResources { hookMore(ctx, seq, h.ReportResourceInstanceDrift, &hooks.ResourceInstanceChange{ Addr: stackaddrs.AbsResourceInstance{ @@ -550,18 +549,7 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla if rsrcChange.Importing != nil { cic.Import++ } - - switch rsrcChange.Action { - case plans.Create: - cic.Add++ - case plans.Delete: - cic.Remove++ - case plans.Update: - cic.Change++ - case plans.CreateThenDelete, plans.DeleteThenCreate: - cic.Add++ - cic.Remove++ - } + cic.CountNewAction(rsrcChange.Action) hookMore(ctx, seq, h.ReportResourceInstancePlanned, &hooks.ResourceInstanceChange{ Addr: stackaddrs.AbsResourceInstance{ @@ -643,14 +631,15 @@ func (c *ComponentInstance) ApplyModuleTreePlan(ctx context.Context, plan *plans return nil, diags } + tfHook := &componentInstanceTerraformHook{ + ctx: ctx, + seq: seq, + hooks: hooksFromContext(ctx), + addr: c.Addr(), + } tfCtx, err := terraform.NewContext(&terraform.ContextOpts{ Hooks: []terraform.Hook{ - &componentInstanceTerraformHook{ - ctx: ctx, - seq: seq, - hooks: hooksFromContext(ctx), - addr: c.Addr(), - }, + tfHook, }, PreloadedProviderSchemas: providerSchemas, }) @@ -724,19 +713,41 @@ func (c *ComponentInstance) ApplyModuleTreePlan(ctx context.Context, plan *plans return nil, 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) if newState != nil { - // TODO: Emit resource-instance-level "applied" events for all - // resource instances that were affected by applying this plan. - - hookMore(ctx, seq, h.ReportComponentInstanceApplied, &hooks.ComponentInstanceChange{ + cic := &hooks.ComponentInstanceChange{ Addr: addr, - // TODO: include counts for each type of change - }) + + // We'll increment these gradually as we visit each change below. + Add: 0, + Change: 0, + Remove: 0, + } + + // We need to report what changes were applied, which is mostly just + // re-announcing what was planned but we'll check to see if our + // terraform.Hook implementation saw a "successfully applied" event + // for each resource instance object before counting it. + applied := tfHook.ResourceInstanceObjectsSuccessfullyApplied() + for _, rioAddr := range applied { + action := tfHook.ResourceInstanceObjectAppliedAction(rioAddr) + + // FIXME: We can't count imports here because they aren't "actions" + // in the sense that our hook gets informed about, and so the + // import number will always be zero in the apply phase. + + cic.CountNewAction(action) + } + + hookMore(ctx, seq, h.ReportComponentInstanceApplied, cic) } if diags.HasErrors() { diff --git a/internal/stacks/stackruntime/internal/stackeval/terraform_hook.go b/internal/stacks/stackruntime/internal/stackeval/terraform_hook.go index 5be60d1c91..be7c29e12d 100644 --- a/internal/stacks/stackruntime/internal/stackeval/terraform_hook.go +++ b/internal/stacks/stackruntime/internal/stackeval/terraform_hook.go @@ -5,6 +5,7 @@ package stackeval import ( "context" + "sync" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/plans" @@ -29,6 +30,10 @@ type componentInstanceTerraformHook struct { seq *hookSeq hooks *Hooks addr stackaddrs.AbsComponentInstance + + mu sync.Mutex + resourceInstanceObjectApplyAction addrs.Map[addrs.AbsResourceInstanceObject, plans.Action] + resourceInstanceObjectApplySuccess addrs.Set[addrs.AbsResourceInstanceObject] } func (h *componentInstanceTerraformHook) resourceInstanceAddr(addr addrs.AbsResourceInstance) stackaddrs.AbsResourceInstance { @@ -61,15 +66,54 @@ func (h *componentInstanceTerraformHook) PreApply(addr addrs.AbsResourceInstance Status: hooks.ResourceInstanceApplying, }) } + + h.mu.Lock() + if h.resourceInstanceObjectApplyAction.Len() == 0 { + h.resourceInstanceObjectApplyAction = addrs.MakeMap[addrs.AbsResourceInstanceObject, plans.Action]() + } + h.resourceInstanceObjectApplyAction.Put( + addrs.AbsResourceInstanceObject{ + ResourceInstance: addr, + DeposedKey: dk, + }, + action, + ) + h.mu.Unlock() + return terraform.HookActionContinue, nil } func (h *componentInstanceTerraformHook) PostApply(addr addrs.AbsResourceInstance, dk addrs.DeposedKey, newState cty.Value, err error) (terraform.HookAction, error) { - // FIXME: need to emit nothing if this was a no-op, which means tracking - // the `action` argument to `PreApply`. See `jsonHook` for more on this. + h.mu.Lock() + action, ok := h.resourceInstanceObjectApplyAction.GetOk(addrs.AbsResourceInstanceObject{ + ResourceInstance: addr, + DeposedKey: dk, + }) + h.mu.Unlock() + if !ok { + // Weird, but we'll just tolerate it to be robust. + return terraform.HookActionContinue, nil + } + + if action == plans.NoOp { + // We don't emit starting hooks for no-op changes and so we shouldn't + // emit ending hooks for them either. + return terraform.HookActionContinue, nil + } + status := hooks.ResourceInstanceApplied if err != nil { status = hooks.ResourceInstanceErrored + } else { + h.mu.Lock() + if h.resourceInstanceObjectApplySuccess == nil { + h.resourceInstanceObjectApplySuccess = addrs.MakeSet[addrs.AbsResourceInstanceObject]() + } + h.resourceInstanceObjectApplySuccess.Add(addrs.AbsResourceInstanceObject{ + ResourceInstance: addr, + DeposedKey: dk, + }) + h.mu.Unlock() } hookMore(h.ctx, h.seq, h.hooks.ReportResourceInstanceStatus, &hooks.ResourceInstanceStatusHookData{ @@ -111,3 +155,17 @@ func (h *componentInstanceTerraformHook) PostProvisionInstanceStep(addr addrs.Ab }) return terraform.HookActionContinue, nil } + +func (h *componentInstanceTerraformHook) ResourceInstanceObjectAppliedAction(addr addrs.AbsResourceInstanceObject) plans.Action { + h.mu.Lock() + ret, ok := h.resourceInstanceObjectApplyAction.GetOk(addr) + h.mu.Unlock() + if !ok { + return plans.NoOp + } + return ret +} + +func (h *componentInstanceTerraformHook) ResourceInstanceObjectsSuccessfullyApplied() addrs.Set[addrs.AbsResourceInstanceObject] { + return h.resourceInstanceObjectApplySuccess +}