Skip to content

Commit

Permalink
Uniform BGP router ID selection for IPv4 and IPv6 (#6605)
Browse files Browse the repository at this point in the history 
Fixes #6550

Signed-off-by: Kumar Atish <kumar.atish@broadcom.com>
  • Loading branch information
@Atish-iaf
Atish-iaf committed Aug 22, 2024
1 parent 2ef21d2 commit c76b38a
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 44 deletions.
18 changes: 12 additions & 6 deletions docs/bgp-policy.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,20 @@ The `bgpPeers` field lists the BGP peers to which the advertisements are sent.

## BGP router ID

The BGP router identifier (ID) is a 4-byte field that is usually represented as an IPv4 address.
The BGP router identifier (ID) is a 4-byte field that is usually represented as an IPv4 address. Antrea uses the following
steps to choose the BGP router ID:

For an IPv4-only or dual-stack Kubernetes cluster, the Node's IPv4 address (assigned to the transport interface) is used.
1. If the `node.antrea.io/bgp-router-id` annotation is present on the Node and its value is a valid IPv4 address string,
we will use the provided value.
2. Otherwise, for an IPv4-only or dual-stack Kubernetes cluster, the Node's IPv4 address (assigned to the transport
interface) is used.
3. Otherwise, for IPv6-only clusters, a 32-bit integer will be generated by hashing the Node's name, then converted to the
string representation of an IPv4 address.

For IPv6-only clusters, if the `node.antrea.io/bgp-router-id` annotation is present on the Node and its value is a valid
IPv4 address string, we will use the provided value. Otherwise, a 32-bit integer will be generated by hashing the Node
name, then converted to the string representation of an IPv4 address, and the `node.antrea.io/bgp-router-id` annotation
is added / updated as necessary to reflect the selected BGP router ID.
After this selection process, the `node.antrea.io/bgp-router-id` annotation is added or updated as necessary to reflect
the selected BGP router ID.

