diff --git a/chk/chk.go b/chk/chk.go index a5509bcc..ceac4757 100644 --- a/chk/chk.go +++ b/chk/chk.go @@ -21,14 +21,19 @@ package chk import ( + "bytes" + "fmt" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/openconfig/gribigo/client" + "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/testing/protocmp" + + spb "google.golang.org/genproto/googleapis/rpc/status" ) // resultOpt is an interface implemented by all options that can be @@ -91,7 +96,13 @@ func HasResult(t testing.TB, res []*client.OpResult, want *client.OpResult, opt } } if !found { - t.Fatalf("results did not contain a result of value %s, got: %v", want, res) + buf := &bytes.Buffer{} + buf.WriteString(fmt.Sprintf("results did not contain a result of value %s\n", want)) + buf.WriteString("got:\n") + for _, r := range res { + buf.WriteString(fmt.Sprintf("\t%s\n", r)) + } + t.Fatalf(buf.String()) } } @@ -123,22 +134,54 @@ func HasNRecvErrors(t testing.TB, err error, count int) { } } +// ErrorOpt is an interface that is implemented by functions that examine errrors. +type ErrorOpt interface { + isErrorOpt() +} + +// allowUnimplemented is the internal representation of an ErrorOpt that allows for +// an error that is unimplemented or a specific error. +type allowUnimplemented struct{} + +// isErrorOpt marks allowUnimplemented as an ErrorOpt. +func (*allowUnimplemented) isErrorOpt() {} + +// AllowUnimplemented specifies that receive error with a particular status can be unimplemented +// OR the specified error type. It can be used to not return a fatal error when a server does +// not support a particular functionality. +func AllowUnimplemented() *allowUnimplemented { + return &allowUnimplemented{} +} + // HasRecvClientErrorWithStatus checks whether the supplied ClientErr ce contains a status with // the code and details set to the values supplied in want. -func HasRecvClientErrorWithStatus(t testing.TB, err error, want *status.Status) { +func HasRecvClientErrorWithStatus(t testing.TB, err error, want *status.Status, opts ...ErrorOpt) { t.Helper() + okMsgs := []*status.Status{want} + for _, o := range opts { + if _, ok := o.(*allowUnimplemented); ok { + uProto := proto.Clone(want.Proto()).(*spb.Status) + uProto.Code = int32(codes.Unimplemented) + unimpl := status.FromProto(uProto) + okMsgs = append(okMsgs, unimpl) + } + } + var found bool ce := clientError(t, err) for _, e := range ce.Recv { - s, ok := status.FromError(e) - if !ok { - continue - } - ns := s.Proto() - ns.Message = "" // blank out message so that we don't compare it. - if proto.Equal(ns, want.Proto()) { - found = true + for _, wo := range okMsgs { + s, ok := status.FromError(e) + if !ok { + continue + } + ns := s.Proto() + ns.Message = "" // blank out message so that we don't compare it. + + if proto.Equal(ns, wo.Proto()) { + found = true + } } } if !found { diff --git a/client/client.go b/client/client.go index 1aa7d487..3f6dc198 100644 --- a/client/client.go +++ b/client/client.go @@ -518,22 +518,12 @@ type OpResult struct { Details *OpDetailsResults } -// OpDetailsResults provides details of an operation for use in the results. -type OpDetailsResults struct { - // Type is the type of the operation (i.e., ADD, MODIFY, DELETE) - Type constants.OpType - - // NextHopIndex is the identifier for a next-hop modified by the operation. - NextHopIndex uint64 - // NextHopGroupID is the identifier for a next-hop-group modified by the - // operation. - NextHopGroupID uint64 - // IPv4Prefix is the IPv4 prefix modified by the operation. - IPv4Prefix string -} - // String returns a string for an OpResult for debugging purposes. func (o *OpResult) String() string { + if o == nil { + return "" + } + buf := &bytes.Buffer{} buf.WriteString("<") buf.WriteString(fmt.Sprintf("%d (%d nsec):", o.Timestamp, o.Latency)) @@ -544,7 +534,14 @@ func (o *OpResult) String() string { } if v := o.OperationID; v != 0 { - buf.WriteString(fmt.Sprintf(" AFTOperation { ID: %d, Type: %s, Status: %s }", v, o.Details.Type, o.ProgrammingResult)) + typ := "Unknown" + if o.Details != nil { + typ = o.Details.Type.String() + } + buf.WriteString(fmt.Sprintf(" AFTOperation { ID: %d, Type: %s, Status: %s }", v, typ, o.ProgrammingResult)) + } else if v := o.ProgrammingResult; v != spb.AFTResult_UNSET { + // Special case for input messages that are just matching on status. + buf.WriteString(fmt.Sprintf(" AFTOperation { Status: %s }", v)) } if v := o.SessionParameters; v != nil { @@ -559,6 +556,40 @@ func (o *OpResult) String() string { return buf.String() } +// OpDetailsResults provides details of an operation for use in the results. +type OpDetailsResults struct { + // Type is the type of the operation (i.e., ADD, MODIFY, DELETE) + Type constants.OpType + + // NextHopIndex is the identifier for a next-hop modified by the operation. + NextHopIndex uint64 + // NextHopGroupID is the identifier for a next-hop-group modified by the + // operation. + NextHopGroupID uint64 + // IPv4Prefix is the IPv4 prefix modified by the operation. + IPv4Prefix string +} + +// String returns a human-readable form of the OpDetailsResults +func (o *OpDetailsResults) String() string { + if o == nil { + return "" + } + buf := &bytes.Buffer{} + buf.WriteString(fmt.Sprintf("") + + return buf.String() +} + // Q enqueues a ModifyRequest to be sent to the target. func (c *Client) Q(m *spb.ModifyRequest) { if err := c.handleModifyRequest(m); err != nil { diff --git a/client/client_test.go b/client/client_test.go index f8b84e6a..fa642d5d 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1106,3 +1106,33 @@ func TestGet(t *testing.T) { }) } } + +func TestOpResultString(t *testing.T) { + tests := []struct { + desc string + inResult *OpResult + want string + }{{ + desc: "nil input", + inResult: nil, + want: "", + }, { + desc: "all fields nil", + inResult: &OpResult{}, + want: "<0 (0 nsec):>", + }, { + desc: "nil type in details", + inResult: &OpResult{ + OperationID: 42, + }, + want: "<0 (0 nsec): AFTOperation { ID: 42, Type: Unknown, Status: UNSET }>", + }} + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + if got := tt.inResult.String(); got != tt.want { + t.Fatalf("did not get expected string, got: %s, want: %s", got, tt.want) + } + }) + } +} diff --git a/cmd/ccli/ccli_test.go b/cmd/ccli/ccli_test.go index f1f727e4..7a3ff8b4 100644 --- a/cmd/ccli/ccli_test.go +++ b/cmd/ccli/ccli_test.go @@ -34,11 +34,14 @@ import ( ) var ( - addr = flag.String("addr", "", "address of the gRIBI server in the format hostname:port") - insecure = flag.Bool("insecure", false, "dial insecure gRPC (no TLS)") - skipverify = flag.Bool("skipverify", true, "allow self-signed TLS certificate; not needed for -insecure") - username = flag.String("username", os.Getenv("USER"), "username to be sent as gRPC metadata") - password = flag.String("password", "", "password to be sent as gRPC metadata") + addr = flag.String("addr", "", "address of the gRIBI server in the format hostname:port") + insecure = flag.Bool("insecure", false, "dial insecure gRPC (no TLS)") + skipVerify = flag.Bool("skip_verify", true, "allow self-signed TLS certificate; not needed for -insecure") + username = flag.String("username", os.Getenv("USER"), "username to be sent as gRPC metadata") + password = flag.String("password", "", "password to be sent as gRPC metadata") + initialElectionID = flag.Uint("initial_electionid", 0, "initial election ID to be used") + skipFIBACK = flag.Bool("skip_fiback", false, "skip tests that rely on FIB ACK") + skipSrvReorder = flag.Bool("skip_reordering", false, "skip tests that rely on server side transaction reordering") ) // flagCred implements credentials.PerRPCCredentials by populating the @@ -64,12 +67,16 @@ func TestCompliance(t *testing.T) { return // Test is part of CI, so do not fail here. } + if *initialElectionID != 0 { + compliance.SetElectionID(uint64(*initialElectionID)) + } + dialOpts := []grpc.DialOption{grpc.WithBlock()} if *insecure { dialOpts = append(dialOpts, grpc.WithInsecure()) - } else if *skipverify { + } else if *skipVerify { tlsc := credentials.NewTLS(&tls.Config{ - InsecureSkipVerify: *skipverify, + InsecureSkipVerify: *skipVerify, }) dialOpts = append(dialOpts, grpc.WithTransportCredentials(tlsc)) } @@ -87,6 +94,14 @@ func TestCompliance(t *testing.T) { stub := spb.NewGRIBIClient(conn) for _, tt := range compliance.TestSuite { + if skip := *skipFIBACK; skip && tt.In.RequiresFIBACK { + continue + } + + if skip := *skipSrvReorder; skip && tt.In.RequiresServerReordering { + continue + } + t.Run(tt.In.ShortName, func(t *testing.T) { c := fluent.NewClient() c.Connection().WithStub(stub) diff --git a/compliance/compliance.go b/compliance/compliance.go index 7d3d3eb8..03d021c4 100644 --- a/compliance/compliance.go +++ b/compliance/compliance.go @@ -44,6 +44,12 @@ func init() { // the last tests', and thus fail due to the state of the server. var electionID = &atomic.Uint64{} +// SetElectionID allows an external caller to specify an election ID to be used for +// subsequent calls. +func SetElectionID(v uint64) { + electionID.Store(v) +} + // Test describes a test within the compliance library. type Test struct { // Fn is the function to be run for a test. @@ -55,6 +61,16 @@ type Test struct { ShortName string // Reference is a unique reference to external data (e.g., test plans) used for the test. Reference string + + // RequiresFIBACK marks a test that requires the implementation of FIB ACK on the server. + // This is expected behaviour of a gRIBI server, but some implementations add this in + // later builds. + RequiresFIBACK bool + // RequiresServerReordering marks a test that requires the implementation of server-side + // reordering of transactions rather than an immediate NACK. Currently, some implementations + // immediately NACK forward references, which causes some tests to fail. The reference + // implementation handles reodering. + RequiresServerReordering bool } // TestSpec is a description of a test. @@ -86,67 +102,106 @@ var ( }, }, { In: Test{ - Fn: AddIPv4EntryRIBACK, + Fn: makeTestWithACK(AddIPv4Entry, fluent.InstalledInRIB), Reference: "TE-2.1.1.1", ShortName: "Add IPv4 entry that can be programmed on the server - with RIB ACK", }, }, { In: Test{ - Fn: AddIPv4EntryFIBACK, - Reference: "TE-2.1.1.2", - ShortName: "Add IPv4 entry that can be programmed on the server - with FIB ACK", + Fn: makeTestWithACK(AddIPv4Entry, fluent.InstalledInFIB), + Reference: "TE-2.1.1.2", + ShortName: "Add IPv4 entry that can be programmed on the server - with FIB ACK", + RequiresFIBACK: true, }, }, { In: Test{ - Fn: AddUnreferencedNextHopGroupFIBACK, + Fn: makeTestWithACK(AddUnreferencedNextHopGroup, fluent.InstalledInRIB), ShortName: "Add next-hop-group entry that can be resolved on the server, no referencing IPv4 entries - with RIB ACK", }, }, { In: Test{ - Fn: AddUnreferencedNextHopGroupRIBACK, - ShortName: "Add next-hop-group entry that can be resolved on the server, no referencing IPv4 entries - with FIB ACK", + Fn: makeTestWithACK(AddUnreferencedNextHopGroup, fluent.InstalledInFIB), + ShortName: "Add next-hop-group entry that can be resolved on the server, no referencing IPv4 entries - with FIB ACK", + RequiresFIBACK: true, + }, + }, { + In: Test{ + Fn: AddIPv4EntryRandom, + ShortName: "Add IPv4 entries that are resolved by NHG and NH, in random order", + RequiresServerReordering: true, }, }, { In: Test{ - Fn: AddIPv4EntryRandom, - ShortName: "Add IPv4 entries that are resolved by NHG and NH, in random order", + Fn: makeTestWithACK(AddIPv4ToMultipleNHsSingleRequest, fluent.InstalledInFIB), + ShortName: "Add IPv4 entries that are resolved to a next-hop-group containing multiple next-hops (single ModifyRequest) - with FIB ACK", + Reference: "TE-2.1.2.1", + RequiresFIBACK: true, }, }, { In: Test{ - Fn: AddIPv4ToMultipleNHsSingleRequest, - ShortName: "Add IPv4 entries that are resolved to a next-hop-group containing multiple next-hops (single ModifyRequest)", + Fn: makeTestWithACK(AddIPv4ToMultipleNHsSingleRequest, fluent.InstalledInRIB), + ShortName: "Add IPv4 entries that are resolved to a next-hop-group containing multiple next-hops (single ModifyRequest) - with RIB ACK", Reference: "TE-2.1.2.1", }, }, { In: Test{ - Fn: AddIPv4ToMultipleNHsMultipleRequests, - ShortName: "Add IPv4 entries that are resolved to a next-hop-group containing multiple next-hops (multiple ModifyRequests)", + Fn: makeTestWithACK(AddIPv4ToMultipleNHsMultipleRequests, fluent.InstalledInFIB), + ShortName: "Add IPv4 entries that are resolved to a next-hop-group containing multiple next-hops (multiple ModifyRequests) - with FIB ACK", + Reference: "TE-2.1.2.2", + RequiresFIBACK: true, + }, + }, { + In: Test{ + Fn: makeTestWithACK(AddIPv4ToMultipleNHsMultipleRequests, fluent.InstalledInRIB), + ShortName: "Add IPv4 entries that are resolved to a next-hop-group containing multiple next-hops (multiple ModifyRequests) - with RIB ACK", Reference: "TE-2.1.2.2", }, }, { In: Test{ - Fn: DeleteIPv4Entry, - ShortName: "Delete IPv4 entry within default network instance", + Fn: makeTestWithACK(DeleteIPv4Entry, fluent.InstalledInRIB), + ShortName: "Delete IPv4 entry within default network instance - RIB ACK", + }, + }, { + In: Test{ + Fn: makeTestWithACK(DeleteReferencedNHGFailure, fluent.InstalledInRIB), + ShortName: "Delete NHG entry that is referenced - failure - RIB ACK", + }, + }, { + In: Test{ + Fn: makeTestWithACK(DeleteReferencedNHFailure, fluent.InstalledInRIB), + ShortName: "Delete NH entry that is referenced - failure - RIB ACK", }, }, { In: Test{ - Fn: DeleteReferencedNHGFailure, - ShortName: "Delete NHG entry that is referenced - failure", + Fn: makeTestWithACK(DeleteNextHopGroup, fluent.InstalledInRIB), + ShortName: "Delete NHG entry successfully - RIB ACK", }, }, { In: Test{ - Fn: DeleteReferencedNHFailure, - ShortName: "Delete NH entry that is referenced - failure", + Fn: makeTestWithACK(DeleteNextHopGroup, fluent.InstalledInFIB), + ShortName: "Delete NHG entry successfully - FIB ACK", + RequiresFIBACK: true, }, }, { In: Test{ - Fn: DeleteNextHopGroup, - ShortName: "Delete NHG entry successfully", + Fn: makeTestWithACK(DeleteNextHop, fluent.InstalledInRIB), + ShortName: "Delete NH entry successfully - RIB ACK", }, }, { In: Test{ - Fn: DeleteNextHop, - ShortName: "Delete NH entry successfully", + Fn: makeTestWithACK(DeleteNextHop, fluent.InstalledInRIB), + ShortName: "Delete NH entry successfully - FIB ACK", + RequiresFIBACK: true, + }, + }, { + In: Test{ + Fn: AddIPv4Metadata, + ShortName: "Add Metadata for IPv4 entry", + }, + }, { + In: Test{ + Fn: makeTestWithACK(AddIPv4EntryDifferentNINHG, fluent.InstalledInRIB), + ShortName: "Add IPv4 Entry that references a NHG in a different network instance", }, }} ) @@ -223,36 +278,12 @@ func ModifyConnectionSinglePrimaryPreserve(c *fluent.GRIBIClient, t testing.TB) WithReason(fluent.UnsupportedParameters). AsStatus(t) - chk.HasRecvClientErrorWithStatus(t, err, want) -} - -// AddIPv4EntryRIBACK adds a simple IPv4 Entry which references a next-hop-group -// to the gRIBI server, requesting a RIB-level ACK. -func AddIPv4EntryRIBACK(c *fluent.GRIBIClient, t testing.TB) { - addIPv4Internal(c, t, fluent.InstalledInRIB) -} - -// AddIPv4EntryFIBACK adds a simple IPv4 Entry which references a next-hop-group -// to the gRIBI server, requesting a FIB-level ACK. -func AddIPv4EntryFIBACK(c *fluent.GRIBIClient, t testing.TB) { - addIPv4Internal(c, t, fluent.InstalledInFIB) -} - -// AddUnreferencedNextHopGroupRIBACK adds an unreferenced next-hop-group that contains -// nexthops to the gRIBI server, requesting a FIB-level ACK. -func AddUnreferencedNextHopGroupRIBACK(c *fluent.GRIBIClient, t testing.TB) { - addNextHopGroupInternal(c, t, fluent.InstalledInRIB) -} - -// AddUnreferencedNextHopGroupFIBACK adds an unreferenced next-hop-group that contains -// nexthops to the gRIBI server, requesting a FIB-level ACK. -func AddUnreferencedNextHopGroupFIBACK(c *fluent.GRIBIClient, t testing.TB) { - addNextHopGroupInternal(c, t, fluent.InstalledInFIB) + chk.HasRecvClientErrorWithStatus(t, err, want, chk.AllowUnimplemented()) } -// addIPv4Internal is an internal test that adds IPv4 entries, and checks -// whether the specified FIB ack is received. -func addIPv4Internal(c *fluent.GRIBIClient, t testing.TB, wantACK fluent.ProgrammingResult) { +// AddIPv4Entry adds a fully referenced IPv4Entry and checks whether the specified ACK +// type (wantACK) is returned. +func AddIPv4Entry(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.TB) { ops := []func(){ func() { c.Modify().AddEntry(t, fluent.NextHopEntry().WithNetworkInstance(server.DefaultNetworkInstanceName).WithIndex(1).WithIPAddress("192.0.2.1")) @@ -338,6 +369,93 @@ func AddIPv4EntryRandom(c *fluent.GRIBIClient, t testing.TB) { // TODO(robjs): add gNMI subscription using generated telemetry library. } +// AddIPv4Metadata adds an IPv4 Entry (and its dependencies) with metadata alongside the +// entry. +func AddIPv4Metadata(c *fluent.GRIBIClient, t testing.TB) { + ops := []func(){ + func() { + c.Modify().AddEntry(t, fluent.IPv4Entry(). + WithPrefix("1.1.1.1/32"). + WithNetworkInstance(server.DefaultNetworkInstanceName). + WithNextHopGroup(1). + WithMetadata([]byte{1, 2, 3, 4, 5, 6, 7, 8}), + ) + c.Modify().AddEntry(t, fluent.NextHopGroupEntry().WithID(1).WithNetworkInstance(server.DefaultNetworkInstanceName).AddNextHop(1, 1)) + c.Modify().AddEntry(t, fluent.NextHopEntry().WithIndex(1).WithNetworkInstance(server.DefaultNetworkInstanceName).WithIPAddress("2.2.2.2")) + }, + } + + res := doOps(c, t, ops, fluent.InstalledInRIB, false) + + chk.HasResult(t, res, + fluent.OperationResult(). + WithIPv4Operation("1.1.1.1/32"). + WithOperationType(constants.Add). + WithProgrammingResult(fluent.InstalledInRIB). + AsResult(), + chk.IgnoreOperationID()) + + chk.HasResult(t, res, + fluent.OperationResult(). + WithNextHopGroupOperation(1). + WithOperationType(constants.Add). + WithProgrammingResult(fluent.InstalledInRIB). + AsResult(), + chk.IgnoreOperationID()) + + chk.HasResult(t, res, + fluent.OperationResult(). + WithNextHopOperation(1). + WithOperationType(constants.Add). + WithProgrammingResult(fluent.InstalledInRIB). + AsResult(), + chk.IgnoreOperationID()) +} + +// AddIPv4EntryDifferentNINHG adds an IPv4 entry that references a next-hop-group within a +// different network instance, and validates that the entry is successfully installed. +func AddIPv4EntryDifferentNINHG(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.TB) { + // TODO(robjs): Server needs to be initialised with >1 VRF. + t.Skip() + ops := []func(){ + func() { + c.Modify().AddEntry(t, fluent.IPv4Entry(). + WithPrefix("1.1.1.1/32"). + WithNetworkInstance("NON-DEFAULT"). + WithNextHopGroup(1). + WithNextHopGroupNetworkInstance(server.DefaultNetworkInstanceName)) + c.Modify().AddEntry(t, fluent.NextHopGroupEntry().WithID(1).AddNextHop(1, 1)) + c.Modify().AddEntry(t, fluent.NextHopEntry().WithIndex(1).WithIPAddress("2.2.2.2")) + }, + } + + res := doOps(c, t, ops, wantACK, false) + + chk.HasResult(t, res, + fluent.OperationResult(). + WithNextHopGroupOperation(1). + WithProgrammingResult(wantACK). + WithOperationType(constants.Add). + AsResult(), + chk.IgnoreOperationID()) + + chk.HasResult(t, res, + fluent.OperationResult(). + WithNextHopOperation(1). + WithProgrammingResult(wantACK). + WithOperationType(constants.Add). + AsResult(), + chk.IgnoreOperationID()) + + chk.HasResult(t, res, + fluent.OperationResult(). + WithIPv4Operation("1.1.1.1/32"). + WithProgrammingResult(wantACK). + WithOperationType(constants.Add). + AsResult(), + chk.IgnoreOperationID()) +} + // doOps performs the series of operations in ops using the context // client c. wantACK specifies the ACK type to request from the // server, and randomise specifies whether the operations should be @@ -378,14 +496,12 @@ func doOps(c *fluent.GRIBIClient, t testing.TB, ops []func(), wantACK fluent.Pro return c.Results(t) } -// addNextHopGroupInternal is an internal implementation that checks that a -// next-hop-group can be added to the gRIBI server with the specified ACK mode. -// The tests does not install an IPv4Entry, so these NHGs are unreferenced. -// We still expect an ACK in this case. -func addNextHopGroupInternal(c *fluent.GRIBIClient, t testing.TB, wantACK fluent.ProgrammingResult) { +// AddUnreferencedNextHopGroup adds a NHG that is not referenced by any other entry. An ACK is expected, +// and is validated to be of the type specified by wantACK. +func AddUnreferencedNextHopGroup(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.TB) { ops := []func(){ func() { - c.Modify().AddEntry(t, fluent.NextHopEntry().WithNetworkInstance(server.DefaultNetworkInstanceName).WithIndex(1)) + c.Modify().AddEntry(t, fluent.NextHopEntry().WithNetworkInstance(server.DefaultNetworkInstanceName).WithIndex(1).WithIPAddress("192.0.2.1")) }, func() { c.Modify().AddEntry(t, fluent.NextHopGroupEntry().WithNetworkInstance(server.DefaultNetworkInstanceName).WithID(42).AddNextHop(1, 1)) @@ -438,13 +554,13 @@ func baseTopologyEntries(c *fluent.GRIBIClient, t testing.TB) { // validateBaseEntries checks that the entries in the base topology are correctly // installed. -func validateBaseTopologyEntries(res []*client.OpResult, t testing.TB) { +func validateBaseTopologyEntries(res []*client.OpResult, wantACK fluent.ProgrammingResult, t testing.TB) { // Check for next-hops 1 and 2. for _, nhopID := range []uint64{1, 2} { chk.HasResult(t, res, fluent.OperationResult(). WithNextHopOperation(nhopID). - WithProgrammingResult(fluent.InstalledInFIB). + WithProgrammingResult(wantACK). WithOperationType(constants.Add). AsResult(), chk.IgnoreOperationID(), @@ -455,7 +571,7 @@ func validateBaseTopologyEntries(res []*client.OpResult, t testing.TB) { chk.HasResult(t, res, fluent.OperationResult(). WithNextHopGroupOperation(1). - WithProgrammingResult(fluent.InstalledInFIB). + WithProgrammingResult(wantACK). WithOperationType(constants.Add). AsResult(), chk.IgnoreOperationID(), @@ -465,17 +581,17 @@ func validateBaseTopologyEntries(res []*client.OpResult, t testing.TB) { chk.HasResult(t, res, fluent.OperationResult(). WithIPv4Operation("1.0.0.0/8"). - WithProgrammingResult(fluent.InstalledInFIB). + WithProgrammingResult(wantACK). WithOperationType(constants.Add). AsResult(), chk.IgnoreOperationID(), ) } -// AddIPv4ToMultipleNHsSingleRequest creates an IPv4 entry which references a NHG containing -// 2 NHs within a single ModifyRequest, validating that they are installed in the FIB. -func AddIPv4ToMultipleNHsSingleRequest(c *fluent.GRIBIClient, t testing.TB) { - +// AddIPv4ToMultipleNHsSingleRequest is the internal implementation of the single request installation +// of a IPv4Entry referencing a NHG that contains multiple NHs. It uses the wantACK parameter to determine the +// type of acknowledgement that is expected from the server. +func AddIPv4ToMultipleNHsSingleRequest(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.TB) { ops := []func(){ func() { c.Modify().AddEntry(t, @@ -486,35 +602,41 @@ func AddIPv4ToMultipleNHsSingleRequest(c *fluent.GRIBIClient, t testing.TB) { }, } - validateBaseTopologyEntries(doOps(c, t, ops, fluent.InstalledInFIB, false), t) + validateBaseTopologyEntries(doOps(c, t, ops, wantACK, false), wantACK, t) } // AddIPv4ToMultipleNHsMultipleRequests creates an IPv4 entry which references a NHG containing -// 2 NHs within multiple ModifyReqests, validating that they are installed in the FIB. -func AddIPv4ToMultipleNHsMultipleRequests(c *fluent.GRIBIClient, t testing.TB) { - +// 2 NHs within multiple ModifyReqests, validating that they are installed in the specified RIB +// or FIB according to wantACK. +func AddIPv4ToMultipleNHsMultipleRequests(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.TB) { ops := []func(){ func() { baseTopologyEntries(c, t) }, } - validateBaseTopologyEntries(doOps(c, t, ops, fluent.InstalledInFIB, false), t) + validateBaseTopologyEntries(doOps(c, t, ops, wantACK, false), wantACK, t) +} + +// makeTestWithACK creates a version of a test function that can be directly executed by the runner +// which has a specific ACK type. +func makeTestWithACK(fn func(*fluent.GRIBIClient, fluent.ProgrammingResult, testing.TB), wantACK fluent.ProgrammingResult) func(*fluent.GRIBIClient, testing.TB) { + return func(c *fluent.GRIBIClient, t testing.TB) { fn(c, wantACK, t) } } // DeleteIPv4Entry deletes an IPv4 entry from the server's RIB. -func DeleteIPv4Entry(c *fluent.GRIBIClient, t testing.TB) { +func DeleteIPv4Entry(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.TB) { ops := []func(){ func() { baseTopologyEntries(c, t) }, func() { c.Modify().DeleteEntry(t, fluent.IPv4Entry().WithPrefix("1.0.0.0/8").WithNetworkInstance(server.DefaultNetworkInstanceName)) }, } - res := doOps(c, t, ops, fluent.InstalledInFIB, false) - validateBaseTopologyEntries(res, t) + res := doOps(c, t, ops, wantACK, false) + validateBaseTopologyEntries(res, wantACK, t) chk.HasResult(t, res, fluent.OperationResult(). WithIPv4Operation("1.0.0.0/8"). WithOperationType(constants.Delete). - WithProgrammingResult(fluent.InstalledInFIB). + WithProgrammingResult(wantACK). AsResult(), chk.IgnoreOperationID(), ) @@ -522,15 +644,15 @@ func DeleteIPv4Entry(c *fluent.GRIBIClient, t testing.TB) { // DeleteReferencedNHGFailure attempts to delete a NextHopGroup entry that is referenced // from the RIB, and expects a failure. -func DeleteReferencedNHGFailure(c *fluent.GRIBIClient, t testing.TB) { +func DeleteReferencedNHGFailure(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.TB) { ops := []func(){ func() { baseTopologyEntries(c, t) }, func() { c.Modify().DeleteEntry(t, fluent.NextHopGroupEntry().WithID(1).WithNetworkInstance(server.DefaultNetworkInstanceName)) }, } - res := doOps(c, t, ops, fluent.InstalledInFIB, false) - validateBaseTopologyEntries(res, t) + res := doOps(c, t, ops, wantACK, false) + validateBaseTopologyEntries(res, wantACK, t) chk.HasResult(t, res, fluent.OperationResult(). @@ -543,7 +665,7 @@ func DeleteReferencedNHGFailure(c *fluent.GRIBIClient, t testing.TB) { // DeleteReferencedNHFailure attempts to delete a NH entry that is referened from the RIB // and expects a failure. -func DeleteReferencedNHFailure(c *fluent.GRIBIClient, t testing.TB) { +func DeleteReferencedNHFailure(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.TB) { ops := []func(){ func() { baseTopologyEntries(c, t) }, func() { @@ -553,8 +675,8 @@ func DeleteReferencedNHFailure(c *fluent.GRIBIClient, t testing.TB) { c.Modify().DeleteEntry(t, fluent.NextHopEntry().WithIndex(2).WithNetworkInstance(server.DefaultNetworkInstanceName)) }, } - res := doOps(c, t, ops, fluent.InstalledInFIB, false) - validateBaseTopologyEntries(res, t) + res := doOps(c, t, ops, wantACK, false) + validateBaseTopologyEntries(res, wantACK, t) for _, i := range []uint64{1, 2} { chk.HasResult(t, res, @@ -568,8 +690,8 @@ func DeleteReferencedNHFailure(c *fluent.GRIBIClient, t testing.TB) { } // DeleteNextHopGroup attempts to delete a NHG entry that is not referenced and expects -// success. -func DeleteNextHopGroup(c *fluent.GRIBIClient, t testing.TB) { +// success. The ACK type expected is validated against wantACK. +func DeleteNextHopGroup(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.TB) { ops := []func(){ func() { baseTopologyEntries(c, t) }, func() { @@ -580,14 +702,14 @@ func DeleteNextHopGroup(c *fluent.GRIBIClient, t testing.TB) { }, } - res := doOps(c, t, ops, fluent.InstalledInFIB, false) - validateBaseTopologyEntries(res, t) + res := doOps(c, t, ops, wantACK, false) + validateBaseTopologyEntries(res, wantACK, t) chk.HasResult(t, res, fluent.OperationResult(). WithNextHopGroupOperation(1). WithOperationType(constants.Delete). - WithProgrammingResult(fluent.InstalledInFIB). + WithProgrammingResult(wantACK). AsResult(), chk.IgnoreOperationID()) @@ -595,17 +717,14 @@ func DeleteNextHopGroup(c *fluent.GRIBIClient, t testing.TB) { fluent.OperationResult(). WithIPv4Operation("1.0.0.0/8"). WithOperationType(constants.Delete). - WithProgrammingResult(fluent.InstalledInFIB). + WithProgrammingResult(wantACK). AsResult(), chk.IgnoreOperationID()) } -// DeleteNextHop attempts to delete the NH entris within the base topology and expects -// success. -// -// TODO(robjs): When traffic and AFT validation is added, ensure that a partial delete -// scenario keeps traffic routed via the remaining NH. -func DeleteNextHop(c *fluent.GRIBIClient, t testing.TB) { +// DeleteNextHop attempts to delete the NH entries within the base topology and expects +// success. The ACK type returned is validated against wantACK. +func DeleteNextHop(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.TB) { ops := []func(){ func() { baseTopologyEntries(c, t) }, func() { @@ -618,25 +737,24 @@ func DeleteNextHop(c *fluent.GRIBIClient, t testing.TB) { }, } - res := doOps(c, t, ops, fluent.InstalledInFIB, false) - validateBaseTopologyEntries(res, t) + res := doOps(c, t, ops, wantACK, false) + validateBaseTopologyEntries(res, wantACK, t) for _, i := range []uint64{1, 2} { chk.HasResult(t, res, fluent.OperationResult(). WithNextHopOperation(i). WithOperationType(constants.Delete). - WithProgrammingResult(fluent.InstalledInFIB). + WithProgrammingResult(wantACK). AsResult(), chk.IgnoreOperationID()) - } chk.HasResult(t, res, fluent.OperationResult(). WithNextHopGroupOperation(1). WithOperationType(constants.Delete). - WithProgrammingResult(fluent.InstalledInFIB). + WithProgrammingResult(wantACK). AsResult(), chk.IgnoreOperationID()) @@ -644,7 +762,7 @@ func DeleteNextHop(c *fluent.GRIBIClient, t testing.TB) { fluent.OperationResult(). WithIPv4Operation("1.0.0.0/8"). WithOperationType(constants.Delete). - WithProgrammingResult(fluent.InstalledInFIB). + WithProgrammingResult(wantACK). AsResult(), chk.IgnoreOperationID()) } diff --git a/demo/ipv4/ipv4_test.go b/demo/ipv4/ipv4_test.go index fe41b45d..77e042a9 100644 --- a/demo/ipv4/ipv4_test.go +++ b/demo/ipv4/ipv4_test.go @@ -38,6 +38,6 @@ func TestDemo(t *testing.T) { } c := fluent.NewClient() c.Connection().WithTarget(*addr) - compliance.AddIPv4EntryRIBACK(c, t) + compliance.AddIPv4Entry(c, fluent.InstalledInRIB, t) time.Sleep(2 * time.Second) } diff --git a/device/device_test.go b/device/device_test.go index 19d8470b..07f6796c 100644 --- a/device/device_test.go +++ b/device/device_test.go @@ -88,7 +88,7 @@ func TestDevice(t *testing.T) { case addr := <-devCh: c := fluent.NewClient() c.Connection().WithTarget(addr) - compliance.AddIPv4EntryRIBACK(c, t) + compliance.AddIPv4Entry(c, fluent.InstalledInRIB, t) _, cidr, err := net.ParseCIDR("1.1.1.1/32") if err != nil { diff --git a/fluent/fluent.go b/fluent/fluent.go index 448c280b..b4e3c043 100644 --- a/fluent/fluent.go +++ b/fluent/fluent.go @@ -464,7 +464,14 @@ func (i *ipv4Entry) WithNextHopGroupNetworkInstance(n string) *ipv4Entry { return i } -// opproto implements the GRIBIEntry interface, building a gRIBI AFTOperation. ID +// WithMetadata specifies a byte slice that is stored as metadata alongside +// the entry on the gRIBI server. +func (i *ipv4Entry) WithMetadata(b []byte) *ipv4Entry { + i.pb.Ipv4Entry.Metadata = &wpb.BytesValue{Value: b} + return i +} + +// opproto implements the gRIBIEntry interface, returning a gRIBI AFTOperation. ID // and ElectionID are explicitly not populated such that they can be populated by // the function (e.g., AddEntry) to which they are an argument. func (i *ipv4Entry) opproto() (*spb.AFTOperation, error) { diff --git a/go.mod b/go.mod index bb850846..2c638862 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/openconfig/grpctunnel v0.0.0-20210610163803-fde4a9dc048d // indirect github.com/openconfig/ygot v0.11.0 go.uber.org/atomic v1.7.0 + google.golang.org/genproto v0.0.0-20201214200347-8c77b98c765d // indirect google.golang.org/grpc v1.37.0 google.golang.org/protobuf v1.26.0 lukechampine.com/uint128 v1.1.1 diff --git a/go.sum b/go.sum index 4081ad08..feea5022 100644 --- a/go.sum +++ b/go.sum @@ -83,6 +83,7 @@ golang.org/x/crypto v0.0.0-20200302210943-78000ba7a073/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= +golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 h1:XQyxROzUlZH+WIQwySDgnISgOivlhjIEwaQaJEJrrN0= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -118,6 +119,7 @@ golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGm golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY= golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= +golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 h1:5Beo0mZN8dRzgrMMkDp0jc8YXQKx9DiJ2k1dkvGsn5A= golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=