Skip to content

Add to Gribi Get() tests validation that the correct status is returned #255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gggsmith wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add to Gribi Get() tests validation that the correct status is returned #255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions chk/chk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/openconfig/gribigo/client"
"github.com/openconfig/gribigo/fluent"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/testing/protocmp"
"github.com/openconfig/gribigo/client"
"github.com/openconfig/gribigo/fluent"

spb "github.com/openconfig/gribi/v1/proto/service"
gspb "google.golang.org/genproto/googleapis/rpc/status"
spb "github.com/openconfig/gribi/v1/proto/service"
)

// resultOpt is an interface implemented by all options that can be
Expand Down Expand Up @@ -374,17 +374,31 @@ func GetResponseHasEntries(t testing.TB, getres *spb.GetResponse, wants ...fluen

switch v := wantProto.Entry.(type) {
case *spb.AFTEntry_NextHopGroup:
if _, ok := ni.nhg[v.NextHopGroup.GetId()]; !ok {
if nhg, ok := ni.nhg[v.NextHopGroup.GetId()]; !ok {
t.Fatalf("did not find entry, did not find nexthop group: %s, got:\n%s", v.NextHopGroup, getres)
} else {
programmedStatusMatch(t, nhg, wantProto)
}
case *spb.AFTEntry_NextHop:
if _, ok := ni.nh[v.NextHop.GetIndex()]; !ok {
if nh, ok := ni.nh[v.NextHop.GetIndex()]; !ok {
t.Fatalf("did not find entry, did not find nexthop: %s, got:\n%s", v.NextHop, getres)
} else {
programmedStatusMatch(t, nh, wantProto)
}
case *spb.AFTEntry_Ipv4:
if _, ok := ni.ipv4[v.Ipv4.GetPrefix()]; !ok {
if ipv4, ok := ni.ipv4[v.Ipv4.GetPrefix()]; !ok {
t.Fatalf("did not find entry, did not find ipv4: %s, got: %s\n", v.Ipv4, getres)
} else {
programmedStatusMatch(t, ipv4, wantProto)
Comment on lines +389 to +392

@robshakir robshakir Mar 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ipv4, ok := ni.ipv4[v.Ipv4.GetPrefix()]; !ok {
t.Fatalf("did not find entry, did not find ipv4: %s, got: %s\n", v.Ipv4, getres)
} else {
programmedStatusMatch(t, ipv4, wantProto)
ipv4, ok := ni.ipv4[v.Ipv4.GetPrefix()]
if !ok {
t.Fatalf("did not find entry, did not find ipv4: %s, got: %s\n", v.Ipv4, getres)
}
programmedStatusMatch(t, ipv4, wantProto)

@robshakir robshakir Mar 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest this layout here and above to avoid the indentation of the other case -- since the !ok case will exit via Fatalf we don't really need the else.

}
}
}
}

// programmedStatusMatch checks if the response's AFT entry FIB status matches the expected status.
// It skips the check if the expected status is UNAVAILABLE (default value).
func programmedStatusMatch(t testing.TB, resp *spb.AFTEntry, want *spb.AFTEntry) {
if want.GetFibStatus() != spb.AFTEntry_UNAVAILABLE && resp.GetFibStatus() != want.GetFibStatus() {
t.Fatalf("FIB status mismatch: %s, got: %s\n", want.GetFibStatus(), resp.GetFibStatus())
}
}
21 changes: 14 additions & 7 deletions compliance/get.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ func GetNH(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.TB
fluent.NextHopEntry().
WithNetworkInstance(defaultNetworkInstanceName).
WithIndex(1).
WithIPAddress("192.0.2.3"))
WithIPAddress("192.0.2.3").
WithFIBProgrammed(wantACK))

@robshakir robshakir Mar 26, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will lead -- in some cases -- to having WithFIBProgrammed(RIB_PROGRAMMED) which seems erroneous right? i.e., we will never have a fib_status field return RIB_PROGRAMMED right?

Instead, should we either have WithProgrammedStatus() which could take arguments of FIB ACKed or RIB ACKed? Or alternatively, WithFIBProgrammedStatus which only allows for FIB ACK or failed?


}

Expand Down Expand Up @@ -129,7 +130,8 @@ func GetNHG(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.T
fluent.NextHopGroupEntry().
WithNetworkInstance(defaultNetworkInstanceName).
WithID(1).
AddNextHop(1, 1),
AddNextHop(1, 1).
WithFIBProgrammed(wantACK),
)
}