The router ID is generated once and will not be updated if the Node configuration changes (e.g., if the Node's IPv4 address is updated).

## BGP Authentication

Expand Down
22 changes: 12 additions & 10 deletions pkg/agent/controller/bgp/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,15 +481,13 @@ func (c *Controller) getRouterID() (string, error) {
// startup and is the same for every local interface and BGP peer.
//
// In goBGP, only an IPv4 address can be used as the BGP Identifier (BGP router ID).
// For IPv4-only or dual-stack Kubernetes clusters, the Node's IPv4 address is used as the BGP router ID, ensuring
// uniqueness.
// For IPv6-only Kubernetes clusters without a Node IPv4 address, the router ID could be specified in the Node
// annotation `node.antrea.io/bgp-router-id`. If the annotation is not present, an IPv4 address will be generated by
// hashing the Node name and updated to the Node annotation `node.antrea.io/bgp-router-id`.

if c.enabledIPv4 {
return c.nodeIPv4Addr, nil
}
// The router ID could be specified in the Node annotation `node.antrea.io/bgp-router-id`.
// For IPv4-only or dual-stack Kubernetes clusters, if the annotation is not present,
// the Node's IPv4 address is used as the BGP router ID, ensuring uniqueness, and updated
// to the Node annotation `node.antrea.io/bgp-router-id`.
// For IPv6-only Kubernetes clusters without a Node IPv4 address, if the annotation is
// not present, an IPv4 address will be generated by hashing the Node name and updated
// to the Node annotation `node.antrea.io/bgp-router-id`.

nodeObj, err := c.nodeLister.Get(c.nodeName)
if err != nil {
Expand All @@ -500,7 +498,11 @@ func (c *Controller) getRouterID() (string, error) {
var routerID string
routerID, exists = nodeObj.GetAnnotations()[types.NodeBGPRouterIDAnnotationKey]
if !exists {
routerID = hashNodeNameToIP(c.nodeName)
if c.enabledIPv4 {
routerID = c.nodeIPv4Addr
} else {
routerID = hashNodeNameToIP(c.nodeName)
}
patch, _ := json.Marshal(map[string]interface{}{
"metadata": map[string]interface{}{
"annotations": map[string]string{
Expand Down
98 changes: 70 additions & 28 deletions pkg/agent/controller/bgp/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func TestBGPPolicyAdd(t *testing.T) {
},
expectedState: generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(clusterIPv4)},
[]bgp.PeerConfig{ipv4Peer1Config},
),
Expand Down Expand Up @@ -296,7 +296,7 @@ func TestBGPPolicyAdd(t *testing.T) {
},
expectedState: generateBGPPolicyState(179,
65000,
"192.168.77.100",
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(externalIPv6)},
[]bgp.PeerConfig{ipv6Peer1Config},
),
Expand Down Expand Up @@ -330,7 +330,7 @@ func TestBGPPolicyAdd(t *testing.T) {
},
expectedState: generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(loadBalancerIPv4), ipStrToPrefix(loadBalancerIPv6)},
[]bgp.PeerConfig{ipv4Peer1Config, ipv6Peer1Config},
),
Expand Down Expand Up @@ -363,7 +363,7 @@ func TestBGPPolicyAdd(t *testing.T) {
},
expectedState: generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(ipv4EgressIP1)},
[]bgp.PeerConfig{ipv4Peer1Config},
),
Expand All @@ -390,7 +390,7 @@ func TestBGPPolicyAdd(t *testing.T) {
objects: []runtime.Object{node},
expectedState: generateBGPPolicyState(179,
65000,
"192.168.77.100",
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{podIPv6CIDR.String()},
[]bgp.PeerConfig{ipv6Peer1Config},
),
Expand Down Expand Up @@ -424,7 +424,7 @@ func TestBGPPolicyAdd(t *testing.T) {
},
expectedState: generateBGPPolicyState(1179,
65001,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
nil,
[]bgp.PeerConfig{ipv4Peer1Config, ipv6Peer1Config},
),
Expand Down Expand Up @@ -462,13 +462,13 @@ func TestBGPPolicyAdd(t *testing.T) {
objects: []runtime.Object{ipv4ClusterIP1, ipv4ClusterIP1Eps, node},
existingState: generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(clusterIPv4)},
[]bgp.PeerConfig{ipv4Peer1Config},
),
expectedState: generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(clusterIPv4)},
[]bgp.PeerConfig{ipv4Peer1Config},
),
Expand Down Expand Up @@ -525,7 +525,7 @@ func TestBGPPolicyUpdate(t *testing.T) {
})
effectivePolicyState := generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(clusterIPv4),
ipStrToPrefix(clusterIPv6),
ipStrToPrefix(loadBalancerIPv4),
Expand Down Expand Up @@ -629,7 +629,7 @@ func TestBGPPolicyUpdate(t *testing.T) {
},
expectedState: generateBGPPolicyState(1179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(clusterIPv4),
ipStrToPrefix(clusterIPv6),
ipStrToPrefix(loadBalancerIPv4),
Expand Down Expand Up @@ -686,7 +686,7 @@ func TestBGPPolicyUpdate(t *testing.T) {
}),
expectedState: generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(externalIPv4),
ipStrToPrefix(externalIPv6),
ipStrToPrefix(ipv4EgressIP1),
Expand Down Expand Up @@ -731,7 +731,7 @@ func TestBGPPolicyUpdate(t *testing.T) {
}),
expectedState: generateBGPPolicyState(179,
65001,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(externalIPv4),
ipStrToPrefix(externalIPv6),
ipStrToPrefix(ipv4EgressIP1),
Expand Down Expand Up @@ -775,7 +775,7 @@ func TestBGPPolicyUpdate(t *testing.T) {
}),
expectedState: generateBGPPolicyState(1179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(clusterIPv4),
ipStrToPrefix(clusterIPv6),
ipStrToPrefix(loadBalancerIPv4),
Expand Down Expand Up @@ -822,7 +822,7 @@ func TestBGPPolicyUpdate(t *testing.T) {
ipv6Peer3}),
expectedState: generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(clusterIPv4),
ipStrToPrefix(clusterIPv6),
ipStrToPrefix(loadBalancerIPv4),
Expand Down Expand Up @@ -945,7 +945,7 @@ func TestBGPPolicyDelete(t *testing.T) {
})
policy1State := generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{
ipStrToPrefix(loadBalancerIPv4),
ipStrToPrefix(loadBalancerIPv6),
Expand All @@ -971,7 +971,7 @@ func TestBGPPolicyDelete(t *testing.T) {
})
policy2State := generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{
ipStrToPrefix(externalIPv4),
ipStrToPrefix(externalIPv6),
Expand All @@ -996,7 +996,7 @@ func TestBGPPolicyDelete(t *testing.T) {
})
policy3State := generateBGPPolicyState(1179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{
ipStrToPrefix(externalIPv4),
ipStrToPrefix(externalIPv6),
Expand Down Expand Up @@ -1119,7 +1119,7 @@ func TestNodeUpdate(t *testing.T) {
[]v1alpha1.BGPPeer{ipv4Peer1, ipv6Peer1})
policy1State := generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{podIPv4CIDR.String(), podIPv6CIDR.String()},
[]bgp.PeerConfig{ipv4Peer1Config, ipv6Peer1Config})
policy2 := generateBGPPolicy(bgpPolicyName2,
Expand All @@ -1135,7 +1135,7 @@ func TestNodeUpdate(t *testing.T) {
[]v1alpha1.BGPPeer{ipv4Peer1, ipv6Peer1})
policy2State := generateBGPPolicyState(1179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{podIPv4CIDR.String(), podIPv6CIDR.String()},
[]bgp.PeerConfig{ipv4Peer1Config, ipv6Peer1Config})
policy3 := generateBGPPolicy(bgpPolicyName3,
Expand Down Expand Up @@ -1210,19 +1210,61 @@ func TestNodeUpdate(t *testing.T) {
},
existingState: deepCopyBGPPolicyState(policy1State),
},
{
name: "Update annotations, effective BGPPolicy router ID is updated",
ipv4Enabled: true,
ipv6Enabled: true,
node: generateNode(localNodeName, nodeLabels1, nodeAnnotations1),
updatedNode: generateNode(localNodeName, nodeLabels1, nodeAnnotations2),
existingState: deepCopyBGPPolicyState(policy1State),
expectedState: generateBGPPolicyState(179,
65000,
nodeAnnotations2[types.NodeBGPRouterIDAnnotationKey],
[]string{podIPv4CIDR.String(), podIPv6CIDR.String()},
[]bgp.PeerConfig{ipv4Peer1Config, ipv6Peer1Config}),
expectedCalls: func(mockBGPServer *bgptest.MockInterfaceMockRecorder) {
mockBGPServer.Start(gomock.Any())
mockBGPServer.Stop(gomock.Any())
mockBGPServer.AddPeer(gomock.Any(), ipv4Peer1Config)
mockBGPServer.AddPeer(gomock.Any(), ipv6Peer1Config)
mockBGPServer.AdvertiseRoutes(gomock.Any(), []bgp.Route{{Prefix: podIPv4CIDR.String()}})
mockBGPServer.AdvertiseRoutes(gomock.Any(), []bgp.Route{{Prefix: podIPv6CIDR.String()}})
},
},
{
name: "Remove annotations, router ID is updated using Node's IPv4 address",
ipv4Enabled: true,
ipv6Enabled: true,
node: generateNode(localNodeName, nodeLabels1, nodeAnnotations1),
updatedNode: generateNode(localNodeName, nodeLabels1, nil),
existingState: deepCopyBGPPolicyState(policy1State),
expectedState: generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
[]string{podIPv4CIDR.String(), podIPv6CIDR.String()},
[]bgp.PeerConfig{ipv4Peer1Config, ipv6Peer1Config}),
expectedCalls: func(mockBGPServer *bgptest.MockInterfaceMockRecorder) {
mockBGPServer.Start(gomock.Any())
mockBGPServer.Stop(gomock.Any())
mockBGPServer.AddPeer(gomock.Any(), ipv4Peer1Config)
mockBGPServer.AddPeer(gomock.Any(), ipv6Peer1Config)
mockBGPServer.AdvertiseRoutes(gomock.Any(), []bgp.Route{{Prefix: podIPv4CIDR.String()}})
mockBGPServer.AdvertiseRoutes(gomock.Any(), []bgp.Route{{Prefix: podIPv6CIDR.String()}})
},
},
{
name: "IPv6 only, update annotations, effective BGPPolicy router ID is updated",
ipv6Enabled: true,
node: generateNode(localNodeName, nodeLabels1, nodeAnnotations1),
updatedNode: generateNode(localNodeName, nodeLabels1, nodeAnnotations2),
existingState: generateBGPPolicyState(179,
65000,
"192.168.77.100",
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{podIPv6CIDR.String()},
[]bgp.PeerConfig{ipv6Peer1Config}),
expectedState: generateBGPPolicyState(179,
65000,
"10.10.0.100",
nodeAnnotations2[types.NodeBGPRouterIDAnnotationKey],
[]string{podIPv6CIDR.String()},
[]bgp.PeerConfig{ipv6Peer1Config}),
expectedCalls: func(mockBGPServer *bgptest.MockInterfaceMockRecorder) {
Expand All @@ -1239,7 +1281,7 @@ func TestNodeUpdate(t *testing.T) {
updatedNode: generateNode(localNodeName, nodeLabels1, nil),
existingState: generateBGPPolicyState(179,
65000,
"192.168.77.100",
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{podIPv6CIDR.String()},
[]bgp.PeerConfig{ipv6Peer1Config}),
expectedState: generateBGPPolicyState(179,
Expand Down Expand Up @@ -1687,7 +1729,7 @@ func TestSyncBGPPolicyFailures(t *testing.T) {

checkBGPPolicyState(t, generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(loadBalancerIPv4)},
[]bgp.PeerConfig{ipv4Peer2Config}),
c.bgpPolicyState)
Expand All @@ -1703,7 +1745,7 @@ func TestSyncBGPPolicyFailures(t *testing.T) {
require.EqualError(t, c.syncBGPPolicy(ctx), "failed to stop current BGP server: failed reason")
checkBGPPolicyState(t, generateBGPPolicyState(179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(loadBalancerIPv4)},
[]bgp.PeerConfig{ipv4Peer2Config}),
c.bgpPolicyState)
Expand All @@ -1720,7 +1762,7 @@ func TestSyncBGPPolicyFailures(t *testing.T) {
require.EqualError(t, c.syncBGPPolicy(ctx), "failed to add BGP peer")
checkBGPPolicyState(t, generateBGPPolicyState(1179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{},
[]bgp.PeerConfig{}),
c.bgpPolicyState)
Expand All @@ -1734,7 +1776,7 @@ func TestSyncBGPPolicyFailures(t *testing.T) {
checkBGPPolicyState(t, generateBGPPolicyState(
1179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{},
[]bgp.PeerConfig{ipv4Peer1Config}),
c.bgpPolicyState)
Expand All @@ -1752,7 +1794,7 @@ func TestSyncBGPPolicyFailures(t *testing.T) {
doneDummyEvent(t, c)
checkBGPPolicyState(t, generateBGPPolicyState(1179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(externalIPv4)},
[]bgp.PeerConfig{ipv4Peer2Config}),
c.bgpPolicyState)
Expand All @@ -1770,7 +1812,7 @@ func TestSyncBGPPolicyFailures(t *testing.T) {
doneDummyEvent(t, c)
checkBGPPolicyState(t, generateBGPPolicyState(1179,
65000,
nodeIPv4Addr.IP.String(),
nodeAnnotations1[types.NodeBGPRouterIDAnnotationKey],
[]string{ipStrToPrefix(loadBalancerIPv4)},
[]bgp.PeerConfig{updatedIPv4Peer2Config}),
c.bgpPolicyState)
Expand Down

0 comments on commit c76b38a

Please sign in to comment.