projectcalico / calico

Cloud native networking and network security
https://docs.tigera.io/calico/latest/about/
Apache License 2.0
5.98k stars 1.33k forks source link

Support for multiple bgp peers with automatic or peer-specific source address #5226

Closed krisiasty closed 2 years ago

krisiasty commented 4 years ago

Current bird template contains global "source address" configuration option in the template bgp bgp_template section, which is not overrided later in the peer-specific section. With multiple L3 interfaces peering with different bgp peers this renders incorrect configuration where calico tries to use source ip address from other subnet than the peer it is trying to connect for all but one interfaces.

Simple solution to this issue would be to provide option to remove the source address from the bgp_template section since it should be automatically selected by bird (unless your peer is not directly connected). In that case, the option should be part of node object, for example "useSourceIP: false" like below:

apiVersion: projectcalico.org/v3
kind: Node
metadata:
  name: storage-1-1
spec:
  bgp:
    asNumber: 4201061
    ipv4Address: 10.10.0.97/31
    useSourceIp: false

Or we could have peer-specific option defined in the calico bgppeer object, for example "localIP: 10.10.0.96" like below:

apiVersion: projectcalico.org/v3
kind: BGPPeer
metadata:
  name: storage-1-1
spec:
  asNumber: 65001
  node: storage-1-1
  peerIP: 10.10.0.96
  localIP: 10.10.0.97

If this option is present in the object, it should be used to render "source address" option for specific peer in bird configuration.

Having both options supported by calico should address all use cases, including those using bgp multihop. Unfortunately, implementing these features needs relevant changes in calico code.

Please also refer to the following issue and my comment: https://github.com/projectcalico/calico/issues/1829#issuecomment-638494464

caseydavenport commented 4 years ago

We probably want to configure this on the BGPConfiguraiton object, since that's meant to configure BGP per-node.

I do think that this belongs in the API, though, rather than configured via env vars.

krisiasty commented 4 years ago

My proposal using env vars was just the quick and easy path which doesn't require API changes, but if you are willing to do this the right way i'm okay with that too (assuming we don't have to wait for it for too long). I'm not sure about BGPConfiguration object though, since this would make the setting global, and it might be better to allow for node-specific configuration. Maybe the best would be to have both options available (global in BGPConfiguration + local BGPPeer override)? In fact, BGPPeer should also allow to specify local IP to use...

caseydavenport commented 4 years ago

BGPConfiguration object though, since this would make the setting global, and it might be better to allow for node-specific configuration

BGPConfiguration has a global default but instances can also be created per-node, so it should satisfy both scenarios.

Would like to get a bit more input from others (CC @neiljerram @Suraiya-Hameed) about this before making a final decision on API, but feel strongly that this belongs in the API and not in an environment variable if we are to do it.

nelljerram commented 4 years ago

In Calico Enterprise we have a sourceAddress field in BGPPeer:

Specifies whether and how to configure a source address for the peerings generated by this BGPPeer resource. Default value “UseNodeIP” means to configure the node IP as the source address. “None” means not to configure a source address.

(from https://docs.tigera.io/reference/resources/bgppeer)

Would that meet the need here?

krisiasty commented 4 years ago

I guess it should do in most cases. I don't know how I could miss that in documentation. I'll give it a try when I came back from vacation. Ideally, sourceAddress could also accept an actual IP address along with UseNodeIP and None values allowing for more advanced configurations.

nelljerram commented 2 years ago

@krisiasty We have the sourceAddress field in open source Calico now, since v3.18, so I'll mark this closed. Please do re-open or raise a new issue if there's still a gap.