Skip to content
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

RT-7.5 Fixed timing of policy application & peergroup configurations #3611

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

RT-7.5 Fixed timing of policy application & peergroup configurations #3611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8ca8670
RT-7.5 Fixed timing of policy & peergroup configurations
snaragund Nov 27, 2024
61588d5
Merge branch 'main' into SND-RT7dot5
snaragund Dec 10, 2024
401428f
Merge branch 'main' into SND-RT7dot5
snaragund Dec 14, 2024
20ffdd4
Merge branch 'main' into SND-RT7dot5
snaragund Dec 23, 2024
0c62454
Merge branch 'main' into SND-RT7dot5
snaragund Jan 15, 2025
16e36f9
Merge branch 'main' into SND-RT7dot5
snaragund Jan 30, 2025
6cb994b
Merge branch 'main' into SND-RT7dot5
snaragund Jan 31, 2025
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
50 changes: 36 additions & 14 deletions feature/bgp/policybase/otg_tests/link_bandwidth_test/link_bandwidth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ func TestBGPLinkBandwidth(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Logf("Description: %s", tc.name)
tc.applyPolicy(t, dut, tc.policyName)
time.Sleep(20 * time.Second)
Copy link
Member

Choose a reason for hiding this comment

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

instead of adding sleep here, use gnmi.Watch with .Await in the appropriate validate function calls.

For reference, see the ONDATRA best practice on avoiding use of sleep in tests.
https://pkg.go.dev/github.com/openconfig/ondatra/gnmi#hdr-Best_Practice__Avoid_time_Sleep

tc.validate(t, dut, tc.policyName)
tc.validateRouteCommunityV4(t, td, tc.routeCommunity)
tc.validateRouteCommunityV6(t, td, tc.routeCommunity)
Expand Down Expand Up @@ -408,37 +409,41 @@ func validateRouteCommunityV4Prefix(t *testing.T, td testData, community, v4Pref
for _, bgpPrefix := range bgpPrefixes {
if bgpPrefix.GetAddress() == v4Prefix {
t.Logf("Prefix recevied on OTG is correct, got Address %s, want prefix %v", bgpPrefix.GetAddress(), v4Prefix)
for _, ec := range bgpPrefix.ExtendedCommunity {
lbSubType := ec.Structured.NonTransitive_2OctetAsType.LinkBandwidthSubtype
t.Logf("LC: Bandwidth from OTG : %v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
}
switch community {
case "none":
if len(bgpPrefix.Community) != 0 || len(bgpPrefix.ExtendedCommunity) != 0 {
t.Errorf("community and ext communituy are not empty it should be none")
t.Errorf("ERROR: community and ext communituy are not empty it should be none")
}
case "100:100":
for _, gotCommunity := range bgpPrefix.Community {
t.Logf("community AS:%d val: %d", gotCommunity.GetCustomAsNumber(), gotCommunity.GetCustomAsValue())
if gotCommunity.GetCustomAsNumber() != 100 || gotCommunity.GetCustomAsValue() != 100 {
t.Errorf("community is not 100:100 got AS number:%d AS value:%d", gotCommunity.GetCustomAsNumber(), gotCommunity.GetCustomAsValue())
t.Errorf("ERROR: community is not 100:100 got AS number:%d AS value:%d", gotCommunity.GetCustomAsNumber(), gotCommunity.GetCustomAsValue())
}
}
default:
if len(bgpPrefix.ExtendedCommunity) == 0 {
t.Errorf("ERROR extended community is empty, expected %v", community)
t.Errorf("ERROR: extended community is empty, expected %v", community)
return
}
for _, ec := range bgpPrefix.ExtendedCommunity {
lbSubType := ec.Structured.NonTransitive_2OctetAsType.LinkBandwidthSubtype
listCommunity := strings.Split(community, ":")
bandwidth := listCommunity[2]
if lbSubType.GetGlobal_2ByteAs() != 23456 && lbSubType.GetGlobal_2ByteAs() != 32002 && lbSubType.GetGlobal_2ByteAs() != 32001 {
t.Errorf("ERROR AS number should be 23456 or %d got %d", ateAS, lbSubType.GetGlobal_2ByteAs())
t.Errorf("ERROR: AS number should be 23456 or %d got %d", ateAS, lbSubType.GetGlobal_2ByteAs())
return
}
if bandwidth == "1000" && ygot.BinaryToFloat32(lbSubType.GetBandwidth()) == 0 {
t.Errorf("ERROR lb Bandwidth want 1000, got:=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
t.Errorf("ERROR: lb Bandwidth want 1000, got:=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
} else if bandwidth == "1000000" && ygot.BinaryToFloat32(lbSubType.GetBandwidth()) != 125000 && ygot.BinaryToFloat32(lbSubType.GetBandwidth()) != 1000000 {
t.Errorf("ERROR lb Bandwidth want :1M, got=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
t.Errorf("ERROR: lb Bandwidth want :1M, got=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
} else if bandwidth == "2000000000" && ygot.BinaryToFloat32(lbSubType.GetBandwidth()) != 2.5e+08 && ygot.BinaryToFloat32(lbSubType.GetBandwidth()) != 2000000000 {
t.Errorf("ERROR lb Bandwidth want :2G, got=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
t.Errorf("ERROR: lb Bandwidth want :2G, got=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
}

if !deviations.BgpExtendedCommunityIndexUnsupported(td.dut) {
Expand Down Expand Up @@ -477,37 +482,41 @@ func validateRouteCommunityV6Prefix(t *testing.T, td testData, community, v6Pref
for _, bgpPrefix := range bgpPrefixes {
if bgpPrefix.GetAddress() == v6Prefix {
t.Logf("Prefix recevied on OTG is correct, got prefix:%v , want prefix %v", bgpPrefix.GetAddress(), v6Prefix)
for _, ec := range bgpPrefix.ExtendedCommunity {
lbSubType := ec.Structured.NonTransitive_2OctetAsType.LinkBandwidthSubtype
t.Logf("LC: Bandwidth from OTG : %v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
}
switch community {
case "none":
if len(bgpPrefix.Community) != 0 || len(bgpPrefix.ExtendedCommunity) != 0 {
t.Errorf("community and ext community are not empty it should be none")
t.Errorf("ERROR: community and ext community are not empty it should be none")
}
case "100:100":
for _, gotCommunity := range bgpPrefix.Community {
t.Logf("community AS:%d val: %d", gotCommunity.GetCustomAsNumber(), gotCommunity.GetCustomAsValue())
if gotCommunity.GetCustomAsNumber() != 100 || gotCommunity.GetCustomAsValue() != 100 {
t.Errorf("community is not 100:100 got AS number:%d AS value:%d", gotCommunity.GetCustomAsNumber(), gotCommunity.GetCustomAsValue())
t.Errorf("ERROR: community is not 100:100 got AS number:%d AS value:%d", gotCommunity.GetCustomAsNumber(), gotCommunity.GetCustomAsValue())
}
}
default:
if len(bgpPrefix.ExtendedCommunity) == 0 {
t.Errorf("ERROR extended community is empty, expected %v", community)
t.Errorf("ERROR: extended community is empty, expected %v", community)
return
}
for _, ec := range bgpPrefix.ExtendedCommunity {
lbSubType := ec.Structured.NonTransitive_2OctetAsType.LinkBandwidthSubtype
listCommunity := strings.Split(community, ":")
bandwidth := listCommunity[2]
if lbSubType.GetGlobal_2ByteAs() != 23456 && lbSubType.GetGlobal_2ByteAs() != 32002 && lbSubType.GetGlobal_2ByteAs() != 32001 {
t.Errorf("ERROR AS number should be 23456 or %d got %d", ateAS, lbSubType.GetGlobal_2ByteAs())
t.Errorf("ERROR: AS number should be 23456 or %d got %d", ateAS, lbSubType.GetGlobal_2ByteAs())
return
}
if bandwidth == "1000" && ygot.BinaryToFloat32(lbSubType.GetBandwidth()) == 0 {
t.Errorf("ERROR lb Bandwidth want 1000, got:=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
t.Errorf("ERROR: lb Bandwidth want 1000, got:=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
} else if bandwidth == "1000000" && ygot.BinaryToFloat32(lbSubType.GetBandwidth()) != 125000 && ygot.BinaryToFloat32(lbSubType.GetBandwidth()) != 1000000 {
t.Errorf("ERROR lb Bandwidth want :1M, got=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
t.Errorf("ERROR: lb Bandwidth want :1M, got=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
} else if bandwidth == "2000000000" && ygot.BinaryToFloat32(lbSubType.GetBandwidth()) != 2.5e+08 && ygot.BinaryToFloat32(lbSubType.GetBandwidth()) != 2000000000 {
t.Errorf("ERROR lb Bandwidth want :2G, got=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
t.Errorf("ERROR: lb Bandwidth want :2G, got=%v", ygot.BinaryToFloat32(lbSubType.GetBandwidth()))
}
if !deviations.BgpExtendedCommunityIndexUnsupported(td.dut) {
verifyExtCommunityIndexV6(t, td, v6Prefix)
Expand Down Expand Up @@ -883,6 +892,7 @@ func createFlowV6(t *testing.T, td testData, fc flowConfig) {
td.ate.OTG().PushConfig(t, td.top)
td.ate.OTG().StartProtocols(t)
otgutils.WaitForARP(t, td.ate.OTG(), td.top, "IPv6")
time.Sleep(3 * time.Second)
}

func checkTraffic(t *testing.T, td testData, flowName string) {
Expand Down Expand Up @@ -917,11 +927,20 @@ func (td *testData) advertiseRoutesWithEBGP(t *testing.T) {
g.GetOrCreateAfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV4_UNICAST).Enabled = ygot.Bool(true)
g.GetOrCreateAfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV6_UNICAST).Enabled = ygot.Bool(true)

// Note: we have to define the peer group even if we aren't setting any policy because it's
// invalid OC for the neighbor to be part of a peer group that doesn't exist.
pg4 := bgp.GetOrCreatePeerGroup(cfgplugins.BGPPeerGroup1)
pg4.SetPeerGroupName(cfgplugins.BGPPeerGroup1)
pg6 := bgp.GetOrCreatePeerGroup(cfgplugins.BGPPeerGroup1)
pg6.SetPeerGroupName(cfgplugins.BGPPeerGroup1)

nV41 := bgp.GetOrCreateNeighbor(atePort1.IPv4)
nV41.SetPeerAs(ateAS)
nV41.GetOrCreateAfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV4_UNICAST).Enabled = ygot.Bool(true)
nV42 := bgp.GetOrCreateNeighbor(atePort2.IPv4)
nV42.SetPeerAs(dutAS)
nV41.SetPeerGroup(cfgplugins.BGPPeerGroup1)
nV42.SetPeerGroup(cfgplugins.BGPPeerGroup1)
if !deviations.SkipBgpSendCommunityType(td.dut) {
nV42.SetSendCommunityType([]oc.E_Bgp_CommunityType{oc.Bgp_CommunityType_STANDARD, oc.Bgp_CommunityType_EXTENDED})
}
Expand All @@ -931,6 +950,8 @@ func (td *testData) advertiseRoutesWithEBGP(t *testing.T) {
nV61.GetOrCreateAfiSafi(oc.BgpTypes_AFI_SAFI_TYPE_IPV6_UNICAST).Enabled = ygot.Bool(true)
nV62 := bgp.GetOrCreateNeighbor(atePort2.IPv6)
nV62.SetPeerAs(dutAS)
nV61.SetPeerGroup(cfgplugins.BGPPeerGroup1)
nV62.SetPeerGroup(cfgplugins.BGPPeerGroup1)
if !deviations.SkipBgpSendCommunityType(td.dut) {
nV62.SetSendCommunityType([]oc.E_Bgp_CommunityType{oc.Bgp_CommunityType_STANDARD, oc.Bgp_CommunityType_EXTENDED})
}
Expand Down Expand Up @@ -1063,6 +1084,7 @@ func baseSetupConfigAndVerification(t *testing.T, td testData) {
td.verifyDUTBGPEstablished(t)
td.verifyOTGBGPEstablished(t)
configureImportRoutingPolicyAllowAll(t, td.dut)
time.Sleep(5 * time.Second)
Copy link
Member

Choose a reason for hiding this comment

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

instead of adding sleep here, use gnmi.Watch with .Await in the validateImportRoutingPolicyAllowAll function below. It seems most of the validate functions are already using await (example), but for some reason, this one is not:

https://github.com/openconfig/featureprofiles/pull/3611/files#diff-e3f393530b8e7c44283201dcf22247b9d7d69e193af76ea8533c16fc0b1e83f6R567

validateImportRoutingPolicyAllowAll(t, td.dut, td.ate)
createFlow(t, td, flowConfig{src: atePort2, dstNw: "v4-bgpNet-dev1", dstIP: v41TrafficStart})
createFlow(t, td, flowConfig{src: atePort2, dstNw: "v4-bgpNet-dev2", dstIP: v42TrafficStart})
Expand Down
11 changes: 11 additions & 0 deletions feature/bgp/policybase/otg_tests/link_bandwidth_test/metadata.textproto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,16 @@ platform_exceptions: {
bgp_explicit_extended_community_enable: true
}
}
platform_exceptions: {
platform: {
vendor: NOKIA
}
deviations: {
interface_enabled: true
explicit_interface_in_default_vrf: true
bgp_extended_community_index_unsupported: true
skip_bgp_send_community_type: true
}
}
tags: TAGS_AGGREGATION
tags: TAGS_DATACENTER_EDGE
Loading