Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
82 changes: 73 additions & 9 deletions feature/gnmi/tests/gnmi_ni_test/gnmi_ni_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package gnmi_ni_test

import (
"fmt"
"math/rand"
"testing"
"time"

"github.com/openconfig/featureprofiles/internal/attrs"
"github.com/openconfig/featureprofiles/internal/cfgplugins"
Expand Down Expand Up @@ -54,17 +57,33 @@ var (
func TestGNMIAdditionalNetworkInstance(t *testing.T) {
// configure DUT
dut := ondatra.DUT(t, "dut")
// Get current ports from the device
usedPorts := GetUsedPorts(t, dut)
// Generate a port number between 10000 and 65535
gNMIPort1, err := GenerateUniquePort(10000, 65535, usedPorts)
if err != nil {
t.Fatalf("Could not generate port: %v", err)
}
t.Logf("Generated unique gNMIport1: %d", gNMIPort1)

usedPorts = append(usedPorts, gNMIPort1)
gNMIPort2, err := GenerateUniquePort(10000, 65535, usedPorts)
if err != nil {
t.Fatalf("Could not generate port: %v", err)
}
t.Logf("Generated unique gNMIport2: %d", gNMIPort2)

batch := &gnmi.SetBatch{}
ConfigureDUT(batch, t, dut)
ConfigureAdditionalNetworkInstance(batch, t, dut, customVRFName)
ConfigureDUT(batch, t, dut, gNMIPort1)
ConfigureAdditionalNetworkInstance(batch, t, dut, customVRFName, gNMIPort2)
t.Log("\nApplying configuration to DUT\n")
batch.Set(t, dut)
t.Log("\nConfiguration applied to DUT\n")
ValidateNetworkInstance(t, dut)
}

// ConfigureDUT configures port1 and port2 on the DUT with default network instance.
func ConfigureDUT(batch *gnmi.SetBatch, t *testing.T, dut *ondatra.DUTDevice) {
func ConfigureDUT(batch *gnmi.SetBatch, t *testing.T, dut *ondatra.DUTDevice, gNMIPort uint16) {

// Configuring basic interface and subinterfaces.
cfgplugins.EnableInterfaceAndSubinterfaces(t, dut, batch, dutPort1)
Expand All @@ -75,15 +94,16 @@ func ConfigureDUT(batch *gnmi.SetBatch, t *testing.T, dut *ondatra.DUTDevice) {
cfgplugins.AssignInterfaceToNetworkInstance(t, batch, dut, dut.Port(t, "port1").Name(), &dutPort1NetworkInstanceIParams, dutPort1.Subinterface)
}

// Configure default network instance.
cfgplugins.NewNetworkInstance(t, dut, batch, &dutPort1NetworkInstanceIParams)
// TBD Not required #### Configure default network instance.
//cfgplugins.NewNetworkInstance(t, dut, batch, &dutPort1NetworkInstanceIParams)

// Configure gNMI server on default network instance.
cfgplugins.CreateGNMIServer(t, dut, batch, &dutPort1NetworkInstanceIParams)
transportSecurity := false
cfgplugins.CreateGNMIServer(t, dut, batch, &dutPort1NetworkInstanceIParams, gNMIPort, transportSecurity)
}

// ConfigureAdditionalNetworkInstance configures a new network instance in DUT and changes the network instance of port2
func ConfigureAdditionalNetworkInstance(batch *gnmi.SetBatch, t *testing.T, dut *ondatra.DUTDevice, ni string) {
func ConfigureAdditionalNetworkInstance(batch *gnmi.SetBatch, t *testing.T, dut *ondatra.DUTDevice, ni string, gNMIPort uint16) {
// Configure interface, non-default network instance
t.Logf("\nConfiguring network instance and gNMI server: Network instance: %s \n", ni)

Expand All @@ -96,7 +116,8 @@ func ConfigureAdditionalNetworkInstance(batch *gnmi.SetBatch, t *testing.T, dut
cfgplugins.NewNetworkInstance(t, dut, batch, &dutPort2NetworkInstanceIParams)

// Configure non-default gNMI server.
cfgplugins.CreateGNMIServer(t, dut, batch, &dutPort2NetworkInstanceIParams)
transportSecurity := false
cfgplugins.CreateGNMIServer(t, dut, batch, &dutPort2NetworkInstanceIParams, gNMIPort, transportSecurity)
}

func ValidateNetworkInstance(t *testing.T, dut *ondatra.DUTDevice) {
Expand Down Expand Up @@ -126,11 +147,17 @@ func ValidateNetworkInstance(t *testing.T, dut *ondatra.DUTDevice) {
customGnmiServerName := "gnxi-" + customVRFName

var defaultValidated, customValidated bool
//if Juniper remove DEFAULT from the list.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This comment is informal and slightly inaccurate as the code skips the entry rather than removing it from the list. Consider rephrasing for clarity and following standard Go comment style (starting with a space after //).


for _, gnmiServer := range gnmiServerList {
serverName := gnmiServer.GetName()
// 1. Strict Skip for the internal Juniper system entry
if dut.Vendor() == ondatra.JUNIPER && serverName == "DEFAULT" {
t.Logf("Skipping internal Juniper system server placeholder: %s", serverName)
continue
}
// Using gnmiServer.GetName() to get the state is better than hardcoding.
serverState := gnmi.Get(t, dut, gnmi.OC().System().GrpcServer(serverName).State())

switch serverName {
case customGnmiServerName:
validateGnmiServerState(t, serverState)
Expand Down Expand Up @@ -165,3 +192,40 @@ func validateGnmiServerState(t *testing.T, state *oc.System_GrpcServer) {
t.Logf("gNMI Server: %s, running on network instance: %s, listening port: %v, Enabled: %t",
state.GetName(), state.GetNetworkInstance(), state.GetPort(), state.GetEnable())
}
func GetUsedPorts(t *testing.T, dut *ondatra.DUTDevice) []uint16 {
t.Helper()
var used []uint16

// Query all gRPC servers in the system
servers := gnmi.GetAll(t, dut, gnmi.OC().System().GrpcServerAny().State())

for _, s := range servers {
if s.Port != nil {
used = append(used, s.GetPort())
}
}

return used
}

// GenerateUniquePort picks a random port between min and max that isn't in the used list.
func GenerateUniquePort(min, max int, usedPorts []uint16) (uint16, error) {
// Seed the random generator
rand.Seed(time.Now().UnixNano())
Comment on lines +210 to +211
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Seeding the random number generator inside a function that is called multiple times is inefficient and can lead to non-random results if calls occur within the same nanosecond. Since Go 1.20, the global random generator is seeded automatically. If a manual seed is required for older Go versions, it should be placed in TestMain or init. Removing this call will also make the time import at line 7 unnecessary.


// Create a map for O(1) lookup
usedMap := make(map[uint16]bool)
for _, p := range usedPorts {
usedMap[p] = true
}

// Try up to 100 times to find a random free port
for i := 0; i < 100; i++ {
p := uint16(rand.Intn(max-min+1) + min)
if !usedMap[p] {
return p, nil
}
}

return 0, fmt.Errorf("failed to find a unique port after 100 attempts")
}
2 changes: 2 additions & 0 deletions feature/gnmi/tests/gnmi_ni_test/metadata.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,7 @@ platform_exceptions: {
}
deviations: {
interface_enabled: true
default_ni_gnmi_server_name: "gnxi-mgmt_junos"
default_network_instance: "mgmt_junos"
}
}
11 changes: 6 additions & 5 deletions internal/cfgplugins/system.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
)

// CreateGNMIServer creates a gNMI server on the DUT on a given network-instance.
func CreateGNMIServer(t testing.TB, d *ondatra.DUTDevice, batch *gnmi.SetBatch, nip *NetworkInstanceParams) {
func CreateGNMIServer(t testing.TB, d *ondatra.DUTDevice, batch *gnmi.SetBatch, nip *NetworkInstanceParams, port uint16, transportSec bool) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The function signature for CreateGNMIServer violates the repository style guide for configuration plugins. It should use a configuration struct to pass parameters (such as port and transportSec) and return the *gnmi.SetBatch object. Additionally, adding parameters directly to the signature is a breaking change for other tests using this internal library.

References
  1. Configuration plugin functions must use a struct for parameters and return a *gnmi.SetBatch. (link)

var niName string
var gnmiServerName string

Expand All @@ -59,10 +59,11 @@ func CreateGNMIServer(t testing.TB, d *ondatra.DUTDevice, batch *gnmi.SetBatch,
t.Logf("Creating gNMI server %s on network instance: %s", gnmiServerName, niName)
gnmiServerPath := gnmi.OC().System().GrpcServer(gnmiServerName)
gnmiServer := &oc.System_GrpcServer{
Name: ygot.String(gnmiServerName),
Port: ygot.Uint16(9339),
Enable: ygot.Bool(true),
NetworkInstance: ygot.String(niName),
Name: ygot.String(gnmiServerName),
Port: ygot.Uint16(port),
Enable: ygot.Bool(true),
NetworkInstance: ygot.String(niName),
TransportSecurity: ygot.Bool(transportSec),
}
if !deviations.GrpcServerServicesUnsupported(d) {
gnmiServer.Services = []oc.E_SystemGrpc_GRPC_SERVICE{oc.SystemGrpc_GRPC_SERVICE_GNMI}
Expand Down
Loading