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
Add retry to update NetworkPolicy status #3134
Conversation
|
/test-all |
Codecov Report
@@ Coverage Diff @@
## main #3134 +/- ##
===========================================
+ Coverage 40.35% 58.28% +17.92%
===========================================
Files 167 295 +128
Lines 20879 24927 +4048
===========================================
+ Hits 8426 14528 +6102
+ Misses 11632 8818 -2814
- Partials 821 1581 +760
Flags with carried forward coverage won't be shown. Click here to find out more.
|
0b295e7
to
399d418
Compare
|
/test-all |
|
/test-integration |
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.
Thanks for making the code more robust. I'd like to know @antoninbas's opinion on the change of the metrics' meaning.
| return err | ||
| } | ||
| klog.V(2).InfoS("Updated Antrea NetworkPolicy", "NetworkPolicy", klog.KObj(toUpdate)) | ||
| metrics.AntreaNetworkPolicyStatusUpdates.Inc() |
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.
@antoninbas do you think it's ok to count successful updates only? Before it counted the number of status update attempt regardless of result.
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 am ok with the change. I think if this is responsible for causing heavy load on the apiserver and etcd, the metric will still reflect that.
|
s/reslove/resolve/ |
To resolve the conflict problem in updating NetworkPolicy status process, add a retry logic in updating NetworkPolicy status function. Signed-off-by: Wu zhengdong <zhengdong.wu@transwarp.io>
399d418
to
442af62
Compare
@tnqn Thanks for the review, updated. |
|
/test-all |
|
/skip-all |
2 similar comments
|
/skip-all |
|
/skip-all |
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
To reslove the conflict problem in updating NetworkPolicy status
process, add a retry logic in updating NetworkPolicy status function.
Fixes: #3133
Signed-off-by: Wu zhengdong zhengdong.wu@transwarp.io