Skip to content

Commit

Permalink
Uniform BGP router ID selection for IPv4 and IPv6
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 9, 2024
1 parent 230c862 commit c746376
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 39 deletions.
4 changes: 1 addition & 3 deletions docs/bgp-policy.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,7 @@ The `bgpPeers` field lists the BGP peers to which the advertisements are sent.

The BGP router identifier (ID) is a 4-byte field that is usually represented as an IPv4 address.

For an IPv4-only or dual-stack Kubernetes cluster, the Node's IPv4 address (assigned to the transport interface) is used.

For IPv6-only clusters, if the `node.antrea.io/bgp-router-id` annotation is present on the Node and its value is a valid
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.
Expand Down
10 changes: 2 additions & 8 deletions pkg/agent/controller/bgp/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,16 +481,10 @@ 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
// 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
}

nodeObj, err := c.nodeLister.Get(c.nodeName)
if err != nil {
return "", fmt.Errorf("failed to get Node object: %w", err)
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 generated from Node name ",
ipv4Enabled: true,
ipv6Enabled: true,
node: generateNode(localNodeName, nodeLabels1, nodeAnnotations1),
updatedNode: generateNode(localNodeName, nodeLabels1, nil),
existingState: deepCopyBGPPolicyState(policy1State),
expectedState: generateBGPPolicyState(179,
65000,
"156.67.103.8",
[]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 c746376

Please sign in to comment.