-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Run post-hooks even if backup actions fail #514
Conversation
9919150
to
b66b90c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was mostly a cut & paste, right?
pkg/backup/item_backupper.go
Outdated
backupErrs := make([]error, 0) | ||
err = ib.executeActions(log, obj, groupResource, name, namespace, metadata) | ||
if err != nil { | ||
log.WithError(err).Error("Error executing backup actions") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd change "backup" to "item"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!
Signed-off-by: Calle Pettersson <[email protected]>
b66b90c
to
b2ec87f
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
LGTM |
Fixes #511
This pulls all the action execution out into a new function (should there be matching changes in unit tests for this?), then ensures post-action hooks are run regardless of errors in the actions, and then returns any errors (including any in the post hook)