Repository navigation
Report accurate resource counts in bundle summaries - #7006
Draft
janniklasrose wants to merge 2 commits into
Draft
janniklasrose wants to merge 2 commits into
janniklasrose wants to merge 2 commits into
Conversation
Make the destroy summary line ("Destroy: N deleted") print even when the
destroy encounters errors partway through. This is a follow-up to PR #6210.
Before this change, early returns on errors (during the resource Apply, DMS
finalization, or file deletion) skipped the summary entirely. Extract the
summary into logDestroySummary and invoke it via defer in destroyCore so it
prints on every return path; partial deletions may have already succeeded.
The count is the planned deletions, not the actually-succeeded ones. Making
the count reflect only successful deletions is a separate follow-up.
Add an acceptance test that injects a failed resource delete and asserts the
summary still prints after the destroy error. The dms/failed-delete golden
gains the same summary line for the same reason.
Co-authored-by: Isaac <no-reply@databricks.com>
The deploy and destroy summaries counted planned operations, so a resource that failed to apply was still reported as created/deleted. Track per-resource outcomes during Apply - Attempted when the graph reaches a node, Applied when its backend operation succeeds - and count what actually happened: Destroy: 2 deleted, 1 failed Resources: 1 created, 0 changed, 0 deleted, 0 unchanged, 2 failed The deploy summary now also prints on a partial resource failure (it was suppressed before). It stays suppressed when no resource failed: a failure before the apply graph runs (e.g. a config error) or after it (e.g. a state-push error) still reports files only, as before. Direct engine only; the terraform engine was removed in v1.20.0. Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: 9e7f1c7
Top 4 slowest tests (at least 2 minutes):
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bundle
deployanddestroysummaries reported the planned operation count, so a resource that failed to apply was still reported as created/deleted — e.g.Destroy: 3 deletedwhen one of three resources actually failed to delete. This makes the summaries report what actually happened:destroyprints its summary even when the destroy errors partway (partial deletions may already have succeeded), with afailedcount.deployprints its resource summary on a partial resource failure too (previously suppressed), with accurate counts.How
Per-resource outcomes are tracked during the direct engine's
Apply:PlanEntry.Attempted— the apply graph reached this node.PlanEntry.Applied— its backend operation then succeeded.Plan.CountApplied()tallies by outcome (plus aFailedtotal). The deploy summary prints on failure only when a resource actually failed, so a failure before the graph runs (a config error) or after a fully successful apply (a state-push error) still reports files only — preserving the existingpartial-summary-on-push-failbehavior.Attemptedis what distinguishes "attempted and failed" from "never reached".Direct engine only; the terraform engine was removed in v1.20.0.
Tests
acceptance/bundle/deploy/summary-on-error(deploy partial failure) and reworkeddestroy/summary-on-error(three chained jobs, one delete fails) — chaining via id references keeps the apply order deterministic.CountAppliedcovering created/changed/deleted/failed, recreate, state-only delete, skip, and the attempted-but-not-reached case.bundle/dms,bundle/resources/*,bundle/resource_deps/*, andbundle/migrate/*to show the new accurate summary line.This pull request and its description were written by Isaac.