Skip to content

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

Merged
StormLiangMS merged 1 commit intosonic-net:202405from
yaqiangz:202405_sanity_skip
Jan 2, 2025
Merged

[202405][sanity][bgp] Skip default route missing recover for multi-asic (#16264)#16293
StormLiangMS merged 1 commit intosonic-net:202405from
yaqiangz:202405_sanity_skip

Conversation

@yaqiangz
Copy link
Copy Markdown
Contributor

@yaqiangz yaqiangz commented Jan 2, 2025

Description of PR

Summary:
Fixes # (issue)

Type of change

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

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

Approach

What is the motivation for this PR?

Manually cherry-pick this PR: #16264
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 it trys to recover 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

…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
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Collaborator

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants