fix(v2): preserve terminal phase on TaskAction delete events#7293
Merged
fix(v2): preserve terminal phase on TaskAction delete events#7293
Conversation
When the controller-runtime informer (added in #7272) delivers a delete event — including DeletedFinalStateUnknown tombstones after a watch gap (e.g. laptop sleep/resume) — buildActionUpdate unconditionally forced the phase to ABORTED. notifyRunService then overwrote the previously recorded Succeeded/Failed status in the DB, so completed runs flipped to Aborted in the UI. Two guards: - DeleteFunc now calls shouldSkipTaskAction so tombstones for actions already labeled terminal-status-recorded are dropped before dispatch. - buildActionUpdate only forces ABORTED when the current phase is not terminal, preserving Succeeded/Failed/TimedOut on legitimate deletes too (e.g. CR GC after the run finished). Signed-off-by: Kevin Su <pingsutw@apache.org>
AdilFayyaz
approved these changes
Apr 27, 2026
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.
Why are the changes needed?
Successful (and failed) runs were flipping to Aborted in the UI some time after completion, even without restarting devbox — most reproducibly after the laptop slept and woke.
DeleteFuncfires for both real deletes andDeletedFinalStateUnknowntombstones (delivered when the informer missed events during a watch gap, e.g. sleep/resume).buildActionUpdatethen unconditionally overwrotephasewithABORTEDfor any delete event, even for TaskActions that had already reachedSucceeded/Failed.notifyRunServicepushed that ABORTED phase to the run service, overwriting the recorded terminal status in the DB → UI shows Aborted.The pre-existing
labelTerminalStatusRecordedguard (shouldSkipTaskAction) was only consulted byAddFunc/UpdateFunc, never byDeleteFunc.What changes were proposed in this pull request?
Two guards in
actions/k8s/client.go:DeleteFuncnow callsshouldSkipTaskActionafter unwrapping the tombstone, so deletes for actions already labeledterminal-status-recordedare dropped before dispatch.buildActionUpdateonly forcesABORTEDwhen the current phase is not terminal. This preservesSucceeded/Failed/TimedOutfor legitimate post-terminal CR cleanup as well.How was this patch tested?
go test ./actions/k8s/... -count=1— passes.New unit tests in
actions/k8s/client_test.go:TestBuildActionUpdate_DeleteAfterTerminalPreservesPhase— Succeeded and Failed cases (the bug scenario).TestBuildActionUpdate_DeleteOnNonTerminalForcesAborted— confirms in-flight deletes still produce ABORTED.Labels
Check all the applicable boxes
Related PRs