[sanity_check][bgp] Add default route check in sanity for single asic#16235
Merged
StormLiangMS merged 4 commits intosonic-net:masterfrom Dec 27, 2024
Merged
[sanity_check][bgp] Add default route check in sanity for single asic#16235StormLiangMS merged 4 commits intosonic-net:masterfrom
StormLiangMS merged 4 commits intosonic-net:masterfrom
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
4cfafc9 to
6a166f8
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
|
Hi @yejianquan could you please help to confirm whether this change is expected in multi-asic? |
Collaborator
|
Hi @cyw233 , could you please help to verify if this new check/recover works well on chassis devices? |
Contributor
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
Author
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mssonicbld
pushed a commit
to mssonicbld/sonic-mgmt
that referenced
this pull request
Dec 27, 2024
…sonic-net#16235) What is the motivation for this PR? BGP routes would be setup during add-topo https://github.com/sonic-net/sonic-mgmt/blob/master/ansible/roles/vm_set/tasks/add_topo.yml#L276. But there are some scenarios that route in DUT has been messed up, but bgp sessions are all up, sanity would treat it as healthy and wouldn't take action to recover it. Loopbackv4 address has been replaced, it would cause all kernel routes from bgp miss In some test cases announce or withdraw routes from ptf but fail to recover (i.e. test_stress_routes) Healthy status: admin@sonic:~$ ip route show default default nhid 282 proto bgp src 10.1.0.32 metric 20 nexthop via 10.0.0.57 dev PortChannel101 weight 1 nexthop via 10.0.0.59 dev PortChannel103 weight 1 nexthop via 10.0.0.61 dev PortChannel105 weight 1 nexthop via 10.0.0.63 dev PortChannel106 weight 1 admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2890 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 763 764 0 0 0 11:46:17 1439 ARISTA01M1 10.0.0.59 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA02M1 10.0.0.61 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA03M1 10.0.0.63 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA04M1 10.0.0.65 4 64001 712 761 0 0 0 11:46:15 2 ARISTA01MX 10.0.0.67 4 64002 712 761 0 0 0 11:46:15 2 ARISTA02MX Total number of neighbors 6 Issue status, no default route, but show ip bgp sum looks good admin@sonic:~$ ip route show default admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2892 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 764 767 0 0 0 11:47:14 1439 ARISTA01M1 10.0.0.59 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA02M1 10.0.0.61 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA03M1 10.0.0.63 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA04M1 10.0.0.65 4 64001 713 764 0 0 0 11:47:12 2 ARISTA01MX 10.0.0.67 4 64002 713 764 0 0 0 11:47:12 2 ARISTA02MX Total number of neighbors 6 How did you do it? Add default routes check in sanity check, and re-announce routes if issue happen How did you verify/test it? Run sanity check
Collaborator
|
Cherry-pick PR to 202405: #16248 |
This was referenced Dec 27, 2024
Merged
Merged
mssonicbld
pushed a commit
that referenced
this pull request
Dec 27, 2024
…#16235) What is the motivation for this PR? BGP routes would be setup during add-topo https://github.com/sonic-net/sonic-mgmt/blob/master/ansible/roles/vm_set/tasks/add_topo.yml#L276. But there are some scenarios that route in DUT has been messed up, but bgp sessions are all up, sanity would treat it as healthy and wouldn't take action to recover it. Loopbackv4 address has been replaced, it would cause all kernel routes from bgp miss In some test cases announce or withdraw routes from ptf but fail to recover (i.e. test_stress_routes) Healthy status: admin@sonic:~$ ip route show default default nhid 282 proto bgp src 10.1.0.32 metric 20 nexthop via 10.0.0.57 dev PortChannel101 weight 1 nexthop via 10.0.0.59 dev PortChannel103 weight 1 nexthop via 10.0.0.61 dev PortChannel105 weight 1 nexthop via 10.0.0.63 dev PortChannel106 weight 1 admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2890 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 763 764 0 0 0 11:46:17 1439 ARISTA01M1 10.0.0.59 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA02M1 10.0.0.61 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA03M1 10.0.0.63 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA04M1 10.0.0.65 4 64001 712 761 0 0 0 11:46:15 2 ARISTA01MX 10.0.0.67 4 64002 712 761 0 0 0 11:46:15 2 ARISTA02MX Total number of neighbors 6 Issue status, no default route, but show ip bgp sum looks good admin@sonic:~$ ip route show default admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2892 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 764 767 0 0 0 11:47:14 1439 ARISTA01M1 10.0.0.59 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA02M1 10.0.0.61 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA03M1 10.0.0.63 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA04M1 10.0.0.65 4 64001 713 764 0 0 0 11:47:12 2 ARISTA01MX 10.0.0.67 4 64002 713 764 0 0 0 11:47:12 2 ARISTA02MX Total number of neighbors 6 How did you do it? Add default routes check in sanity check, and re-announce routes if issue happen How did you verify/test it? Run sanity check
mssonicbld
pushed a commit
that referenced
this pull request
Dec 30, 2024
…#16235) What is the motivation for this PR? BGP routes would be setup during add-topo https://github.com/sonic-net/sonic-mgmt/blob/master/ansible/roles/vm_set/tasks/add_topo.yml#L276. But there are some scenarios that route in DUT has been messed up, but bgp sessions are all up, sanity would treat it as healthy and wouldn't take action to recover it. Loopbackv4 address has been replaced, it would cause all kernel routes from bgp miss In some test cases announce or withdraw routes from ptf but fail to recover (i.e. test_stress_routes) Healthy status: admin@sonic:~$ ip route show default default nhid 282 proto bgp src 10.1.0.32 metric 20 nexthop via 10.0.0.57 dev PortChannel101 weight 1 nexthop via 10.0.0.59 dev PortChannel103 weight 1 nexthop via 10.0.0.61 dev PortChannel105 weight 1 nexthop via 10.0.0.63 dev PortChannel106 weight 1 admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2890 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 763 764 0 0 0 11:46:17 1439 ARISTA01M1 10.0.0.59 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA02M1 10.0.0.61 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA03M1 10.0.0.63 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA04M1 10.0.0.65 4 64001 712 761 0 0 0 11:46:15 2 ARISTA01MX 10.0.0.67 4 64002 712 761 0 0 0 11:46:15 2 ARISTA02MX Total number of neighbors 6 Issue status, no default route, but show ip bgp sum looks good admin@sonic:~$ ip route show default admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2892 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 764 767 0 0 0 11:47:14 1439 ARISTA01M1 10.0.0.59 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA02M1 10.0.0.61 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA03M1 10.0.0.63 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA04M1 10.0.0.65 4 64001 713 764 0 0 0 11:47:12 2 ARISTA01MX 10.0.0.67 4 64002 713 764 0 0 0 11:47:12 2 ARISTA02MX Total number of neighbors 6 How did you do it? Add default routes check in sanity check, and re-announce routes if issue happen How did you verify/test it? Run sanity check
8 tasks
yaqiangz
added a commit
to yaqiangz/sonic-mgmt
that referenced
this pull request
Dec 31, 2024
…sonic-net#16235) What is the motivation for this PR? BGP routes would be setup during add-topo https://github.com/sonic-net/sonic-mgmt/blob/master/ansible/roles/vm_set/tasks/add_topo.yml#L276. But there are some scenarios that route in DUT has been messed up, but bgp sessions are all up, sanity would treat it as healthy and wouldn't take action to recover it. Loopbackv4 address has been replaced, it would cause all kernel routes from bgp miss In some test cases announce or withdraw routes from ptf but fail to recover (i.e. test_stress_routes) Healthy status: admin@sonic:~$ ip route show default default nhid 282 proto bgp src 10.1.0.32 metric 20 nexthop via 10.0.0.57 dev PortChannel101 weight 1 nexthop via 10.0.0.59 dev PortChannel103 weight 1 nexthop via 10.0.0.61 dev PortChannel105 weight 1 nexthop via 10.0.0.63 dev PortChannel106 weight 1 admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2890 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 763 764 0 0 0 11:46:17 1439 ARISTA01M1 10.0.0.59 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA02M1 10.0.0.61 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA03M1 10.0.0.63 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA04M1 10.0.0.65 4 64001 712 761 0 0 0 11:46:15 2 ARISTA01MX 10.0.0.67 4 64002 712 761 0 0 0 11:46:15 2 ARISTA02MX Total number of neighbors 6 Issue status, no default route, but show ip bgp sum looks good admin@sonic:~$ ip route show default admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2892 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 764 767 0 0 0 11:47:14 1439 ARISTA01M1 10.0.0.59 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA02M1 10.0.0.61 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA03M1 10.0.0.63 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA04M1 10.0.0.65 4 64001 713 764 0 0 0 11:47:12 2 ARISTA01MX 10.0.0.67 4 64002 713 764 0 0 0 11:47:12 2 ARISTA02MX Total number of neighbors 6 How did you do it? Add default routes check in sanity check, and re-announce routes if issue happen How did you verify/test it? Run sanity check
8 tasks
mssonicbld
pushed a commit
that referenced
this pull request
Dec 31, 2024
…#16235) What is the motivation for this PR? BGP routes would be setup during add-topo https://github.com/sonic-net/sonic-mgmt/blob/master/ansible/roles/vm_set/tasks/add_topo.yml#L276. But there are some scenarios that route in DUT has been messed up, but bgp sessions are all up, sanity would treat it as healthy and wouldn't take action to recover it. Loopbackv4 address has been replaced, it would cause all kernel routes from bgp miss In some test cases announce or withdraw routes from ptf but fail to recover (i.e. test_stress_routes) Healthy status: admin@sonic:~$ ip route show default default nhid 282 proto bgp src 10.1.0.32 metric 20 nexthop via 10.0.0.57 dev PortChannel101 weight 1 nexthop via 10.0.0.59 dev PortChannel103 weight 1 nexthop via 10.0.0.61 dev PortChannel105 weight 1 nexthop via 10.0.0.63 dev PortChannel106 weight 1 admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2890 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 763 764 0 0 0 11:46:17 1439 ARISTA01M1 10.0.0.59 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA02M1 10.0.0.61 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA03M1 10.0.0.63 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA04M1 10.0.0.65 4 64001 712 761 0 0 0 11:46:15 2 ARISTA01MX 10.0.0.67 4 64002 712 761 0 0 0 11:46:15 2 ARISTA02MX Total number of neighbors 6 Issue status, no default route, but show ip bgp sum looks good admin@sonic:~$ ip route show default admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2892 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 764 767 0 0 0 11:47:14 1439 ARISTA01M1 10.0.0.59 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA02M1 10.0.0.61 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA03M1 10.0.0.63 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA04M1 10.0.0.65 4 64001 713 764 0 0 0 11:47:12 2 ARISTA01MX 10.0.0.67 4 64002 713 764 0 0 0 11:47:12 2 ARISTA02MX Total number of neighbors 6 How did you do it? Add default routes check in sanity check, and re-announce routes if issue happen How did you verify/test it? Run sanity check
wangxin
pushed a commit
that referenced
this pull request
Dec 31, 2024
What is the motivation for this PR? Default route check in sanity is added by #16235 It only supports single asic for now. But constraint for single asic in recover stage is missed It would cause keyError in multi-asic if there is bgp sanity check failure def adaptive_recover(dut, localhost, fanouthosts, nbrhosts, tbinfo, check_results, wait_time): outstanding_action = None for result in check_results: if result['failed']: if result['check_item'] == 'interfaces': action = _recover_interfaces(dut, fanouthosts, result, wait_time) elif result['check_item'] == 'services': action = _recover_services(dut, result) elif result['check_item'] == 'bgp': # If there is only default route missing issue, only need to re-announce routes to recover > if ("no_v4_default_route" in result['bgp'] and len(result['bgp']) == 1 or "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 1 or ("no_v4_default_route" in result['bgp'] and "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 2)): E KeyError: 'bgp' check_results = [{'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...}] dut = <MultiAsicSonicHost vlab-08> fanouthosts = {} localhost = <tests.common.devices.local.Localhost object at 0x77f1b9270a90> nbrhosts = {'ARISTA01T0': <EosHost VM0129>, 'ARISTA01T2': <EosHost VM0128>} outstanding_action = None result = {'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...} tbinfo = {'auto_recover': 'False', 'comment': 'Tests multi-asic virtual switch vm', 'conf-name': 'vms-kvm-four-asic-t1-lag', 'duts': ['vlab-08'], ...} wait_time = 30 How did you do it? Add single asic constraint in recover How did you verify/test it? Run test
mssonicbld
pushed a commit
to mssonicbld/sonic-mgmt
that referenced
this pull request
Dec 31, 2024
…c-net#16264) What is the motivation for this PR? Default route check in sanity is added by sonic-net#16235 It only supports single asic for now. But constraint for single asic in recover stage is missed It would cause keyError in multi-asic if there is bgp sanity check failure def adaptive_recover(dut, localhost, fanouthosts, nbrhosts, tbinfo, check_results, wait_time): outstanding_action = None for result in check_results: if result['failed']: if result['check_item'] == 'interfaces': action = _recover_interfaces(dut, fanouthosts, result, wait_time) elif result['check_item'] == 'services': action = _recover_services(dut, result) elif result['check_item'] == 'bgp': # If there is only default route missing issue, only need to re-announce routes to recover > if ("no_v4_default_route" in result['bgp'] and len(result['bgp']) == 1 or "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 1 or ("no_v4_default_route" in result['bgp'] and "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 2)): E KeyError: 'bgp' check_results = [{'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...}] dut = <MultiAsicSonicHost vlab-08> fanouthosts = {} localhost = <tests.common.devices.local.Localhost object at 0x77f1b9270a90> nbrhosts = {'ARISTA01T0': <EosHost VM0129>, 'ARISTA01T2': <EosHost VM0128>} outstanding_action = None result = {'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...} tbinfo = {'auto_recover': 'False', 'comment': 'Tests multi-asic virtual switch vm', 'conf-name': 'vms-kvm-four-asic-t1-lag', 'duts': ['vlab-08'], ...} wait_time = 30 How did you do it? Add single asic constraint in recover How did you verify/test it? Run test
mssonicbld
pushed a commit
to mssonicbld/sonic-mgmt
that referenced
this pull request
Dec 31, 2024
…c-net#16264) What is the motivation for this PR? Default route check in sanity is added by sonic-net#16235 It only supports single asic for now. But constraint for single asic in recover stage is missed It would cause keyError in multi-asic if there is bgp sanity check failure def adaptive_recover(dut, localhost, fanouthosts, nbrhosts, tbinfo, check_results, wait_time): outstanding_action = None for result in check_results: if result['failed']: if result['check_item'] == 'interfaces': action = _recover_interfaces(dut, fanouthosts, result, wait_time) elif result['check_item'] == 'services': action = _recover_services(dut, result) elif result['check_item'] == 'bgp': # If there is only default route missing issue, only need to re-announce routes to recover > if ("no_v4_default_route" in result['bgp'] and len(result['bgp']) == 1 or "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 1 or ("no_v4_default_route" in result['bgp'] and "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 2)): E KeyError: 'bgp' check_results = [{'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...}] dut = <MultiAsicSonicHost vlab-08> fanouthosts = {} localhost = <tests.common.devices.local.Localhost object at 0x77f1b9270a90> nbrhosts = {'ARISTA01T0': <EosHost VM0129>, 'ARISTA01T2': <EosHost VM0128>} outstanding_action = None result = {'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...} tbinfo = {'auto_recover': 'False', 'comment': 'Tests multi-asic virtual switch vm', 'conf-name': 'vms-kvm-four-asic-t1-lag', 'duts': ['vlab-08'], ...} wait_time = 30 How did you do it? Add single asic constraint in recover How did you verify/test it? Run test
mssonicbld
pushed a commit
to mssonicbld/sonic-mgmt
that referenced
this pull request
Dec 31, 2024
…c-net#16264) What is the motivation for this PR? Default route check in sanity is added by sonic-net#16235 It only supports single asic for now. But constraint for single asic in recover stage is missed It would cause keyError in multi-asic if there is bgp sanity check failure def adaptive_recover(dut, localhost, fanouthosts, nbrhosts, tbinfo, check_results, wait_time): outstanding_action = None for result in check_results: if result['failed']: if result['check_item'] == 'interfaces': action = _recover_interfaces(dut, fanouthosts, result, wait_time) elif result['check_item'] == 'services': action = _recover_services(dut, result) elif result['check_item'] == 'bgp': # If there is only default route missing issue, only need to re-announce routes to recover > if ("no_v4_default_route" in result['bgp'] and len(result['bgp']) == 1 or "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 1 or ("no_v4_default_route" in result['bgp'] and "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 2)): E KeyError: 'bgp' check_results = [{'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...}] dut = <MultiAsicSonicHost vlab-08> fanouthosts = {} localhost = <tests.common.devices.local.Localhost object at 0x77f1b9270a90> nbrhosts = {'ARISTA01T0': <EosHost VM0129>, 'ARISTA01T2': <EosHost VM0128>} outstanding_action = None result = {'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...} tbinfo = {'auto_recover': 'False', 'comment': 'Tests multi-asic virtual switch vm', 'conf-name': 'vms-kvm-four-asic-t1-lag', 'duts': ['vlab-08'], ...} wait_time = 30 How did you do it? Add single asic constraint in recover How did you verify/test it? Run test
This was referenced Dec 31, 2024
mssonicbld
pushed a commit
that referenced
this pull request
Dec 31, 2024
What is the motivation for this PR? Default route check in sanity is added by #16235 It only supports single asic for now. But constraint for single asic in recover stage is missed It would cause keyError in multi-asic if there is bgp sanity check failure def adaptive_recover(dut, localhost, fanouthosts, nbrhosts, tbinfo, check_results, wait_time): outstanding_action = None for result in check_results: if result['failed']: if result['check_item'] == 'interfaces': action = _recover_interfaces(dut, fanouthosts, result, wait_time) elif result['check_item'] == 'services': action = _recover_services(dut, result) elif result['check_item'] == 'bgp': # If there is only default route missing issue, only need to re-announce routes to recover > if ("no_v4_default_route" in result['bgp'] and len(result['bgp']) == 1 or "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 1 or ("no_v4_default_route" in result['bgp'] and "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 2)): E KeyError: 'bgp' check_results = [{'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...}] dut = <MultiAsicSonicHost vlab-08> fanouthosts = {} localhost = <tests.common.devices.local.Localhost object at 0x77f1b9270a90> nbrhosts = {'ARISTA01T0': <EosHost VM0129>, 'ARISTA01T2': <EosHost VM0128>} outstanding_action = None result = {'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...} tbinfo = {'auto_recover': 'False', 'comment': 'Tests multi-asic virtual switch vm', 'conf-name': 'vms-kvm-four-asic-t1-lag', 'duts': ['vlab-08'], ...} wait_time = 30 How did you do it? Add single asic constraint in recover How did you verify/test it? Run test
mssonicbld
pushed a commit
that referenced
this pull request
Dec 31, 2024
What is the motivation for this PR? Default route check in sanity is added by #16235 It only supports single asic for now. But constraint for single asic in recover stage is missed It would cause keyError in multi-asic if there is bgp sanity check failure def adaptive_recover(dut, localhost, fanouthosts, nbrhosts, tbinfo, check_results, wait_time): outstanding_action = None for result in check_results: if result['failed']: if result['check_item'] == 'interfaces': action = _recover_interfaces(dut, fanouthosts, result, wait_time) elif result['check_item'] == 'services': action = _recover_services(dut, result) elif result['check_item'] == 'bgp': # If there is only default route missing issue, only need to re-announce routes to recover > if ("no_v4_default_route" in result['bgp'] and len(result['bgp']) == 1 or "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 1 or ("no_v4_default_route" in result['bgp'] and "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 2)): E KeyError: 'bgp' check_results = [{'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...}] dut = <MultiAsicSonicHost vlab-08> fanouthosts = {} localhost = <tests.common.devices.local.Localhost object at 0x77f1b9270a90> nbrhosts = {'ARISTA01T0': <EosHost VM0129>, 'ARISTA01T2': <EosHost VM0128>} outstanding_action = None result = {'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...} tbinfo = {'auto_recover': 'False', 'comment': 'Tests multi-asic virtual switch vm', 'conf-name': 'vms-kvm-four-asic-t1-lag', 'duts': ['vlab-08'], ...} wait_time = 30 How did you do it? Add single asic constraint in recover How did you verify/test it? Run test
yaqiangz
added a commit
to yaqiangz/sonic-mgmt
that referenced
this pull request
Jan 2, 2025
…c-net#16264) What is the motivation for this PR? Default route check in sanity is added by sonic-net#16235 It only supports single asic for now. But constraint for single asic in recover stage is missed It would cause keyError in multi-asic if there is bgp sanity check failure def adaptive_recover(dut, localhost, fanouthosts, nbrhosts, tbinfo, check_results, wait_time): outstanding_action = None for result in check_results: if result['failed']: if result['check_item'] == 'interfaces': action = _recover_interfaces(dut, fanouthosts, result, wait_time) elif result['check_item'] == 'services': action = _recover_services(dut, result) elif result['check_item'] == 'bgp': # If there is only default route missing issue, only need to re-announce routes to recover > if ("no_v4_default_route" in result['bgp'] and len(result['bgp']) == 1 or "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 1 or ("no_v4_default_route" in result['bgp'] and "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 2)): E KeyError: 'bgp' check_results = [{'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...}] dut = <MultiAsicSonicHost vlab-08> fanouthosts = {} localhost = <tests.common.devices.local.Localhost object at 0x77f1b9270a90> nbrhosts = {'ARISTA01T0': <EosHost VM0129>, 'ARISTA01T2': <EosHost VM0128>} outstanding_action = None result = {'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...} tbinfo = {'auto_recover': 'False', 'comment': 'Tests multi-asic virtual switch vm', 'conf-name': 'vms-kvm-four-asic-t1-lag', 'duts': ['vlab-08'], ...} wait_time = 30 How did you do it? Add single asic constraint in recover How did you verify/test it? Run test
8 tasks
StormLiangMS
pushed a commit
that referenced
this pull request
Jan 2, 2025
…) (#16293) What is the motivation for this PR? Default route check in sanity is added by #16235 It only supports single asic for now. But constraint for single asic in recover stage is missed It would cause keyError in multi-asic if there is bgp sanity check failure def adaptive_recover(dut, localhost, fanouthosts, nbrhosts, tbinfo, check_results, wait_time): outstanding_action = None for result in check_results: if result['failed']: if result['check_item'] == 'interfaces': action = _recover_interfaces(dut, fanouthosts, result, wait_time) elif result['check_item'] == 'services': action = _recover_services(dut, result) elif result['check_item'] == 'bgp': # If there is only default route missing issue, only need to re-announce routes to recover > if ("no_v4_default_route" in result['bgp'] and len(result['bgp']) == 1 or "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 1 or ("no_v4_default_route" in result['bgp'] and "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 2)): E KeyError: 'bgp' check_results = [{'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...}] dut = <MultiAsicSonicHost vlab-08> fanouthosts = {} localhost = <tests.common.devices.local.Localhost object at 0x77f1b9270a90> nbrhosts = {'ARISTA01T0': <EosHost VM0129>, 'ARISTA01T2': <EosHost VM0128>} outstanding_action = None result = {'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...} tbinfo = {'auto_recover': 'False', 'comment': 'Tests multi-asic virtual switch vm', 'conf-name': 'vms-kvm-four-asic-t1-lag', 'duts': ['vlab-08'], ...} wait_time = 30 How did you do it? Add single asic constraint in recover How did you verify/test it? Run test
nnelluri-cisco
pushed a commit
to nnelluri-cisco/sonic-mgmt
that referenced
this pull request
Mar 15, 2025
…sonic-net#16235) What is the motivation for this PR? BGP routes would be setup during add-topo https://github.com/sonic-net/sonic-mgmt/blob/master/ansible/roles/vm_set/tasks/add_topo.yml#L276. But there are some scenarios that route in DUT has been messed up, but bgp sessions are all up, sanity would treat it as healthy and wouldn't take action to recover it. Loopbackv4 address has been replaced, it would cause all kernel routes from bgp miss In some test cases announce or withdraw routes from ptf but fail to recover (i.e. test_stress_routes) Healthy status: admin@sonic:~$ ip route show default default nhid 282 proto bgp src 10.1.0.32 metric 20 nexthop via 10.0.0.57 dev PortChannel101 weight 1 nexthop via 10.0.0.59 dev PortChannel103 weight 1 nexthop via 10.0.0.61 dev PortChannel105 weight 1 nexthop via 10.0.0.63 dev PortChannel106 weight 1 admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2890 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 763 764 0 0 0 11:46:17 1439 ARISTA01M1 10.0.0.59 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA02M1 10.0.0.61 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA03M1 10.0.0.63 4 65200 763 765 0 0 0 11:46:17 1439 ARISTA04M1 10.0.0.65 4 64001 712 761 0 0 0 11:46:15 2 ARISTA01MX 10.0.0.67 4 64002 712 761 0 0 0 11:46:15 2 ARISTA02MX Total number of neighbors 6 Issue status, no default route, but show ip bgp sum looks good admin@sonic:~$ ip route show default admin@sonic:~$ show ip bgp sum IPv4 Unicast Summary: BGP router identifier 10.1.0.32, local AS number 65100 vrf-id 0 BGP table version 2892 RIB entries 2893, using 648032 bytes of memory Peers 6, using 4451856 KiB of memory Peer groups 4, using 256 bytes of memory Neighbhor V AS MsgRcvd MsgSent TblVer InQ OutQ Up/Down State/PfxRcd NeighborName ----------- --- ----- --------- --------- -------- ----- ------ --------- -------------- -------------- 10.0.0.57 4 65200 764 767 0 0 0 11:47:14 1439 ARISTA01M1 10.0.0.59 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA02M1 10.0.0.61 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA03M1 10.0.0.63 4 65200 764 768 0 0 0 11:47:14 1439 ARISTA04M1 10.0.0.65 4 64001 713 764 0 0 0 11:47:12 2 ARISTA01MX 10.0.0.67 4 64002 713 764 0 0 0 11:47:12 2 ARISTA02MX Total number of neighbors 6 How did you do it? Add default routes check in sanity check, and re-announce routes if issue happen How did you verify/test it? Run sanity check
nnelluri-cisco
pushed a commit
to nnelluri-cisco/sonic-mgmt
that referenced
this pull request
Mar 15, 2025
…c-net#16264) What is the motivation for this PR? Default route check in sanity is added by sonic-net#16235 It only supports single asic for now. But constraint for single asic in recover stage is missed It would cause keyError in multi-asic if there is bgp sanity check failure def adaptive_recover(dut, localhost, fanouthosts, nbrhosts, tbinfo, check_results, wait_time): outstanding_action = None for result in check_results: if result['failed']: if result['check_item'] == 'interfaces': action = _recover_interfaces(dut, fanouthosts, result, wait_time) elif result['check_item'] == 'services': action = _recover_services(dut, result) elif result['check_item'] == 'bgp': # If there is only default route missing issue, only need to re-announce routes to recover > if ("no_v4_default_route" in result['bgp'] and len(result['bgp']) == 1 or "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 1 or ("no_v4_default_route" in result['bgp'] and "no_v6_default_route" in result['bgp'] and len(result['bgp']) == 2)): E KeyError: 'bgp' check_results = [{'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...}] dut = <MultiAsicSonicHost vlab-08> fanouthosts = {} localhost = <tests.common.devices.local.Localhost object at 0x77f1b9270a90> nbrhosts = {'ARISTA01T0': <EosHost VM0129>, 'ARISTA01T2': <EosHost VM0128>} outstanding_action = None result = {'bgp0': {'down_neighbors': ['2603:10e2:400:1::5', 'fc00::2']}, 'bgp3': {'down_neighbors': ['2603:10e2:400:1::6']}, 'check_item': 'bgp', 'failed': True, ...} tbinfo = {'auto_recover': 'False', 'comment': 'Tests multi-asic virtual switch vm', 'conf-name': 'vms-kvm-four-asic-t1-lag', 'duts': ['vlab-08'], ...} wait_time = 30 How did you do it? Add single asic constraint in recover How did you verify/test it? Run test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Description of PR
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
BGP routes would be setup during add-topo https://github.com/sonic-net/sonic-mgmt/blob/master/ansible/roles/vm_set/tasks/add_topo.yml#L276.
But there are some scenarios that route in DUT has been messed up, but bgp sessions are all up, sanity would treat it as healthy and wouldn't take action to recover it.
Healthy status:
Issue status, no default route, but
show ip bgp sumlooks goodHow did you do it?
Add default routes check in sanity check, and re-announce routes if issue happen
How did you verify/test it?
Run sanity check
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation