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
Adding support for WinProxyEbpfMode in Windows KubeProxy #124092
base: master
Are you sure you want to change the base?
Adding support for WinProxyEbpfMode in Windows KubeProxy #124092
Conversation
Please note that we're already in Test Freeze for the Fast forwards are scheduled to happen every 6 hours, whereas the most recent run was: Thu Mar 28 02:11:27 UTC 2024. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: princepereira The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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. |
Hi @princepereira. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
dd74563
to
5cb686e
Compare
e4489eb
to
7ffe4fc
Compare
7ffe4fc
to
d081c20
Compare
// owner: @princepereira | ||
// alpha: v1.27 | ||
// | ||
// Allows windows kube-proxy to switch between old and new windows networking stack |
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.
// Allows windows kube-proxy to switch between old and new windows networking stack | |
// Allows windows kube-proxy to switch between "vswitch" and "ebpf" windows networking stack |
// This will be cleanup by cleanupStaleLoadbalancer fnction. | ||
proxier.mapStaleLoadbalancers[*lbHnsID] = true | ||
// configureLoadbalancer takes care of deleting and creating lb policy in old hns architecture and update policy in | ||
// new architecture. If there are no backend endpoints, then code will only delete existing loadbalancer. |
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.
spelling
return | ||
} | ||
|
||
klog.V(3).InfoS("Create and delete loadbalancer called.", "lbHnsID", *lbHnsID, "sourceVip", sourceVip, "vip", vip, "port", targetPort, "lbType", lbType) |
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.
Nit: reverse these.. i.e., "delete and create"
svcPortMap[svcPortName] = true | ||
proxier.onEndpointsMapChange(&svcPortName, false) | ||
} | ||
|
||
for svcPortName, eps := range newEndpointsMap { | ||
logFormattedEndpoints("endpointsMapChange newEndpointsMap", logLevel, svcPortName, eps) | ||
if proxier.winProxyEbpfMode { | ||
for _, ep := range eps { | ||
if !ep.IsLocal() && proxier.remoteEPsToCleanup[ep.IP()] { |
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.
Probably isLocal check is not needed here. ep in remoteEPsToCleanup shouldn't be local per check in line 371. And if at all ep is local and in remoteEPsToCleanup, we would ideally like to remove it from remoteEPsToCleanup?
@@ -348,14 +362,30 @@ func (proxier *Proxier) endpointsMapChange(oldEndpointsMap, newEndpointsMap prox | |||
// This will optimize remote endpoint and loadbalancer deletion based on the annotation |
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.
As discussed, please add a comment on when endpointsMapChange gets called.
/hold is there a KEP? I can not find it, also windows proxy is not well maintained so we should understand the implications of adding more functionality if there is no commitment from people to maintain it |
What type of PR is this?
This pr provides support for WinProxyEbpfMode in windows Kube proxy.
/kind feature
What this PR does / why we need it:
Earlier windows networking stack didn't support modify load balancer policy. Update of load balancer policy was always done through delete and create policy. This was slightly time consuming, and this will increase the lag when the number of policies is in lot in number. The other draw back was change in the load balancer id even for the update policy. The sequence is also error prune in cases where delete is passed and create is failed.
With the introduction of eBPF mode in Windows Container Networking Stack, will add api for load balancer update which will be lot faster, less error prune and no change in lb id after update. Also have added changes to support proxy terminating endpoints in ebpf mode.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
NONE
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: