-
Notifications
You must be signed in to change notificati 8000 on settings - Fork 936
Do not create ApplyPolicyFailed for conflicts #6267
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
Comments
Hi @grosser, do you mean to minimize the incident by retrying? |
yes, that would cut down on the noise a lot, maybe even eliminate it |
Thanks @grosser Can you help point out specific code changes and change logic? Others may also be concerned about this optimization. |
here needs to ignore when the object already exists (I assume that is not the same as a conflict) I did not find where the actual object update is happening, but if that does not have a retry on conflict then it should be added and it it already has one then either it needs to reload to avoid the conflict or have it's retry steps spaced out a bit more |
I'm sorry, I didn't quite understand you.
You mean to reduce the report of this event or
These two don't seem to be the same thing, they deal with different scenes. |
Yes correct, might be better as separate issues, I was trying to fix ApplyPolicyFailed events and found these 2 examples. |
Okay, let's focus on one of them first |
the "already exists" seems easy, would you agree that the fix would be to ignore if an already exist error occurs here |
I think the retry here should not be "already exists", it should be an update conflict. That doesn't seem to match your description. |
yeah don't know then, I'd leave the issue open for others to find, but don't have time atm to dig deeper |
Yeah, this is indeed not good. Anyway, who can analyze the root cause? Where exactly does the conflict come from? |
What would you like to be added:
when ApplyPolicyFailed happens due to a conflict, either issue it as "Normal" or retry a few times first
Why is this needed:
this made our alerts spammy that alert on "Warn events" for all apps, and not random apps get these events.
We had to ignore the events, but ideally we want to still alert when "non-conflict" sync fails (were retry does not help and someone needs to fix something)
The text was updated successfully, but these errors were encountered: