diff --git a/go.mod b/go.mod index b2a13121..df56d0d3 100644 --- a/go.mod +++ b/go.mod @@ -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 ) diff --git a/go.sum b/go.sum index e707d59a..813cd26f 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/rib/reconciler/reconcile.go b/rib/reconciler/reconcile.go index 63c7f995..cbcac6ee 100644 --- a/rib/reconciler/reconcile.go +++ b/rib/reconciler/reconcile.go @@ -27,6 +27,7 @@ import ( "context" "fmt" "reflect" + "sync/atomic" "github.com/openconfig/gribigo/aft" "github.com/openconfig/gribigo/rib" @@ -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 @@ -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 @@ -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. @@ -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{}, } } @@ -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) { if src == nil || dst == nil { return nil, fmt.Errorf("invalid nil input RIBs, src: %v, dst: %v", src, dst) } @@ -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 { @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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{ @@ -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{ @@ -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{ @@ -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{ diff --git a/rib/reconciler/reconcile_test.go b/rib/reconciler/reconcile_test.go index 32ce206a..8ccb8b93 100644 --- a/rib/reconciler/reconcile_test.go +++ b/rib/reconciler/reconcile_test.go @@ -2,7 +2,9 @@ package reconciler import ( "context" + "sync/atomic" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -44,7 +46,7 @@ func TestDiff(t *testing.T) { inSrc *rib.RIB inDst *rib.RIB inExplicitReplace map[spb.AFTType]bool - wantOps *reconcileOps + wantOps *ReconcileOps wantErr bool }{{ desc: "VRF NI in src, but not in dst", @@ -66,8 +68,8 @@ func TestDiff(t *testing.T) { return r.RIB() }(), inDst: rib.NewFake(dn).RIB(), - wantOps: &reconcileOps{ - Add: &ops{ + wantOps: &ReconcileOps{ + Add: &Ops{ NH: []*spb.AFTOperation{{ Id: 3, NetworkInstance: "FOO", @@ -144,8 +146,8 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: &reconcileOps{ - Add: &ops{ + wantOps: &ReconcileOps{ + Add: &Ops{ TopLevel: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -160,7 +162,7 @@ func TestDiff(t *testing.T) { }, }}, }, - Delete: &ops{ + Delete: &Ops{ TopLevel: []*spb.AFTOperation{{ Id: 2, NetworkInstance: dn, @@ -207,8 +209,8 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: &reconcileOps{ - Add: &ops{ + wantOps: &ReconcileOps{ + Add: &Ops{ NHG: []*spb.AFTOperation{{ Id: 2, NetworkInstance: dn, @@ -228,7 +230,7 @@ func TestDiff(t *testing.T) { }, }}, }, - Replace: &ops{ + Replace: &Ops{ TopLevel: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -278,8 +280,8 @@ func TestDiff(t *testing.T) { inExplicitReplace: map[spb.AFTType]bool{ spb.AFTType_IPV4: true, }, - wantOps: &reconcileOps{ - Add: &ops{ + wantOps: &ReconcileOps{ + Add: &Ops{ NHG: []*spb.AFTOperation{{ Id: 2, NetworkInstance: dn, @@ -299,7 +301,7 @@ func TestDiff(t *testing.T) { }, }}, }, - Replace: &ops{ + Replace: &Ops{ TopLevel: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -334,8 +336,8 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: &reconcileOps{ - Add: &ops{ + wantOps: &ReconcileOps{ + Add: &Ops{ NHG: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -375,8 +377,8 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: &reconcileOps{ - Delete: &ops{ + wantOps: &ReconcileOps{ + Delete: &Ops{ NHG: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -428,8 +430,8 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: &reconcileOps{ - Replace: &ops{ + wantOps: &ReconcileOps{ + Replace: &Ops{ NHG: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -489,8 +491,8 @@ func TestDiff(t *testing.T) { inExplicitReplace: map[spb.AFTType]bool{ spb.AFTType_NEXTHOP_GROUP: true, }, - wantOps: &reconcileOps{ - Replace: &ops{ + wantOps: &ReconcileOps{ + Replace: &Ops{ NHG: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -535,8 +537,8 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: &reconcileOps{ - Add: &ops{ + wantOps: &ReconcileOps{ + Add: &Ops{ NH: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -573,8 +575,8 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: &reconcileOps{ - Delete: &ops{ + wantOps: &ReconcileOps{ + Delete: &Ops{ NH: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -608,8 +610,8 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: &reconcileOps{ - Replace: &ops{ + wantOps: &ReconcileOps{ + Replace: &Ops{ NH: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -646,8 +648,8 @@ func TestDiff(t *testing.T) { inExplicitReplace: map[spb.AFTType]bool{ spb.AFTType_NEXTHOP: true, }, - wantOps: &reconcileOps{ - Replace: &ops{ + wantOps: &ReconcileOps{ + Replace: &Ops{ NH: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -693,8 +695,8 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: &reconcileOps{ - Add: &ops{ + wantOps: &ReconcileOps{ + Add: &Ops{ TopLevel: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -737,8 +739,8 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: &reconcileOps{ - Delete: &ops{ + wantOps: &ReconcileOps{ + Delete: &Ops{ TopLevel: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -790,8 +792,8 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: &reconcileOps{ - Replace: &ops{ + wantOps: &ReconcileOps{ + Replace: &Ops{ TopLevel: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -846,8 +848,8 @@ func TestDiff(t *testing.T) { inExplicitReplace: map[spb.AFTType]bool{ spb.AFTType_MPLS: true, }, - wantOps: &reconcileOps{ - Replace: &ops{ + wantOps: &ReconcileOps{ + Replace: &Ops{ TopLevel: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -909,8 +911,8 @@ func TestDiff(t *testing.T) { inExplicitReplace: map[spb.AFTType]bool{ spb.AFTType_ALL: true, }, - wantOps: &reconcileOps{ - Replace: &ops{ + wantOps: &ReconcileOps{ + Replace: &Ops{ TopLevel: []*spb.AFTOperation{{ Id: 1, NetworkInstance: dn, @@ -977,7 +979,8 @@ func TestDiff(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - got, err := diff(tt.inSrc, tt.inDst, tt.inExplicitReplace) + id := &atomic.Uint64{} + got, err := diff(tt.inSrc, tt.inDst, tt.inExplicitReplace, id) if (err != nil) != tt.wantErr { t.Fatalf("did not get expected error, got: %v, wantErr? %v", err, tt.wantErr) } @@ -989,13 +992,13 @@ func TestDiff(t *testing.T) { // Keep the test input as pithy as possible whilst ensuring safe adds // in the real implementation. if tt.wantOps.Add == nil { - tt.wantOps.Add = &ops{} + tt.wantOps.Add = &Ops{} } if tt.wantOps.Replace == nil { - tt.wantOps.Replace = &ops{} + tt.wantOps.Replace = &Ops{} } if tt.wantOps.Delete == nil { - tt.wantOps.Delete = &ops{} + tt.wantOps.Delete = &Ops{} } if diff := cmp.Diff(got, tt.wantOps, @@ -1008,3 +1011,325 @@ func TestDiff(t *testing.T) { }) } } + +func TestReconcile(t *testing.T) { + dn := "DEFAULT" + tests := []struct { + desc string + inIntended RIBTarget + inTarget RIBTarget + inID *atomic.Uint64 + wantOps *ReconcileOps + wantErr bool + }{{ + desc: "local-local: one entry in intended, not in target", + inIntended: func() RIBTarget { + r := rib.NewFake(dn, rib.DisableRIBCheckFn()) + if err := r.InjectIPv4(dn, "1.0.0.0/24", 1); err != nil { + t.Fatalf("cannot add prefix to intended, %v", err) + } + return NewLocalRIB(r.RIB()) + }(), + inTarget: func() RIBTarget { + r := rib.NewFake(dn, rib.DisableRIBCheckFn()) + return NewLocalRIB(r.RIB()) + }(), + inID: &atomic.Uint64{}, + wantOps: &ReconcileOps{ + Add: &Ops{ + TopLevel: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_Ipv4{ + Ipv4: &aftpb.Afts_Ipv4EntryKey{ + Prefix: "1.0.0.0/24", + Ipv4Entry: &aftpb.Afts_Ipv4Entry{ + NextHopGroup: &wpb.UintValue{Value: 1}, + }, + }, + }, + }}, + }, + Replace: &Ops{}, + Delete: &Ops{}, + }, + }, { + desc: "local-local: no differences between intended and target", + inIntended: func() RIBTarget { + r := rib.NewFake(dn, rib.DisableRIBCheckFn()) + if err := r.InjectNHG(dn, 100, map[uint64]uint64{ + 1: 10, + 2: 20, + 3: 30, + }); err != nil { + t.Fatalf("cannot add NHG, err: %v", err) + } + return NewLocalRIB(r.RIB()) + }(), + inTarget: func() RIBTarget { + r := rib.NewFake(dn, rib.DisableRIBCheckFn()) + if err := r.InjectNHG(dn, 100, map[uint64]uint64{ + 1: 10, + 2: 20, + 3: 30, + }); err != nil { + t.Fatalf("cannot add NHG, err: %v", err) + } + return NewLocalRIB(r.RIB()) + }(), + inID: &atomic.Uint64{}, + wantOps: &ReconcileOps{ + Add: &Ops{}, + Replace: &Ops{}, + Delete: &Ops{}, + }, + }, { + desc: "local-local: entry in target that is not in intended", + inIntended: NewLocalRIB(rib.NewFake(dn, rib.DisableRIBCheckFn()).RIB()), + inTarget: func() RIBTarget { + r := rib.NewFake(dn, rib.DisableRIBCheckFn()) + if err := r.InjectNH(dn, 1, "eth0"); err != nil { + t.Fatalf("cannot add NH, err: %v", err) + } + return NewLocalRIB(r.RIB()) + }(), + inID: &atomic.Uint64{}, + wantOps: &ReconcileOps{ + Add: &Ops{}, + Replace: &Ops{}, + Delete: &Ops{ + NH: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_DELETE, + Entry: &spb.AFTOperation_NextHop{ + NextHop: &aftpb.Afts_NextHopKey{ + Index: 1, + NextHop: &aftpb.Afts_NextHop{ + InterfaceRef: &aftpb.Afts_NextHop_InterfaceRef{ + Interface: &wpb.StringValue{Value: "eth0"}, + }, + }, + }, + }, + }}, + }, + }, + }, { + desc: "local-local: non-zero starting ID", + inIntended: func() RIBTarget { + r := rib.NewFake(dn, rib.DisableRIBCheckFn()) + if err := r.InjectIPv4(dn, "1.0.0.0/24", 1); err != nil { + t.Fatalf("cannot add prefix to intended, %v", err) + } + return NewLocalRIB(r.RIB()) + }(), + inTarget: func() RIBTarget { + r := rib.NewFake(dn, rib.DisableRIBCheckFn()) + return NewLocalRIB(r.RIB()) + }(), + inID: func() *atomic.Uint64 { + u := &atomic.Uint64{} + u.Store(42) + return u + }(), + wantOps: &ReconcileOps{ + Add: &Ops{ + TopLevel: []*spb.AFTOperation{{ + Id: 43, + NetworkInstance: dn, + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_Ipv4{ + Ipv4: &aftpb.Afts_Ipv4EntryKey{ + Prefix: "1.0.0.0/24", + Ipv4Entry: &aftpb.Afts_Ipv4Entry{ + NextHopGroup: &wpb.UintValue{Value: 1}, + }, + }, + }, + }}, + }, + Replace: &Ops{}, + Delete: &Ops{}, + }, + }} + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + rr := New(tt.inIntended, tt.inTarget) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + got, err := rr.Reconcile(ctx, tt.inID) + + if (err != nil) != tt.wantErr { + t.Fatalf("did not get expected error, got: %v, wantErr? %v", err, tt.wantErr) + } + + if diff := cmp.Diff(got, tt.wantOps, + protocmp.Transform(), + cmpopts.EquateEmpty(), + ); diff != "" { + t.Fatalf("did not get expected RIB, diff(-got,+want):\n%s", diff) + } + }) + } +} + +func TestReconcileRemote(t *testing.T) { + dn := "DEFAULT" + tests := []struct { + desc string + inIntendedServer func(*testing.T, *rib.RIB) (string, func()) + inTargetServer func(*testing.T, *rib.RIB) (string, func()) + inInjectedIntendedRIB *rib.RIB + inInjectedRemoteRIB *rib.RIB + inID *atomic.Uint64 + wantOps *ReconcileOps + wantErr bool + }{{ + desc: "no routes to add", + inIntendedServer: newServer, + inTargetServer: newServer, + inInjectedIntendedRIB: rib.New(dn, rib.DisableRIBCheckFn()), + inInjectedRemoteRIB: rib.New(dn, rib.DisableRIBCheckFn()), + inID: &atomic.Uint64{}, + wantOps: &ReconcileOps{ + Add: &Ops{}, + Delete: &Ops{}, + Replace: &Ops{}, + }, + }, { + desc: "route to add from intended to target", + inIntendedServer: newServer, + inTargetServer: newServer, + inInjectedIntendedRIB: func() *rib.RIB { + r := rib.NewFake(dn, rib.DisableRIBCheckFn()) + for i, a := range []string{"84.18.210.139/32", "142.250.72.174/32"} { + if err := r.InjectIPv4(dn, a, uint64(i+1)); err != nil { + t.Fatalf("cannot inject %s to intended, %v", a, err) + } + } + return r.RIB() + }(), + inInjectedRemoteRIB: func() *rib.RIB { + r := rib.NewFake(dn, rib.DisableRIBCheckFn()) + if err := r.InjectIPv4(dn, "84.18.210.139/32", 1); err != nil { + t.Fatalf("cannot inject IPV4 prefix to intended, %v", err) + } + return r.RIB() + }(), + inID: func() *atomic.Uint64 { + u := &atomic.Uint64{} + u.Store(41) + return u + }(), + wantOps: &ReconcileOps{ + Add: &Ops{ + TopLevel: []*spb.AFTOperation{{ + Id: 42, + NetworkInstance: dn, + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_Ipv4{ + Ipv4: &aftpb.Afts_Ipv4EntryKey{ + Prefix: "142.250.72.174/32", + Ipv4Entry: &aftpb.Afts_Ipv4Entry{ + NextHopGroup: &wpb.UintValue{Value: 2}, + }, + }, + }, + }}, + }, + Replace: &Ops{}, + Delete: &Ops{}, + }, + }, { + desc: "delete from target", + inIntendedServer: newServer, + inTargetServer: newServer, + inInjectedIntendedRIB: rib.New(dn, rib.DisableRIBCheckFn()), + inInjectedRemoteRIB: func() *rib.RIB { + r := rib.NewFake(dn, rib.DisableRIBCheckFn()) + if err := r.InjectIPv4(dn, "84.18.210.139/32", 1); err != nil { + t.Fatalf("cannot inject IPV4 prefix to intended, %v", err) + } + return r.RIB() + }(), + inID: func() *atomic.Uint64 { + u := &atomic.Uint64{} + u.Store(41) + return u + }(), + wantOps: &ReconcileOps{ + Add: &Ops{}, + Replace: &Ops{}, + Delete: &Ops{ + TopLevel: []*spb.AFTOperation{{ + Id: 42, + NetworkInstance: dn, + Op: spb.AFTOperation_DELETE, + Entry: &spb.AFTOperation_Ipv4{ + Ipv4: &aftpb.Afts_Ipv4EntryKey{ + Prefix: "84.18.210.139/32", + Ipv4Entry: &aftpb.Afts_Ipv4Entry{ + NextHopGroup: &wpb.UintValue{Value: 1}, + }, + }, + }, + }}, + }, + }, + }, { + desc: "badly behaving target", + inIntendedServer: newServer, + inTargetServer: newBadServer, + inInjectedIntendedRIB: rib.New(dn, rib.DisableRIBCheckFn()), + inInjectedRemoteRIB: rib.New(dn, rib.DisableRIBCheckFn()), + inID: &atomic.Uint64{}, + wantErr: true, + }, { + desc: "sleepy intended server", + inIntendedServer: newHangingServer, + inTargetServer: newServer, + inInjectedIntendedRIB: rib.New(dn, rib.DisableRIBCheckFn()), + inInjectedRemoteRIB: rib.New(dn, rib.DisableRIBCheckFn()), + inID: &atomic.Uint64{}, + wantErr: true, + }} + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + intendedAddr, intendedStop := tt.inIntendedServer(t, tt.inInjectedIntendedRIB) + defer intendedStop() + + targetAddr, targetStop := tt.inTargetServer(t, tt.inInjectedRemoteRIB) + defer targetStop() + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + + intendedTarget, err := NewRemoteRIB(ctx, dn, intendedAddr) + if err != nil { + t.Fatalf("cannot create intended RIBTarget, %v", err) + } + + targetTarget, err := NewRemoteRIB(ctx, dn, targetAddr) + if err != nil { + t.Fatalf("cannot create target RIBTarget, %v", err) + } + + rr := New(intendedTarget, targetTarget) + + got, err := rr.Reconcile(ctx, tt.inID) + if (err != nil) != tt.wantErr { + t.Fatalf("(reconciler.Reconcile()): cannot calculate difference, got unexpected err: %v", err) + } + + if diff := cmp.Diff(got, tt.wantOps, protocmp.Transform()); diff != "" { + t.Fatalf("reconciler.Reconcile(): did not get expected ops, diff(-got,+want):\n%s", diff) + } + }) + } +} diff --git a/rib/reconciler/remote_test.go b/rib/reconciler/remote_test.go index c3efb5c3..1feb8e04 100644 --- a/rib/reconciler/remote_test.go +++ b/rib/reconciler/remote_test.go @@ -101,7 +101,33 @@ func newBadServer(t *testing.T, r *rib.RIB) (string, func()) { go srv.Serve(l) return l.Addr().String(), srv.Stop +} + +type hangingGRIBI struct { + *spb.UnimplementedGRIBIServer +} +func (h *hangingGRIBI) Get(_ *spb.GetRequest, _ spb.GRIBI_GetServer) error { + time.Sleep(600 * time.Second) + return nil +} + +func newHangingServer(t *testing.T, r *rib.RIB) (string, func()) { + creds, err := credentials.NewServerTLSFromFile(testcommon.TLSCreds()) + if err != nil { + t.Fatalf("cannot load TLS credentials, got err: %v", err) + } + srv := grpc.NewServer(grpc.Creds(creds)) + s := &hangingGRIBI{} + + l, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("cannot create listener, got err: %v", err) + } + spb.RegisterGRIBIServer(srv, s) + + go srv.Serve(l) + return l.Addr().String(), srv.Stop } func TestGet(t *testing.T) { @@ -145,6 +171,11 @@ func TestGet(t *testing.T) { return r }(), }, + }, { + desc: "hanging gRIBI", + inDefName: dn, + inServer: newHangingServer, + wantErr: true, }} for _, tt := range tests {