Add option to disable announcement of aggregated LoadBalancerIP ranges

[Lucaber]

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. The serviceLoadBalancerIPs list still acts as a filter.

Todos

• Tests
• Documentation
• Release note

Release Note

TBD

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.

[CLAassistant]

All committers have signed the CLA.

[caseydavenport]

@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.

[Lucaber]

Hi @caseydavenport ,
thanks for the suggestion, i didn't know about the BGPFilter resource.
Blocking the announcement via a BGPFilter would only partially solve the problem.

First, calico creates cali-cidr-block block rules for all loadbalancerservice cidrs which i'm now preventing in this
commit. So traffic from inside the cluster would be blocked by these iptables rules.
Btw: Shouldn't we also block the creation of these rules for /32 cidrs, as /32 cidrs are only being announced when a service exists (see here)

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.

[caseydavenport]

Secondly, with the BGPFilter, the routes would probably still be created locally on all nodes

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 😅

First, calico creates cali-cidr-block block rules for all loadbalancerservice cidrs which i'm now preventing in this
commit. So traffic from inside the cluster would be blocked by these iptables rules.

Yep, this is fair and something I hadn't spotted. So, IIUC, we need to:

• Prevent export of the full CIDR (can be achieved with BGPFilter)
• Remove the service loop prevention rules that drop packets to LoadBalancer IPs (configurable via the FelixConfiguration ServiceLookPrevention option)

So I believe that this is already achievable today, albeit with two pieces of configuration.

Btw: Shouldn't we also block the creation of these rules for /32 cidrs, as /32 cidrs are only being announced when a service exists (see here)

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).

[Lucaber]

Yes you are right but I forgot a third change that is implemented by this PR.
We need to export /32 routes for LoadBalancers with externalTrafficPolicy: cluster. These are normally handled by the full cidr route on every node.