[flytepropeller] Preserve deck URI when a node is aborted#7611
Open
moldhouse wants to merge 1 commit into
Open
[flytepropeller] Preserve deck URI when a node is aborted#7611moldhouse wants to merge 1 commit into
moldhouse wants to merge 1 commit into
Conversation
Abort builds its own node event and never set DeckUri, so aborting a run wiped the deck from the UI even though deck.html still existed. Re-attach it when the file is present, matching the recovery path. Fixes flyteorg#7610 Signed-off-by: Moritz Althaus <moritzalthaus@gmx.de>
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
1 participant
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?
Aborting a running node wipes its Flyte Deck from the UI, even though the deck was showing a moment earlier and
deck.htmlstill exists in blob storage. Abort takes a different path than the other terminal states:nodeExecutor.Abortbuilds its ownNodeExecutionEventwith no deck URI, and flyteadmin then overwrites the stored URI with that empty value.This is a hard blocker for eager workflows. Each eager child is its own execution, and the eager task's deck is the only place the UI surfaces the children and their links. Aborting the parent, the normal way to stop a run, wipes the deck and with it the trail to every child execution.
What changes were proposed in this pull request?
On abort, re-attach the deck URI to the ABORTED event when the deck file exists, using the same
GetDeckPath()the running task writes to. This mirrors the recovery path, which already checks the deck file and re-attaches it. It only sets the URI when the file is present, so it never resurrects a deck for a disabled or never-published task, and aHeaderror is tolerated so abort always proceeds.How was this patch tested?
Added a unit test that drives
Abortand asserts the emitted event carries the deck URI; confirmed it fails without the change and passes with it. The fullflytepropeller/pkg/controller/nodessuite passes.Fixes #7610