Skip to content
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.

Sign up for GitHub

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

Uniform BGP router ID selection for IPv4 and IPv6 #6605

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Uniform BGP router ID selection for IPv4 and IPv6 #6605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions docs/bgp-policy.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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