diff --git a/go.mod b/go.mod index f4b5b14a..1aef9bc4 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,6 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d google.golang.org/grpc v1.58.0-dev google.golang.org/protobuf v1.31.0 - k8s.io/klog v1.0.0 lukechampine.com/uint128 v1.2.0 ) diff --git a/go.sum b/go.sum index b223ff32..1d0f4239 100644 --- a/go.sum +++ b/go.sum @@ -79,7 +79,6 @@ github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeME github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= -github.com/go-logr/logr v0.1.0/go.mod h1:ixOQHD9gLJUVQQ2ZOR7zLEifBX6tGkNJF4QyIY7sIas= github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0= github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= @@ -606,8 +605,6 @@ 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 v1.0.0 h1:Pt+yjF5aB1xDSVbau4VsWe+dQNzA0qv1LlXdC2dF6Q8= -k8s.io/klog v1.0.0/go.mod h1:4Bi6QPql/J/LkTDqv7R/cd3hPo4k2DG6Ptcz060Ez5I= k8s.io/klog/v2 v2.90.1 h1:m4bYOKall2MmOiRaR1J+We67Do7vm9KiQVlT96lnHUw= k8s.io/klog/v2 v2.90.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= lukechampine.com/uint128 v1.2.0 h1:mBi/5l91vocEN8otkC5bDLhi2KdCticRiwbdB0O+rjI= diff --git a/rib/helpers.go b/rib/helpers.go index 717a94ab..5768493e 100644 --- a/rib/helpers.go +++ b/rib/helpers.go @@ -143,15 +143,19 @@ func (f *fakeRIB) InjectNHG(ni string, nhgId uint64, nhs map[uint64]uint64) erro // InjectNH adds a next-hop entry to network instance ni, with the specified // index (nhIdx). An error is returned if it cannot be added. -func (f *fakeRIB) InjectNH(ni string, nhIdx uint64) error { +func (f *fakeRIB) InjectNH(ni string, nhIdx uint64, intName string) error { niR, ok := f.r.NetworkInstanceRIB(ni) if !ok { return fmt.Errorf("unknown NI, %s", ni) } if _, _, err := niR.AddNextHop(&aftpb.Afts_NextHopKey{ - Index: nhIdx, - NextHop: &aftpb.Afts_NextHop{}, + Index: nhIdx, + NextHop: &aftpb.Afts_NextHop{ + InterfaceRef: &aftpb.Afts_NextHop_InterfaceRef{ + Interface: &wpb.StringValue{Value: intName}, + }, + }, }, false); err != nil { return fmt.Errorf("cannot add NH entry, err: %v", err) } diff --git a/rib/helpers_test.go b/rib/helpers_test.go index cee5a58a..49398e07 100644 --- a/rib/helpers_test.go +++ b/rib/helpers_test.go @@ -267,7 +267,7 @@ func TestFakeRIB(t *testing.T) { desc: "nh only", inBuild: func() *fakeRIB { f := NewFake(dn) - if err := f.InjectNH(dn, 1); err != nil { + if err := f.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, err: %v", err) } return f @@ -280,7 +280,12 @@ func TestFakeRIB(t *testing.T) { r: &aft.RIB{ Afts: &aft.Afts{ NextHop: map[uint64]*aft.Afts_NextHop{ - 1: {Index: ygot.Uint64(1)}, + 1: { + Index: ygot.Uint64(1), + InterfaceRef: &aft.Afts_NextHop_InterfaceRef{ + Interface: ygot.String("int42"), + }, + }, }, }, }, @@ -348,7 +353,7 @@ func TestFakeRIB(t *testing.T) { inBuild: func() *fakeRIB { f := NewFake(dn) // Discard the errors, since the test will check whether the entries are there. - f.InjectNH(dn, 1) + f.InjectNH(dn, 1, "int42") f.InjectNHG(dn, 1, map[uint64]uint64{1: 1}) f.InjectIPv4(dn, "192.0.2.1/32", 1) return f @@ -363,6 +368,9 @@ func TestFakeRIB(t *testing.T) { NextHop: map[uint64]*aft.Afts_NextHop{ 1: { Index: ygot.Uint64(1), + InterfaceRef: &aft.Afts_NextHop_InterfaceRef{ + Interface: ygot.String("int42"), + }, }, }, NextHopGroup: map[uint64]*aft.Afts_NextHopGroup{ diff --git a/rib/reconciler/reconcile.go b/rib/reconciler/reconcile.go index 8a280dd5..78b23ff6 100644 --- a/rib/reconciler/reconcile.go +++ b/rib/reconciler/reconcile.go @@ -30,7 +30,6 @@ import ( "github.com/openconfig/gribigo/aft" "github.com/openconfig/gribigo/rib" - "k8s.io/klog" spb "github.com/openconfig/gribi/v1/proto/service" ) @@ -116,6 +115,39 @@ func (r *R) Reconcile(ctx context.Context) error { } +// 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 { + // NH stores the next-hop operations in the operation set. + NH []*spb.AFTOperation + // NHG stores the next-hop-group operations in the operation set. + NHG []*spb.AFTOperation + // TopLevel stores the IPv4, IPv6, and MPLS operations in the operation set. + TopLevel []*spb.AFTOperation +} + +// reconcile ops stores the operations that are required for a specific reconciliation +// run. +type reconcileOps struct { + // Add stores the operations that are explicitly adding new entries. + Add *ops + // Replace stores the operations that are implicit or explicit replaces of + // existing entries. + Replace *ops + // Delete stores the operations that are removing entries. + Delete *ops +} + +// newReconcileOps returns a new reconcileOps struct with the fields initialised. +func newReconcileOps() *reconcileOps { + return &reconcileOps{ + Add: &ops{}, + Replace: &ops{}, + Delete: &ops{}, + } +} + // diff returns the difference between the src and dst RIBs expressed as gRIBI // AFTOperations. That is to say, for each network instance RIB within the RIBs: // @@ -129,7 +161,10 @@ func (r *R) Reconcile(ctx context.Context) error { // // 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) ([]*spb.AFTOperation, error) { +func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOps, error) { + if src == nil || dst == nil { + return nil, fmt.Errorf("invalid nil input RIBs, src: %v, dst: %v", src, dst) + } srcContents, err := src.RIBContents() if err != nil { return nil, fmt.Errorf("cannot copy source RIB contents, err: %v", err) @@ -139,37 +174,16 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp return nil, fmt.Errorf("cannot copy destination RIB contents, err: %v", err) } - // Store the "top-level" operations (i.e., IPv4, IPv6, MPLS) and then the NHG and NHs - // separately. This allows us to return the operations separately so that they can be - // ordered in terms of programming. NHs need to be installed/replaced before NHGs, and - // then subsequently top-level entries. - topLevelOps, nhgOps, nhOps := []*spb.AFTOperation{}, []*spb.AFTOperation{}, []*spb.AFTOperation{} + ops := newReconcileOps() + var id uint64 for srcNI, srcNIEntries := range srcContents { dstNIEntries, ok := dstContents[srcNI] if !ok { - // The network instance does not exist in the destination therefore - // all entries are ADDs. - for pfx, e := range srcNIEntries.GetAfts().Ipv4Entry { - id++ - op, err := v4Operation(spb.AFTOperation_ADD, srcNI, pfx, id, e) - if err != nil { - return nil, err - } - topLevelOps = append(topLevelOps, op) - } - - for nhgID, e := range srcNIEntries.GetAfts().NextHopGroup { - id++ - op, err := nhgOperation(spb.AFTOperation_ADD, srcNI, nhgID, id, e) - if err != nil { - return nil, err - } - nhgOps = append(nhgOps, op) - } - - continue + dstNIEntries = &aft.RIB{} + dstNIEntries.GetOrCreateAfts() } + // For each AFT: // * if a key is present in src but not in dst -> generate an ADD // * if a key is present in src and in dst -> diff, and generate an ADD if the contents differ. @@ -185,7 +199,14 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp if err != nil { return nil, err } - topLevelOps = append(topLevelOps, op) + + // If this entry already exists then this is an addition rather than a replace. + switch ok { + case true: + ops.Replace.TopLevel = append(ops.Replace.TopLevel, op) + case false: + ops.Add.TopLevel = append(ops.Add.TopLevel, op) + } } } @@ -200,10 +221,40 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp if err != nil { return nil, err } - nhgOps = append(nhgOps, op) + + // If this entry already exists then this is an addition rather than a replace. + switch ok { + case true: + ops.Replace.NHG = append(ops.Replace.NHG, op) + case false: + ops.Add.NHG = append(ops.Add.NHG, op) + } } } + for nhID, srcE := range srcNIEntries.GetAfts().NextHop { + if dstE, ok := dstNIEntries.GetAfts().NextHop[nhID]; !ok || !reflect.DeepEqual(srcE, dstE) { + opType := spb.AFTOperation_ADD + if ok && explicitReplace[spb.AFTType_NEXTHOP] { + opType = spb.AFTOperation_REPLACE + } + id++ + op, err := nhOperation(opType, srcNI, nhID, id, srcE) + if err != nil { + return nil, err + } + + // If this entry already exists then this is an addition rather than a replace. + switch ok { + case true: + ops.Replace.NH = append(ops.Replace.NH, op) + case false: + ops.Add.NH = append(ops.Add.NH, op) + } + } + } + + // Delete operations. for pfx, dstE := range dstNIEntries.GetAfts().Ipv4Entry { if _, ok := srcNIEntries.GetAfts().Ipv4Entry[pfx]; !ok { id++ @@ -211,7 +262,7 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp if err != nil { return nil, err } - topLevelOps = append(topLevelOps, op) + ops.Delete.TopLevel = append(ops.Delete.TopLevel, op) } } @@ -222,20 +273,22 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp if err != nil { return nil, err } - nhgOps = append(nhgOps, op) + ops.Delete.NHG = append(ops.Delete.NHG, op) } } - if srcN, dstN := len(srcNIEntries.GetAfts().NextHop), len(dstNIEntries.GetAfts().NextHop); srcN != 0 || dstN != 0 { - // TODO(robjs): Implement mapping of NH entries. - klog.Warningf("next-hop reconcilation unimplemented, NHG entries, src: %d, dst: %d", srcN, dstN) + 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) + if err != nil { + return nil, err + } + ops.Delete.NH = append(ops.Delete.NH, op) + } } } - ops := append([]*spb.AFTOperation{}, nhOps...) - ops = append(ops, nhgOps...) - ops = append(ops, topLevelOps...) - return ops, nil } @@ -274,3 +327,21 @@ 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 +// 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) { + p, err := rib.ConcreteNextHopProto(e) + if err != nil { + return nil, fmt.Errorf("cannot create operation for NH %d, %v", nhID, err) + } + return &spb.AFTOperation{ + Id: id, + NetworkInstance: ni, + Op: method, + Entry: &spb.AFTOperation_NextHop{ + NextHop: p, + }, + }, nil +} diff --git a/rib/reconciler/reconcile_test.go b/rib/reconciler/reconcile_test.go index 8942cfd4..e4e87f25 100644 --- a/rib/reconciler/reconcile_test.go +++ b/rib/reconciler/reconcile_test.go @@ -44,7 +44,7 @@ func TestDiff(t *testing.T) { inSrc *rib.RIB inDst *rib.RIB inExplicitReplace map[spb.AFTType]bool - wantOps []*spb.AFTOperation + wantOps *reconcileOps wantErr bool }{{ desc: "VRF NI in src, but not in dst", @@ -54,7 +54,7 @@ func TestDiff(t *testing.T) { if err := r.RIB().AddNetworkInstance(vrf); err != nil { t.Fatalf("cannot create NI, %v", err) } - if err := r.InjectNH(vrf, 1); err != nil { + if err := r.InjectNH(vrf, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(vrf, 1, map[uint64]uint64{1: 1}); err != nil { @@ -66,41 +66,61 @@ func TestDiff(t *testing.T) { return r.RIB() }(), inDst: rib.NewFake(dn).RIB(), - wantOps: []*spb.AFTOperation{{ - Id: 2, - NetworkInstance: "FOO", - Op: spb.AFTOperation_ADD, - Entry: &spb.AFTOperation_NextHopGroup{ - NextHopGroup: &aftpb.Afts_NextHopGroupKey{ - Id: 1, - NextHopGroup: &aftpb.Afts_NextHopGroup{ - NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ + wantOps: &reconcileOps{ + Add: &ops{ + NH: []*spb.AFTOperation{{ + Id: 3, + NetworkInstance: "FOO", + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_NextHop{ + NextHop: &aftpb.Afts_NextHopKey{ Index: 1, - NextHop: &aftpb.Afts_NextHopGroup_NextHop{ - Weight: &wpb.UintValue{Value: 1}, + NextHop: &aftpb.Afts_NextHop{ + InterfaceRef: &aftpb.Afts_NextHop_InterfaceRef{ + Interface: &wpb.StringValue{Value: "int42"}, + }, }, - }}, + }, }, - }, - }, - }, { - Id: 1, - NetworkInstance: "FOO", - 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}, + }}, + NHG: []*spb.AFTOperation{{ + Id: 2, + NetworkInstance: "FOO", + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_NextHopGroup{ + NextHopGroup: &aftpb.Afts_NextHopGroupKey{ + Id: 1, + NextHopGroup: &aftpb.Afts_NextHopGroup{ + NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ + Index: 1, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 1}, + }, + }}, + }, + }, + }, + }}, + TopLevel: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: "FOO", + 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}, + }, + }, }, - }, + }}, }, - }}, + }, }, { desc: "default NI with added and removed IPv4 entries", inSrc: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { @@ -113,7 +133,7 @@ func TestDiff(t *testing.T) { }(), inDst: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { @@ -124,36 +144,43 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: []*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}, + 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}, + }, + }, }, - }, + }}, }, - }, { - Id: 2, - NetworkInstance: dn, - Op: spb.AFTOperation_DELETE, - Entry: &spb.AFTOperation_Ipv4{ - Ipv4: &aftpb.Afts_Ipv4EntryKey{ - Prefix: "2.0.0.0/24", - Ipv4Entry: &aftpb.Afts_Ipv4Entry{ - NextHopGroup: &wpb.UintValue{Value: 1}, + Delete: &ops{ + TopLevel: []*spb.AFTOperation{{ + Id: 2, + NetworkInstance: dn, + Op: spb.AFTOperation_DELETE, + Entry: &spb.AFTOperation_Ipv4{ + Ipv4: &aftpb.Afts_Ipv4EntryKey{ + Prefix: "2.0.0.0/24", + Ipv4Entry: &aftpb.Afts_Ipv4Entry{ + NextHopGroup: &wpb.UintValue{Value: 1}, + }, + }, }, - }, + }}, }, - }}, + }, }, { desc: "default NI with IPv4 entry with different contents", inSrc: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { @@ -169,7 +196,7 @@ func TestDiff(t *testing.T) { }(), inDst: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { @@ -180,41 +207,48 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: []*spb.AFTOperation{{ - Id: 2, - NetworkInstance: dn, - Op: spb.AFTOperation_ADD, - Entry: &spb.AFTOperation_NextHopGroup{ - NextHopGroup: &aftpb.Afts_NextHopGroupKey{ - Id: 2, - NextHopGroup: &aftpb.Afts_NextHopGroup{ - NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ - Index: 1, - NextHop: &aftpb.Afts_NextHopGroup_NextHop{ - Weight: &wpb.UintValue{Value: 2}, + wantOps: &reconcileOps{ + Add: &ops{ + NHG: []*spb.AFTOperation{{ + Id: 2, + NetworkInstance: dn, + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_NextHopGroup{ + NextHopGroup: &aftpb.Afts_NextHopGroupKey{ + Id: 2, + NextHopGroup: &aftpb.Afts_NextHopGroup{ + NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ + Index: 1, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 2}, + }, + }}, }, - }}, + }, }, - }, + }}, }, - }, { - 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: 2}, + Replace: &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: 2}, + }, + }, }, - }, + }}, }, - }}, + }, }, { desc: "default NI with IPv4 entry with different contents, explicit replace", inSrc: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { @@ -230,7 +264,7 @@ func TestDiff(t *testing.T) { }(), inDst: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { @@ -244,41 +278,48 @@ func TestDiff(t *testing.T) { inExplicitReplace: map[spb.AFTType]bool{ spb.AFTType_IPV4: true, }, - wantOps: []*spb.AFTOperation{{ - Id: 2, - NetworkInstance: dn, - Op: spb.AFTOperation_ADD, - Entry: &spb.AFTOperation_NextHopGroup{ - NextHopGroup: &aftpb.Afts_NextHopGroupKey{ - Id: 2, - NextHopGroup: &aftpb.Afts_NextHopGroup{ - NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ - Index: 1, - NextHop: &aftpb.Afts_NextHopGroup_NextHop{ - Weight: &wpb.UintValue{Value: 2}, + wantOps: &reconcileOps{ + Add: &ops{ + NHG: []*spb.AFTOperation{{ + Id: 2, + NetworkInstance: dn, + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_NextHopGroup{ + NextHopGroup: &aftpb.Afts_NextHopGroupKey{ + Id: 2, + NextHopGroup: &aftpb.Afts_NextHopGroup{ + NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ + Index: 1, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 2}, + }, + }}, }, - }}, + }, }, - }, + }}, }, - }, { - Id: 1, - NetworkInstance: dn, - Op: spb.AFTOperation_REPLACE, - Entry: &spb.AFTOperation_Ipv4{ - Ipv4: &aftpb.Afts_Ipv4EntryKey{ - Prefix: "1.0.0.0/24", - Ipv4Entry: &aftpb.Afts_Ipv4Entry{ - NextHopGroup: &wpb.UintValue{Value: 2}, + Replace: &ops{ + TopLevel: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_REPLACE, + Entry: &spb.AFTOperation_Ipv4{ + Ipv4: &aftpb.Afts_Ipv4EntryKey{ + Prefix: "1.0.0.0/24", + Ipv4Entry: &aftpb.Afts_Ipv4Entry{ + NextHopGroup: &wpb.UintValue{Value: 2}, + }, + }, }, - }, + }}, }, - }}, + }, }, { desc: "NHG installed", inSrc: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { @@ -288,41 +329,45 @@ func TestDiff(t *testing.T) { }(), inDst: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } return r.RIB() }(), - wantOps: []*spb.AFTOperation{{ - Id: 1, - NetworkInstance: dn, - Op: spb.AFTOperation_ADD, - Entry: &spb.AFTOperation_NextHopGroup{ - NextHopGroup: &aftpb.Afts_NextHopGroupKey{ - Id: 1, - NextHopGroup: &aftpb.Afts_NextHopGroup{ - NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ - Index: 1, - NextHop: &aftpb.Afts_NextHopGroup_NextHop{ - Weight: &wpb.UintValue{Value: 1}, + wantOps: &reconcileOps{ + Add: &ops{ + NHG: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_NextHopGroup{ + NextHopGroup: &aftpb.Afts_NextHopGroupKey{ + Id: 1, + NextHopGroup: &aftpb.Afts_NextHopGroup{ + NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ + Index: 1, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 1}, + }, + }}, }, - }}, + }, }, - }, + }}, }, - }}, + }, }, { desc: "NHG deleted", inSrc: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } return r.RIB() }(), inDst: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { @@ -330,32 +375,36 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: []*spb.AFTOperation{{ - Id: 1, - NetworkInstance: dn, - Op: spb.AFTOperation_DELETE, - Entry: &spb.AFTOperation_NextHopGroup{ - NextHopGroup: &aftpb.Afts_NextHopGroupKey{ - Id: 1, - NextHopGroup: &aftpb.Afts_NextHopGroup{ - NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ - Index: 1, - NextHop: &aftpb.Afts_NextHopGroup_NextHop{ - Weight: &wpb.UintValue{Value: 1}, + wantOps: &reconcileOps{ + Delete: &ops{ + NHG: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_DELETE, + Entry: &spb.AFTOperation_NextHopGroup{ + NextHopGroup: &aftpb.Afts_NextHopGroupKey{ + Id: 1, + NextHopGroup: &aftpb.Afts_NextHopGroup{ + NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ + Index: 1, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 1}, + }, + }}, }, - }}, + }, }, - }, + }}, }, - }}, + }, }, { desc: "NHG implicitly replaced", inSrc: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } - if err := r.InjectNH(dn, 2); err != nil { + if err := r.InjectNH(dn, 2, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{ @@ -368,10 +417,10 @@ func TestDiff(t *testing.T) { }(), inDst: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } - if err := r.InjectNH(dn, 2); err != nil { + if err := r.InjectNH(dn, 2, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { @@ -379,37 +428,41 @@ func TestDiff(t *testing.T) { } return r.RIB() }(), - wantOps: []*spb.AFTOperation{{ - Id: 1, - NetworkInstance: dn, - Op: spb.AFTOperation_ADD, - Entry: &spb.AFTOperation_NextHopGroup{ - NextHopGroup: &aftpb.Afts_NextHopGroupKey{ - Id: 1, - NextHopGroup: &aftpb.Afts_NextHopGroup{ - NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ - Index: 1, - NextHop: &aftpb.Afts_NextHopGroup_NextHop{ - Weight: &wpb.UintValue{Value: 2}, - }, - }, { - Index: 2, - NextHop: &aftpb.Afts_NextHopGroup_NextHop{ - Weight: &wpb.UintValue{Value: 4}, + wantOps: &reconcileOps{ + Replace: &ops{ + NHG: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_NextHopGroup{ + NextHopGroup: &aftpb.Afts_NextHopGroupKey{ + Id: 1, + NextHopGroup: &aftpb.Afts_NextHopGroup{ + NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ + Index: 1, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 2}, + }, + }, { + Index: 2, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 4}, + }, + }}, }, - }}, + }, }, - }, + }}, }, - }}, + }, }, { desc: "NHG explicitly replaced", inSrc: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } - if err := r.InjectNH(dn, 2); err != nil { + if err := r.InjectNH(dn, 2, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{ @@ -422,10 +475,10 @@ func TestDiff(t *testing.T) { }(), inDst: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } - if err := r.InjectNH(dn, 2); err != nil { + if err := r.InjectNH(dn, 2, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { @@ -436,29 +489,185 @@ func TestDiff(t *testing.T) { inExplicitReplace: map[spb.AFTType]bool{ spb.AFTType_NEXTHOP_GROUP: true, }, - wantOps: []*spb.AFTOperation{{ - Id: 1, - NetworkInstance: dn, - Op: spb.AFTOperation_REPLACE, - Entry: &spb.AFTOperation_NextHopGroup{ - NextHopGroup: &aftpb.Afts_NextHopGroupKey{ - Id: 1, - NextHopGroup: &aftpb.Afts_NextHopGroup{ - NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ - Index: 1, - NextHop: &aftpb.Afts_NextHopGroup_NextHop{ - Weight: &wpb.UintValue{Value: 2}, + wantOps: &reconcileOps{ + Replace: &ops{ + NHG: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_REPLACE, + Entry: &spb.AFTOperation_NextHopGroup{ + NextHopGroup: &aftpb.Afts_NextHopGroupKey{ + Id: 1, + NextHopGroup: &aftpb.Afts_NextHopGroup{ + NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ + Index: 1, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 2}, + }, + }, { + Index: 2, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 4}, + }, + }}, + }, + }, + }, + }}, + }, + }, + }, { + desc: "NH added", + inSrc: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1, "int42"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNH(dn, 2, "int42"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + return r.RIB() + }(), + inDst: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1, "int42"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + return r.RIB() + }(), + wantOps: &reconcileOps{ + Add: &ops{ + NH: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_NextHop{ + NextHop: &aftpb.Afts_NextHopKey{ + Index: 2, + NextHop: &aftpb.Afts_NextHop{ + InterfaceRef: &aftpb.Afts_NextHop_InterfaceRef{ + Interface: &wpb.StringValue{Value: "int42"}, + }, }, - }, { + }, + }, + }}, + }, + }, + }, { + desc: "NH deleted", + inSrc: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1, "int42"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + return r.RIB() + }(), + inDst: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1, "int42"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNH(dn, 2, "int42"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + return r.RIB() + }(), + wantOps: &reconcileOps{ + Delete: &ops{ + NH: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_DELETE, + Entry: &spb.AFTOperation_NextHop{ + NextHop: &aftpb.Afts_NextHopKey{ Index: 2, - NextHop: &aftpb.Afts_NextHopGroup_NextHop{ - Weight: &wpb.UintValue{Value: 4}, + NextHop: &aftpb.Afts_NextHop{ + InterfaceRef: &aftpb.Afts_NextHop_InterfaceRef{ + Interface: &wpb.StringValue{Value: "int42"}, + }, + }, + }, + }, + }}, + }, + }, + }, { + desc: "NH replaced", + inSrc: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1, "INTERFACE1"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + return r.RIB() + }(), + inDst: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1, "INTERFACE42"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + return r.RIB() + }(), + wantOps: &reconcileOps{ + Replace: &ops{ + NH: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_NextHop{ + NextHop: &aftpb.Afts_NextHopKey{ + Index: 1, + NextHop: &aftpb.Afts_NextHop{ + InterfaceRef: &aftpb.Afts_NextHop_InterfaceRef{ + Interface: &wpb.StringValue{Value: "INTERFACE1"}, + }, + }, + }, + }, + }}, + }, + }, + }, { + desc: "NH replaced - explicitly", + inSrc: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1, "INTERFACE1"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + return r.RIB() + }(), + inDst: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1, "INTERFACE42"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + return r.RIB() + }(), + inExplicitReplace: map[spb.AFTType]bool{ + spb.AFTType_NEXTHOP: true, + }, + wantOps: &reconcileOps{ + Replace: &ops{ + NH: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_REPLACE, + Entry: &spb.AFTOperation_NextHop{ + NextHop: &aftpb.Afts_NextHopKey{ + Index: 1, + NextHop: &aftpb.Afts_NextHop{ + InterfaceRef: &aftpb.Afts_NextHop_InterfaceRef{ + Interface: &wpb.StringValue{Value: "INTERFACE1"}, + }, }, - }}, + }, }, - }, + }}, }, - }}, + }, + }, { + desc: "nil input", + wantErr: true, }} for _, tt := range tests { @@ -467,7 +676,25 @@ func TestDiff(t *testing.T) { if (err != nil) != tt.wantErr { t.Fatalf("did not get expected error, got: %v, wantErr? %v", err, tt.wantErr) } + + if err != nil { + return + } + + // 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{} + } + if tt.wantOps.Replace == nil { + tt.wantOps.Replace = &ops{} + } + if tt.wantOps.Delete == nil { + tt.wantOps.Delete = &ops{} + } + if diff := cmp.Diff(got, tt.wantOps, + cmpopts.EquateEmpty(), protocmp.Transform(), protocmp.SortRepeatedFields(&aftpb.Afts_NextHopGroup{}, "next_hop"), ); diff != "" { diff --git a/rib/reconciler/remote_test.go b/rib/reconciler/remote_test.go index 468fa52b..c3efb5c3 100644 --- a/rib/reconciler/remote_test.go +++ b/rib/reconciler/remote_test.go @@ -124,7 +124,7 @@ func TestGet(t *testing.T) { inServer: newServer, inInjectedRIB: func() *rib.RIB { r := rib.NewFake(dn) - if err := r.InjectNH(dn, 1); err != nil { + if err := r.InjectNH(dn, 1, "int42"); err != nil { t.Fatalf("cannot add NH, %v", err) } if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { @@ -141,7 +141,7 @@ func TestGet(t *testing.T) { a := r.GetOrCreateAfts() a.GetOrCreateIpv4Entry("1.0.0.0/24").NextHopGroup = ygot.Uint64(1) a.GetOrCreateNextHopGroup(1).GetOrCreateNextHop(1).Weight = ygot.Uint64(1) - a.GetOrCreateNextHop(1) + a.GetOrCreateNextHop(1).GetOrCreateInterfaceRef().Interface = ygot.String("int42") return r }(), },