-
Notifications
You must be signed in to change notification settings - Fork 39.3k
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 Topology Aware Hints support for Kube-Proxy #100804
Conversation
/assign @robscott |
a618036
to
6d3a4b2
Compare
Thanks @aojea! Although there's lots of good improvements in here, I'd rather keep this fix small in scope so we can justify getting it into 1.21. If I'm understanding the issue, the true fix is contained in the small I think we should pull out the EndpointSlice v1 upgrade from this PR. The scope is a bit larger than the diff suggests, it's not just a 1:1 mapping between topology and deprecatedTopology. I think that will take more work and testing and does not feel like it's required as part of this fix. |
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 fixing this! I'm a bit hesitant about the scope of this PR, but really appreciate the core bug fix here.
Yeah, I just added everything in different commits to the PR so you can pick the important stuff, I wasn't sure what was needed and what can be postponed. I will keep only the fix here and we can discuss the other stuff for 1.22 |
/area kube-proxy |
/retest Review the full test history for this PR. Silence the bot with an |
/retest |
/retest Review the full test history for this PR. Silence the bot with an |
5 similar comments
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest |
2 similar comments
/retest |
/retest |
the job has an issue, hold until we fix it |
/retest Review the full test history for this PR. Silence the bot with an |
4 similar comments
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
this is constantly failing, can it be the cause? |
/retest |
/retest Review the full test history for this PR. Silence the bot with an |
1 similar comment
/retest Review the full test history for this PR. Silence the bot with an |
@aojea: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
/retest Review the full test history for this PR. Silence the bot with an |
…0804-release-1.21 Automated cherry pick of #100804: add node handlers to the metaproxier
/kind bug
What this PR does / why we need it:
Fix kube-proxy implementation of Topology Aware Hints,
implemented in #99522
KEP: https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/2433-topology-aware-hints
Enhancement Issue: kubernetes/enhancements#2433
/sig network
/priority critical-urgent