Fix RT-7.1: BGP default policies test for Arista (#3024)

* Fix RT-7.1: BGP default policies test for Arista the diff contains the following changes to make the test pass for Arista - Add "bgp_default_policy_unsupported" deviation for Arista since default bgp policy action will not be applied for prefixes that do not match on any condition in an inbound/outbound policy applied for a given neighbor. - Fix the "configureRoutingPolicyDefaultAction()" to not create raw json object to create the default accept/reject rule in the policy when "bgp_default_policy_unsupported" is present. - set mode if the prefix-set definition ( which is a requirement for arista ) for the prefix-set to be applied on a policy - Fix another issue for 7.1.3 and 7.1.4 case to make sure the prefix-set match condition is deleted before applying the default accept/deny rule * Address review comments * address review comments * Fix readme to add OpenConfig Path and RPC Coverage section * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md --------- Co-authored-by: Pramod Maurya <pramodmaurya@google.com>
openconfig · Jun 10, 2024 · b3130b5 · b3130b5
1 parent 7d87efa
commit b3130b5
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 87 deletions.
diff --git a/feature/bgp/policybase/otg_tests/default_policies_test/README.md b/feature/bgp/policybase/otg_tests/default_policies_test/README.md
@@ -96,44 +96,34 @@ B <-- IBGP+IS-IS --> C[Port2:OTG];
     * DUT:Port2 wouldn't export routes to IPv4-prefix1, IPv4-prefix2, IPv4-prefix3, IPv6-prefix1, IPv6-prefix2 and IPv6-prefix3 since they are missing from the DUT's forwarding table.
     * IS-IS and static routes shouldn't be advertised to the EBGP and IBGP peers.
 
-### Config Parameter Coverage
 
-  * Defined Sets
-    * /routing-policy/defined-sets/prefix-sets/prefix-set/
-    * /routing-policy/defined-sets/prefix-sets/prefix-set/prefixes/prefix/config/ip-prefix
-    * /routing-policy/defined-sets/prefix-sets/prefix-set/prefixes/prefix/config/masklength-range/exact
-
-  * Policy-Definition
-    * /routing-policy/policy-definitions/policy-definition/config/name
-    * /routing-policy/policy-definitions/policy-definition/statements/statement/config/name
-    * /routing-policy/policy-definitions/policy-definition/statements/statement/conditions/match-prefix-set/config/prefix-set
-    * /routing-policy/policy-definitions/policy-definition/statements/statement/conditions/match-prefix-set/config/match-set-options
-    * /routing-policy/policy-definitions/policy-definition/statements/statement/actions/config/policy-result/ACCEPT_ROUTE
-    * /routing-policy/policy-definitions/policy-definition/statements/statement/actions/config/policy-result/REJECT_ROUTE
-
-  * Path to Neighbor or Peer-Group level
-    * /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/
-    * /network-instances/network-instance/protocols/protocol/bgp/neighbors/peer-group/
-
-  * Apply Policy at Neighbor or Peer-Group level
-    * afi-safis/afi-safi/apply-policy/config/import-policy
-    * afi-safis/afi-safi/apply-policy/config/export-policy
-    * afi-safis/afi-safi/apply-policy/config/default-import-policy/ACCEPT-ALL
-    * afi-safis/afi-safi/apply-policy/config/default-export-policy/ACCEPT-ALL
-    * afi-safis/afi-safi/apply-policy/config/default-import-policy/REJECT-ALL
-    * afi-safis/afi-safi/apply-policy/config/default-export-policy/REJECT-ALL
-
-
-### Telemetry Parameter Coverage
-
-  * Path to Neighbor or Peer-Group level:
-    * /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor
-    * /network-instances/network-instance/protocols/protocol/bgp/peer-groups/peer-group
-
-  * Paths under Neighbor and Peer-Group level:
-    * afi-safis/afi-safi/apply-policy/state/export-policy
-    * afi-safis/afi-safi/apply-policy/state/import-policy
-    * afi-safis/afi-safi/state/prefixes/installed
-    * afi-safis/afi-safi/state/prefixes/received
-    * afi-safis/afi-safi/state/prefixes/received-pre-policy
-    * afi-safis/afi-safi/state/prefixes/sent
+## OpenConfig Path and RPC Coverage
+```yaml
+paths:
+  # Defined Sets
+  /routing-policy/defined-sets/prefix-sets/prefix-set/prefixes/prefix/config/ip-prefix:
+  /routing-policy/defined-sets/prefix-sets/prefix-set/prefixes/prefix/config/masklength-range:
+  # Policy-Definition
+  /routing-policy/policy-definitions/policy-definition/config/name:
+  /routing-policy/policy-definitions/policy-definition/statements/statement/config/name:
+  /routing-policy/policy-definitions/policy-definition/statements/statement/conditions/match-prefix-set/config/prefix-set:
+  /routing-policy/policy-definitions/policy-definition/statements/statement/conditions/match-prefix-set/config/match-set-options:
+  /routing-policy/policy-definitions/policy-definition/statements/statement/actions/config/policy-result:  
+  # Apply Policy at Neighbor or Peer-Group level
+  /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/apply-policy/config/import-policy:
+  /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/apply-policy/config/export-policy:
+  /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/apply-policy/config/default-import-policy:
+  /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/apply-policy/config/default-export-policy:
+  # Paths under Neighbor and Peer-Group level
+  /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/apply-policy/state/export-policy:
+  /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/apply-policy/state/import-policy:
+  /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/state/prefixes/installed:
+  /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/state/prefixes/received:
+  /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/state/prefixes/received-pre-policy:
+  /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/state/prefixes/sent:
+rpcs:
+  gnmi:
+    gNMI.Get:
+    gNMI.Set:
+    gNMI.Subscribe:
+```
diff --git a/feature/bgp/policybase/otg_tests/default_policies_test/bgp_default_policies_test.go b/feature/bgp/policybase/otg_tests/default_policies_test/bgp_default_policies_test.go
@@ -15,8 +15,6 @@
 package bgp_default_policies_test
 
 import (
-	"context"
-	"encoding/json"
 	"fmt"
 	"testing"
 	"time"
@@ -26,7 +24,6 @@ import (
 	"github.com/openconfig/featureprofiles/internal/attrs"
 	"github.com/openconfig/featureprofiles/internal/deviations"
 	"github.com/openconfig/featureprofiles/internal/fptest"
-	gpb "github.com/openconfig/gnmi/proto/gnmi"
 	"github.com/openconfig/ondatra"
 	"github.com/openconfig/ondatra/gnmi"
 	"github.com/openconfig/ondatra/gnmi/oc"
@@ -195,6 +192,13 @@ func configurePrefixMatchPolicy(t *testing.T, dut *ondatra.DUTDevice, prefixSet,
 	pset := rp.GetOrCreateDefinedSets().GetOrCreatePrefixSet(prefixSet)
 	for _, pref := range ipPrefixSet {
 		pset.GetOrCreatePrefix(pref+"/"+maskLen, prefixSubnetRange)
+		mode := oc.PrefixSet_Mode_IPV4
+		if maskLen == maskLen128 {
+			mode = oc.PrefixSet_Mode_IPV6
+		}
+		if !deviations.SkipPrefixSetMode(dut) {
+			pset.SetMode(mode)
+		}
 	}
 
 	pdef := rp.GetOrCreatePolicyDefinition(prefixSet)
@@ -584,54 +588,23 @@ func deleteBGPPolicy(t *testing.T, dut *ondatra.DUTDevice, nbrList []*bgpNbrList
 }
 
 func configureRoutingPolicyDefaultAction(t *testing.T, dut *ondatra.DUTDevice, action string, replace bool) {
-
-	var routingPolicyDefaultAction = []any{
-		map[string]any{
-			"default-action": map[string]any{
-				"policy-result": action,
-			},
-		},
-	}
-	defaultAction, err := json.Marshal(routingPolicyDefaultAction)
-	if err != nil {
-		t.Fatalf("Error with json Marshal: %v", err)
-	}
-
-	var updates []*gpb.Update
+	d := &oc.Root{}
+	rp := d.GetOrCreateRoutingPolicy()
 	for _, policy := range []string{ebgpExportIPv4, ebgpExportIPv6, ibgpExportIPv4, ibgpExportIPv6, ebgpImportIPv4, ebgpImportIPv6, ibgpImportIPv4, ibgpImportIPv6} {
-		update := gpb.Update{
-			Path: &gpb.Path{
-				Elem: []*gpb.PathElem{
-					{Name: "routing-policy"},
-					{Name: "policy", Key: map[string]string{"name": policy}},
-				},
-			},
-			Val: &gpb.TypedValue{
-				Value: &gpb.TypedValue_JsonIetfVal{
-					JsonIetfVal: defaultAction,
-				},
-			},
+		pdef := rp.GetOrCreatePolicyDefinition(policy)
+		stmt, err := pdef.AppendNewStatement("50")
+		if err != nil {
+			t.Fatal(err)
 		}
-		updates = append(updates, &update)
-	}
-	gpbSetRequest := &gpb.SetRequest{
-		Prefix: &gpb.Path{
-			Origin: "native",
-		},
-		Update: updates,
-	}
-	if replace {
-		gpbSetRequest = &gpb.SetRequest{
-			Prefix: &gpb.Path{
-				Origin: "native",
-			},
-			Replace: updates,
+		stmt.GetOrCreateActions().PolicyResult = oc.RoutingPolicy_PolicyResultType_ACCEPT_ROUTE
+		if action == "reject" {
+			stmt.GetOrCreateActions().PolicyResult = oc.RoutingPolicy_PolicyResultType_REJECT_ROUTE
 		}
 	}
-
-	gnmiClient := dut.RawAPIs().GNMI(t)
-	if _, err := gnmiClient.Set(context.Background(), gpbSetRequest); err != nil {
-		t.Fatalf("Unexpected error updating SRL routing-policy default-action: %v", err)
+	if replace {
+		gnmi.Replace(t, dut, gnmi.OC().RoutingPolicy().Config(), rp)
+	} else {
+		gnmi.Update(t, dut, gnmi.OC().RoutingPolicy().Config(), rp)
 	}
 	time.Sleep(5 * time.Second)
 }

diff --git a/feature/bgp/policybase/otg_tests/default_policies_test/metadata.textproto b/feature/bgp/policybase/otg_tests/default_policies_test/metadata.textproto
@@ -26,6 +26,7 @@ platform_exceptions:  {
     isis_interface_afi_unsupported: true
     isis_instance_enabled_required: true
     default_import_export_policy: true
+    bgp_default_policy_unsupported: true
   }
 }
 platform_exceptions:  {