From 373cdd44e0f98a789f886ee4bee04177966df3cf Mon Sep 17 00:00:00 2001 From: Pramod Maurya Date: Tue, 13 Aug 2024 13:05:30 +0530 Subject: [PATCH] using OC to create prefix-list and gnoi to terminate Octa (#3368) * using OC to create prefix-list * PR comment fixes * added changes for Nokia --- .../otg_tests/prefix_set_test/README.md | 35 ++--- .../prefix_set_test/prefix_set_test.go | 75 +++++------ internal/gnoi/gnoi.go | 124 ++++++++++++++++++ internal/system/system.go | 39 ++++++ 4 files changed, 216 insertions(+), 57 deletions(-) create mode 100644 internal/gnoi/gnoi.go create mode 100644 internal/system/system.go diff --git a/feature/experimental/policy/otg_tests/prefix_set_test/README.md b/feature/experimental/policy/otg_tests/prefix_set_test/README.md index 59789d7cd6f..20ccedcaaa8 100644 --- a/feature/experimental/policy/otg_tests/prefix_set_test/README.md +++ b/feature/experimental/policy/otg_tests/prefix_set_test/README.md @@ -2,7 +2,9 @@ ## Summary -- Prefix list is updated and replaced correctly +- Prefix list is updated and replaced correctly after restarting the process + with supports gNOI to validate that internal state of OC agent is in sync + with the running configuration. ## Testbed type @@ -67,23 +69,23 @@ the DUT in one gnmi.Set using the `replace` option ### RT-1.53.4 [TODO:https://github.com/openconfig/featureprofiles/issues/3306] -### Add prefix-list from cli and then replace with gnmi +### Create prefix list and replace with gnmi. -* Configure Prefix-list through CLI Config Sessions. +* Send a gNMI SET request that contains below prefixes under TAG_3_IPV4 prefix-set ``` - ip prefix-list TAG_3_IPV4 - seq 10 permit 10.240.31.48/28 - seq 20 permit 10.244.187.32/28 - seq 30 permit 173.36.128.0/20 - seq 40 permit 173.37.128.0/20 - seq 50 permit 173.38.128.0/20 - seq 60 permit 173.39.128.0/20 - seq 70 permit 173.40.128.0/20 - seq 80 permit 173.41.128.0/20 - seq 90 permit 173.42.128.0/20 - seq 100 permit 173.43.128.0/20 + 10.240.31.48/28 + 10.244.187.32/28 + 173.36.128.0/20 + 173.37.128.0/20 + 173.38.128.0/20 + 173.39.128.0/20 + 173.40.128.0/20 + 173.41.128.0/20 + 173.42.128.0/20 + 173.43.128.0/20 ``` -* Perform octa restart or reboot the device. +* Validate that the prefix-list is created correctly with 10 prefixes. +* Use gNOI to kill the process supporting gNMI. * Send a gNMI SET request that contains additional prefixes within the same prefix-set, TAG_3_IPV4. ``` @@ -110,6 +112,7 @@ the DUT in one gnmi.Set using the `replace` option 173.48.128.0/20 173.45.128.0/20 ``` +* Validate that the prefix-list is created correctly with 22 prefixes. ## OpenConfig Path and RPC Coverage @@ -136,6 +139,8 @@ rpcs: gnmi: gNMI.Set: gNMI.Subscribe: + gnoi: + system.System.KillProcess: ``` ## Required DUT platform diff --git a/feature/experimental/policy/otg_tests/prefix_set_test/prefix_set_test.go b/feature/experimental/policy/otg_tests/prefix_set_test/prefix_set_test.go index 7dab71b0d34..d61f664c2e3 100644 --- a/feature/experimental/policy/otg_tests/prefix_set_test/prefix_set_test.go +++ b/feature/experimental/policy/otg_tests/prefix_set_test/prefix_set_test.go @@ -15,12 +15,11 @@ package prefix_set_test import ( - "context" "testing" "github.com/openconfig/featureprofiles/internal/deviations" "github.com/openconfig/featureprofiles/internal/fptest" - "github.com/openconfig/featureprofiles/internal/helpers" + "github.com/openconfig/featureprofiles/internal/gnoi" "github.com/openconfig/ondatra" "github.com/openconfig/ondatra/gnmi" "github.com/openconfig/ondatra/gnmi/oc" @@ -28,6 +27,7 @@ import ( const ( prefixSetA = "PFX_SET_A" + tag3IPv4 = "TAG_3_IPV4" pfx1 = "10.240.31.48/28" pfx2 = "173.36.128.0/20" pfx3 = "173.36.144.0/20" @@ -107,48 +107,39 @@ func TestPrefixSet(t *testing.T) { } } -func TestPrefixSetWithCLIConfig(t *testing.T) { +func TestPrefixSetWithOCAgentRestart(t *testing.T) { dut := ondatra.DUT(t, "dut") - ctx := context.Background() - cli := dut.RawAPIs().CLI(t) - - switch dut.Vendor() { - case ondatra.ARISTA: - helpers.GnmiCLIConfig(t, dut, ` - ip prefix-list TAG_3_IPV4 - seq 10 permit 10.240.31.48/28 - seq 20 permit 10.244.187.32/28 - seq 30 permit 173.36.128.0/20 - seq 40 permit 173.37.128.0/20 - seq 50 permit 173.38.128.0/20 - seq 60 permit 173.39.128.0/20 - seq 70 permit 173.40.128.0/20 - seq 80 permit 173.41.128.0/20 - seq 90 permit 173.42.128.0/20 - seq 100 permit 173.43.128.0/20 - `, - ) - helpers.GnmiCLIConfig(t, dut, ` - management api gnmi - transport grpc default - operation set persistence - `, - ) - cmd := "agent Octa terminate" - res, err := cli.RunCommand(ctx, "agent Octa terminate") - if err != nil { - t.Errorf("error executing command %q:\n%v", cmd, err) - } - if res.Error() != "" { - t.Errorf("error executing command %q:\n%v", cmd, res.Error()) - } - } dutOcRoot := &oc.Root{} rp := dutOcRoot.GetOrCreateRoutingPolicy() ds := rp.GetOrCreateDefinedSets() - v4PrefixSet := ds.GetOrCreatePrefixSet("TAG_3_IPV4") + v4PrefixSet := ds.GetOrCreatePrefixSet(tag3IPv4) + if !deviations.SkipPrefixSetMode(dut) { + v4PrefixSet.SetMode(oc.PrefixSet_Mode_IPV4) + } + v4PrefixSet.GetOrCreatePrefix("10.240.31.48/28", mskLen) + v4PrefixSet.GetOrCreatePrefix("10.244.187.32/28", mskLen) + v4PrefixSet.GetOrCreatePrefix("173.36.128.0/20", mskLen) + v4PrefixSet.GetOrCreatePrefix("173.37.128.0/20", mskLen) + v4PrefixSet.GetOrCreatePrefix("173.38.128.0/20", mskLen) + v4PrefixSet.GetOrCreatePrefix("173.39.128.0/20", mskLen) + v4PrefixSet.GetOrCreatePrefix("173.40.128.0/20", mskLen) + v4PrefixSet.GetOrCreatePrefix("173.41.128.0/20", mskLen) + v4PrefixSet.GetOrCreatePrefix("173.42.128.0/20", mskLen) + v4PrefixSet.GetOrCreatePrefix("173.43.128.0/20", mskLen) + gnmi.Replace(t, dut, gnmi.OC().RoutingPolicy().DefinedSets().PrefixSet(tag3IPv4).Config(), v4PrefixSet) + prefixSet := gnmi.Get[*oc.RoutingPolicy_DefinedSets_PrefixSet](t, dut, gnmi.OC().RoutingPolicy().DefinedSets().PrefixSet(tag3IPv4).State()) + if got, want := len(prefixSet.Prefix), 10; got != want { + t.Errorf("Prefix set has %v prefixes, want %v", got, want) + } + + gnoi.KillProcess(t, dut, gnoi.OCAGENT, true) + + v4PrefixSet = ds.GetOrCreatePrefixSet(tag3IPv4) v4PrefixSet.SetMode(oc.PrefixSet_Mode_IPV4) + if !deviations.SkipPrefixSetMode(dut) { + v4PrefixSet.SetMode(oc.PrefixSet_Mode_IPV4) + } v4PrefixSet.GetOrCreatePrefix("173.49.128.0/20", mskLen) v4PrefixSet.GetOrCreatePrefix("173.46.128.0/20", mskLen) v4PrefixSet.GetOrCreatePrefix("10.240.31.48/28", mskLen) @@ -172,9 +163,9 @@ func TestPrefixSetWithCLIConfig(t *testing.T) { v4PrefixSet.GetOrCreatePrefix("173.48.128.0/20", mskLen) v4PrefixSet.GetOrCreatePrefix("173.45.128.0/20", mskLen) - gnmi.Replace(t, dut, gnmi.OC().RoutingPolicy().DefinedSets().PrefixSet("TAG_3_IPV4").Config(), v4PrefixSet) - prefixSet := gnmi.Get[*oc.RoutingPolicy_DefinedSets_PrefixSet](t, dut, gnmi.OC().RoutingPolicy().DefinedSets().PrefixSet("TAG_3_IPV4").State()) - if len(prefixSet.Prefix) != 22 { - t.Errorf("Prefix set has %v prefixes, want 22", len(prefixSet.Prefix)) + gnmi.Replace(t, dut, gnmi.OC().RoutingPolicy().DefinedSets().PrefixSet(tag3IPv4).Config(), v4PrefixSet) + prefixSet = gnmi.Get[*oc.RoutingPolicy_DefinedSets_PrefixSet](t, dut, gnmi.OC().RoutingPolicy().DefinedSets().PrefixSet(tag3IPv4).State()) + if got, want := len(prefixSet.Prefix), 22; got != want { + t.Errorf("Prefix set has %v prefixes, want %v", got, want) } } diff --git a/internal/gnoi/gnoi.go b/internal/gnoi/gnoi.go new file mode 100644 index 00000000000..f308fb956d0 --- /dev/null +++ b/internal/gnoi/gnoi.go @@ -0,0 +1,124 @@ +// Copyright 2024 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 gnoi provides utilities for interacting with the gNOI API. +package gnoi + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/openconfig/featureprofiles/internal/system" + gpb "github.com/openconfig/gnmi/proto/gnmi" + spb "github.com/openconfig/gnoi/system" + "github.com/openconfig/ondatra" + "github.com/openconfig/ondatra/gnmi" + "github.com/openconfig/ondatra/gnmi/oc" + "github.com/openconfig/ygnmi/ygnmi" +) + +var ( + daemonProcessNames = map[ondatra.Vendor]map[Daemon]string{ + ondatra.ARISTA: { + GRIBI: "Gribi", + OCAGENT: "Octa", + P4RT: "P4Runtime", + ROUTING: "Bgp-main", + }, + ondatra.CISCO: { + GRIBI: "emsd", + P4RT: "emsd", + ROUTING: "emsd", + }, + ondatra.JUNIPER: { + GRIBI: "rpd", + P4RT: "p4-switch", + ROUTING: "rpd", + }, + ondatra.NOKIA: { + GRIBI: "sr_grpc_server", + OCAGENT: "oc_mgmt_server", + P4RT: "sr_grpc_server", + ROUTING: "sr_bgp_mgr", + }, + } +) + +// Daemon is the type of the daemon on the device. +type Daemon string + +const ( + // GRIBI is the gRIBI daemon. + GRIBI Daemon = "GRIBI" + // OCAGENT is the OpenConfig agent daemon. + OCAGENT Daemon = "OCAGENT" + // P4RT is the P4RT daemon. + P4RT Daemon = "P4RT" + // ROUTING is the routing daemon. + ROUTING Daemon = "ROUTING" +) + +// KillProcess terminates the daemon on the DUT. +func KillProcess(t *testing.T, dut *ondatra.DUTDevice, daemon Daemon, waitForRestart bool) { + t.Helper() + + daemonName, err := FetchProcessName(dut, daemon) + if err != nil { + t.Fatalf("Daemon %s not defined for vendor %s", daemon, dut.Vendor().String()) + } + pid := system.FindProcessIDByName(t, dut, daemonName) + if pid == 0 { + t.Fatalf("process %s not found on device", daemonName) + } + + gnoiClient := dut.RawAPIs().GNOI(t) + killProcessRequest := &spb.KillProcessRequest{ + Signal: spb.KillProcessRequest_SIGNAL_KILL, + Name: daemonName, + Pid: uint32(pid), + Restart: true, + } + gnoiClient.System().KillProcess(context.Background(), killProcessRequest) + + if waitForRestart { + gnmi.WatchAll( + t, + dut.GNMIOpts().WithYGNMIOpts(ygnmi.WithSubscriptionMode(gpb.SubscriptionMode_ON_CHANGE)), + gnmi.OC().System().ProcessAny().State(), + time.Minute, + func(p *ygnmi.Value[*oc.System_Process]) bool { + val, ok := p.Val() + if !ok { + return false + } + return val.GetName() == daemonName && val.GetPid() != pid + }, + ) + } +} + +// FetchProcessName returns the name of the daemon on the DUT based on the vendor. +func FetchProcessName(dut *ondatra.DUTDevice, daemon Daemon) (string, error) { + daemons, ok := daemonProcessNames[dut.Vendor()] + if !ok { + return "", fmt.Errorf("unsupported vendor: %s", dut.Vendor().String()) + } + d, ok := daemons[daemon] + if !ok { + return "", fmt.Errorf("daemon %s not defined for vendor %s", daemon, dut.Vendor().String()) + } + return d, nil +} diff --git a/internal/system/system.go b/internal/system/system.go new file mode 100644 index 00000000000..d21116e2750 --- /dev/null +++ b/internal/system/system.go @@ -0,0 +1,39 @@ +// Copyright 2024 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 system provides helper functions for gNMI system related operations. +package system + +import ( + "testing" + + "github.com/openconfig/ondatra" + "github.com/openconfig/ondatra/gnmi" + "github.com/openconfig/ondatra/gnmi/oc" +) + +// FindProcessIDByName uses telemetry to find out the PID of a process. +func FindProcessIDByName(t *testing.T, dut *ondatra.DUTDevice, pName string) uint64 { + t.Helper() + + var pid uint64 + pList := gnmi.GetAll[*oc.System_Process](t, dut, gnmi.OC().System().ProcessAny().State()) + for _, proc := range pList { + if proc.GetName() == pName { + pid = proc.GetPid() + break + } + } + return pid +}