Expand Down Expand Up @@ -208,7 +210,8 @@ func GetIPv4(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.
WithNetworkInstance(defaultNetworkInstanceName).
WithNextHopGroup(1).
WithPrefix("42.42.42.42/32").
WithMetadata([]byte{1, 2, 3, 4, 5, 6, 7, 8}),
WithMetadata([]byte{1, 2, 3, 4, 5, 6, 7, 8}).
WithFIBProgrammed(wantACK),
)
}

Expand Down Expand Up @@ -287,7 +290,8 @@ func GetIPv6(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t testing.
WithNetworkInstance(defaultNetworkInstanceName).
WithNextHopGroup(1).
WithPrefix("2001:db8::/32").
WithMetadata([]byte{1, 2, 3, 4, 5, 6, 7, 8}),
WithMetadata([]byte{1, 2, 3, 4, 5, 6, 7, 8}).
WithFIBProgrammed(wantACK),
)
}

Expand Down Expand Up @@ -365,15 +369,18 @@ func GetIPv4Chain(c *fluent.GRIBIClient, wantACK fluent.ProgrammingResult, t tes
fluent.IPv4Entry().
WithNetworkInstance(defaultNetworkInstanceName).
WithNextHopGroup(1).
WithPrefix("42.42.42.42/32"),
WithPrefix("42.42.42.42/32").
WithFIBProgrammed(wantACK),
fluent.NextHopGroupEntry().
WithNetworkInstance(defaultNetworkInstanceName).
WithID(1).
AddNextHop(1, 1),
AddNextHop(1, 1).
WithFIBProgrammed(wantACK),
fluent.NextHopEntry().
WithNetworkInstance(defaultNetworkInstanceName).
WithIndex(1).
WithIPAddress("192.0.2.3"),
WithIPAddress("192.0.2.3").
WithFIBProgrammed(wantACK),
)
}

Expand Down
48 changes: 48 additions & 0 deletions fluent/fluent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,8 @@ type ipv4Entry struct {
// electionID is an explicit election ID to be used for an
// operation using the entry.
electionID *spb.Uint128
// fibStatus is the FIB programming status of the entry.
fibStatus spb.AFTEntry_Status
}

// IPv4Entry returns a new gRIBI IPv4Entry builder.
Expand Down Expand Up @@ -583,6 +585,15 @@ func (i *ipv4Entry) WithElectionID(low, high uint64) *ipv4Entry {
return i
}

// WithFIBProgrammed sets the FIB programming status of the entry to PROGRAMMED if
// the specified operation programming result is InstalledInFIB.
func (i *ipv4Entry) WithFIBProgrammed(r ProgrammingResult) *ipv4Entry {
if r == InstalledInFIB {
i.fibStatus = spb.AFTEntry_PROGRAMMED
}
return i
}

// OpProto implements the gRIBIEntry interface, returning a gRIBI AFTOperation. ID
// is explicitly not populated such that they can be populated by
// the function (e.g., AddEntry) to which they are an argument.
Expand All @@ -603,6 +614,7 @@ func (i *ipv4Entry) EntryProto() (*spb.AFTEntry, error) {
Entry: &spb.AFTEntry_Ipv4{
Ipv4: proto.Clone(i.pb).(*aftpb.Afts_Ipv4EntryKey),
},
FibStatus: i.fibStatus,
}, nil
}

Expand All @@ -615,6 +627,8 @@ type ipv6Entry struct {
// electionID is an explicit election ID to be used for an
// operation using the entry.
electionID *spb.Uint128
// fibStatus is the FIB programming status of the entry.
fibStatus spb.AFTEntry_Status
}

