Skip to content
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

Enable panic recovery #859

Merged
merged 1 commit into from
Aug 11, 2022
Merged

Enable panic recovery #859

merged 1 commit into from
Aug 11, 2022

Conversation

Santosh1176
Copy link
Contributor

Implements RecoverPanic option on reconcilers.
This is a part fixes to issue #2952

Signed-off-by: Santosh Kaluskar dtshbl@gmail.com

main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
@stefanprodan stefanprodan changed the title Implementing RecoverPanic on reconcilers to ensure it recovers from panic instead of crashing the controller. Enable panic recovery Aug 11, 2022
@@ -125,6 +125,7 @@ type BucketReconciler struct {
type BucketReconcilerOptions struct {
MaxConcurrentReconciles int
RateLimiter ratelimiter.RateLimiter
RecoverPanic bool
Copy link
Contributor

@darkowlzz darkowlzz Aug 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer need to add this option as we don't make this configurable.

@darkowlzz
Copy link
Contributor

@Santosh1176 Along with the last requested change, can you please squash all the commits into a single commit?

…anic instead of crashing the controller and Squashed commits.

Signed-off-by: Santosh Kaluskar <dtshbl@gmail.com>
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks @Santosh1176 🏅

Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for working on it.

Copy link
Member

@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

thanks for working on this @Santosh1176! 🙇

@Santosh1176
Copy link
Contributor Author

@stefanprodan @pjbgf @darkowlzz Thank you for the opportunity and guidance.

@pjbgf pjbgf merged commit 3e2de43 into fluxcd:main Aug 11, 2022
@Santosh1176 Santosh1176 deleted the fix-2952/sc branch August 30, 2022 18:03
@Santosh1176 Santosh1176 restored the fix-2952/sc branch August 30, 2022 18:03
@Santosh1176 Santosh1176 deleted the fix-2952/sc branch August 30, 2022 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants