Skip to content

Commit

Permalink
BGPv1: Introduce generic bgp manager layer
Browse files Browse the repository at this point in the history 
Introduce new manager layer, which decouples itself from gobgp
structures. Also, using UUID in identifying paths instead of gobgp path pointers.

This change is mostly refactoring code and not introducing any
functionality.

Signed-off-by: harsimran pabla <hpabla@isovalent.com>
  • Loading branch information
@harsimran-pabla
harsimran-pabla committed May 2, 2023
1 parent db3e015 commit c7f4cbc
Show file tree
Hide file tree
Showing 16 changed files with 588 additions and 322 deletions.
76 changes: 56 additions & 20 deletions pkg/bgpv1/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,25 +253,53 @@ sequenceDiagram
```
Above is a high level sequence diagram describing the control flow of the `Agent-Side BGP Control Plane`

##### Architecture Diagram
```mermaid
flowchart
subgraph agent
agent:node_spec("node event")
agent:policy_spec("policy event")
agent:signal("control loop")
end
agent:node_spec---agent:signal
agent:policy_spec---agent:signal
subgraph manager
manager:router_manager("router manager")
manager:reconcilers("reconcilers")
manager:servers("server configs")
end
agent:signal---manager:router_manager
manager:router_manager-->manager:servers
manager:router_manager-->manager:reconcilers
subgraph types
types:bgp("cilium bgp")
types:router("bgp interface")
end
subgraph gobgp
gobgp:gobgp("gobgp client")
end
manager:reconcilers-->gobgp:gobgp
```

*Note*: We summarize the Kubernetes events which trigger the `Controller` to just a `CiliumBGPPeeringPolicy` event, however the `Controller` will wake on other events which influence changes in the `Agent-Side BGP Control Plane`. See the source code for full details.

#### Controller
The `Agent-Side Control Plane` implements a controller located in `pkg/bgpv1/agent/controller.go`.

The controller listens for `CiliumBGPPeeringPolicy`, determines if a policy applies to its current host and if it does, captures some information about Cilium's current state then calls down to the implemented `BGPRouterManager`.
The controller listens for `CiliumBGPPeeringPolicy`, determines if a policy applies to its current host and if it does, captures some information about Cilium's current state then calls down to the implemented `Manager`.

#### BGPRouterManager
The `BGPRouterManager` is an interface used to define a declarative API between the `Controller` and instantiated BGP routers.
#### Manager
The `Manager` is an interface used to define a declarative API between the `Controller` and instantiated BGP routers.

The interface defines a single declarative method whose argument is the desired `CiliumBGPPeeringPolicy` (among a few others).

The `BGPRouterManager` is in charge of pushing the `BGP Control Plane` to the desired `CiliumBGPPeeringPolicy` or returning an error if it is not possible.

##### GoBGP Implementation
The `Manager` is in charge of pushing the `BGP Control Plane` to the desired `CiliumBGPPeeringPolicy` or returning an error if it is not possible.

The first implementation of `BGPRouterManager` utilizes the `gobgp` package.

You can find this implementation in `pkg/bgpv1/gobgp`.
You can find this implementation in `pkg/bgpv1/manager`.

This implementation will
* evaluate the desired `CiliumBGPPeeringPolicy`
Expand All @@ -280,33 +308,41 @@ This implementation will
* enable/disable any BGP server specific features
* inform the caller if the policy cannot be applied

The GoBGP implementation is capable of evaluating each `CiliumBGPVirtualRouter` in isolation.
The `Manager` implementation is capable of evaluating each `CiliumBGPVirtualRouter` in isolation.

This means when applying a `CiliumBGPPeeringPolicy` the GoBGP `BGPRouterManager` will attempt to create each `CiliumBGPVirtualRouter`.
This means when applying a `CiliumBGPPeeringPolicy` the `Manager` will attempt to create each `CiliumBGPVirtualRouter`.

If a particular `CiliumBGPVirtualRouter` fails to instantiate the error is logged and the `BGPRouterManager` will continue to the next `CiliumBGPVirtualRouter`, utilizing the aforementioned logic.
If a particular `CiliumBGPVirtualRouter` fails to instantiate the error is logged and the `Manager` will continue to the next `CiliumBGPVirtualRouter`, utilizing the aforementioned logic.

###### GoBGP BGPRouterManager Architecture
###### Manager Architecture

It's worth expanding on how the `gobgp` implementation of the `BGPRouterManager` works internally.
It's worth expanding on how the implementation of the `Manager` works internally.

This `BGPRouterManager` views each `CiliumBGPVirtualRouter` as a BGP router instance.
This `Manager` views each `CiliumBGPVirtualRouter` as a BGP router instance.

Each `CiliumBGPVirtualRouter` defines a local ASN, a router ID and a list of `CiliumBGPNeighbors` to peer with.

This is enough for the `BGPRouterManager` to create a `BgpServer` instance, which is the nomenclature defining a BGP speaker in `gobgp`-package-parlance.
This is enough for the `Manager` to create a `BgpServer` instance, which is the nomenclature defining a BGP speaker in `gobgp`-package-parlance.

This `BGPRouterManager` groups `BgpServer` instances by their local ASNs.
This `Manager` groups `BgpServer` instances by their local ASNs.

This leads to the following rule:
* A `CiliumBGPPeeringPolicy` applying to node `A` must not have two or more `CiliumBGPVirtualRouters` with the same `localASN` fields.

The `gobgp` `BGPRouterManager` employs a set of `ConfigReconcilerFunc`(s) which perform the order-dependent reconciliation actions for each `BgpServer` it must reconcile.
The `Manager` employs a set of `ConfigReconcilerFunc`(s) which perform the order-dependent reconciliation actions for each `BgpServer` it must reconcile.

A `ConfigReconcilerFunc` is simply a function with a typed signature.

```go
type ConfigReconcilerFunc func(ctx context.Context, m *BGPRouterManager, sc *ServerWithConfig, newc *v2alpha1api.CiliumBGPVirtualRouter, cstate *agent.ControlPlaneState) error
type ConfigReconcilerFunc func(ctx context.Context, params ReconcileParams) error
```

See the source code at `pkg/bgpv1/gobgp/reconcile.go` for a more in depth explanation of how each `ConfigReconcilerFunc` is called.
See the source code at `pkg/bgpv1/manager/reconcile.go` for a more in depth explanation of how each `ConfigReconcilerFunc` is called.

#### Router
Underlying router implementation exposes imperative API for BGP related configuration, such as add/remove neighbor, add/remove routes etc. Currently, only gobgp is supported as
underlying routing implementation.

This shim layer provides translation between cilium specific BGP types and gobgp types.

See the source code at `pkg/bgpv1/gobgp` for more details.
8 changes: 4 additions & 4 deletions pkg/bgpv1/cell.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ package bgpv1

import (
"github.com/cilium/cilium/pkg/bgpv1/agent"
"github.com/cilium/cilium/pkg/bgpv1/gobgp"
"github.com/cilium/cilium/pkg/bgpv1/manager"
"github.com/cilium/cilium/pkg/hive"
"github.com/cilium/cilium/pkg/hive/cell"
v2alpha1api "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2alpha1"
Expand All @@ -31,12 +31,12 @@ var Cell = cell.Module(
newBGPPeeringPolicyResource,
// goBGP is currently the only supported RouterManager, if more are
// implemented, provide the manager via a Cell that pics implementation based on configuration.
gobgp.NewBGPRouterManager,
manager.NewBGPRouterManager,
// Create a slim service DiffStore
gobgp.NewDiffStore[*slim_core_v1.Service],
manager.NewDiffStore[*slim_core_v1.Service],
),
// Provides the reconcilers used by the route manager to update the config
gobgp.ConfigReconcilers,
manager.ConfigReconcilers,
)

func newBGPPeeringPolicyResource(lc hive.Lifecycle, c client.Clientset, dc *option.DaemonConfig) resource.Resource[*v2alpha1api.CiliumBGPPeeringPolicy] {
Expand Down
130 changes: 62 additions & 68 deletions pkg/bgpv1/gobgp/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import (

gobgp "github.com/osrg/gobgp/v3/api"
"github.com/osrg/gobgp/v3/pkg/server"
"github.com/sirupsen/logrus"
apb "google.golang.org/protobuf/types/known/anypb"

v2alpha1api "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2alpha1"
"github.com/cilium/cilium/pkg/k8s/resource"
"github.com/cilium/cilium/pkg/bgpv1/types"
)

var (
Expand All @@ -31,55 +31,39 @@ var (
}
)

// Advertisement is a container object which associates a net.IPNet with a
// gobgp.Path.
//
// The `Net` field makes comparing this Advertisement with another IPNet encoded
// prefixes simple.
//
// The `Path` field is a gobgp.Path object which can be forwarded to our server's
// WithdrawPath method, making withdrawing an advertised route simple.
type Advertisement struct {
Net *net.IPNet
Path *gobgp.Path
}
// GoBGPServer is wrapper on top of go bgp server implementation
type GoBGPServer struct {
logger *logrus.Entry

// asn is local AS number
asn uint32

// ServerWithConfig is a container for grouping a gobgp BgpServer with the
// Cilium's BGP control plane related configuration.
//
// It exports a method set for manipulating the BgpServer. However, this
// struct is a dumb object. The calling code is required to keep the BgpServer's
// configuration and associated configuration fields in sync.
type ServerWithConfig struct {
// a gobgp backed BgpServer configured in accordance to the accompanying
// CiliumBGPVirtualRouter configuration.
Server *server.BgpServer
// The CiliumBGPVirtualRouter configuration which drives the configuration
// of the above BgpServer.
//
// If this field is nil it means the above BgpServer has had no
// configuration applied to it.
Config *v2alpha1api.CiliumBGPVirtualRouter
// Holds any announced PodCIDR routes.
PodCIDRAnnouncements []Advertisement
// Holds any announced Service routes.
ServiceAnnouncements map[resource.Key][]Advertisement
server *server.BgpServer
}

// NewServerWithConfig will start an underlying BgpServer utilizing startReq
// for its initial configuration.
//
// The returned ServerWithConfig has a nil CiliumBGPVirtualRouter config, and is
// ready to be provided to ReconcileBGPConfig.
//
// Canceling the provided context will kill the BgpServer along with calling the
// underlying BgpServer's Stop() method.
func NewServerWithConfig(ctx context.Context, startReq *gobgp.StartBgpRequest) (*ServerWithConfig, error) {
logger := NewServerLogger(log.Logger, startReq.Global.Asn)
// NewGoBGPServerWithConfig returns instance of go bgp router wrapper.
func NewGoBGPServerWithConfig(ctx context.Context, log *logrus.Entry, params types.ServerParameters) (types.Router, error) {
logger := NewServerLogger(log.Logger, params.Global.ASN)

s := server.NewBgpServer(server.LoggerOption(logger))
go s.Serve()

startReq := &gobgp.StartBgpRequest{
Global: &gobgp.Global{
Asn: params.Global.ASN,
RouterId: params.Global.RouterID,
ListenPort: params.Global.ListenPort,
},
}

if params.Global.RouteSelectionOptions != nil {
startReq.Global.RouteSelectionOptions = &gobgp.RouteSelectionOptionsConfig{
AdvertiseInactiveRoutes: params.Global.RouteSelectionOptions.AdvertiseInactiveRoutes,
}
}

if err := s.StartBgp(ctx, startReq); err != nil {
return nil, fmt.Errorf("failed starting BGP server: %w", err)
}
Expand All @@ -97,29 +81,28 @@ func NewServerWithConfig(ctx context.Context, startReq *gobgp.StartBgpRequest) (
return nil, fmt.Errorf("failed to configure logging for virtual router with local-asn %v: %w", startReq.Global.Asn, err)
}

return &ServerWithConfig{
Server: s,
Config: nil,
PodCIDRAnnouncements: []Advertisement{},
ServiceAnnouncements: make(map[resource.Key][]Advertisement),
return &GoBGPServer{
logger: log,
asn: params.Global.ASN,
server: s,
}, nil
}

// AddNeighbor will add the CiliumBGPNeighbor to the gobgp.BgpServer, creating
// a BGP peering connection.
func (sc *ServerWithConfig) AddNeighbor(ctx context.Context, n *v2alpha1api.CiliumBGPNeighbor) error {
func (g *GoBGPServer) AddNeighbor(ctx context.Context, n types.NeighborRequest) error {
// cilium neighbor uses CIDR string, gobgp neighbor uses IP string, convert.
var ip net.IP
var err error
if ip, _, err = net.ParseCIDR(n.PeerAddress); err != nil {
if ip, _, err = net.ParseCIDR(n.Neighbor.PeerAddress); err != nil {
// unlikely, we validate this on CR write to k8s api.
return fmt.Errorf("failed to parse PeerAddress: %w", err)
}
peerReq := &gobgp.AddPeerRequest{
Peer: &gobgp.Peer{
Conf: &gobgp.PeerConf{
NeighborAddress: ip.String(),
PeerAsn: uint32(n.PeerASN),
PeerAsn: uint32(n.Neighbor.PeerASN),
},
// tells the peer we are capable of unicast IPv4 and IPv6
// advertisements.
Expand All @@ -137,27 +120,27 @@ func (sc *ServerWithConfig) AddNeighbor(ctx context.Context, n *v2alpha1api.Cili
},
},
}
if err = sc.Server.AddPeer(ctx, peerReq); err != nil {
return fmt.Errorf("failed while adding peer %v %v: %w", n.PeerAddress, n.PeerASN, err)
if err = g.server.AddPeer(ctx, peerReq); err != nil {
return fmt.Errorf("failed while adding peer %v %v: %w", n.Neighbor.PeerAddress, n.Neighbor.PeerASN, err)
}
return nil
}

// RemoveNeighbor will remove the CiliumBGPNeighbor from the gobgp.BgpServer,
// disconnecting the BGP peering connection.
func (sc *ServerWithConfig) RemoveNeighbor(ctx context.Context, n *v2alpha1api.CiliumBGPNeighbor) error {
func (g *GoBGPServer) RemoveNeighbor(ctx context.Context, n types.NeighborRequest) error {
// cilium neighbor uses CIDR string, gobgp neighbor uses IP string, convert.
var ip net.IP
var err error
if ip, _, err = net.ParseCIDR(n.PeerAddress); err != nil {
if ip, _, err = net.ParseCIDR(n.Neighbor.PeerAddress); err != nil {
// unlikely, we validate this on CR write to k8s api.
return fmt.Errorf("failed to parse PeerAddress: %w", err)
}
peerReq := &gobgp.DeletePeerRequest{
Address: ip.String(),
}
if err := sc.Server.DeletePeer(ctx, peerReq); err != nil {
return fmt.Errorf("failed while reconciling neighbor %v %v: %w", n.PeerAddress, n.PeerASN, err)
if err := g.server.DeletePeer(ctx, peerReq); err != nil {
return fmt.Errorf("failed while reconciling neighbor %v %v: %w", n.Neighbor.PeerAddress, n.Neighbor.PeerASN, err)
}
return nil
}
Expand All @@ -176,9 +159,12 @@ func (sc *ServerWithConfig) RemoveNeighbor(ctx context.Context, n *v2alpha1api.C
//
// An Advertisement is returned which may be passed to WithdrawPath to remove
// this Advertisement.
func (sc *ServerWithConfig) AdvertisePath(ctx context.Context, ip *net.IPNet) (Advertisement, error) {
func (g *GoBGPServer) AdvertisePath(ctx context.Context, p types.PathRequest) (types.PathResponse, error) {
var err error
var path *gobgp.Path
var resp *gobgp.AddPathResponse
ip := p.Advert.Net

origin, _ := apb.New(&gobgp.OriginAttribute{
Origin: 0,
})
Expand Down Expand Up @@ -212,7 +198,7 @@ func (sc *ServerWithConfig) AdvertisePath(ctx context.Context, ip *net.IPNet) (A
Nlri: nlri,
Pattrs: []*apb.Any{nextHop, origin},
}
_, err = sc.Server.AddPath(ctx, &gobgp.AddPathRequest{
resp, err = g.server.AddPath(ctx, &gobgp.AddPathRequest{
Path: path,
})
case ip.IP.To16() != nil:
Expand All @@ -232,27 +218,35 @@ func (sc *ServerWithConfig) AdvertisePath(ctx context.Context, ip *net.IPNet) (A
Nlri: nlri,
Pattrs: []*apb.Any{nlriAttrs, origin},
}
_, err = sc.Server.AddPath(ctx, &gobgp.AddPathRequest{
resp, err = g.server.AddPath(ctx, &gobgp.AddPathRequest{
Path: path,
})
default:
return Advertisement{}, fmt.Errorf("provided IP returned nil for both IPv4 and IPv6 lengths: %v", len(ip.IP))
return types.PathResponse{}, fmt.Errorf("provided IP returned nil for both IPv4 and IPv6 lengths: %v", len(ip.IP))
}
if err != nil {
return Advertisement{}, err
return types.PathResponse{}, err
}
return Advertisement{
ip,
path,
return types.PathResponse{
Advert: types.Advertisement{
Net: ip,
GoBGPPathUUID: resp.Uuid,
},
}, err
}

// WithdrawPath withdraws an Advertisement produced by AdvertisePath from this
// BgpServer.
func (sc *ServerWithConfig) WithdrawPath(ctx context.Context, advert Advertisement) error {
err := sc.Server.DeletePath(ctx, &gobgp.DeletePathRequest{
Family: advert.Path.Family,
Path: advert.Path,
func (g *GoBGPServer) WithdrawPath(ctx context.Context, p types.PathRequest) error {
err := g.server.DeletePath(ctx, &gobgp.DeletePathRequest{
Uuid: p.Advert.GoBGPPathUUID,
})
return err
}

// Stop closes gobgp server
func (g *GoBGPServer) Stop() {
if g.server != nil {
g.server.Stop()
}
}

0 comments on commit c7f4cbc

Please sign in to comment.