diff --git a/go.mod b/go.mod index 32ff70c0..1aef9bc4 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/openconfig/gribi v1.0.0 github.com/openconfig/lemming v0.3.2-0.20230914210403-c6484d12af0a github.com/openconfig/testt v0.0.0-20220311054427-efbb1a32ec07 - github.com/openconfig/ygot v0.29.10 + github.com/openconfig/ygot v0.29.11-0.20230922181344-6c3af4108a57 go.uber.org/atomic v1.10.0 google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d google.golang.org/grpc v1.58.0-dev diff --git a/go.sum b/go.sum index bca00634..1d0f4239 100644 --- a/go.sum +++ b/go.sum @@ -208,8 +208,8 @@ github.com/openconfig/ygnmi v0.8.7/go.mod h1:7up6qc9l9G4+Cfo37gzO0D7+2g4yqyW+xzh github.com/openconfig/ygot v0.6.0/go.mod h1:o30svNf7O0xK+R35tlx95odkDmZWS9JyWWQSmIhqwAs= github.com/openconfig/ygot v0.10.4/go.mod h1:oCQNdXnv7dWc8scTDgoFkauv1wwplJn5HspHcjlxSAQ= github.com/openconfig/ygot v0.13.2/go.mod h1:kJN0yCXIH07dOXvNBEFm3XxXdnDD5NI6K99tnD5x49c= -github.com/openconfig/ygot v0.29.10 h1:FRZXxyeCdiJXz6uat5uOm3Hlg+PUu2N0mY+eiva12MI= -github.com/openconfig/ygot v0.29.10/go.mod h1:RNnn1ytQ8GZV5LPts36l0cyoRjsYYpruiruJEvmU2sg= +github.com/openconfig/ygot v0.29.11-0.20230922181344-6c3af4108a57 h1:z7bNYTNR1HzxYQNVwBUaf0veA4j4vuTBW7mTLRW4M1w= +github.com/openconfig/ygot v0.29.11-0.20230922181344-6c3af4108a57/go.mod h1:RNnn1ytQ8GZV5LPts36l0cyoRjsYYpruiruJEvmU2sg= github.com/p4lang/p4runtime v1.4.0-rc.5.0.20220728214547-13f0d02a521e h1:AfZKoikDXbZ7zWvO/lvCRzLo7i6lM+gNleYVMxPiWyQ= github.com/p4lang/p4runtime v1.4.0-rc.5.0.20220728214547-13f0d02a521e/go.mod h1:m9laObIMXM9N1ElGXijc66/MSM5eheZJLRLxg/TG+fU= github.com/pborman/getopt v0.0.0-20190409184431-ee0cd42419d3/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= diff --git a/rib/helpers.go b/rib/helpers.go index 836c87f6..717a94ab 100644 --- a/rib/helpers.go +++ b/rib/helpers.go @@ -17,14 +17,16 @@ package rib import ( "fmt" + "github.com/openconfig/ygot/ygot" + aftpb "github.com/openconfig/gribi/v1/proto/gribi_aft" spb "github.com/openconfig/gribi/v1/proto/service" - "github.com/openconfig/ygot/ygot" + wpb "github.com/openconfig/ygot/proto/ywrapper" ) -// RIBFromGetResponses returns a RIB from a slice of gRIBI GetResponse messages. +// FromGetResponses returns a RIB from a slice of gRIBI GetResponse messages. // The supplied defaultName is used as the default network instance name. -func RIBFromGetResponses(defaultName string, responses []*spb.GetResponse) (*RIB, error) { +func FromGetResponses(defaultName string, responses []*spb.GetResponse, opt ...RIBOpt) (*RIB, error) { r := New(defaultName) niAFTs := map[string]*aftpb.Afts{} @@ -70,3 +72,89 @@ func RIBFromGetResponses(defaultName string, responses []*spb.GetResponse) (*RIB return r, nil } + +// fakeRIB is a RIB for use in testing which exposes methods that can be used to more easily +// construct a RIB's contents. +type fakeRIB struct { + r *RIB +} + +// NewFake returns a new Fake RIB. +func NewFake(defaultName string, opt ...RIBOpt) *fakeRIB { + return &fakeRIB{ + r: New(defaultName, opt...), + } +} + +// RIB returns the constructed fake RIB to the caller. +func (f *fakeRIB) RIB() *RIB { + return f.r +} + +// InjectIPv4 adds an IPv4 entry to network instance ni, with the specified +// prefix (pfx), and referencing the specified next-hop-group with index nhg. +// It returns an error if the entry cannot be injected. +func (f *fakeRIB) InjectIPv4(ni, pfx string, nhg uint64) error { + niR, ok := f.r.NetworkInstanceRIB(ni) + if !ok { + return fmt.Errorf("unknown NI, %s", ni) + } + if _, _, err := niR.AddIPv4(&aftpb.Afts_Ipv4EntryKey{ + Prefix: pfx, + Ipv4Entry: &aftpb.Afts_Ipv4Entry{ + NextHopGroup: &wpb.UintValue{Value: nhg}, + }, + }, false); err != nil { + return fmt.Errorf("cannot add IPv4 entry, err: %v", err) + } + + return nil +} + +// 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, +// 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 { + niR, ok := f.r.NetworkInstanceRIB(ni) + if !ok { + return fmt.Errorf("unknown NI, %s", ni) + } + + nhg := &aftpb.Afts_NextHopGroupKey{ + Id: nhgId, + NextHopGroup: &aftpb.Afts_NextHopGroup{}, + } + for nh, weight := range nhs { + nhg.NextHopGroup.NextHop = append(nhg.NextHopGroup.NextHop, &aftpb.Afts_NextHopGroup_NextHopKey{ + Index: nh, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: weight}, + }, + }) + } + + if _, _, err := niR.AddNextHopGroup(nhg, false); err != nil { + return fmt.Errorf("cannot add NHG entry, err: %v", err) + } + + return nil +} + +// 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 { + 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{}, + }, false); err != nil { + return fmt.Errorf("cannot add NH entry, err: %v", err) + } + + return nil +} diff --git a/rib/helpers_test.go b/rib/helpers_test.go index cb0fb461..cee5a58a 100644 --- a/rib/helpers_test.go +++ b/rib/helpers_test.go @@ -27,7 +27,7 @@ import ( "github.com/openconfig/ygot/ygot" ) -func TestRIBFromGetResponses(t *testing.T) { +func TestFromGetResponses(t *testing.T) { defaultName := "DEFAULT" tests := []struct { desc string @@ -240,9 +240,9 @@ func TestRIBFromGetResponses(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - got, err := RIBFromGetResponses(tt.inDefaultName, tt.inResponses) + got, err := FromGetResponses(tt.inDefaultName, tt.inResponses) if (err != nil) != tt.wantErr { - t.Fatalf("RIBFromGetResponses(...): did not get expected error, got: %v, wantErr? %v", err, tt.wantErr) + t.Fatalf("FromGetResponses(...): did not get expected error, got: %v, wantErr? %v", err, tt.wantErr) } if diff := cmp.Diff(got, tt.wantRIB, @@ -251,7 +251,154 @@ func TestRIBFromGetResponses(t *testing.T) { cmp.AllowUnexported(RIBHolder{}), cmpopts.IgnoreFields(RIBHolder{}, "mu", "refCounts", "checkFn"), ); diff != "" { - t.Fatalf("RIBFromGetResponses(...): did not get expected RIB, diff(-got,+want):\n%s", diff) + t.Fatalf("FromGetResponses(...): did not get expected RIB, diff(-got,+want):\n%s", diff) + } + }) + } +} + +func TestFakeRIB(t *testing.T) { + dn := "DEFAULT" + tests := []struct { + desc string + inBuild func() *fakeRIB + wantRIB *RIB + }{{ + desc: "nh only", + inBuild: func() *fakeRIB { + f := NewFake(dn) + if err := f.InjectNH(dn, 1); err != nil { + t.Fatalf("cannot add NH, err: %v", err) + } + return f + }, + wantRIB: &RIB{ + defaultName: dn, + niRIB: map[string]*RIBHolder{ + dn: { + name: dn, + r: &aft.RIB{ + Afts: &aft.Afts{ + NextHop: map[uint64]*aft.Afts_NextHop{ + 1: {Index: ygot.Uint64(1)}, + }, + }, + }, + }, + }, + }, + }, { + desc: "ipv4 only", + inBuild: func() *fakeRIB { + f := NewFake(dn, DisableRIBCheckFn()) + if err := f.InjectIPv4(dn, "1.0.0.0/24", 1); err != nil { + t.Fatalf("cannot add IPv4, err: %v", err) + } + return f + }, + wantRIB: &RIB{ + defaultName: dn, + niRIB: map[string]*RIBHolder{ + dn: { + name: dn, + r: &aft.RIB{ + Afts: &aft.Afts{ + Ipv4Entry: map[string]*aft.Afts_Ipv4Entry{ + "1.0.0.0/24": {Prefix: ygot.String("1.0.0.0/24"), NextHopGroup: ygot.Uint64(1)}, + }, + }, + }, + }, + }, + }, + }, { + desc: "nhg only", + inBuild: func() *fakeRIB { + f := NewFake(dn, DisableRIBCheckFn()) + if err := f.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil { + t.Fatalf("cannot add NHG, err: %v", err) + } + return f + }, + wantRIB: &RIB{ + defaultName: dn, + niRIB: map[string]*RIBHolder{ + dn: { + name: dn, + r: &aft.RIB{ + Afts: &aft.Afts{ + NextHopGroup: map[uint64]*aft.Afts_NextHopGroup{ + 1: { + Id: ygot.Uint64(1), + NextHop: map[uint64]*aft.Afts_NextHopGroup_NextHop{ + 1: { + Index: ygot.Uint64(1), + Weight: ygot.Uint64(1), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, { + desc: "full ipv4 chain", + inBuild: func() *fakeRIB { + f := NewFake(dn) + // Discard the errors, since the test will check whether the entries are there. + f.InjectNH(dn, 1) + f.InjectNHG(dn, 1, map[uint64]uint64{1: 1}) + f.InjectIPv4(dn, "192.0.2.1/32", 1) + return f + }, + wantRIB: &RIB{ + defaultName: dn, + niRIB: map[string]*RIBHolder{ + dn: { + name: dn, + r: &aft.RIB{ + Afts: &aft.Afts{ + NextHop: map[uint64]*aft.Afts_NextHop{ + 1: { + Index: ygot.Uint64(1), + }, + }, + NextHopGroup: map[uint64]*aft.Afts_NextHopGroup{ + 1: { + Id: ygot.Uint64(1), + NextHop: map[uint64]*aft.Afts_NextHopGroup_NextHop{ + 1: { + Index: ygot.Uint64(1), + Weight: ygot.Uint64(1), + }, + }, + }, + }, + Ipv4Entry: map[string]*aft.Afts_Ipv4Entry{ + "192.0.2.1/32": { + Prefix: ygot.String("192.0.2.1/32"), + NextHopGroup: ygot.Uint64(1), + }, + }, + }, + }, + }, + }, + }, + }} + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + got := tt.inBuild().RIB() + if diff := cmp.Diff(got, tt.wantRIB, + cmpopts.EquateEmpty(), cmp.AllowUnexported(RIB{}), + cmpopts.IgnoreFields(RIB{}, "nrMu", "pendMu", "ribCheck"), + cmp.AllowUnexported(RIBHolder{}), + cmpopts.IgnoreFields(RIBHolder{}, "mu", "refCounts", "checkFn"), + ); diff != "" { + t.Fatalf("FakeRIB.RIB(...): did not get expected RIB, diff(-got,+want):\n%s", diff) } }) } diff --git a/rib/reconciler/reconcile.go b/rib/reconciler/reconcile.go new file mode 100644 index 00000000..6bc75cc0 --- /dev/null +++ b/rib/reconciler/reconcile.go @@ -0,0 +1,127 @@ +// Copyright 2023 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package reconciler reconciles the contents of two gRIBI RIBs -- the intended RIB +// is assumed to contain the desired RIB entries, whereas the 'target' RIB is to be +// programmed. The reconciler: +// +// - Uses the messages that are returned from the `Get` RPC to build the contents of +// an external RIB. +// - Calculates a diff between the two RIBs. +// - Sends gRIBI operations to the target RIB to make it consistent with the +// intended RIB. +package reconciler + +import ( + "context" + "fmt" + + "github.com/openconfig/gribigo/rib" + + spb "github.com/openconfig/gribi/v1/proto/service" +) + +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 +// reconciler. It allows the RIB contents to be retrieved and programmed either +// via gRIBI or from a local RIB cache. +type RIBTarget interface { + // Get returns a RIB containing all network-instances and AFTs that are + // supported by the RIB. + Get(context.Context) (*rib.RIB, error) + // CleanUp is called to indicate that the RIBTarget should remove any + // state or external connections as it is no longer required. + CleanUp() +} + +// LocalRIB wraps a RIB that is locally available on the system as a gRIBIgo +// RIB type. +type LocalRIB struct { + r *rib.RIB +} + +// Get returns the contents of the local RIB. +func (l *LocalRIB) Get(_ context.Context) (*rib.RIB, error) { + return l.r, nil +} + +// CleanUp implements the RIBTarget interface. No local cleanup is required. +func (l *LocalRIB) CleanUp() {} + +var ( + // Compile time check that LocalRIB implements the RIBTarget interface. + _ RIBTarget = &LocalRIB{} +) + +// New returns a new reconciler with the specified intended and target RIBs. +func New(intended, target RIBTarget) *R { + return &R{ + intended: intended, + target: target, + } +} + +// Reconcile performs a reconciliation operation between the intended and specified +// remote RIB. +func (r *R) Reconcile(ctx context.Context) 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) + } + + tRIB, err := r.target.Get(ctx) + if err != nil { + return fmt.Errorf("cannot reconcile RIBs, cannot get contents of target, %v", err) + } + + // Perform diff on their contents. + + diffs, err := diff(iRIB, tRIB) + if err != nil { + return 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") + +} + +// 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: +// +// - entries that are present in src but not dst are returned as ADD +// operations. +// - entries that are present in src but not dst and their contents diff are +// returned as MODIFY operations. +// - entries that are not present in src but are present in dst are returned +// as DELETE operations. +func diff(src, dst *rib.RIB) ([]*spb.AFTOperation, error) { + return nil, fmt.Errorf("unimplemented") +} diff --git a/rib/reconciler/reconcile_test.go b/rib/reconciler/reconcile_test.go new file mode 100644 index 00000000..39170435 --- /dev/null +++ b/rib/reconciler/reconcile_test.go @@ -0,0 +1,35 @@ +package reconciler + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/openconfig/gribigo/rib" +) + +func TestLocalRIB(t *testing.T) { + + t.Run("get", func(t *testing.T) { + l := &LocalRIB{ + r: rib.New("DEFAULT"), + } + + want := rib.New("DEFAULT") + got, err := l.Get(context.Background()) + if err != nil { + t.Fatalf("(*LocalRIB).Get(): did not get expected error, got: %v, want: nil", err) + } + + if diff := cmp.Diff(got, want, + cmpopts.EquateEmpty(), cmp.AllowUnexported(rib.RIB{}), + cmpopts.IgnoreFields(rib.RIB{}, "nrMu", "pendMu", "ribCheck"), + cmp.AllowUnexported(rib.RIBHolder{}), + cmpopts.IgnoreFields(rib.RIBHolder{}, "mu", "refCounts", "checkFn"), + ); diff != "" { + t.Fatalf("(*LocalRIB).Get(): did not get expected results, diff(-got,+want):\n%s", diff) + } + }) + +} diff --git a/rib/reconciler/remote.go b/rib/reconciler/remote.go new file mode 100644 index 00000000..cdce9643 --- /dev/null +++ b/rib/reconciler/remote.go @@ -0,0 +1,67 @@ +package reconciler + +import ( + "context" + "fmt" + + "github.com/openconfig/gribigo/client" + "github.com/openconfig/gribigo/rib" + + spb "github.com/openconfig/gribi/v1/proto/service" +) + +// RemoteRIB implements the RIBTarget interface and wraps a remote gRIBI RIB. +// The contents are accessed via the gRIBI gRPC API. +type RemoteRIB struct { + c *client.Client + + defaultName string +} + +// NewRemoteRIB returns a new remote gRIBI RIB. The context supplied is used to +// dial the remote gRIBI server at the address 'addr'. the 'defName' argument +// is used to identify the name of the default network instance on the server. +func NewRemoteRIB(ctx context.Context, defName, addr string) (*RemoteRIB, error) { + gc, err := client.New() + if err != nil { + return nil, fmt.Errorf("cannot create gRIBI client, %v", err) + } + + r := &RemoteRIB{ + c: gc, + defaultName: defName, + } + + if err := r.c.Dial(ctx, addr); err != nil { + return nil, fmt.Errorf("cannot dial remote server, %v", err) + } + return r, nil +} + +// CleanUp closes the remote connection to the gRIBI server. +func (r *RemoteRIB) CleanUp() { + r.c.Close() +} + +// Get retrieves the contents of the remote gRIBI server's RIB and returns it as a +// gRIBIgo RIB struct. The context is used for a Get RPC call to the remote server. +func (r *RemoteRIB) Get(ctx context.Context) (*rib.RIB, error) { + resp, err := r.c.Get(ctx, &spb.GetRequest{ + NetworkInstance: &spb.GetRequest_All{ + All: &spb.Empty{}, + }, + Aft: spb.AFTType_ALL, + }) + if err != nil { + return nil, fmt.Errorf("cannot get remote RIB, %v", err) + } + + // We always disable the RIB checking function because we want to see entries that have + // not got valid references so that we can reconcile them. + remRIB, err := rib.FromGetResponses(r.defaultName, []*spb.GetResponse{resp}, rib.DisableRIBCheckFn()) + if err != nil { + return nil, fmt.Errorf("cannot build remote RIB from responses, %v", err) + } + + return remRIB, nil +} diff --git a/rib/reconciler/remote_test.go b/rib/reconciler/remote_test.go new file mode 100644 index 00000000..468fa52b --- /dev/null +++ b/rib/reconciler/remote_test.go @@ -0,0 +1,184 @@ +package reconciler + +import ( + "context" + "net" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/openconfig/gribigo/aft" + "github.com/openconfig/gribigo/rib" + "github.com/openconfig/gribigo/server" + "github.com/openconfig/gribigo/testcommon" + "github.com/openconfig/ygot/ygot" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/status" + + spb "github.com/openconfig/gribi/v1/proto/service" +) + +func newServer(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, err := server.NewFake() + if err != nil { + t.Fatalf("cannot create server, got err: %v", err) + } + s.InjectRIB(r) + + 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 TestNewRemoteRIB(t *testing.T) { + tests := []struct { + desc string + inDefName string + inAddrOverride string + wantErr bool + }{{ + desc: "successful dial", + inDefName: "DEFAULT", + }, { + desc: "unsuccessful dial", + inDefName: "DEFAULT", + inAddrOverride: "invalid.addr:999999", + wantErr: true, + }} + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + addr, stop := newServer(t, nil) + defer stop() + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + + if tt.inAddrOverride != "" { + addr = tt.inAddrOverride + } + + if _, err := NewRemoteRIB(ctx, tt.inDefName, addr); (err != nil) != tt.wantErr { + t.Fatalf("NewRemoteRIB(ctx, %s, %s): did not get expected error, got: %v, wantErr? %v", tt.inDefName, addr, err, tt.wantErr) + } + }) + } +} + +type badGRIBI struct { + *spb.UnimplementedGRIBIServer +} + +func (b *badGRIBI) Get(_ *spb.GetRequest, _ spb.GRIBI_GetServer) error { + return status.Errorf(codes.Unimplemented, "RPC unimplemented") +} + +func newBadServer(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 := &badGRIBI{} + + 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) { + dn := "DEFAULT" + tests := []struct { + desc string + inDefName string + inServer func(*testing.T, *rib.RIB) (string, func()) + inInjectedRIB *rib.RIB + wantRIBContents map[string]*aft.RIB + wantErr bool + }{{ + desc: "cannot get RIB", + inDefName: "DEFAULT", + inServer: newBadServer, + wantErr: true, + }, { + desc: "successfully got RIB", + inDefName: dn, + inServer: newServer, + inInjectedRIB: 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, %v", err) + } + if err := r.InjectIPv4(dn, "1.0.0.0/24", 1); err != nil { + t.Fatalf("cannot add IPv4, %v", err) + } + return r.RIB() + }(), + wantRIBContents: map[string]*aft.RIB{ + dn: func() *aft.RIB { + r := &aft.RIB{} + 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) + return r + }(), + }, + }} + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + addr, stop := tt.inServer(t, tt.inInjectedRIB) + defer stop() + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + + rr, err := NewRemoteRIB(ctx, tt.inDefName, addr) + if err != nil { + t.Fatalf("NewRemoteRIB(_, %s, %s): cannot connect, got err: %v", tt.inDefName, addr, err) + } + + got, err := rr.Get(ctx) + if (err != nil) != tt.wantErr { + t.Fatalf("(*RemoteRIB).Get(ctx): did not get expected err, got: %v, wantErr? %v", err, tt.wantErr) + } + + if err != nil { + return + } + + // We can't introspect the private RIB contents, so make a copy to introspect. + gotContents, err := got.RIBContents() + if err != nil { + t.Fatalf("(*RemoteRIB).Get(ctx).RIBContents(): can't introspect RIB, err: %v", err) + } + + if diff := cmp.Diff(gotContents, tt.wantRIBContents); diff != "" { + t.Fatalf("(*RemoteRIB).Get(ctx): did not get expected contents, diff(-got,+want):\n%s", diff) + } + + }) + } +} diff --git a/rib/rib.go b/rib/rib.go index b0b17a51..8a70373b 100644 --- a/rib/rib.go +++ b/rib/rib.go @@ -324,6 +324,15 @@ func (r *RIB) KnownNetworkInstances() []string { return names } +// RIBContents returns the contents of the RIB in a manner that an external +// caller can interact with. It returns a map, keyed by network instance name, +// with a deep copy of the RIB contents. Since copying large RIBs may be expensive +// care should be taken with when it is used. A copy is used since the RIB continues +// to handle concurrent changes to the contents from multiple sources. +func (r *RIB) RIBContents() (map[string]*aft.RIB, error) { + return r.copyRIBs() +} + // String returns a string representation of the RIB. func (r *RIB) String() string { r.nrMu.RLock() @@ -542,6 +551,11 @@ func (r *RIB) callResolvedEntryHook(optype constants.OpType, netinst string, aft // AFT struct, of the set of RIBs stored by the instance r. A DeepCopy of the RIBs is returned, // along with an error that indicates whether the entries could be copied. func (r *RIB) copyRIBs() (map[string]*aft.RIB, error) { + // TODO(robjs): Consider whether we need finer grained locking for each network + // instance RIB rather than holding the lock whilst we clone the contents. + r.nrMu.RLock() + defer r.nrMu.RUnlock() + rib := map[string]*aft.RIB{} for name, niR := range r.niRIB { // this is likely expensive on very large RIBs, but with today's implementatiom @@ -1836,7 +1850,8 @@ func protoFromGoStruct(s ygot.ValidatedGoStruct, prefix *gpb.Path, pb proto.Mess if err := protomap.ProtoFromPaths(pb, vals, protomap.ProtobufMessagePrefix(prefix), protomap.ValuePathPrefix(prefix), - protomap.IgnoreExtraPaths()); err != nil { + protomap.IgnoreExtraPaths(), + ); err != nil { return fmt.Errorf("cannot unmarshal gNMI paths, %v", err) } diff --git a/rib/rib_test.go b/rib/rib_test.go index 73040268..817785ab 100644 --- a/rib/rib_test.go +++ b/rib/rib_test.go @@ -851,6 +851,46 @@ func TestIndividualDeleteEntryFunctions(t *testing.T) { } } +func TestConcreteNHGProto(t *testing.T) { + tests := []struct { + desc string + inEntry *aft.Afts_NextHopGroup + want *aftpb.Afts_NextHopGroupKey + wantErr bool + }{{ + desc: "populated nhg", + inEntry: func() *aft.Afts_NextHopGroup { + a := &aft.Afts_NextHopGroup{} + a.Id = ygot.Uint64(1) + a.GetOrCreateNextHop(1).Weight = ygot.Uint64(1) + return a + }(), + want: &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}, + }, + }}, + }, + }, + }} + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + got, err := concreteNextHopGroupProto(tt.inEntry) + if (err != nil) != tt.wantErr { + t.Fatalf("did not get expected error, got: %v, want: %v", err, tt.wantErr) + } + if diff := cmp.Diff(got, tt.want, protocmp.Transform(), cmpopts.EquateEmpty()); diff != "" { + t.Fatalf("did not get expected proto, diff(-got,+want):\n%s", diff) + } + }) + } +} + func TestConcreteIPv4Proto(t *testing.T) { tests := []struct { desc string @@ -3176,8 +3216,15 @@ func TestGetRIB(t *testing.T) { NetworkInstance: "VRF-42", Entry: &spb.AFTEntry_NextHopGroup{ NextHopGroup: &aftpb.Afts_NextHopGroupKey{ - Id: 42, - NextHopGroup: &aftpb.Afts_NextHopGroup{}, + Id: 42, + NextHopGroup: &aftpb.Afts_NextHopGroup{ + NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{ + Index: 1, + NextHop: &aftpb.Afts_NextHopGroup_NextHop{ + Weight: &wpb.UintValue{Value: 1}, + }, + }}, + }, }, }, }}, @@ -4229,3 +4276,102 @@ func TestFlush(t *testing.T) { } } } + +func TestRIBContents(t *testing.T) { + tests := []struct { + desc string + inRIB *RIB + want map[string]*aft.RIB + wantErr bool + }{{ + desc: "one NI", + inRIB: &RIB{ + niRIB: map[string]*RIBHolder{ + "default": { + r: &aft.RIB{ + Afts: &aft.Afts{ + Ipv4Entry: map[string]*aft.Afts_Ipv4Entry{ + "1.0.0.0/24": { + Prefix: ygot.String("1.0.0.0/24"), + }, + }, + }, + }, + }, + }, + }, + want: map[string]*aft.RIB{ + "default": { + Afts: &aft.Afts{ + Ipv4Entry: map[string]*aft.Afts_Ipv4Entry{ + "1.0.0.0/24": { + Prefix: ygot.String("1.0.0.0/24"), + }, + }, + }, + }, + }, + }, { + desc: "two NIs", + inRIB: &RIB{ + niRIB: map[string]*RIBHolder{ + "default": { + r: &aft.RIB{ + Afts: &aft.Afts{ + Ipv4Entry: map[string]*aft.Afts_Ipv4Entry{ + "1.0.0.0/24": { + Prefix: ygot.String("1.0.0.0/24"), + }, + }, + }, + }, + }, + "pepsicola": { + r: &aft.RIB{ + Afts: &aft.Afts{ + NextHop: map[uint64]*aft.Afts_NextHop{ + 1: { + Index: ygot.Uint64(1), + }, + }, + }, + }, + }, + }, + }, + want: map[string]*aft.RIB{ + "default": { + Afts: &aft.Afts{ + Ipv4Entry: map[string]*aft.Afts_Ipv4Entry{ + "1.0.0.0/24": { + Prefix: ygot.String("1.0.0.0/24"), + }, + }, + }, + }, + "pepsicola": { + Afts: &aft.Afts{ + NextHop: map[uint64]*aft.Afts_NextHop{ + 1: { + Index: ygot.Uint64(1), + }, + }, + }, + }, + }, + }} + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + got, err := tt.inRIB.RIBContents() + if (err != nil) != tt.wantErr { + t.Fatalf("(*RIB).RIBContents(): did not get expected error, got: %v, wantErr? %v", err, tt.wantErr) + } + + if diff := cmp.Diff(got, tt.want); diff != "" { + t.Fatalf("(*RIB).RIBContents(): did not get expected contents, diff(-got,+want):\n%s", diff) + } + }) + } + +} diff --git a/server/server.go b/server/server.go index aaa953bc..1e8d6f19 100644 --- a/server/server.go +++ b/server/server.go @@ -1009,7 +1009,7 @@ func checkElectionForModify(opID uint64, opElecID *spb.Uint128, election *electi return nil, true, nil } -// doGet implents the Get RPC for the gRIBI server. It handles the input GetRequest, writing +// doGet implements the Get RPC for the gRIBI server. It handles the input GetRequest, writing // the set of GetResponses to the specified msgCh. When the Get is done, the function writes to // doneCh such that the caller knows that the work that is being done is complete. If a message // is received on stopCh the function returns. Any errors that are experienced are written to