diff --git a/cmd/antrea-agent/agent.go b/cmd/antrea-agent/agent.go index f4b0e6eea8d..5846e1e27da 100644 --- a/cmd/antrea-agent/agent.go +++ b/cmd/antrea-agent/agent.go @@ -16,7 +16,6 @@ package main import ( "antrea.io/antrea/pkg/agent/bgp" - "antrea.io/antrea/pkg/agent/bgp/gobgp" "context" "fmt" "net" @@ -753,8 +752,7 @@ func run(o *Options) error { k8sClient, o.config.BGPPolicy.SecretName, nodeConfig, - networkConfig, - gobgp.NewBGPServer) + networkConfig) if err != nil { return err } diff --git a/pkg/agent/bgp/controller.go b/pkg/agent/bgp/controller.go index e934a8c0c41..bc8aa584dbf 100644 --- a/pkg/agent/bgp/controller.go +++ b/pkg/agent/bgp/controller.go @@ -15,6 +15,7 @@ package bgp import ( + "antrea.io/antrea/pkg/agent/bgp/gobgp" "context" "fmt" "reflect" @@ -62,7 +63,9 @@ const ( ) const ( - defaultBGPListenPort uint32 = 179 + defaultBGPListenPort int32 = 179 + + defaultBGPASN int32 = 64512 ) const ( @@ -72,6 +75,8 @@ const ( var ( protocolIPv4 = netutils.IPv4 protocolIPv6 = netutils.IPv6 + + newBGPServerFn = gobgp.NewBGPServer ) type nodeToBGPPolicyBinding struct { @@ -83,9 +88,9 @@ type bgpPolicyState struct { // The local BGP process created for the BGPPolicy bgpServer types.Interface // The port on which local the BGP process listens. - listenPort uint32 + listenPort int32 // The AS number used by the local BGP process. - localASN uint32 + localASN int32 // The router ID used byt the local BGP process. routerID string // Routes to be advertised to external BGP peers. @@ -137,8 +142,6 @@ type Controller struct { egressEnabled bool - bgpServerFn func(uint32, string, int32) (types.Interface, error) - queue workqueue.RateLimitingInterface } @@ -150,8 +153,7 @@ func NewBGPPolicyController(nodeInformer coreinformers.NodeInformer, k8sClient kubernetes.Interface, bgpPeerPasswordsSecret string, nodeConfig *config.NodeConfig, - networkConfig *config.NetworkConfig, - bgpServerFn func(uint32, string, int32) (types.Interface, error)) (*Controller, error) { + networkConfig *config.NetworkConfig) (*Controller, error) { c := &Controller{ nodeInformer: nodeInformer.Informer(), nodeLister: nodeInformer.Lister(), @@ -179,7 +181,6 @@ func NewBGPPolicyController(nodeInformer coreinformers.NodeInformer, podIPv6CIDR: nodeConfig.PodIPv6CIDR.String(), nodeIPv4Addr: nodeConfig.NodeIPv4Addr.IP.String(), egressEnabled: features.DefaultFeatureGate.Enabled(features.Egress), - bgpServerFn: bgpServerFn, queue: workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(minRetryDelay, maxRetryDelay), "bgpPolicyGroup"), } c.bgpPolicyInformer.AddEventHandlerWithResyncPeriod( @@ -334,45 +335,84 @@ func (c *Controller) syncBGPPolicy(bpName string) error { if err != nil && !apierrors.IsNotFound(err) { return err } - if apierrors.IsNotFound(err) || bp != nil && !c.matchedCurrentNode(bp) { - // If the BGPPolicy is deleted and the corresponding state doesn't exist, just return. + // If the BGPPolicy is deleted or not applied to the current Node, do some cleanup. + if bp == nil || !c.matchedCurrentNode(bp) { bpState, exists := c.getBGPPolicyState(bpName) - if !exists { - return nil + // If this is the effective BGPPolicy for the current Node, the BGPPolicy state should exist, and do some cleanup. + if exists { + // Stop the BGP server process on the current Node. + if err := bpState.bgpServer.Stop(); err != nil { + return err + } + // Delete the BGPPolicy state. + c.deleteBGPPolicyState(bpName) } - // If a BGPPolicy is deleted but the corresponding state exists, do some cleanup for the deleted BGPPolicy. - if err = c.uninstallBGPPolicy(bpName, bpState); err != nil { - return err + + // Unbind the BGPPolicy from the current Node. + // If the BGPPolicy is the effective one for the current Node, try to pop a new effective BGPPolicy from the + // alternatives list to the queue. If the BGPPolicy is an alternative, remove it from the alternatives list. + if newEffectiveBP := c.unbindNodeFromBGPPolicy(bpName); newEffectiveBP != "" { + c.queue.Add(newEffectiveBP) } - // Delete the state of the deleted BGPPolicy. - c.deleteBGPPolicyState(bpName) return nil } + // Bind the BGPPolicy to the current Node. If the BGPPolicy is not as the effective one, just return. + if asEffective := c.bindNodeToBGPPolicy(bpName); !asEffective { + klog.InfoS("The Node has already got an effective BGPPolicy. This is as an alternative", "BGPPolicy", bpName) + return nil + } + + // Retrieve the listen port, local AS number, and router ID from the current BGPPolicy to start the BGP server. + routerID, err := c.getRouterID() + if err != nil { + return err + } + listenPort := defaultBGPListenPort + if bp.Spec.ListenPort != nil { + listenPort = *bp.Spec.ListenPort + } + localASN := defaultBGPASN + if bp.Spec.LocalASN != nil { + localASN = *bp.Spec.LocalASN + } + + var needUpdateBGPServer bool // Get the BGPPolicy state. bpState, exists := c.getBGPPolicyState(bpName) - // If the BGPPolicy exists and corresponding state doesn't exist, create state for the BGPPolicy. if !exists { - if bpState, err = c.newBGPPolicyState(bp); err != nil { - return err - } - } - - if !c.bindNodeToBGPPolicy(bpName) { - klog.InfoS("The Node has already got an effective BGPPolicy") - return nil + // If the BGPPolicy state doesn't exist, meaning the BGPPolicy has not been enforced on the current Node, create + // state for the BGPPolicy, and then start the BGP server. + bpState = c.newBGPPolicyState(bp) + needUpdateBGPServer = true + } else { + // Check the listen port, local AS number and routerID. If any of them have changed, then start a new BGP server + // and stop the stale one. + needUpdateBGPServer = bpState.listenPort != listenPort || + bpState.localASN != localASN || + bpState.routerID != routerID } - // Start BGP process if it didn't start. - if bpState.bgpServer == nil { - bgpServer, err := c.bgpServerFn(uint32(bp.Spec.LocalASN), bpState.routerID, *bp.Spec.ListenPort) + if needUpdateBGPServer { + // Start the new BGP server. + bgpServer, err := newBGPServerFn(localASN, routerID, listenPort) if err != nil { return err } + // Stop the stale BGP server if it exists. + if bpState.bgpServer != nil { + if err := bpState.bgpServer.Stop(); err != nil { + klog.ErrorS(err, "Failed to stop stale BGP Server", "BGPPolicy", bpName) + } + } + // Update the BGPPolicy state. bpState.bgpServer = bgpServer + bpState.listenPort = listenPort + bpState.localASN = localASN + bpState.routerID = routerID } - // TODO: ASN, local port, router ID cannot be changed. + // Reconcile BGP peers. peers, peerConfigs, err := c.getPeers(bp.Spec.BGPPeers) if err != nil { return err @@ -381,16 +421,16 @@ func (c *Controller) syncBGPPolicy(bpName string) error { return err } - // Update advertisements. + // Reconcile advertisements. routes, err := c.getRoutes(bp.Spec.Advertisements) if err != nil { return err } - if err := c.reconcileRoutes(routes, bpState); err != nil { return err } + // Update the BGPPolicy state. bpState.routes = routes bpState.peers = peers bpState.peerConfigs = peerConfigs @@ -398,6 +438,23 @@ func (c *Controller) syncBGPPolicy(bpName string) error { return nil } +func bgpServerIdentifyChanged(prevBPStatus *bgpPolicyState, curBP *v1alpha1.BGPPolicy, routerID string) bool { + listenPort := defaultBGPListenPort + if curBP.Spec.ListenPort != nil { + listenPort = *curBP.Spec.ListenPort + } + localASN := defaultBGPASN + if curBP.Spec.LocalASN != nil { + listenPort = *curBP.Spec.LocalASN + } + if prevBPStatus.listenPort != listenPort || + prevBPStatus.localASN != localASN || + prevBPStatus.routerID != routerID { + return false + } + return true +} + func (c *Controller) getBGPRouterID() (string, error) { return "", nil } @@ -486,42 +543,40 @@ func (c *Controller) deleteBGPPolicyState(bpName string) { delete(c.bgpPolicyStates, bpName) } -func (c *Controller) newBGPPolicyState(bp *v1alpha1.BGPPolicy) (*bgpPolicyState, error) { - c.bgpPolicyStatesMutex.Lock() - defer c.bgpPolicyStatesMutex.Unlock() - - listenPort := defaultBGPListenPort - if bp.Spec.ListenPort != nil { - listenPort = uint32(*bp.Spec.ListenPort) - } - routes := make(map[netutils.IPFamily]sets.Set[string]) - peers := make(map[netutils.IPFamily]sets.Set[string]) - for _, ipProtocol := range c.ipProtocols { - routes[ipProtocol] = sets.New[string]() - peers[ipProtocol] = sets.New[string]() - } - +func (c *Controller) getRouterID() (string, error) { var routerID string if !c.enabledIPv4 && c.enabledIPv6 { nodeObj, _ := c.nodeLister.Get(c.nodeName) var exists bool if routerID, exists = nodeObj.GetAnnotations()[bgpRouteIDAnnotation]; !exists { - return nil, fmt.Errorf("BGP routerID should be assigned by annotation manually when IPv6 is only enabled") + return "", fmt.Errorf("BGP routerID should be assigned by annotation manually when IPv6 is only enabled") + } + if !netutils.IsIPv4String(routerID) { + return "", fmt.Errorf("BGP routerID should be an IPv4 address") } } else { routerID = c.nodeIPv4Addr } + return routerID, nil +} +func (c *Controller) newBGPPolicyState(bp *v1alpha1.BGPPolicy) *bgpPolicyState { + c.bgpPolicyStatesMutex.Lock() + defer c.bgpPolicyStatesMutex.Unlock() + routes := make(map[netutils.IPFamily]sets.Set[string]) + peers := make(map[netutils.IPFamily]sets.Set[string]) + peerConfigs := make(map[string]*types.BGPPeerConfig) + for _, ipProtocol := range c.ipProtocols { + routes[ipProtocol] = sets.New[string]() + peers[ipProtocol] = sets.New[string]() + } state := &bgpPolicyState{ - localASN: uint32(bp.Spec.LocalASN), - listenPort: listenPort, routes: routes, peers: peers, - peerConfigs: make(map[string]*types.BGPPeerConfig), - routerID: routerID, + peerConfigs: peerConfigs, } c.bgpPolicyStates[bp.Name] = state - return state, nil + return state } func (c *Controller) getRoutes(advertisements v1alpha1.Advertisements) (map[netutils.IPFamily]sets.Set[string], error) { @@ -548,7 +603,7 @@ func (c *Controller) getRoutes(advertisements v1alpha1.Advertisements) (map[netu } func (c *Controller) addServiceRoutes(advertisement *v1alpha1.ServiceAdvertisement, allRoutes map[netutils.IPFamily]sets.Set[string]) error { - if !advertisement.ClusterIPs && !advertisement.ExternalIPs && !advertisement.LoadBalancerIPs { + if advertisement.ClusterIPs == nil && advertisement.ExternalIPs == nil && advertisement.LoadBalancerIPs == nil { return nil } services, err := c.serviceLister.List(labels.Everything()) @@ -570,21 +625,21 @@ func (c *Controller) addServiceRoutes(advertisement *v1alpha1.ServiceAdvertiseme } } - if advertisement.ClusterIPs { + if advertisement.ClusterIPs != nil && *advertisement.ClusterIPs { if internalLocal && hasLocalEndpoints || !internalLocal { for _, clusterIP := range svc.Spec.ClusterIPs { ips = append(ips, clusterIP) } } } - if advertisement.ExternalIPs { + if advertisement.ExternalIPs != nil && *advertisement.ExternalIPs { if externalLocal && hasLocalEndpoints || !externalLocal { for _, externalIP := range svc.Spec.ExternalIPs { ips = append(ips, externalIP) } } } - if advertisement.LoadBalancerIPs && svc.Spec.Type == corev1.ServiceTypeLoadBalancer { + if advertisement.LoadBalancerIPs != nil && *advertisement.LoadBalancerIPs && svc.Spec.Type == corev1.ServiceTypeLoadBalancer { if externalLocal && hasLocalEndpoints || !externalLocal { for _, ingressIP := range svc.Status.LoadBalancer.Ingress { if ingressIP.IP != "" { @@ -663,16 +718,16 @@ func (c *Controller) hasLocalEndpoints(svc *corev1.Service) (bool, error) { func (c *Controller) generateBGPPeerConfig(peer *v1alpha1.BGPPeer) (*types.BGPPeerConfig, error) { bgpPeerConfig := &types.BGPPeerConfig{ Address: peer.Address, - ASN: uint32(peer.ASN), + ASN: peer.ASN, } if peer.Port != nil { - bgpPeerConfig.Port = uint32(*peer.Port) + bgpPeerConfig.Port = *peer.Port } else { bgpPeerConfig.Port = defaultBGPListenPort } if peer.GracefulRestartTime != nil { - bgpPeerConfig.GracefulRestartTime = uint32(*peer.GracefulRestartTime) + bgpPeerConfig.GracefulRestartTime = *peer.GracefulRestartTime } else { bgpPeerConfig.GracefulRestartTime = 120 } @@ -719,18 +774,6 @@ func generateBGPPeerKey(address string, asn int32) string { return fmt.Sprintf("%s-%d", address, asn) } -func (c *Controller) uninstallBGPPolicy(bpName string, bpState *bgpPolicyState) error { - if bpState.bgpServer != nil { - if err := bpState.bgpServer.Stop(); err != nil { - return err - } - } - if newEffectiveBP := c.unbindNodeFromBGPPolicy(bpName); newEffectiveBP != "" { - c.queue.Add(newEffectiveBP) - } - return nil -} - func (c *Controller) addBGPPolicy(obj interface{}) { bp := obj.(*v1alpha1.BGPPolicy) if !c.matchedCurrentNode(bp) { @@ -806,9 +849,9 @@ func (c *Controller) filterAffectedBPsByService(svc *corev1.Service) sets.Set[st if svcAdvertisement == nil { continue } - if svcAdvertisement.ClusterIPs && len(svc.Spec.ClusterIPs) != 0 || - svcAdvertisement.ExternalIPs && len(svc.Spec.ExternalIPs) != 0 || - svcAdvertisement.LoadBalancerIPs && len(getIngressIPs(svc)) != 0 { + if svcAdvertisement.ClusterIPs != nil && *svcAdvertisement.ClusterIPs && len(svc.Spec.ClusterIPs) != 0 || + svcAdvertisement.ExternalIPs != nil && *svcAdvertisement.ExternalIPs && len(svc.Spec.ExternalIPs) != 0 || + svcAdvertisement.LoadBalancerIPs != nil && *svcAdvertisement.LoadBalancerIPs && len(getIngressIPs(svc)) != 0 { affectedBPs.Insert(bp.GetName()) } } @@ -926,6 +969,9 @@ func (c *Controller) deleteEgress(obj interface{}) { func (c *Controller) updateNode(oldObj, obj interface{}) { oldNode := oldObj.(*corev1.Node) node := obj.(*corev1.Node) + if node.GetName() != c.nodeName { + return + } if reflect.DeepEqual(node.GetLabels(), oldNode.GetLabels()) && reflect.DeepEqual(node.GetAnnotations(), oldNode.GetAnnotations()) { return } @@ -964,7 +1010,6 @@ func (c *Controller) unbindNodeFromBGPPolicy(bpName string) string { binding := c.bgpPolicyBinding if binding.effectiveBP == bpName { - // TODO: stop the bgpService var popped bool // Select a new effective BGPPolicy. binding.effectiveBP, popped = binding.alternativeBPs.PopAny() diff --git a/pkg/agent/bgp/controller_test.go b/pkg/agent/bgp/controller_test.go index 22b855186bb..a932de76dd4 100644 --- a/pkg/agent/bgp/controller_test.go +++ b/pkg/agent/bgp/controller_test.go @@ -15,9 +15,9 @@ package bgp import ( - "antrea.io/antrea/pkg/util/ip" "context" "fmt" + "k8s.io/utils/ptr" "testing" "time" @@ -43,6 +43,7 @@ import ( crdv1b1 "antrea.io/antrea/pkg/apis/crd/v1beta1" fakeversioned "antrea.io/antrea/pkg/client/clientset/versioned/fake" crdinformers "antrea.io/antrea/pkg/client/informers/externalversions" + "antrea.io/antrea/pkg/util/ip" ) const testAntreaBGPPasswordSecret = "antrea-bgp-passwords" @@ -52,6 +53,13 @@ var ( podIPv6CIDR = ip.MustParseCIDR("fec0:10:10::/64") nodeIPv4Addr = ip.MustParseCIDR("192.168.77.100/24") + testNodeConfig = &config.NodeConfig{ + PodIPv4CIDR: podIPv4CIDR, + PodIPv6CIDR: podIPv6CIDR, + NodeIPv4Addr: nodeIPv4Addr, + Name: localNodeName, + } + peerASN = 65535 ipv4PeerAddr = "192.168.77.254" ipv6PeerAddr = "fec0::196:168:77:254" @@ -70,14 +78,14 @@ var ( ipv4PeerConfig = &types.BGPPeerConfig{ Address: ipv4PeerAddr, Port: 179, - ASN: uint32(peerASN), + ASN: int32(peerASN), GracefulRestartTime: 120, AuthPassword: "", } ipv6PeerConfig = &types.BGPPeerConfig{ Address: ipv6PeerAddr, Port: 179, - ASN: uint32(peerASN), + ASN: int32(peerASN), GracefulRestartTime: 120, AuthPassword: "", } @@ -234,10 +242,6 @@ func newFakeController(t *testing.T, objects []runtime.Object, crdObjects []runt endpointSliceInformer := informerFactory.Discovery().V1().EndpointSlices() bgpPolicyInformer := crdInformerFactory.Crd().V1alpha1().BGPPolicies() - newMockBGPServerFn := func(uint32, string, int32) (types.Interface, error) { - return mockBGPServer, nil - } - bgpController, _ := NewBGPPolicyController(nodeInformer, serviceInformer, egressInformer, @@ -245,17 +249,11 @@ func newFakeController(t *testing.T, objects []runtime.Object, crdObjects []runt endpointSliceInformer, client, testAntreaBGPPasswordSecret, - &config.NodeConfig{ - PodIPv4CIDR: podIPv4CIDR, - PodIPv6CIDR: podIPv6CIDR, - NodeIPv4Addr: nodeIPv4Addr, - Name: localNodeName, - }, + testNodeConfig, &config.NetworkConfig{ IPv4Enabled: ipv4Enabled, IPv6Enabled: ipv6Enabled, - }, - newMockBGPServerFn) + }) bgpController.egressEnabled = true if existingEffectiveBP != "" { bgpController.bgpPolicyBinding.effectiveBP = existingEffectiveBP @@ -479,16 +477,8 @@ func TestBGPPolicyAdd(t *testing.T) { false, false, []v1alpha1.BGPPeer{ipv4Peer}), - existingEffectiveBP: bgpPolicyName2, - objects: []runtime.Object{ipv4ClusterIP1, ipv4ClusterIP1Eps1, node}, - expectedBGPPolicyState: generateBGPPolicyState(true, - false, - 179, - 65000, - nodeIPv4Addr.IP.String(), - nil, - nil, - ), + existingEffectiveBP: bgpPolicyName2, + objects: []runtime.Object{ipv4ClusterIP1, ipv4ClusterIP1Eps1, node}, expectedEffectiveBP: bgpPolicyName2, expectedAlternativeBPs: sets.New[string](bgpPolicyName1), }, @@ -498,6 +488,7 @@ func TestBGPPolicyAdd(t *testing.T) { t.Run(tt.name, func(t *testing.T) { crdObjects := append(tt.crdObjects, tt.bp) c := newFakeController(t, tt.objects, crdObjects, tt.ipv4Enabled, tt.ipv6Enabled, tt.existingEffectiveBP) + defer mockNewBGPServer(c.mockBGPServer)() stopCh := make(chan struct{}) defer close(stopCh) @@ -518,6 +509,16 @@ func TestBGPPolicyAdd(t *testing.T) { } } +func mockNewBGPServer(mockBGPServer types.Interface) func() { + originBGPServerFn := newBGPServerFn + newBGPServerFn = func(_ int32, _ string, _ int32) (types.Interface, error) { + return mockBGPServer, nil + } + return func() { + newBGPServerFn = originBGPServerFn + } +} + func TestBGPPolicyUpdate(t *testing.T) { bp1 := generateBGPPolicy(bgpPolicyName1, nodeLabels, @@ -585,10 +586,7 @@ func TestBGPPolicyUpdate(t *testing.T) { expectedEffectiveBP: "", expectedAlternativeBPs: sets.New[string](), expectedCalls: func(mockBGPServer *bgptest.MockInterfaceMockRecorder) { - mockBGPServer.AdvertiseRoutes(gomock.Any(), gomock.InAnyOrder([]string{ipStrToCIDStr(externalIPv4), ipStrToCIDStr(ipv4EgressIP1)}), protocolIPv4).Times(1) - mockBGPServer.AdvertiseRoutes(gomock.Any(), gomock.InAnyOrder([]string{ipStrToCIDStr(externalIPv6), ipStrToCIDStr(ipv6EgressIP1)}), protocolIPv6).Times(1) - mockBGPServer.WithdrawRoutes(gomock.Any(), gomock.InAnyOrder([]string{ipStrToCIDStr(clusterIPv4), ipStrToCIDStr(loadBalancerIPv4), podIPv4CIDR.String()}), protocolIPv4).Times(1) - mockBGPServer.WithdrawRoutes(gomock.Any(), gomock.InAnyOrder([]string{ipStrToCIDStr(clusterIPv6), ipStrToCIDStr(loadBalancerIPv6), podIPv6CIDR.String()}), protocolIPv6).Times(1) + mockBGPServer.Stop().Times(1) }, }, { @@ -639,6 +637,7 @@ func TestBGPPolicyUpdate(t *testing.T) { t.Run(tt.name, func(t *testing.T) { c := newFakeController(t, objects, crdObjects, true, true, bp1.Name) c.bgpPolicyStates[bp1.Name] = bp1Status + c.bgpPolicyStates[bp1.Name].bgpServer = c.mockBGPServer c.bgpPolicyBinding.effectiveBP = bp1.Name c.bgpPolicyBinding.alternativeBPs = sets.New[string]() @@ -663,8 +662,7 @@ func TestBGPPolicyUpdate(t *testing.T) { assert.NoError(t, c.syncBGPPolicy(tt.updatedBP.Name)) assert.Equal(t, tt.expectedEffectiveBP, c.bgpPolicyBinding.effectiveBP) assert.Equal(t, tt.expectedAlternativeBPs, c.bgpPolicyBinding.alternativeBPs) - _, exists := c.bgpPolicyStates[tt.updatedBP.GetName()] - assert.Equal(t, false, exists) + checkBGPPolicyState(t, tt.expectedBGPPolicyState, c.bgpPolicyStates[tt.updatedBP.GetName()]) }) } @@ -697,8 +695,8 @@ func TestServiceUpdate(t *testing.T) { func generateBGPPolicyState(ipv4Enabled bool, ipv6Enabled bool, - listenPort uint32, - localASN uint32, + listenPort int32, + localASN int32, routerID string, routes []string, peerConfigs []*types.BGPPeerConfig) *bgpPolicyState { @@ -744,12 +742,15 @@ func generateBGPPolicyState(ipv4Enabled bool, } func checkBGPPolicyState(t *testing.T, expected, got *bgpPolicyState) { - assert.Equal(t, expected.listenPort, got.listenPort) - assert.Equal(t, expected.localASN, got.localASN) - assert.Equal(t, expected.routerID, got.routerID) - assert.Equal(t, expected.routes, got.routes) - assert.Equal(t, expected.peers, got.peers) - assert.Equal(t, expected.peerConfigs, got.peerConfigs) + require.Equal(t, expected != nil, got != nil) + if expected != nil { + assert.Equal(t, expected.listenPort, got.listenPort) + assert.Equal(t, expected.localASN, got.localASN) + assert.Equal(t, expected.routerID, got.routerID) + assert.Equal(t, expected.routes, got.routes) + assert.Equal(t, expected.peers, got.peers) + assert.Equal(t, expected.peerConfigs, got.peerConfigs) + } } func generateBGPPolicy(name string, @@ -766,13 +767,13 @@ func generateBGPPolicy(name string, if advertiseClusterIP || advertiseExternalIP || advertiseLoadBalancerIP { advertisement.Service = &v1alpha1.ServiceAdvertisement{} if advertiseClusterIP { - advertisement.Service.ClusterIPs = true + advertisement.Service.ClusterIPs = ptr.To(true) } if advertiseExternalIP { - advertisement.Service.ExternalIPs = true + advertisement.Service.ExternalIPs = ptr.To(true) } if advertiseLoadBalancerIP { - advertisement.Service.LoadBalancerIPs = true + advertisement.Service.LoadBalancerIPs = ptr.To(true) } } if advertiseEgressIP { @@ -786,7 +787,7 @@ func generateBGPPolicy(name string, ObjectMeta: metav1.ObjectMeta{Name: name, UID: "test-uid"}, Spec: v1alpha1.BGPPolicySpec{ NodeSelector: &metav1.LabelSelector{MatchLabels: nodeSelector}, - LocalASN: localASN, + LocalASN: &localASN, ListenPort: &listenPort, Advertisements: advertisement, BGPPeers: externalPeers, diff --git a/pkg/agent/bgp/gobgp/server.go b/pkg/agent/bgp/gobgp/server.go index dd4afbb5bf0..6c65358d3e6 100644 --- a/pkg/agent/bgp/gobgp/server.go +++ b/pkg/agent/bgp/gobgp/server.go @@ -31,12 +31,13 @@ type GoBGPServer struct { server *gobgp.BgpServer } -func NewBGPServer(asn uint32, routerID string, listenPort int32) (types.Interface, error) { +// TODO: add watcher!!!!! +func NewBGPServer(asn int32, routerID string, listenPort int32) (types.Interface, error) { server := gobgp.NewBgpServer() go server.Serve() global := &gobgpapi.Global{ - Asn: asn, + Asn: uint32(asn), RouterId: routerID, ListenPort: listenPort, } @@ -72,11 +73,11 @@ func (g *GoBGPServer) AddPeers(ctx context.Context, peerConfigs []*types.BGPPeer peer := &gobgpapi.Peer{ Conf: &gobgpapi.PeerConf{ NeighborAddress: peerConfig.Address, - PeerAsn: peerConfig.ASN, + PeerAsn: uint32(peerConfig.ASN), AuthPassword: peerConfig.AuthPassword, }, Transport: &gobgpapi.Transport{ - RemotePort: peerConfig.Port, + RemotePort: uint32(peerConfig.Port), }, AfiSafis: []*gobgpapi.AfiSafi{ { @@ -100,7 +101,7 @@ func (g *GoBGPServer) AddPeers(ctx context.Context, peerConfigs []*types.BGPPeer if peerConfig.GracefulRestartTime != 0 { peer.GracefulRestart = &gobgpapi.GracefulRestart{ Enabled: true, - RestartTime: peerConfig.GracefulRestartTime, + RestartTime: uint32(peerConfig.GracefulRestartTime), } } diff --git a/pkg/agent/bgp/types/types.go b/pkg/agent/bgp/types/types.go index e803f174d4c..f39d645d7d0 100644 --- a/pkg/agent/bgp/types/types.go +++ b/pkg/agent/bgp/types/types.go @@ -16,10 +16,10 @@ package types type BGPPeerConfig struct { Address string - Port uint32 - ASN uint32 + Port int32 + ASN int32 AuthPassword string - GracefulRestartTime uint32 + GracefulRestartTime int32 } // ResetDirection defines the direction in which a BGP soft reset should be performed