[sanity][bgp] Skip default route missing recover for multi-asic by yaqiangz · Pull Request #16264 · sonic-net/sonic-mgmt

yaqiangz · 2024-12-31T03:59:40Z

Description of PR

Summary:
Fixes # (issue)

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Back port request

Approach

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

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

mssonicbld · 2024-12-31T03:59:43Z

/azp run

azure-pipelines · 2024-12-31T03:59:54Z

Azure Pipelines successfully started running 1 pipeline(s).

…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 · 2024-12-31T13:26:35Z

Cherry-pick PR to 202405: #16267

mssonicbld · 2024-12-31T13:26:36Z

Cherry-pick PR to 202411: #16268

mssonicbld · 2024-12-31T13:26:37Z

Cherry-pick PR to 202311: #16269

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

…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

…) (#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

yejianquan · 2025-01-03T03:57:39Z

merged to 202405: #16293

…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

[sanity][bgp] Skip default route missing recover for multi-asic

3fcaef7

yaqiangz added Request for 202311 branch Request for 202405 branch Request for 202411 branch labels Dec 31, 2024

yaqiangz mentioned this pull request Dec 31, 2024

[202405][sanity_check][bgp] Add default route check in sanity for single asic #16265

Closed

8 tasks

Blueve approved these changes Dec 31, 2024

View reviewed changes

wangxin approved these changes Dec 31, 2024

View reviewed changes

wangxin merged commit d45bde5 into sonic-net:master Dec 31, 2024

yaqiangz added Approved for 202311 branch Approved for 202405 branch Approved for 202411 branch labels Dec 31, 2024

mssonicbld added the Created PR to 202405 branch label Dec 31, 2024

mssonicbld added the Created PR to 202411 branch label Dec 31, 2024

mssonicbld added the Created PR to 202311 branch label Dec 31, 2024

mssonicbld added Included in 202311 branch and removed Created PR to 202311 branch labels Dec 31, 2024

mssonicbld added the Included in 202411 branch label Dec 31, 2024

mssonicbld removed the Created PR to 202411 branch label Dec 31, 2024

yaqiangz mentioned this pull request Jan 2, 2025

[202405][sanity][bgp] Skip default route missing recover for multi-asic (#16264) #16293

Merged

8 tasks

yejianquan added Included in 202405 branch and removed Created PR to 202405 branch labels Jan 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[sanity][bgp] Skip default route missing recover for multi-asic#16264

[sanity][bgp] Skip default route missing recover for multi-asic#16264
wangxin merged 1 commit intosonic-net:masterfrom
yaqiangz:azure-master_sanity

yaqiangz commented Dec 31, 2024 •

edited

Loading

Uh oh!

mssonicbld commented Dec 31, 2024

Uh oh!

azure-pipelines bot commented Dec 31, 2024

Uh oh!

mssonicbld commented Dec 31, 2024

Uh oh!

mssonicbld commented Dec 31, 2024

Uh oh!

mssonicbld commented Dec 31, 2024

Uh oh!

yejianquan commented Jan 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

yaqiangz commented Dec 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

mssonicbld commented Dec 31, 2024

Uh oh!

azure-pipelines bot commented Dec 31, 2024

Uh oh!

mssonicbld commented Dec 31, 2024

Uh oh!

mssonicbld commented Dec 31, 2024

Uh oh!

mssonicbld commented Dec 31, 2024

Uh oh!

yejianquan commented Jan 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yaqiangz commented Dec 31, 2024 •

edited

Loading