bgpv1: pass router state to gobgp

pkg/bgpv1/gobgp/server.go

@@ -13,8 +13,8 @@ import (
 	"github.com/sirupsen/logrus"
 	apb "google.golang.org/protobuf/types/known/anypb"
 
+	"github.com/cilium/cilium/pkg/bgpv1/agent"
 	"github.com/cilium/cilium/pkg/bgpv1/types"
-	v2alpha1api "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2alpha1"
 )
 
 const (
@@ -53,7 +53,7 @@ type GoBGPServer struct {
 }
 
 // NewGoBGPServerWithConfig returns instance of go bgp router wrapper.
-func NewGoBGPServerWithConfig(ctx context.Context, log *logrus.Entry, params types.ServerParameters) (types.Router, error) {
+func NewGoBGPServerWithConfig(ctx context.Context, log *logrus.Entry, params types.ServerParameters, _ *agent.ControlPlaneState) (types.Router, error) {
 	logger := NewServerLogger(log.Logger, LogParams{
 		AS:        params.Global.ASN,
 		Component: "gobgp.BgpServerInstance",
@@ -104,7 +104,7 @@ func NewGoBGPServerWithConfig(ctx context.Context, log *logrus.Entry, params typ
 // AddNeighbor will add the CiliumBGPNeighbor to the gobgp.BgpServer, creating
 // a BGP peering connection.
 func (g *GoBGPServer) AddNeighbor(ctx context.Context, n types.NeighborRequest) error {
-	peer, _, err := g.getPeerConfig(ctx, n.Neighbor, false)
+	peer, _, err := g.getPeerConfig(ctx, n, false)
 	if err != nil {
 		return err
 	}
@@ -119,7 +119,7 @@ func (g *GoBGPServer) AddNeighbor(ctx context.Context, n types.NeighborRequest)
 
 // UpdateNeighbor will update the existing CiliumBGPNeighbor in the gobgp.BgpServer.
 func (g *GoBGPServer) UpdateNeighbor(ctx context.Context, n types.NeighborRequest) error {
-	peer, needsHardReset, err := g.getPeerConfig(ctx, n.Neighbor, true)
+	peer, needsHardReset, err := g.getPeerConfig(ctx, n, true)
 	if err != nil {
 		return err
 	}
@@ -153,23 +153,23 @@ func (g *GoBGPServer) UpdateNeighbor(ctx context.Context, n types.NeighborReques
 }
 
 // getPeerConfig returns GoBGP Peer configuration for the provided CiliumBGPNeighbor.
-func (g *GoBGPServer) getPeerConfig(ctx context.Context, n *v2alpha1api.CiliumBGPNeighbor, isUpdate bool) (peer *gobgp.Peer, needsReset bool, err error) {
+func (g *GoBGPServer) getPeerConfig(ctx context.Context, n types.NeighborRequest, isUpdate bool) (peer *gobgp.Peer, needsReset bool, err error) {
 	// cilium neighbor uses prefix string, gobgp neighbor uses IP string, convert.
-	prefix, err := netip.ParsePrefix(n.PeerAddress)
+	prefix, err := netip.ParsePrefix(n.Neighbor.PeerAddress)
 	if err != nil {
 		// unlikely, we validate this on CR write to k8s api.
 		return peer, needsReset, fmt.Errorf("failed to parse PeerAddress: %w", err)
 	}
 	peerAddr := prefix.Addr()
-	peerPort := uint32(*n.PeerPort)
+	peerPort := uint32(*n.Neighbor.PeerPort)
 
 	var existingPeer *gobgp.Peer
 	if isUpdate {
 		// If this is an update, try retrieving the existing Peer.
 		// This is necessary as many Peer fields are defaulted internally in GoBGP,
 		// and if they were not set, the update would always cause BGP peer reset.
 		// This will not fail if the peer is not found for whatever reason.
-		existingPeer, err = g.getExistingPeer(ctx, peerAddr, uint32(n.PeerASN))
+		existingPeer, err = g.getExistingPeer(ctx, peerAddr, uint32(n.Neighbor.PeerASN))
 		if err != nil {
 			return peer, needsReset, fmt.Errorf("failed retrieving peer: %w", err)
 		}
@@ -188,7 +188,7 @@ func (g *GoBGPServer) getPeerConfig(ctx context.Context, n *v2alpha1api.CiliumBG
 		peer = &gobgp.Peer{
 			Conf: &gobgp.PeerConf{
 				NeighborAddress: peerAddr.String(),
-				PeerAsn:         uint32(n.PeerASN),
+				PeerAsn:         uint32(n.Neighbor.PeerASN),
 			},
 			Transport: &gobgp.Transport{
 				RemotePort: peerPort,
@@ -220,32 +220,33 @@ func (g *GoBGPServer) getPeerConfig(ctx context.Context, n *v2alpha1api.CiliumBG
 	}
 
 	// Enable multi-hop for eBGP if non-zero TTL is provided
-	if g.asn != uint32(n.PeerASN) && *n.EBGPMultihopTTL > 1 {
+	if g.asn != uint32(n.Neighbor.PeerASN) && *n.Neighbor.EBGPMultihopTTL > 1 {
 		peer.EbgpMultihop = &gobgp.EbgpMultihop{
 			Enabled:     true,
-			MultihopTtl: uint32(*n.EBGPMultihopTTL),
+			MultihopTtl: uint32(*n.Neighbor.EBGPMultihopTTL),
 		}
 	}
 
 	if peer.Timers == nil {
 		peer.Timers = &gobgp.Timers{}
 	}
 	peer.Timers.Config = &gobgp.TimersConfig{
-		ConnectRetry:           uint64(*n.ConnectRetryTimeSeconds),
-		HoldTime:               uint64(*n.HoldTimeSeconds),
-		KeepaliveInterval:      uint64(*n.KeepAliveTimeSeconds),
+		ConnectRetry:           uint64(*n.Neighbor.ConnectRetryTimeSeconds),
+		HoldTime:               uint64(*n.Neighbor.HoldTimeSeconds),
+		KeepaliveInterval:      uint64(*n.Neighbor.KeepAliveTimeSeconds),
 		IdleHoldTimeAfterReset: idleHoldTimeAfterResetSeconds,
 	}
 
 	// populate graceful restart config
 	if peer.GracefulRestart == nil {
 		peer.GracefulRestart = &gobgp.GracefulRestart{}
 	}
-	if n.GracefulRestart != nil && n.GracefulRestart.Enabled {
+	if n.Neighbor.GracefulRestart != nil && n.Neighbor.GracefulRestart.Enabled {
 		peer.GracefulRestart.Enabled = true
-		peer.GracefulRestart.RestartTime = uint32(*n.GracefulRestart.RestartTimeSeconds)
+		peer.GracefulRestart.RestartTime = uint32(*n.Neighbor.GracefulRestart.RestartTimeSeconds)
 		peer.GracefulRestart.NotificationEnabled = true
 	}
+
 	for _, afiConf := range peer.AfiSafis {
 		if afiConf.MpGracefulRestart == nil {
 			afiConf.MpGracefulRestart = &gobgp.MpGracefulRestart{}

pkg/bgpv1/gobgp/state_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/cilium/cilium/api/v1/models"
+	"github.com/cilium/cilium/pkg/bgpv1/agent"
 	"github.com/cilium/cilium/pkg/bgpv1/types"
 	v2alpha1api "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2alpha1"
 	"github.com/cilium/cilium/pkg/logging"
@@ -255,18 +256,29 @@ func TestGetPeerState(t *testing.T) {
 			},
 		}
 		t.Run(tt.name, func(t *testing.T) {
-			testSC, err := NewGoBGPServerWithConfig(context.Background(), log, srvParams)
+			testSC, err := NewGoBGPServerWithConfig(context.Background(), log, srvParams, &agent.ControlPlaneState{})
 			require.NoError(t, err)
 
 			t.Cleanup(func() {
 				testSC.Stop()
 			})
+			// create current vRouter config and add neighbors
+			router := &v2alpha1api.CiliumBGPVirtualRouter{
+				LocalASN:  int64(tt.localASN),
+				Neighbors: []v2alpha1api.CiliumBGPNeighbor{},
+			}
 
 			// add neighbours
 			for _, n := range tt.neighbors {
 				n.SetDefaults()
+
+				router.Neighbors = append(router.Neighbors, v2alpha1api.CiliumBGPNeighbor{
+					PeerAddress: n.PeerAddress,
+					PeerASN:     n.PeerASN,
+				})
 				err = testSC.AddNeighbor(context.Background(), types.NeighborRequest{
 					Neighbor: n,
+					VR:       router,
 				})
 				if tt.errStr != "" {
 					require.EqualError(t, err, tt.errStr)
@@ -290,6 +302,7 @@ func TestGetPeerState(t *testing.T) {
 				n.SetDefaults()
 				err = testSC.UpdateNeighbor(context.Background(), types.NeighborRequest{
 					Neighbor: n,
+					VR:       router,
 				})
 				if tt.updateErrStr != "" {
 					require.EqualError(t, err, tt.updateErrStr)

pkg/bgpv1/manager/instance.go

@@ -6,6 +6,7 @@ package manager
 import (
 	"context"
 
+	"github.com/cilium/cilium/pkg/bgpv1/agent"
 	"github.com/cilium/cilium/pkg/bgpv1/gobgp"
 	"github.com/cilium/cilium/pkg/bgpv1/types"
 	v2alpha1api "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2alpha1"
@@ -45,8 +46,8 @@ type ServerWithConfig struct {
 //
 // Canceling the provided context will kill the BgpServer along with calling the
 // underlying BgpServer's Stop() method.
-func NewServerWithConfig(ctx context.Context, params types.ServerParameters) (*ServerWithConfig, error) {
-	s, err := gobgp.NewGoBGPServerWithConfig(ctx, log, params)
+func NewServerWithConfig(ctx context.Context, params types.ServerParameters, cstate *agent.ControlPlaneState) (*ServerWithConfig, error) {
+	s, err := gobgp.NewGoBGPServerWithConfig(ctx, log, params, cstate)
 	if err != nil {
 		return nil, err
 	}

pkg/bgpv1/manager/manager.go

@@ -226,7 +226,7 @@ func (m *BGPRouterManager) registerBGPServer(ctx context.Context, c *v2alpha1api
 		},
 	}
 
-	if s, err = NewServerWithConfig(ctx, globalConfig); err != nil {
+	if s, err = NewServerWithConfig(ctx, globalConfig, cstate); err != nil {
 		return fmt.Errorf("failed to start BGP server for config with local ASN %v: %w", c.LocalASN, err)
 	}
 

pkg/bgpv1/manager/reconcile.go

@@ -155,7 +155,7 @@ func preflightReconciler(
 	sc.Server.Stop()
 
 	// create a new one via ServerWithConfig constructor
-	s, err := NewServerWithConfig(ctx, globalConfig)
+	s, err := NewServerWithConfig(ctx, globalConfig, cstate)
 	if err != nil {
 		l.WithError(err).Errorf("Failed to start BGP server for virtual router with local ASN %v", newc.LocalASN)
 		return fmt.Errorf("failed to start BGP server for virtual router with local ASN %v: %w", newc.LocalASN, err)
@@ -300,23 +300,23 @@ func neighborReconciler(
 	// create new neighbors
 	for _, n := range toCreate {
 		l.Infof("Adding peer %v %v to local ASN %v", n.PeerAddress, n.PeerASN, newc.LocalASN)
-		if err := sc.Server.AddNeighbor(ctx, types.NeighborRequest{Neighbor: n}); err != nil {
+		if err := sc.Server.AddNeighbor(ctx, types.NeighborRequest{Neighbor: n, VR: newc}); err != nil {
 			return fmt.Errorf("failed while reconciling neighbor %v %v: %w", n.PeerAddress, n.PeerASN, err)
 		}
 	}
 
 	// update neighbors
 	for _, n := range toUpdate {
 		l.Infof("Updating peer %v %v in local ASN %v", n.PeerAddress, n.PeerASN, newc.LocalASN)
-		if err := sc.Server.UpdateNeighbor(ctx, types.NeighborRequest{Neighbor: n}); err != nil {
+		if err := sc.Server.UpdateNeighbor(ctx, types.NeighborRequest{Neighbor: n, VR: newc}); err != nil {
 			return fmt.Errorf("failed while reconciling neighbor %v %v: %w", n.PeerAddress, n.PeerASN, err)
 		}
 	}
 
 	// remove neighbors
 	for _, n := range toRemove {
 		l.Infof("Removing peer %v %v from local ASN %v", n.PeerAddress, n.PeerASN, newc.LocalASN)
-		if err := sc.Server.RemoveNeighbor(ctx, types.NeighborRequest{Neighbor: n}); err != nil {
+		if err := sc.Server.RemoveNeighbor(ctx, types.NeighborRequest{Neighbor: n, VR: newc}); err != nil {
 			return fmt.Errorf("failed while reconciling neighbor %v %v: %w", n.PeerAddress, n.PeerASN, err)
 		}
 	}

pkg/bgpv1/manager/reconcile_test.go

@@ -105,7 +105,7 @@ func TestPreflightReconciler(t *testing.T) {
 					ListenPort: tt.localPort,
 				},
 			}
-			testSC, err := NewServerWithConfig(context.Background(), srvParams)
+			testSC, err := NewServerWithConfig(context.Background(), srvParams, &agent.ControlPlaneState{})
 			if err != nil {
 				t.Fatalf("failed to create test BgpServer: %v", err)
 			}
@@ -313,7 +313,7 @@ func TestNeighborReconciler(t *testing.T) {
 					ListenPort: -1,
 				},
 			}
-			testSC, err := NewServerWithConfig(context.Background(), srvParams)
+			testSC, err := NewServerWithConfig(context.Background(), srvParams, &agent.ControlPlaneState{})
 			if err != nil {
 				t.Fatalf("failed to create test BgpServer: %v", err)
 			}
@@ -330,6 +330,7 @@ func TestNeighborReconciler(t *testing.T) {
 				oldc.Neighbors = append(oldc.Neighbors, n)
 				testSC.Server.AddNeighbor(context.Background(), types.NeighborRequest{
 					Neighbor: &n,
+					VR:       oldc,
 				})
 			}
 			testSC.Config = oldc
@@ -468,7 +469,7 @@ func TestExportPodCIDRReconciler(t *testing.T) {
 				ExportPodCIDR: pointer.Bool(tt.enabled),
 				Neighbors:     []v2alpha1api.CiliumBGPNeighbor{},
 			}
-			testSC, err := NewServerWithConfig(context.Background(), srvParams)
+			testSC, err := NewServerWithConfig(context.Background(), srvParams, &agent.ControlPlaneState{})
 			if err != nil {
 				t.Fatalf("failed to create test bgp server: %v", err)
 			}
@@ -1055,7 +1056,7 @@ func TestLBServiceReconciler(t *testing.T) {
 				Neighbors:       []v2alpha1api.CiliumBGPNeighbor{},
 				ServiceSelector: tt.oldServiceSelector,
 			}
-			testSC, err := NewServerWithConfig(context.Background(), srvParams)
+			testSC, err := NewServerWithConfig(context.Background(), srvParams, &agent.ControlPlaneState{})
 			if err != nil {
 				t.Fatalf("failed to create test bgp server: %v", err)
 			}
@@ -1198,7 +1199,7 @@ func TestReconcileAfterServerReinit(t *testing.T) {
 		},
 	}
 
-	testSC, err := NewServerWithConfig(context.Background(), srvParams)
+	testSC, err := NewServerWithConfig(context.Background(), srvParams, &agent.ControlPlaneState{})
 	require.NoError(t, err)
 
 	originalServer := testSC.Server

pkg/bgpv1/types/bgp.go

@@ -40,6 +40,7 @@ type Advertisement struct {
 // NeighborRequest contains neighbor parameters used when enabling or disabling peer
 type NeighborRequest struct {
 	Neighbor *v2alpha1api.CiliumBGPNeighbor
+	VR       *v2alpha1api.CiliumBGPVirtualRouter
 }
 
 // PathRequest contains parameters for advertising or withdrawing routes