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.

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

Add testing for reconcile function. #202

Merged
merged 3 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,5 @@ require (
golang.org/x/text v0.13.0 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/klog/v2 v2.90.1 // indirect
k8s.io/klog/v2 v2.100.1 // indirect
)
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -605,8 +605,8 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh
honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
k8s.io/klog/v2 v2.90.1 h1:m4bYOKall2MmOiRaR1J+We67Do7vm9KiQVlT96lnHUw=
k8s.io/klog/v2 v2.90.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0=
k8s.io/klog/v2 v2.100.1 h1:7WCHKK6K8fNhTqfBhISHQ97KrnJNFZMcQvKp7gP/tmg=
k8s.io/klog/v2 v2.100.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0=
lukechampine.com/uint128 v1.2.0 h1:mBi/5l91vocEN8otkC5bDLhi2KdCticRiwbdB0O+rjI=
lukechampine.com/uint128 v1.2.0/go.mod h1:c4eWIwlEGaxC/+H1VguhU4PHXNWDCDMUlWdIWl2j1gk=
rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
Expand Down
150 changes: 78 additions & 72 deletions rib/reconciler/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"context"
"fmt"
"reflect"
"sync/atomic"

"github.com/openconfig/gribigo/aft"
"github.com/openconfig/gribigo/rib"
Expand All @@ -38,13 +39,6 @@ type R struct {
// intended and target are the mechanisms by which to access the intended
// RIB (source of truth) and the target it is to be reconciled with.
intended, target RIBTarget

// intended is a RIB containing the AFT entries that are intended to be
// programmed by the reconciler.
lastIntended *rib.RIB
// lastTarget is a cache of the last RIB entries that were returned from
// the target RIB.
lastTarget *rib.RIB
}

// RIBTarget is an interface that abstracts a local and remote RIB in the
Expand All @@ -65,6 +59,11 @@ type LocalRIB struct {
r *rib.RIB
}

// NewLocalRIB returns a new LocalRIB instance.
func NewLocalRIB(r *rib.RIB) *LocalRIB {
return &LocalRIB{r: r}
}

// Get returns the contents of the local RIB.
func (l *LocalRIB) Get(_ context.Context) (*rib.RIB, error) {
return l.r, nil
Expand All @@ -86,39 +85,47 @@ func New(intended, target RIBTarget) *R {
}
}

