-
Notifications
You must be signed in to change notification settings - Fork 217
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
fix: catch exceptions from updating the status #2752
Conversation
closes: operator-framework#2751 Signed-off-by: Steve Hawkins <[email protected]>
This makes more sense to me, considering the following situation having a patch from error handler that fails and not to retry raises some concerns of consistency. This way might be at least eventual consistent in general. But not retrying lead to an unexpected state: reconciliation failed and even status is not updated. |
Why would you not retry? Wouldn't that mean the core reconciliation action has no retry or retires left? An independent retry for patching the status doesn't make as much sense for me. Since there is already retry logic built in the client, the most likely causes for the patching to fail would be:
|
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.
It makes sense to me like this. If there are requests to patch the status, we could put this under config.
Ideally, it would make sense to always try to patch the status but it might not be possible, especially if it already failed at least once… |
Thank you @shawkins ! |
There are two possible handlings here:
I opted for the former, but I'm open to the latter.
closes: #2751