-
Notifications
You must be signed in to change notification settings - Fork 140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Report action cleanup errors to the user & various cleanup fixes for recipe action #301
Draft
obbardc
wants to merge
8
commits into
go-debos:main
Choose a base branch
from
obbardc:wip/obbardc/fix-recipe-cleanup
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
This comment was marked as outdated.
This comment was marked as outdated.
obbardc
force-pushed
the
wip/obbardc/fix-recipe-cleanup
branch
from
November 2, 2022 18:35
4d83abc
to
e711d59
Compare
This comment was marked as outdated.
This comment was marked as outdated.
obbardc
force-pushed
the
wip/obbardc/fix-recipe-cleanup
branch
4 times, most recently
from
July 26, 2023 15:05
4c5d534
to
db61042
Compare
Rather than panicing on error, bubble up an error so that the execution may recover correctly and the error shown to the user. Fixes: go-debos#413 Signed-off-by: Christopher Obbard <[email protected]>
Currently the debos error handling sets an exitcode during the run, then exits with that code once execution has finished. This paradigm is quite fragile, so let's switch around the error handling to use context.State to track errors and check the state of this variable after the run has completed. While we are here, rename the checkError function to handleError to better describe the function's intent and make it return a boolean to show that an error has been handled. Rework the do_run function to also return a boolean to show that an error occurred during the run. Signed-off-by: Christopher Obbard <[email protected]>
Signed-off-by: Christopher Obbard <[email protected]>
Rework the error handling to show where an error occured during the run. Also print all error messages to the log rather than stdout. Signed-off-by: Christopher Obbard <[email protected]>
Currently any errors from the cleanup of an action are discarded and not reported to the user. So let's report errors during the cleanup to the user and indicate that the overall execution actually failed by setting the exitcode appropriately. Signed-off-by: Christopher Obbard <[email protected]>
obbardc
force-pushed
the
wip/obbardc/fix-recipe-cleanup
branch
from
January 10, 2024 17:38
db61042
to
6897f33
Compare
Other parts of the codebase will need to use the handleError function so refactor it into the main debos namespace. Signed-off-by: Christopher Obbard <[email protected]>
Unify the recipe action to handle action cleanup errors in the same way as Debos handles the cleanup functions. Specifically: - run all cleanup functions, even if a previous one failed, - run cleanup functions in the reverse order to the action, - report all errors during cleanup. Fixes: 74df488 ("actions: Add recipe action") Signed-off-by: Christopher Obbard <[email protected]>
When running nested recipes using the recipe action, if an action in the child recipe exits early (e.g. on failure) the remaining actions do not run but the cleanup functions for these remaining actions still run even though there is nothing to cleanup. This doesn't follow how Debos handles running standalone recipes, so modify the recipe action to only run action cleanup functions when the associated action needs to be cleaned up. Fixes: 74df488 ("actions: Add recipe action") Signed-off-by: Christopher Obbard <[email protected]>
obbardc
force-pushed
the
wip/obbardc/fix-recipe-cleanup
branch
from
January 13, 2024 13:39
6897f33
to
a9919aa
Compare
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.
Draft as depends on #303