[occm]: support providers that do not require nodeports#3071
[occm]: support providers that do not require nodeports#3071oblazek wants to merge 2 commits intokubernetes:masterfrom
Conversation
k8s-ci-robot
added
the
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @oblazek! |
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @oblazek. 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. DetailsInstructions 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-sigs/prow repository. |
k8s-ci-robot
added
size/S
|
added/fixed tests so that the defaults are overwritten when needed |
|
/test |
|
@oblazek: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
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-sigs/prow repository. |
Add a new loadBalancer option `ProviderRequiresNodeports` which by default is true to keep the existing behavior but when set to false allows user to not have to allocate nodeports in case there is support by the loadbalancer provider. Signed-off-by: Ondrej Blazek <ondrej.blazek@firma.seznam.cz>
Signed-off-by: Ondrej Blazek <ondrej.blazek@firma.seznam.cz>
| newMembers := sets.New[string]() | ||
|
|
||
| for _, node := range nodes { | ||
| addr, err := nodeAddressForLB(node, svcConf.preferredIPFamily) |
There was a problem hiding this comment.
wouldn't you also need to use the pod addr instead of the node one in case no nodeport is required?
There was a problem hiding this comment.
Well usually not, as OCCM does not watch endpoints. That's the purpose of CNI (like cilium) imo. In our case we do ipip tunneling from our externalLB (the one that receives configuration from OCCM in the end) to k8s nodes where cilium listens.
There was a problem hiding this comment.
so in our case you have: extLB -> node1IP: clientIP -> svcIP (outer IP header: inner IP header)
There was a problem hiding this comment.
so that's why there is no need for nodeport
There was a problem hiding this comment.
I see, we have a slightly different use-case. we use native routing with cilium, so the pod network is directly reachable from external LB, therefore no additional ( nodePort ) hop would be required, so the LB member address could be the pod IP.
There was a problem hiding this comment.
Interesting, that's a lot of updates your externalLB needs to process. Anyway regarding the pod CIDR, how would you pass that to the OCCM? AFAICS it does not watch pods/endpoints.
There was a problem hiding this comment.
not necessarily a lot more updates if you use externalTrafficPolicy: local, which means the traffic is only sent to the nodes where the upstream pods are running. so the amount of updates would be higher only if the new pods are being rescheduled on the same nodes ( so no update of the LB required with nodePort) , otherwise it would be the same amount....
I'm not sure about the best way to have the podIPs in the OCCM, would it feasible to watch the Endpoints/EndpointSlices?
There was a problem hiding this comment.
Yeah that totally would, we have a similar controller that does that (but that's meant for other LB - not the external one). But I suppose OCCM maintainers don't want to do that.
3 participants
Add a new loadBalancer option
ProviderRequiresNodeportswhich by default is true to keep the existing behavior but when set to false allows user to not have to allocate nodeports in case there is support by the loadbalancer provider.This allows usage of services of type LoadBalancer with
allocateLoadBalancerNodePorts=falsein case provider like ours supports that. In our case we send traffic to our k8s nodes (in openstack) using ipip tunneling so there is no need to allocate nodeports.