// IPv6Entry returns a new gRIBI IPv6Entry builder.
Expand Down Expand Up @@ -671,6 +685,15 @@ func (i *ipv6Entry) WithElectionID(low, high uint64) *ipv6Entry {
return i
}

// WithFIBProgrammed sets the FIB programming status of the entry to PROGRAMMED if
// the specified operation programming result is InstalledInFIB.
func (i *ipv6Entry) WithFIBProgrammed(r ProgrammingResult) *ipv6Entry {
if r == InstalledInFIB {
i.fibStatus = spb.AFTEntry_PROGRAMMED
}
return i
}

// OpProto implements the gRIBIEntry interface, returning a gRIBI AFTOperation. ID
// is explicitly not populated such that they can be populated by
// the function (e.g., AddEntry) to which they are an argument.
Expand All @@ -691,6 +714,7 @@ func (i *ipv6Entry) EntryProto() (*spb.AFTEntry, error) {
Entry: &spb.AFTEntry_Ipv6{
Ipv6: proto.Clone(i.pb).(*aftpb.Afts_Ipv6EntryKey),
},
FibStatus: i.fibStatus,
}, nil
}

Expand Down Expand Up @@ -790,6 +814,8 @@ type nextHopEntry struct {
// electionID is an explicit electionID to be used when the next-hop entry
// is programmed.
electionID *spb.Uint128
// fibStatus is the FIB programming status of the entry.
fibStatus spb.AFTEntry_Status
}

// NextHopEntry returns a builder that can be used to build up a NextHop within
Expand Down Expand Up @@ -963,6 +989,15 @@ func (n *nextHopEntry) WithElectionID(low, high uint64) *nextHopEntry {
return n
}

// WithFIBProgrammed sets the FIB programming status of the entry to PROGRAMMED if
// the specified operation programming result is InstalledInFIB.
func (n *nextHopEntry) WithFIBProgrammed(r ProgrammingResult) *nextHopEntry {
if r == InstalledInFIB {
n.fibStatus = spb.AFTEntry_PROGRAMMED
}
return n
}

// TODO(robjs): add additional NextHopEntry fields.

// OpProto implements the GRIBIEntry interface, building a gRIBI AFTOperation. ID
Expand All @@ -985,6 +1020,7 @@ func (n *nextHopEntry) EntryProto() (*spb.AFTEntry, error) {
Entry: &spb.AFTEntry_NextHop{
NextHop: proto.Clone(n.pb).(*aftpb.Afts_NextHopKey),
},
FibStatus: n.fibStatus,
}, nil
}

Expand All @@ -998,6 +1034,8 @@ type nextHopGroupEntry struct {
// electionID is the explicit election ID to be used when this entry is used
// in an AFTOperation.
electionID *spb.Uint128
// fibStatus is the FIB programming status of the entry.
fibStatus spb.AFTEntry_Status
}

// NextHopGroupEntry returns a builder that can be used to build up a NextHopGroup within
Expand Down Expand Up @@ -1052,6 +1090,15 @@ func (n *nextHopGroupEntry) WithElectionID(low, high uint64) *nextHopGroupEntry
return n
}

// WithFIBProgrammed sets the FIB programming status of the entry to PROGRAMMED if
// the specified operation programming result is InstalledInFIB.
func (n *nextHopGroupEntry) WithFIBProgrammed(r ProgrammingResult) *nextHopGroupEntry {
if r == InstalledInFIB {
n.fibStatus = spb.AFTEntry_PROGRAMMED
}
return n
}

// OpProto implements the GRIBIEntry interface, building 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.
Expand All @@ -1072,6 +1119,7 @@ func (n *nextHopGroupEntry) EntryProto() (*spb.AFTEntry, error) {
Entry: &spb.AFTEntry_NextHopGroup{
NextHopGroup: proto.Clone(n.pb).(*aftpb.Afts_NextHopGroupKey),
},
FibStatus: n.fibStatus,
}, nil
}

Expand Down