// Reconcile performs a reconciliation operation between the intended and specified
// remote RIB.
func (r *R) Reconcile(ctx context.Context) error {
// Reconcile calculates the required gRIBI actions to institute a reconciliation
// operation between the intended and remote RIB. The specified ID is used as the
// base for the operation ID within gRIBI. Reconcile returns a set of operations
// in the form of a ReconcileOps struct.
//
// Within the returned ReconcileOps:
// - The Add field indicates entries that are to be newly added to the remote RIB.
// - The Replace field indicates entries that are replacing entries within the remote RIB,
// the replaces will be implicit (i.e., expressed as gRIBI ADD operations) unless the
// ExplicitReplace option is provided.
// - The Delete field indicates entries that are to be deleted from the remote RIB.
//
// Within each of these fields, entries are broken down into "top-level" entries which are from
// the IPv4, IPv6, or MPLS AFTs, and those that correspond to next-hop-group (NHG) or next-hop (NH)
// entries. This allows the client receiving these entries to enqueue them in the correct order
// to ensure that there are no forward references, and implement make-before-break.
func (r *R) Reconcile(ctx context.Context, id *atomic.Uint64) (*ReconcileOps, error) {
// Get the current contents of intended and target.
iRIB, err := r.intended.Get(ctx)
if err != nil {
return fmt.Errorf("cannot reconcile RIBs, cannot get contents of intended, %v", err)
return nil, fmt.Errorf("cannot reconcile RIBs, cannot get contents of intended, %v", err)
}

tRIB, err := r.target.Get(ctx)
if err != nil {
return fmt.Errorf("cannot reconcile RIBs, cannot get contents of target, %v", err)
return nil, fmt.Errorf("cannot reconcile RIBs, cannot get contents of target, %v", err)
}

// Perform diff on their contents.
// TODO(robjs): Plumb through explicitReplace map.
diffs, err := diff(iRIB, tRIB, nil)
diffs, err := diff(iRIB, tRIB, nil, id)
if err != nil {
return fmt.Errorf("cannot reconcile RIBs, cannot calculate diff, %v", err)
return nil, fmt.Errorf("cannot reconcile RIBs, cannot calculate diff, %v", err)
}
_ = diffs
_, _ = r.lastIntended, r.lastTarget

// Enqueue the operations towards target that bring it in-line with intended.
// TODO(robjs): Implement enqueuing in client.
return fmt.Errorf("reconciliation unimplemented")

return diffs, nil
}

// ops stores a set of operations with their corresponding types. Operations
// Ops stores a set of operations with their corresponding types. Operations
// are stored as NH (nexthop), NHG (next-hop-group) and top-level (MPLS, IPv4,
// IPv6). This allows a gRIBI client to sequence the ops suitably.
type ops struct {
// IPv6). This allows a gRIBI client to sequence the Ops suitably.
type Ops struct {
// NH stores the next-hop operations in the operation set.
NH []*spb.AFTOperation
// NHG stores the next-hop-group operations in the operation set.
Expand All @@ -127,24 +134,24 @@ type ops struct {
TopLevel []*spb.AFTOperation
}

// reconcile ops stores the operations that are required for a specific reconciliation
// ReconcileOps stores the operations that are required for a specific reconciliation
// run.
type reconcileOps struct {
type ReconcileOps struct {
// Add stores the operations that are explicitly adding new entries.
Add *ops
Add *Ops
// Replace stores the operations that are implicit or explicit replaces of
// existing entries.
Replace *ops
Replace *Ops
// Delete stores the operations that are removing entries.
Delete *ops
Delete *Ops
}

// newReconcileOps returns a new reconcileOps struct with the fields initialised.
func newReconcileOps() *reconcileOps {
return &reconcileOps{
Add: &ops{},
Replace: &ops{},
Delete: &ops{},
// NewReconcileOps returns a new reconcileOps struct with the fields initialised.
func NewReconcileOps() *ReconcileOps {
return &ReconcileOps{
Add: &Ops{},
Replace: &Ops{},
Delete: &Ops{},
}
}

Expand All @@ -161,7 +168,7 @@ func newReconcileOps() *reconcileOps {
//
// If an entry within the explicitReplace map is set to true then explicit, rather
// than implicit replaces are generated for that function.
func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOps, error) {
func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool, id *atomic.Uint64) (*ReconcileOps, error) {
robshakir marked this conversation as resolved.
Show resolved Hide resolved
if src == nil || dst == nil {
return nil, fmt.Errorf("invalid nil input RIBs, src: %v, dst: %v", src, dst)
}
Expand All @@ -185,9 +192,8 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp
return nil, fmt.Errorf("cannot copy destination RIB contents, err: %v", err)
}

ops := newReconcileOps()
ops := NewReconcileOps()

var id uint64
for srcNI, srcNIEntries := range srcContents {
dstNIEntries, ok := dstContents[srcNI]
if !ok {
Expand All @@ -205,8 +211,8 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp
if ok && explicitReplace[spb.AFTType_IPV4] {
opType = spb.AFTOperation_REPLACE
}
id++
op, err := v4Operation(opType, srcNI, pfx, id, srcE)
id.Add(1)
op, err := v4Operation(opType, srcNI, id, srcE)
if err != nil {
return nil, err
}
Expand All @@ -227,8 +233,8 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp
if ok && explicitReplace[spb.AFTType_MPLS] {
opType = spb.AFTOperation_REPLACE
}
id++
op, err := mplsOperation(opType, srcNI, lbl, id, srcE)
id.Add(1)
op, err := mplsOperation(opType, srcNI, id, srcE)
if err != nil {
return nil, err
}
Expand All @@ -249,8 +255,8 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp
if ok && explicitReplace[spb.AFTType_NEXTHOP_GROUP] {
opType = spb.AFTOperation_REPLACE
}
id++
op, err := nhgOperation(opType, srcNI, nhgID, id, srcE)
id.Add(1)
op, err := nhgOperation(opType, srcNI, id, srcE)
if err != nil {
return nil, err
}
Expand All @@ -271,8 +277,8 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp
if ok && explicitReplace[spb.AFTType_NEXTHOP] {
opType = spb.AFTOperation_REPLACE
}
id++
op, err := nhOperation(opType, srcNI, nhID, id, srcE)
id.Add(1)
op, err := nhOperation(opType, srcNI, id, srcE)
if err != nil {
return nil, err
}
Expand All @@ -290,8 +296,8 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp
// Delete operations.
for pfx, dstE := range dstNIEntries.GetAfts().Ipv4Entry {
if _, ok := srcNIEntries.GetAfts().Ipv4Entry[pfx]; !ok {
id++
op, err := v4Operation(spb.AFTOperation_DELETE, srcNI, pfx, id, dstE)
id.Add(1)
op, err := v4Operation(spb.AFTOperation_DELETE, srcNI, id, dstE)
if err != nil {
return nil, err
}
Expand All @@ -301,8 +307,8 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp

for lbl, dstE := range dstNIEntries.GetAfts().LabelEntry {
if _, ok := srcNIEntries.GetAfts().LabelEntry[lbl]; !ok {
id++
op, err := mplsOperation(spb.AFTOperation_DELETE, srcNI, lbl, id, dstE)
id.Add(1)
op, err := mplsOperation(spb.AFTOperation_DELETE, srcNI, id, dstE)
if err != nil {
return nil, err
}
Expand All @@ -312,8 +318,8 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp

for nhgID, dstE := range dstNIEntries.GetAfts().NextHopGroup {
if _, ok := srcNIEntries.GetAfts().NextHopGroup[nhgID]; !ok {
id++
op, err := nhgOperation(spb.AFTOperation_DELETE, srcNI, nhgID, id, dstE)
id.Add(1)
op, err := nhgOperation(spb.AFTOperation_DELETE, srcNI, id, dstE)
if err != nil {
return nil, err
}
Expand All @@ -323,8 +329,8 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp

for nhID, dstE := range dstNIEntries.GetAfts().NextHop {
if _, ok := srcNIEntries.GetAfts().NextHop[nhID]; !ok {
id++
op, err := nhOperation(spb.AFTOperation_DELETE, srcNI, nhID, id, dstE)
id.Add(1)
op, err := nhOperation(spb.AFTOperation_DELETE, srcNI, id, dstE)
if err != nil {
return nil, err
}
Expand All @@ -336,16 +342,16 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp
return ops, nil
}

// v4Operation builds a gRIBI IPv4 operation with the specified method corresponding to the
// prefix pfx in network instance ni, using the specified ID for the operation. The contents
// v4Operation builds a gRIBI IPv4 operation with the specified method corresponding to a
// prefix in the network instance ni, using the specified ID for the operation. The contents
// of the operation are the entry e.
func v4Operation(method spb.AFTOperation_Operation, ni, pfx string, id uint64, e *aft.Afts_Ipv4Entry) (*spb.AFTOperation, error) {
func v4Operation(method spb.AFTOperation_Operation, ni string, id *atomic.Uint64, e *aft.Afts_Ipv4Entry) (*spb.AFTOperation, error) {
p, err := rib.ConcreteIPv4Proto(e)
if err != nil {
return nil, fmt.Errorf("cannot create operation for prefix %s, %v", pfx, err)
return nil, fmt.Errorf("cannot create operation for prefix %s, %v", e.GetPrefix(), err)
}
return &spb.AFTOperation{
Id: id,
Id: id.Load(),
NetworkInstance: ni,
Op: method,
Entry: &spb.AFTOperation_Ipv4{
Expand All @@ -354,16 +360,16 @@ func v4Operation(method spb.AFTOperation_Operation, ni, pfx string, id uint64, e
}, nil
}

// nhgOperation builds a gRIBI NHG operation with the specified method, corresponding to the
// NHG ID nhgID, in network instance ni, using the specified ID for the operation. The
// nhgOperation builds a gRIBI NHG operation with the specified method, corresponding to a
// NHG in network instance ni, using the specified ID for the operation. The
// contents of the operation are the entry e.
func nhgOperation(method spb.AFTOperation_Operation, ni string, nhgID, id uint64, e *aft.Afts_NextHopGroup) (*spb.AFTOperation, error) {
func nhgOperation(method spb.AFTOperation_Operation, ni string, id *atomic.Uint64, e *aft.Afts_NextHopGroup) (*spb.AFTOperation, error) {
p, err := rib.ConcreteNextHopGroupProto(e)
if err != nil {
return nil, fmt.Errorf("cannot create operation for NHG %d, %v", nhgID, err)
return nil, fmt.Errorf("cannot create operation for NHG %d, %v", e.GetId(), err)
}
return &spb.AFTOperation{
Id: id,
Id: id.Load(),
NetworkInstance: ni,
Op: method,
Entry: &spb.AFTOperation_NextHopGroup{
Expand All @@ -372,16 +378,16 @@ func nhgOperation(method spb.AFTOperation_Operation, ni string, nhgID, id uint64
}, nil
}

// nhOperation builds a gRIBI NH operation with the specified method, corresponding to the
// NH ID nhID, in network instance ni, using the specified ID for the operation. The contents
// nhOperation builds a gRIBI NH operation with the specified method, corresponding to a
// NH in network instance ni, using the specified ID for the operation. The contents
// of the operation are the entry e.
func nhOperation(method spb.AFTOperation_Operation, ni string, nhID, id uint64, e *aft.Afts_NextHop) (*spb.AFTOperation, error) {
func nhOperation(method spb.AFTOperation_Operation, ni string, id *atomic.Uint64, e *aft.Afts_NextHop) (*spb.AFTOperation, error) {
p, err := rib.ConcreteNextHopProto(e)
if err != nil {
return nil, fmt.Errorf("cannot create operation for NH %d, %v", nhID, err)
return nil, fmt.Errorf("cannot create operation for NH %d, %v", e.GetIndex(), err)
}
return &spb.AFTOperation{
Id: id,
Id: id.Load(),
NetworkInstance: ni,
Op: method,
Entry: &spb.AFTOperation_NextHop{
Expand All @@ -393,13 +399,13 @@ func nhOperation(method spb.AFTOperation_Operation, ni string, nhID, id uint64,
// mplsOperation builds a gRIBI LabelEntry operation with the specified method corresponding to
// the MPLS label entry lbl. The operation is targeted at network instance ni, and uses the specified
// ID. The contents of the operation are the entry e.
func mplsOperation(method spb.AFTOperation_Operation, ni string, lbl aft.Afts_LabelEntry_Label_Union, id uint64, e *aft.Afts_LabelEntry) (*spb.AFTOperation, error) {
func mplsOperation(method spb.AFTOperation_Operation, ni string, id *atomic.Uint64, e *aft.Afts_LabelEntry) (*spb.AFTOperation, error) {
p, err := rib.ConcreteMPLSProto(e)
if err != nil {
return nil, fmt.Errorf("cannot create operation for label %d, %v", lbl, err)
return nil, fmt.Errorf("cannot create operation for label %d, %v", e.GetLabel(), err)
}
return &spb.AFTOperation{
Id: id,
Id: id.Load(),
NetworkInstance: ni,
Op: method,
Entry: &spb.AFTOperation_Mpls{
Expand Down
Loading
Loading