From 2c0e1f757648317dc6f7a12e0487118baebd286e Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Mon, 23 Oct 2023 15:39:45 -0700 Subject: [PATCH 1/4] Add support for next-hop-group into reconciler. * (M) rib/reconciler/reconcile.go * (M) rib/reconciler/reconcile_test.go - support add/delete/replace for NextHopGroup entries within a RIB. --- rib/reconciler/reconcile.go | 71 ++++++++- rib/reconciler/reconcile_test.go | 241 ++++++++++++++++++++++++++++++- 2 files changed, 304 insertions(+), 8 deletions(-) diff --git a/rib/reconciler/reconcile.go b/rib/reconciler/reconcile.go index 05d476fa..8a280dd5 100644 --- a/rib/reconciler/reconcile.go +++ b/rib/reconciler/reconcile.go @@ -139,7 +139,11 @@ 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) } - ops := []*spb.AFTOperation{} + // 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{} var id uint64 for srcNI, srcNIEntries := range srcContents { dstNIEntries, ok := dstContents[srcNI] @@ -152,8 +156,18 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp if err != nil { return nil, err } - ops = append(ops, op) + 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 } // For each AFT: @@ -171,7 +185,22 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp if err != nil { return nil, err } - ops = append(ops, op) + topLevelOps = append(topLevelOps, op) + } + } + + for nhgID, srcE := range srcNIEntries.GetAfts().NextHopGroup { + if dstE, ok := dstNIEntries.GetAfts().NextHopGroup[nhgID]; !ok || !reflect.DeepEqual(srcE, dstE) { + opType := spb.AFTOperation_ADD + if ok && explicitReplace[spb.AFTType_NEXTHOP_GROUP] { + opType = spb.AFTOperation_REPLACE + } + id++ + op, err := nhgOperation(opType, srcNI, nhgID, id, srcE) + if err != nil { + return nil, err + } + nhgOps = append(nhgOps, op) } } @@ -182,13 +211,19 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp if err != nil { return nil, err } - ops = append(ops, op) + topLevelOps = append(topLevelOps, op) } } - if srcN, dstN := len(srcNIEntries.GetAfts().NextHopGroup), len(dstNIEntries.GetAfts().NextHopGroup); srcN != 0 || dstN != 0 { - // TODO(robjs): Implement diffing of NHG entries. - klog.Warningf("next-hop-group reconcilation unimplemented, NHG entries, src: %d, dst: %d", srcN, dstN) + 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) + if err != nil { + return nil, err + } + nhgOps = append(nhgOps, op) + } } if srcN, dstN := len(srcNIEntries.GetAfts().NextHop), len(dstNIEntries.GetAfts().NextHop); srcN != 0 || dstN != 0 { @@ -197,6 +232,10 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp } } + ops := append([]*spb.AFTOperation{}, nhOps...) + ops = append(ops, nhgOps...) + ops = append(ops, topLevelOps...) + return ops, nil } @@ -217,3 +256,21 @@ 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 +// 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) { + p, err := rib.ConcreteNextHopGroupProto(e) + if err != nil { + return nil, fmt.Errorf("cannot create operation for NHG %d, %v", nhgID, err) + } + return &spb.AFTOperation{ + Id: id, + NetworkInstance: ni, + Op: method, + Entry: &spb.AFTOperation_NextHopGroup{ + NextHopGroup: p, + }, + }, nil +} diff --git a/rib/reconciler/reconcile_test.go b/rib/reconciler/reconcile_test.go index ae2c594f..8942cfd4 100644 --- a/rib/reconciler/reconcile_test.go +++ b/rib/reconciler/reconcile_test.go @@ -67,6 +67,23 @@ func TestDiff(t *testing.T) { }(), 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{{ + Index: 1, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 1}, + }, + }}, + }, + }, + }, + }, { Id: 1, NetworkInstance: "FOO", Op: spb.AFTOperation_ADD, @@ -164,6 +181,23 @@ 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}, + }, + }}, + }, + }, + }, + }, { Id: 1, NetworkInstance: dn, Op: spb.AFTOperation_ADD, @@ -211,6 +245,23 @@ func TestDiff(t *testing.T) { 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}, + }, + }}, + }, + }, + }, + }, { Id: 1, NetworkInstance: dn, Op: spb.AFTOperation_REPLACE, @@ -223,6 +274,191 @@ func TestDiff(t *testing.T) { }, }, }}, + }, { + desc: "NHG installed", + inSrc: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG 1, %v", err) + } + return r.RIB() + }(), + inDst: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1); 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}, + }, + }}, + }, + }, + }, + }}, + }, { + desc: "NHG deleted", + inSrc: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1); 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 { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG 1, %v", err) + } + 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}, + }, + }}, + }, + }, + }, + }}, + }, { + desc: "NHG implicitly replaced", + inSrc: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNH(dn, 2); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNHG(dn, 1, map[uint64]uint64{ + 1: 2, + 2: 4, + }); err != nil { + t.Fatalf("cannot add NHG 1, %v", err) + } + return r.RIB() + }(), + inDst: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNH(dn, 2); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG 1, %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: 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 { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNH(dn, 2); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNHG(dn, 1, map[uint64]uint64{ + 1: 2, + 2: 4, + }); err != nil { + t.Fatalf("cannot add NHG 1, %v", err) + } + return r.RIB() + }(), + inDst: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNH(dn, 2); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG 1, %v", err) + } + return r.RIB() + }(), + 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}, + }, + }, { + Index: 2, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 4}, + }, + }}, + }, + }, + }, + }}, }} for _, tt := range tests { @@ -231,7 +467,10 @@ 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 diff := cmp.Diff(got, tt.wantOps, protocmp.Transform()); diff != "" { + if diff := cmp.Diff(got, tt.wantOps, + protocmp.Transform(), + protocmp.SortRepeatedFields(&aftpb.Afts_NextHopGroup{}, "next_hop"), + ); diff != "" { t.Fatalf("diff(%s, %s): did not get expected operations, diff(-got,+want):\n%s", tt.inSrc, tt.inDst, diff) } }) From e3063f8c800c4b8261f51468010fce9e94e1d0b5 Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Mon, 23 Oct 2023 17:24:03 -0700 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=A7=B9:=20tidy=20go.mod?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index db2747dd..f4b5b14a 100644 --- a/go.mod +++ b/go.mod @@ -16,6 +16,7 @@ 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 ) @@ -59,6 +60,5 @@ require ( golang.org/x/text v0.12.0 // indirect gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/klog v1.0.0 // indirect k8s.io/klog/v2 v2.90.1 // indirect ) From 9e70c94e8686fcc0ce3a54e1ca92a9b875418890 Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Mon, 23 Oct 2023 16:36:07 -0700 Subject: [PATCH 3/4] Add support for next-hop reconcilation and refactor return values. * (M) rib/helpers.go * (M) rib/helpers_test.go - Allow NH details to be injected into the fake RIB. * (M) rib/reconciler/reconcile.go * (M) rib/reconciler/reconcile_test.go - Restructure return values to allow a caller to stage their operations into the gRIBI server - e.g., perform adds to install new entries, then replaces, then deletes. Keep types separate to allow for more robust ordering. - Add support for NH diffing. * (M) rib/reconciler/remote_test.go - Adopt new fake RIB helper method. --- go.mod | 1 - go.sum | 3 - rib/helpers.go | 10 +- rib/helpers_test.go | 14 +- rib/reconciler/reconcile.go | 149 ++++++-- rib/reconciler/reconcile_test.go | 595 +++++++++++++++++++++---------- rib/reconciler/remote_test.go | 4 +- 7 files changed, 541 insertions(+), 235 deletions(-) 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 }(), }, From 253b9ae15656736d155fbcf4c32859d3a63ee3f8 Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Tue, 24 Oct 2023 14:26:27 -0700 Subject: [PATCH 4/4] Add support for MPLS reconciliation. (#201) * Add support for MPLS reconciliation. * (M) rib/helpers.go * (M) rib/helpers_test.go - Add fake RIB helper methods for MPLS. * (M) rib/reconciler/reconcile.go * (M) rib/reconciler/reconcile_test.go - Add support for reconciliation of label entries. * Add support for `ALL` address families in explicitReplace. * (M) rib/reconciler/reconcile.go * (M) rib/reconciler/reconcile_test.go - Ensure that the `ALL` `AFTType` is mapped to the correct address families when reconciling. --- rib/helpers.go | 31 +++- rib/helpers_test.go | 27 +++ rib/reconciler/reconcile.go | 62 +++++++ rib/reconciler/reconcile_test.go | 305 +++++++++++++++++++++++++++++++ 4 files changed, 421 insertions(+), 4 deletions(-) diff --git a/rib/helpers.go b/rib/helpers.go index 5768493e..e68ce94e 100644 --- a/rib/helpers.go +++ b/rib/helpers.go @@ -112,17 +112,17 @@ func (f *fakeRIB) InjectIPv4(ni, pfx string, nhg uint64) error { } // InjectNHG adds a next-hop-group entry to network instance ni, with the specified -// ID (nhgId). The next-hop-group contains the next hops specified in the nhs map, +// ID (nhgID). The next-hop-group contains the next hops specified in the nhs map, // with the key of the map being the next-hop ID and the value being the weight within // the group. -func (f *fakeRIB) InjectNHG(ni string, nhgId uint64, nhs map[uint64]uint64) error { +func (f *fakeRIB) InjectNHG(ni string, nhgID uint64, nhs map[uint64]uint64) error { niR, ok := f.r.NetworkInstanceRIB(ni) if !ok { return fmt.Errorf("unknown NI, %s", ni) } nhg := &aftpb.Afts_NextHopGroupKey{ - Id: nhgId, + Id: nhgID, NextHopGroup: &aftpb.Afts_NextHopGroup{}, } for nh, weight := range nhs { @@ -142,7 +142,8 @@ 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. +// index (nhIdx), and interface ref to intName. An error is returned if it cannot +// be added. func (f *fakeRIB) InjectNH(ni string, nhIdx uint64, intName string) error { niR, ok := f.r.NetworkInstanceRIB(ni) if !ok { @@ -162,3 +163,25 @@ func (f *fakeRIB) InjectNH(ni string, nhIdx uint64, intName string) error { return nil } + +// InjectMPLS adds an MPLS (Label) entry to network instance ni, with the +// specified next-hop-group. An error is returned if it cannot be added. +func (f *fakeRIB) InjectMPLS(ni string, label, nhg uint64) error { + niR, ok := f.r.NetworkInstanceRIB(ni) + if !ok { + return fmt.Errorf("unknown NI, %s", ni) + } + + if _, _, err := niR.AddMPLS(&aftpb.Afts_LabelEntryKey{ + Label: &aftpb.Afts_LabelEntryKey_LabelUint64{ + LabelUint64: label, + }, + LabelEntry: &aftpb.Afts_LabelEntry{ + NextHopGroup: &wpb.UintValue{Value: nhg}, + }, + }, false); err != nil { + return fmt.Errorf("cannot add MPLS entry, err: %v", err) + } + + return nil +} diff --git a/rib/helpers_test.go b/rib/helpers_test.go index 49398e07..2c9ab32b 100644 --- a/rib/helpers_test.go +++ b/rib/helpers_test.go @@ -316,6 +316,33 @@ func TestFakeRIB(t *testing.T) { }, }, }, + }, { + desc: "mpls only", + inBuild: func() *fakeRIB { + f := NewFake(dn, DisableRIBCheckFn()) + if err := f.InjectMPLS(dn, 42, 1); err != nil { + t.Fatalf("cannot add MPLS, err: %v", err) + } + return f + }, + wantRIB: &RIB{ + defaultName: dn, + niRIB: map[string]*RIBHolder{ + dn: { + name: dn, + r: &aft.RIB{ + Afts: &aft.Afts{ + LabelEntry: map[aft.Afts_LabelEntry_Label_Union]*aft.Afts_LabelEntry{ + aft.UnionUint32(42): { + Label: aft.UnionUint32(42), + NextHopGroup: ygot.Uint64(1), + }, + }, + }, + }, + }, + }, + }, }, { desc: "nhg only", inBuild: func() *fakeRIB { diff --git a/rib/reconciler/reconcile.go b/rib/reconciler/reconcile.go index 78b23ff6..63c7f995 100644 --- a/rib/reconciler/reconcile.go +++ b/rib/reconciler/reconcile.go @@ -165,6 +165,17 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp if src == nil || dst == nil { return nil, fmt.Errorf("invalid nil input RIBs, src: %v, dst: %v", src, dst) } + + // Re-map ALL into the supported address families. + if _, ok := explicitReplace[spb.AFTType_ALL]; ok { + explicitReplace = map[spb.AFTType]bool{ + spb.AFTType_IPV4: true, + spb.AFTType_MPLS: true, + spb.AFTType_NEXTHOP: true, + spb.AFTType_NEXTHOP_GROUP: true, + } + } + srcContents, err := src.RIBContents() if err != nil { return nil, fmt.Errorf("cannot copy source RIB contents, err: %v", err) @@ -210,6 +221,28 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) (*reconcileOp } } + for lbl, srcE := range srcNIEntries.GetAfts().LabelEntry { + if dstE, ok := dstNIEntries.GetAfts().LabelEntry[lbl]; !ok || !reflect.DeepEqual(srcE, dstE) { + opType := spb.AFTOperation_ADD + if ok && explicitReplace[spb.AFTType_MPLS] { + opType = spb.AFTOperation_REPLACE + } + id++ + op, err := mplsOperation(opType, srcNI, lbl, 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.TopLevel = append(ops.Replace.TopLevel, op) + case false: + ops.Add.TopLevel = append(ops.Add.TopLevel, op) + } + } + } + for nhgID, srcE := range srcNIEntries.GetAfts().NextHopGroup { if dstE, ok := dstNIEntries.GetAfts().NextHopGroup[nhgID]; !ok || !reflect.DeepEqual(srcE, dstE) { opType := spb.AFTOperation_ADD @@ -266,6 +299,17 @@ 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) + if err != nil { + return nil, err + } + ops.Delete.TopLevel = append(ops.Delete.TopLevel, op) + } + } + for nhgID, dstE := range dstNIEntries.GetAfts().NextHopGroup { if _, ok := srcNIEntries.GetAfts().NextHopGroup[nhgID]; !ok { id++ @@ -345,3 +389,21 @@ func nhOperation(method spb.AFTOperation_Operation, ni string, nhID, id uint64, }, }, nil } + +// 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) { + p, err := rib.ConcreteMPLSProto(e) + if err != nil { + return nil, fmt.Errorf("cannot create operation for label %d, %v", lbl, err) + } + return &spb.AFTOperation{ + Id: id, + NetworkInstance: ni, + Op: method, + Entry: &spb.AFTOperation_Mpls{ + Mpls: p, + }, + }, nil +} diff --git a/rib/reconciler/reconcile_test.go b/rib/reconciler/reconcile_test.go index e4e87f25..32ce206a 100644 --- a/rib/reconciler/reconcile_test.go +++ b/rib/reconciler/reconcile_test.go @@ -668,6 +668,311 @@ func TestDiff(t *testing.T) { }, { desc: "nil input", wantErr: true, + }, { + desc: "MPLS 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.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectMPLS(dn, 42, 1); err != nil { + t.Fatalf("cannot add label entry, %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.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + return r.RIB() + }(), + wantOps: &reconcileOps{ + Add: &ops{ + TopLevel: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_Mpls{ + Mpls: &aftpb.Afts_LabelEntryKey{ + Label: &aftpb.Afts_LabelEntryKey_LabelUint64{ + LabelUint64: 42, + }, + LabelEntry: &aftpb.Afts_LabelEntry{ + NextHopGroup: &wpb.UintValue{Value: 1}, + }, + }, + }, + }}, + }, + }, + }, { + desc: "MPLS delete", + 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.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %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.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectMPLS(dn, 42, 1); err != nil { + t.Fatalf("cannot add label entry, %v", err) + } + return r.RIB() + }(), + wantOps: &reconcileOps{ + Delete: &ops{ + TopLevel: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_DELETE, + Entry: &spb.AFTOperation_Mpls{ + Mpls: &aftpb.Afts_LabelEntryKey{ + Label: &aftpb.Afts_LabelEntryKey_LabelUint64{ + LabelUint64: 42, + }, + LabelEntry: &aftpb.Afts_LabelEntry{ + NextHopGroup: &wpb.UintValue{Value: 1}, + }, + }, + }, + }}, + }, + }, + }, { + desc: "MPLS implicit replace", + 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.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectNHG(dn, 2, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectMPLS(dn, 42, 2); err != nil { + t.Fatalf("cannot add label entry, %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.InjectNHG(dn, 2, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectMPLS(dn, 42, 1); err != nil { + t.Fatalf("cannot add label entry, %v", err) + } + return r.RIB() + }(), + wantOps: &reconcileOps{ + Replace: &ops{ + TopLevel: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_ADD, + Entry: &spb.AFTOperation_Mpls{ + Mpls: &aftpb.Afts_LabelEntryKey{ + Label: &aftpb.Afts_LabelEntryKey_LabelUint64{ + LabelUint64: 42, + }, + LabelEntry: &aftpb.Afts_LabelEntry{ + NextHopGroup: &wpb.UintValue{Value: 2}, + }, + }, + }, + }}, + }, + }, + }, { + desc: "MPLS explicit replace", + 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.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectNHG(dn, 2, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectMPLS(dn, 42, 2); err != nil { + t.Fatalf("cannot add label entry, %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.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectNHG(dn, 2, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectMPLS(dn, 42, 1); err != nil { + t.Fatalf("cannot add label entry, %v", err) + } + return r.RIB() + }(), + inExplicitReplace: map[spb.AFTType]bool{ + spb.AFTType_MPLS: true, + }, + wantOps: &reconcileOps{ + Replace: &ops{ + TopLevel: []*spb.AFTOperation{{ + Id: 1, + NetworkInstance: dn, + Op: spb.AFTOperation_REPLACE, + Entry: &spb.AFTOperation_Mpls{ + Mpls: &aftpb.Afts_LabelEntryKey{ + Label: &aftpb.Afts_LabelEntryKey_LabelUint64{ + LabelUint64: 42, + }, + LabelEntry: &aftpb.Afts_LabelEntry{ + NextHopGroup: &wpb.UintValue{Value: 2}, + }, + }, + }, + }}, + }, + }, + }, { + desc: "explicit replace for all types", + inSrc: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1, "int42"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + // NHG ID = 1 is unchanged. + if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectNHG(dn, 2, map[uint64]uint64{1: 64}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectMPLS(dn, 42, 2); err != nil { + t.Fatalf("cannot add label entry, %v", err) + } + if err := r.InjectIPv4(dn, "1.0.0.0/24", 2); err != nil { + t.Fatalf("cannot add IPv4 entry, %v", err) + } + return r.RIB() + }(), + inDst: func() *rib.RIB { + r := rib.NewFake(dn) + if err := r.InjectNH(dn, 1, "int10"); err != nil { + t.Fatalf("cannot add NH, %v", err) + } + if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectNHG(dn, 2, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, %v", err) + } + if err := r.InjectMPLS(dn, 42, 1); err != nil { + t.Fatalf("cannot add label entry, %v", err) + } + if err := r.InjectIPv4(dn, "1.0.0.0/24", 1); err != nil { + t.Fatalf("cannot add IPv4 entry, %v", err) + } + return r.RIB() + }(), + inExplicitReplace: map[spb.AFTType]bool{ + spb.AFTType_ALL: true, + }, + wantOps: &reconcileOps{ + 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}, + }, + }, + }, + }, { + Id: 2, + NetworkInstance: dn, + Op: spb.AFTOperation_REPLACE, + Entry: &spb.AFTOperation_Mpls{ + Mpls: &aftpb.Afts_LabelEntryKey{ + Label: &aftpb.Afts_LabelEntryKey_LabelUint64{ + LabelUint64: 42, + }, + LabelEntry: &aftpb.Afts_LabelEntry{ + NextHopGroup: &wpb.UintValue{Value: 2}, + }, + }, + }, + }}, + NHG: []*spb.AFTOperation{{ + Id: 3, + NetworkInstance: dn, + Op: spb.AFTOperation_REPLACE, + 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: 64}, + }, + }}, + }, + }, + }, + }}, + NH: []*spb.AFTOperation{{ + Id: 4, + 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: "int42"}, + }, + }, + }, + }, + }}, + }, + }, }} for _, tt := range tests {