-
Notifications
You must be signed in to change notification settings - Fork 1.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
Add option to disable announcement of aggregated LoadBalancerIP ranges #8837
base: master
Are you sure you want to change the base?
Add option to disable announcement of aggregated LoadBalancerIP ranges #8837
Conversation
This MR allows to disable the default announcement of all LoadBalancerIP ranges by setting `serviceLoadBalancerRouteAggregationEnabled: false` in the BGPConfiguration. We are advertising IPs of the same `/24` in multiple clusters. Creating LoadBalancer services with externalTrafficPolicy Local are reachable because additional /32 route are being announced only from the cluster where the LoadBalancer exists. But LoadBalancers with externalTrafficPolicy Cluster are not working since the /24 route is being announced from multiple clusters and traffic not reaching the correct cluster. To mitigate this we would have to add every ip as a /32 to the BGPConfiguration. This MR adds an additional option to the BGPConfiguration to disable the default annoncement of all prefixes in the `serviceLoadBalancerIPs` list. In which case we are also announcing /32 routes for LoadBalancers with externalTrafficPolicy Cluster.
c09420e
to
ce377b5
Compare
@Lucaber thanks for the PR! I'm mulling this one over. In the meantime, I wonder if you have tried using the BGPFilter resource to achieve the same result? https://docs.tigera.io/calico/latest/reference/resources/bgpfilter I believe it would allow you to configure export filters for Calico that block the advertisement of the aggregated ranges (albeit in a more verbose way). Not necessarily against something like this PR, but ideally would be good to avoid implementing two ways to achieve the same end result. Let me know what you think. |
Hi @caseydavenport , First, calico creates Secondly, with the BGPFilter, the routes would probably still be created locally on all nodes, so traffic originating from inside the cluster would not be routed via the default route. |
Calico shouldn't actually program the Service LoadBalancer IP addresses / CIDRs into the routing table of the node, so I don't think they would impact routing on the cluster nodes - unless I'm misunderstanding something 😅
Yep, this is fair and something I hadn't spotted. So, IIUC, we need to:
So I believe that this is already achievable today, albeit with two pieces of configuration.
I think there is still a theoretical case where traffic ends up on a node that isn't hosting that service, and will be routed using the host's default gateway which isn't necessarily a loop but we probably still want to prevent. This might just be in theory though, and I agree for the most part we can expect /32 IP address to be real services that kube-proxy will be handling (at least on nodes that are hosting that service). |
Yes you are right but I forgot a third change that is implemented by this PR. |
Sorry for the delay on this.. Right, yes that makes sense to me. So there are a collection of three changes - two of which are expressible using existing config options, but one of them isn't:
Perhaps I have forgotten the context here... why do pods within the cluster want to talk via the LoadBalancer service IP? Just trying to remember / understand why we need to stop programming the service loop prevention rules. |
Description
This MR allows to disable the default announcement of all LoadBalancerIP ranges by setting
serviceLoadBalancerRouteAggregationEnabled: false
in the BGPConfiguration.We are advertising IPs of the same subnets (eg /24) in multiple clusters. LoadBalancer services with externalTrafficPolicy Local are reachable because additional /32 route are being announced only from the cluster where the LoadBalancer exists. But LoadBalancers with externalTrafficPolicy Cluster are not working since the /24 route is being announced from multiple clusters and traffic not reaching the correct cluster.
To mitigate this we would have to dynamically add every ip as a /32 to the BGPConfiguration because /32 routes are not advertised by default.
This MR adds an additional option to the BGPConfiguration to disable the default announcement of all prefixes in the
serviceLoadBalancerIPs
list. In which case we are also announcing /32 routes for LoadBalancers with externalTrafficPolicy Cluster. TheserviceLoadBalancerIPs
list still acts as a filter.Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*
label.docs-pr-required
: This change requires a change to the documentation that has not been completed yet.docs-completed
: This change has all necessary documentation completed.docs-not-required
: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*
label.release-note-required
: This PR has user-facing changes. Most PRs should have this label.release-note-not-required
: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate
: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr
: This PR is related to install and requires a corresponding change to the operator.