Repository navigation
fix(workflow-recipes): tear down a terminal run child once - #1510
Draft
alfredolopez80 wants to merge 4 commits into
Draft
alfredolopez80 wants to merge 4 commits into
alfredolopez80 wants to merge 4 commits into
Conversation
A terminal run-scoped workflow recipe repeated its compute teardown on every pass that reached the terminal branch: three Pod deletes, the transport delegation deletes and label sweeps, and the transport Deployment deletes. Every status-only MODIFIED event of such a child forced a full reconcile, so the record-less teardown ran again each time. - Add status.terminalRunTeardown (completedAt, noSecretBearingWorkloads) and status.secretOwnershipRevocations to the WorkflowRecipe CRD and types. The status schema is closed, so both must be declared. - Write terminalRunTeardown.completedAt once the teardown completed and skip the teardown while the record is present. A record write checks that the API server stored the fields; a response without them means an older CRD and is logged as an error naming the CRD. - A failed teardown step, or a record that was not stored, records nothing and requeues the pass. Transport Deployment deletes now fail loud instead of being swallowed. - Without the workflow subsystem the compute pods are not deleted, so no record is written and the pass requeues. - Clear the record before a run-scoped recipe enters the deploy path (live read, then a merge patch with null). - A status-only event of a recorded terminal child only syncs the DB; a child latched failed by a transient error is still forced. Test-only changes: the shared status-patch mock echoes the patched status; the CR-store fakes merge the run records by key and delete null keys; fixture stubs gain the methods the terminal branch calls; two status-conflict tests and the legacy retry test account for the record write (one more resourceVersion), and the latter's final event now only syncs the DB. Refs #1349 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t version A workload whose Secret is ownership-denied was torn down again on every pass by all three callers (the non-workflow deploy loop, the workflow deploy path and the steady enforcer behind the active and terminal short-circuits). For a denied StatefulSet the teardown also created the StatefulSet and its headless Service when they were absent, then deleted its Pods, so a terminal run child kept restarting a Pod that should not exist. - classifySecretAccess returns the denied read as <namespace>/<resourceVersion>; combineSecretAccess keeps the versions of both denied reads, and an unversioned read makes the denial unversioned. - deniedSecretRevocationDigest: sha256 over a workload's denied Secret refs and their sorted versions; undefined when a denied ref has no version, so the revocation then runs as before. - teardownDeniedWorkload takes the gate and returns at once when status.secretOwnershipRevocations[workloadId] equals the digest. The revocation log lines are emitted only when the teardown runs. - Each caller records the workloads it tore down in this pass with one status record patch, also when another workload's teardown failed. A record that is not stored asks for a requeue and keeps the pass result. - A denied StatefulSet is torn down only when it is live: the teardown reads it first and creates nothing when it is absent. Tests: T2.1 on the real watcher bridge (pass 2 sends no write, and the record event runs no pass), T2.2-T2.7 and T2.10-T2.12 per caller, T2.9 in secretOwnership.test.ts. The StatefulSet revocation test is rewritten on a live StatefulSet and the teardown-failure test now seeds one. The global beforeEach resets the Pod deletecollection mock, which leaked a rejection into later tests. Refs #1349 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The external egress refresh selected recipes by spec only and forced a full reconcile every tick. A terminal run child whose teardown is recorded returns from the terminal block before the egress accumulator runs, so the forced pass folded nothing and only repeated the terminal work. The refresh now skips a terminal run child (workflow, run-id label, terminal execution phase, not deleting) whose teardown is recorded and that is not latched by a transient error. A latched child falls through to the deploy path, where the accumulator runs, so it is still selected. recipeHasExternalExactHostEgress is unchanged. Refs #1349 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t-bearing workloads The ownership backstop selected every workflow recipe whose spec names a Secret, every 30 s, for as long as its CR exists. A run child copies the parent's workloads and Secret references, so a terminal run child was re-reconciled forever even when nothing it references is deployed. The terminal pass now records `terminalRunTeardown.noSecretBearingWorkloads` when every workload that names a Secret was denied in that pass or is deleted by the terminal teardown (a transport that is neither a StatefulSet nor PVC-backed). It is written only by the terminal block, after the teardown record and a clean ownership pass, and a failed write requeues. The fact lives inside the teardown record, so the clear before a re-deploy removes it too. `workflowNeedsOwnershipBackstop` skips a recorded, non-latched terminal run child that carries the fact. Every other recipe is selected as before; Secret changes still reach the child through the SecretWatcher. The steady enforcer now returns the denied workload ids, and the result shaping of `revokeOrRequeueSteadyWorkflow` moved to a pure function so the terminal block can write the fact between the two. The other three short-circuits keep calling the wrapper. The transport rule of the terminal teardown is shared through `isRemovedByTerminalRunTeardown`. Test harness: the CR store fake can prune one field of a record (`terminalRunTeardown.noSecretBearingWorkloads`), to cover a CRD that lacks the fact. Refs #1349 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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
Fixes #1349. A run-scoped workflow child repeated its compute teardown and its Secret-revocation deletes on every reconcile pass until its CR was deleted. Two periodic loops (external egress refresh and the ownership backstop) kept forcing those passes.
One commit per root cause:
b1440669c). Newstatus.terminalRunTeardown.completedAt, written once the teardown completed and checked against what the API server stored. While it is present, the teardown is skipped. A failed step or a record that was not stored writes nothing and requeues. The record is cleared before a run-scoped recipe is deployed again. A status-only event of a recorded terminal child only syncs the DB.6f55fcd58). Newstatus.secretOwnershipRevocations[workloadId], a sha256 over the denied Secret refs and their resourceVersions. All three callers skip a teardown whose digest is already recorded. A denied StatefulSet is torn down only when it is live, so it is no longer re-created and then deleted.7dd7da816). The forced pass folded nothing for a recorded terminal child. A child latched by a transient error is still selected.c5efc1e91). The terminal pass recordsterminalRunTeardown.noSecretBearingWorkloadswhen every workload that names a Secret was denied in that pass or is deleted by the terminal teardown. A deferred ownership pass records nothing and requeues. Secret changes still reach the child through the SecretWatcher.Both status fields are declared in the WorkflowRecipe CRD, whose status schema is closed. A record write against an older CRD is logged as an error naming the CRD, and the pass requeues.
Testing
npm testpasses for changed services: workflow-recipes, 3384 passed / 29 skipped (local T0 atc5efc1e91,T0_EXIT=0, includingtsc -b)npx tsc --noEmitclean (EXIT=0)c5efc1e91: 28/28 killed, none survived. Each removes one guard, record write, record check or CRD field, and the targeted tests turn red.Checklist
🤖 Generated with Claude Code