search

sonic-net / sonic-mgmt

Configuration management examples for SONiC
Other
201 stars 727 forks source link

[Multi-asic/Multi-Dut/T2]: route/test_route_flap.py::test_route_flap fails due to use of BP PC #8612

Closed vperumal closed 1 year ago

vperumal commented 1 year ago

Description

Testcase passes when front facing ports are chosen for finding out the Exabgp port

show ip int | grep -w Ethernet60 | awk '{print $4}'

But fails when it choses a Backplane PC, which is expected.

The test should skip BP ports, which in this case it doesn't, hence hitting the key error.

duthost = tbinfo = {'auto_recover': 'True', 'comment': 'Tests SFD T2 - Closed PID setup', 'conf-name': 'sfd-lt2', 'duts': ['sfd-lt2-lc0', 'sfd-lt2-lc1', 'sfd-lt2-lc2', 'sfd-lt2-sup'], ...} dev_port = 'PortChannel01'

def get_exabgp_port(duthost, tbinfo, dev_port):
    tor1 = duthost.shell("show ip int | grep -w {} | awk '{{print $4}}'".format(dev_port))['stdout']
  tor1_offset = tbinfo['topo']['properties']['topology']['VMs'][tor1]['vm_offset']

E KeyError: u''

dev_port = 'PortChannel01' duthost = tbinfo = {'auto_recover': 'True', 'comment': 'Tests SFD T2 - Closed PID setup', 'conf-name': 'sfd-lt2', 'duts': ['sfd-lt2-lc0', 'sfd-lt2-lc1', 'sfd-lt2-lc2', 'sfd-lt2-sup'], ...} tor1 = u''

route/test_route_flap.py:191: KeyError ==================================== PASSES ==================================== ____ test_route_flap[sfd-lt2-lc1-0] ____

vperumal commented 1 year ago

FYI @abdosi

abdosi commented 1 year ago

@vperumal can you look into fixng this ?

cc @wenyiz2021 can you see if this is quick fix you can do ?

wenyiz2021 commented 1 year ago

@vperumal @abdosi I'll take a look

wenyiz2021 commented 1 year ago

@vperumal in current design, path that origins from internal should be excluded already here: https://github.com/sonic-net/sonic-mgmt/blob/a0ab44ccc8e16c33d183dd66412ed1f6b64800fe/tests/route/test_route_flap.py#L174

this should be a more strict criteria that even some external routes may origins from 'internal'. I ran this on a Cisco linecard that has portchannel01-30 as internal interfaces, after filtering there are only 3 routes left, and they are all external ones:

(Pdb) dev[route_to_ping][0]['nexthops']
[{u'interfaceName': u'Ethernet72', ...}, {u'interfaceName': u'Ethernet84',...}, {u'interfaceName': u'Ethernet120', ...'}]
(Pdb) len(dev[route_to_ping][0]['nexthops'])
3

can you check if you testing branch is latest? if yes, can you put a breakpoint here https://github.com/sonic-net/sonic-mgmt/blob/a0ab44ccc8e16c33d183dd66412ed1f6b64800fe/tests/route/test_route_flap.py#L269, print route_to_ping and dev[route_to_ping][0]['nexthops'] ? thanks..

vperumal commented 1 year ago

Hi @wenyiz2021, Yes the testing branch was the latest, but I had tested it on the chassis and there the PCs 01-48 face backplane. External facing PCs are 101 onwards. I will add the breakpoint and get u the details needed

wenyiz2021 commented 1 year ago

hi @vperumal, I had this PR merge yday, can you please try with the PR fix? (https://github.com/sonic-net/sonic-mgmt/pull/8623)

the issue is previous design had 2 filters: one makes sure routes origins from external, the other makes sure interface name does not have 'IB'. the 1st filter could not guarantee paths origin from external are also external routes, the 2nd is incomplete filter. e.g. Cisco chassis has 'BP' and 'PortChannel01' as interface names, and are BP intf. new change is on 2nd filter, to get interfaces IP also, and make sure their IP are not one of internal BGP IPs.

vperumal commented 1 year ago

Thanks @wenyiz2021 - will pick it up and let u know the result

abdosi commented 1 year ago

can you check back ?