From d411abfbc067300ed799372381c1b64a2a96969e Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Thu, 11 Jul 2024 18:45:36 -0700 Subject: [PATCH 1/9] Add support for plumbing gRIBI server options through lemming. * (M) lemming.go * (M) gribi/gribi.go - for some behaviours, there is a need to change the underlying RIB functionality in the gRIBI server as a lemming is created. this change plumbs some options through to allow lemmings to be created using this method. --- gribi/gribi.go | 15 ++++++++++----- lemming.go | 21 ++++++++++++++++++++- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/gribi/gribi.go b/gribi/gribi.go index d86ed3e64..7ea8cdb93 100644 --- a/gribi/gribi.go +++ b/gribi/gribi.go @@ -53,8 +53,10 @@ type Server struct { // installed. // - root, if specified, will be used to populate connected routes into the RIB // manager. Note this is intended to be used for unit/standalone device testing. -func New(s *grpc.Server, gClient gpb.GNMIClient, target string, root *oc.Root, sysribAddr string) (*Server, error) { - gs, err := createGRIBIServer(gClient, target, root, sysribAddr) +// - opts, if specified, will be used to control the underlying gRIBI server's +// behaviours. +func New(s *grpc.Server, gClient gpb.GNMIClient, target string, root *oc.Root, sysribAddr string, opts ...server.ServerOpt) (*Server, error) { + gs, err := createGRIBIServer(gClient, target, root, sysribAddr, opts...) if err != nil { return nil, fmt.Errorf("cannot create gRIBI server, %v", err) } @@ -73,7 +75,10 @@ func New(s *grpc.Server, gClient gpb.GNMIClient, target string, root *oc.Root, s // // - root, if specified, will be used to populate connected routes into the RIB // manager. Note this is intended to be used for unit/standalone device testing. -func createGRIBIServer(gClient gpb.GNMIClient, target string, root *oc.Root, sysribAddr string) (*server.Server, error) { +// +// The ServerOpt slice provided is handed to the gRIBI fake server to control its +// behaviour. +func createGRIBIServer(gClient gpb.GNMIClient, target string, root *oc.Root, sysribAddr string, opts ...server.ServerOpt) (*server.Server, error) { gzebraConn, err := grpc.Dial(sysribAddr, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { return nil, fmt.Errorf("cannot dial to sysrib, %v", err) @@ -153,11 +158,11 @@ func createGRIBIServer(gClient gpb.GNMIClient, target string, root *oc.Root, sys log.Infof("Sent route %v with response %v", routeReq, resp) } - return server.New( + return server.New(append([]server.ServerOpt{ server.WithPostChangeRIBHook(ribHookfn), server.WithRIBResolvedEntryHook(ribAddfn), server.WithVRFs(networkInstances), - ) + }, opts...)) } // createSetRouteRequest converts a Route to a sysrib SetRouteRequest diff --git a/lemming.go b/lemming.go index 261ecbe4c..c3f03b982 100644 --- a/lemming.go +++ b/lemming.go @@ -27,6 +27,8 @@ import ( "google.golang.org/grpc/reflection" "k8s.io/klog/v2" + gribis "github.com/openconfig/gribigo/server" + "github.com/openconfig/lemming/bgp" "github.com/openconfig/lemming/dataplane" "github.com/openconfig/lemming/dataplane/dplaneopts" @@ -85,6 +87,10 @@ type opt struct { bgpPort uint16 dataplane bool dataplaneOpts []dplaneopts.Option + // disableRIBForwardReferences indicates that the lemming's RIB should not + // allow for forward references (e.g., next-hop-groups that reference + // next-hops that do not yet exist). + disableRIBForwardReferences bool } // resolveOpts applies all the options and returns a struct containing the result. @@ -176,6 +182,15 @@ func WithSysribAddr(sysribAddr string) Option { } } +// WithNoRIBForwardReferences specifies that the lemming's RIB should not +// allow forward references - e.g., a next-hop-group that references a next-hop +// that does not exist. +func WithNoRIBForwardReferences() Option { + return func(o *opt) { + o.disableRIBForwardReferences = true + } +} + // New returns a new initialized device. func New(targetName, zapiURL string, opts ...Option) (*Device, error) { var dplane *dataplane.Dataplane @@ -248,7 +263,11 @@ func New(targetName, zapiURL string, opts ...Option) (*Device, error) { log.Info("starting gRIBI") // TODO(wenbli): Use gRIBIs once we change lemming's KNE config to use different ports. // gRIBIs := grpc.NewServer() - gribiServer, err := fgribi.New(s, cacheClient, targetName, root, fmt.Sprintf("unix:%s", resolvedOpts.sysribAddr)) + gribiOpts := []gribis.ServerOpt{} + if resolvedOpts.disableRIBForwardReferences { + gribiOpts = append(gribiOpts, gribis.WithNoRIBForwardReferences()) + } + gribiServer, err := fgribi.New(s, cacheClient, targetName, root,fmt.Sprintf("unix:%s", resolvedOpts.sysribAddr) gribiOpts...) if err != nil { return nil, err } From 39507a1c1ab2929c31791ae5a4ca6e93922d54d7 Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Thu, 11 Jul 2024 18:49:40 -0700 Subject: [PATCH 2/9] Move to development gribigo commit. --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index fc30dfdbf..e1124e0aa 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,7 @@ require ( github.com/openconfig/gnsi v1.4.5 github.com/openconfig/goyang v1.4.5 github.com/openconfig/gribi v1.0.0 - github.com/openconfig/gribigo v0.0.0-20240314002941-b59d04db23bd + github.com/openconfig/gribigo v0.0.0-20240712014900-6e0dad60c17a github.com/openconfig/kne v0.1.18 github.com/openconfig/magna v0.0.0-20240326180454-518e16696c84 github.com/openconfig/ondatra v0.5.8 diff --git a/go.sum b/go.sum index 3cf955d8d..51a409072 100644 --- a/go.sum +++ b/go.sum @@ -1781,6 +1781,8 @@ github.com/openconfig/gribi v1.0.0 h1:xMwEg0mBD+21mOxuFOw0d9dBKuIPwJEhMUUeUulZdL github.com/openconfig/gribi v1.0.0/go.mod h1:VFqGH2ZPFIfnKTimP4/AQB4OK0eySW5muJNFxXAwP6k= github.com/openconfig/gribigo v0.0.0-20240314002941-b59d04db23bd h1:MwzM4PieAaHBYDTUd5wMFqzVTzerf3OvHdLmjv7mD+g= github.com/openconfig/gribigo v0.0.0-20240314002941-b59d04db23bd/go.mod h1:oMzEmz+ba6QQGNSPMwxJhZxx6gF5GBFnuuQPv6rd0oU= +github.com/openconfig/gribigo v0.0.0-20240712014900-6e0dad60c17a h1:KTuSH6nuZUQxarrctK/Lw5EzjALKpBYX+U0wqum1/S4= +github.com/openconfig/gribigo v0.0.0-20240712014900-6e0dad60c17a/go.mod h1:wMFhIe5tmza/FLGS5XtY3JCZfF7U81yyLCWbm8cTETM= github.com/openconfig/grpctunnel v0.0.0-20220819142823-6f5422b8ca70 h1:t6SvvdfWCMlw0XPlsdxO8EgO+q/fXnTevDjdYREKFwU= github.com/openconfig/grpctunnel v0.0.0-20220819142823-6f5422b8ca70/go.mod h1:OmTWe7RyZj2CIzIgy4ovEBzCLBJzRvWSZmn7u02U9gU= github.com/openconfig/kne v0.1.18 h1:8D9SexWhj6knxfvEficyVj0F13GIvF1pQz7TKwVDSUI= From b7aa4717b5b87c5a900baac910d2c372e1191f1f Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Thu, 11 Jul 2024 19:03:11 -0700 Subject: [PATCH 3/9] Fix syntax error. --- gribi/gribi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gribi/gribi.go b/gribi/gribi.go index 7ea8cdb93..ad95accba 100644 --- a/gribi/gribi.go +++ b/gribi/gribi.go @@ -162,7 +162,7 @@ func createGRIBIServer(gClient gpb.GNMIClient, target string, root *oc.Root, sys server.WithPostChangeRIBHook(ribHookfn), server.WithRIBResolvedEntryHook(ribAddfn), server.WithVRFs(networkInstances), - }, opts...)) + }, opts...)...) } // createSetRouteRequest converts a Route to a sysrib SetRouteRequest From 9934aad2acfcd89e62c3886cdebc01044e3b0ebe Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Thu, 11 Jul 2024 19:05:57 -0700 Subject: [PATCH 4/9] Fix BUILD. --- BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/BUILD b/BUILD index a883bce5f..94541ba62 100644 --- a/BUILD +++ b/BUILD @@ -29,6 +29,7 @@ go_library( "//p4rt", "//sysrib", "@com_github_golang_glog//:glog", + "@com_github_openconfig_gribigo//server" "@io_k8s_klog_v2//:klog", "@org_golang_google_grpc//:go_default_library", "@org_golang_google_grpc//credentials", From 0bf85eb50752c0dbc1cdf15fcb84a21fa48464b1 Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Thu, 11 Jul 2024 19:06:45 -0700 Subject: [PATCH 5/9] Fix typo. --- lemming.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemming.go b/lemming.go index c3f03b982..536dc2ce0 100644 --- a/lemming.go +++ b/lemming.go @@ -267,7 +267,7 @@ func New(targetName, zapiURL string, opts ...Option) (*Device, error) { if resolvedOpts.disableRIBForwardReferences { gribiOpts = append(gribiOpts, gribis.WithNoRIBForwardReferences()) } - gribiServer, err := fgribi.New(s, cacheClient, targetName, root,fmt.Sprintf("unix:%s", resolvedOpts.sysribAddr) gribiOpts...) + gribiServer, err := fgribi.New(s, cacheClient, targetName, root,fmt.Sprintf("unix:%s", resolvedOpts.sysribAddr), gribiOpts...) if err != nil { return nil, err } From 42c5d356d3136e143702a33f07b725463c5647f2 Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Thu, 11 Jul 2024 19:19:45 -0700 Subject: [PATCH 6/9] Fix BUILD again. --- BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BUILD b/BUILD index 94541ba62..7bae965fa 100644 --- a/BUILD +++ b/BUILD @@ -29,7 +29,7 @@ go_library( "//p4rt", "//sysrib", "@com_github_golang_glog//:glog", - "@com_github_openconfig_gribigo//server" + "@com_github_openconfig_gribigo//server", "@io_k8s_klog_v2//:klog", "@org_golang_google_grpc//:go_default_library", "@org_golang_google_grpc//credentials", From 3984457bddbe12c38863ffdfd20207ae675dcadb Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Thu, 11 Jul 2024 19:20:34 -0700 Subject: [PATCH 7/9] gofmt. --- lemming.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemming.go b/lemming.go index 536dc2ce0..6c1f3b569 100644 --- a/lemming.go +++ b/lemming.go @@ -267,7 +267,7 @@ func New(targetName, zapiURL string, opts ...Option) (*Device, error) { if resolvedOpts.disableRIBForwardReferences { gribiOpts = append(gribiOpts, gribis.WithNoRIBForwardReferences()) } - gribiServer, err := fgribi.New(s, cacheClient, targetName, root,fmt.Sprintf("unix:%s", resolvedOpts.sysribAddr), gribiOpts...) + gribiServer, err := fgribi.New(s, cacheClient, targetName, root, fmt.Sprintf("unix:%s", resolvedOpts.sysribAddr), gribiOpts...) if err != nil { return nil, err } From 8ec146e5fc82727efcc3757210b2750adc514e2d Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Fri, 12 Jul 2024 20:45:04 +0000 Subject: [PATCH 8/9] Fix BUILD and test. --- BUILD | 2 +- go.sum | 2 -- repositories.bzl | 14 ++++---------- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/BUILD b/BUILD index 7bae965fa..d15abac49 100644 --- a/BUILD +++ b/BUILD @@ -29,7 +29,7 @@ go_library( "//p4rt", "//sysrib", "@com_github_golang_glog//:glog", - "@com_github_openconfig_gribigo//server", + "@com_github_openconfig_gribigo//server", "@io_k8s_klog_v2//:klog", "@org_golang_google_grpc//:go_default_library", "@org_golang_google_grpc//credentials", diff --git a/go.sum b/go.sum index 51a409072..98fab232b 100644 --- a/go.sum +++ b/go.sum @@ -1779,8 +1779,6 @@ github.com/openconfig/goyang v1.4.5/go.mod h1:sdNZi/wdTZyLNBNfgLzmmbi7kISm7FskMD github.com/openconfig/gribi v0.1.1-0.20210423184541-ce37eb4ba92f/go.mod h1:OoH46A2kV42cIXGyviYmAlGmn6cHjGduyC2+I9d/iVs= github.com/openconfig/gribi v1.0.0 h1:xMwEg0mBD+21mOxuFOw0d9dBKuIPwJEhMUUeUulZdLg= github.com/openconfig/gribi v1.0.0/go.mod h1:VFqGH2ZPFIfnKTimP4/AQB4OK0eySW5muJNFxXAwP6k= -github.com/openconfig/gribigo v0.0.0-20240314002941-b59d04db23bd h1:MwzM4PieAaHBYDTUd5wMFqzVTzerf3OvHdLmjv7mD+g= -github.com/openconfig/gribigo v0.0.0-20240314002941-b59d04db23bd/go.mod h1:oMzEmz+ba6QQGNSPMwxJhZxx6gF5GBFnuuQPv6rd0oU= github.com/openconfig/gribigo v0.0.0-20240712014900-6e0dad60c17a h1:KTuSH6nuZUQxarrctK/Lw5EzjALKpBYX+U0wqum1/S4= github.com/openconfig/gribigo v0.0.0-20240712014900-6e0dad60c17a/go.mod h1:wMFhIe5tmza/FLGS5XtY3JCZfF7U81yyLCWbm8cTETM= github.com/openconfig/grpctunnel v0.0.0-20220819142823-6f5422b8ca70 h1:t6SvvdfWCMlw0XPlsdxO8EgO+q/fXnTevDjdYREKFwU= diff --git a/repositories.bzl b/repositories.bzl index 8b33825d2..fb655cc23 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -1437,8 +1437,8 @@ def go_repositories(): go_repository( name = "com_github_openconfig_gribigo", importpath = "github.com/openconfig/gribigo", - sum = "h1:MwzM4PieAaHBYDTUd5wMFqzVTzerf3OvHdLmjv7mD+g=", - version = "v0.0.0-20240314002941-b59d04db23bd", + sum = "h1:KTuSH6nuZUQxarrctK/Lw5EzjALKpBYX+U0wqum1/S4=", + version = "v0.0.0-20240712014900-6e0dad60c17a", ) go_repository( name = "com_github_openconfig_grpctunnel", @@ -1882,12 +1882,6 @@ def go_repositories(): sum = "h1:Oeaw1EM2JMxD51g9uhtC0D7erkIjgmj8+JZc26m1YX8=", version = "v0.0.4", ) - go_repository( - name = "com_github_wenovus_gobgp_v3", - importpath = "github.com/wenovus/gobgp/v3", - sum = "h1:jse5eORjbrlTIOPzOO3cpm4feJ16ZCntxzAHSdcWuy4=", - version = "v3.0.0-20230831013712-6d33842cbf42", - ) go_repository( name = "com_github_wi2l_jsondiff", importpath = "github.com/wI2L/jsondiff", @@ -3078,8 +3072,8 @@ def go_repositories(): name = "org_golang_google_grpc", build_file_proto_mode = "disable", importpath = "google.golang.org/grpc", - sum = "h1:KH3VH9y/MgNQg1dE7b3XfVK0GsPSIzJwdF617gUSbvY=", - version = "v1.64.0", + sum = "h1:LKtvyfbX3UGVPFcGqJ9ItpVWW6oN/2XqTxfAnwRRXiA=", + version = "v1.64.1", ) go_repository( name = "org_golang_google_grpc_cmd_protoc_gen_go_grpc", From 8bc4aabf2c7157c4ec4171df9a2e4c0d276f23a3 Mon Sep 17 00:00:00 2001 From: Rob Shakir Date: Tue, 16 Jul 2024 12:40:50 -0700 Subject: [PATCH 9/9] Switch to a cleaner gRIBI options passing mechanism. --- lemming.go | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/lemming.go b/lemming.go index 6c1f3b569..11f5ea25e 100644 --- a/lemming.go +++ b/lemming.go @@ -87,10 +87,7 @@ type opt struct { bgpPort uint16 dataplane bool dataplaneOpts []dplaneopts.Option - // disableRIBForwardReferences indicates that the lemming's RIB should not - // allow for forward references (e.g., next-hop-groups that reference - // next-hops that do not yet exist). - disableRIBForwardReferences bool + gribiOpts []gribis.ServerOpt } // resolveOpts applies all the options and returns a struct containing the result. @@ -182,12 +179,11 @@ func WithSysribAddr(sysribAddr string) Option { } } -// WithNoRIBForwardReferences specifies that the lemming's RIB should not -// allow forward references - e.g., a next-hop-group that references a next-hop -// that does not exist. -func WithNoRIBForwardReferences() Option { +// WithGRIBIOpts specifies the set of gRIBI options that should be passed to +// the gRIBI server that lemming runs. +func WithGRIBIOpts(opts ...gribis.ServerOpt) Option { return func(o *opt) { - o.disableRIBForwardReferences = true + o.gribiOpts = opts } } @@ -263,11 +259,7 @@ func New(targetName, zapiURL string, opts ...Option) (*Device, error) { log.Info("starting gRIBI") // TODO(wenbli): Use gRIBIs once we change lemming's KNE config to use different ports. // gRIBIs := grpc.NewServer() - gribiOpts := []gribis.ServerOpt{} - if resolvedOpts.disableRIBForwardReferences { - gribiOpts = append(gribiOpts, gribis.WithNoRIBForwardReferences()) - } - gribiServer, err := fgribi.New(s, cacheClient, targetName, root, fmt.Sprintf("unix:%s", resolvedOpts.sysribAddr), gribiOpts...) + gribiServer, err := fgribi.New(s, cacheClient, targetName, root, fmt.Sprintf("unix:%s", resolvedOpts.sysribAddr), resolvedOpts.gribiOpts...) if err != nil { return nil, err }