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.

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

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

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

yaqiangz
Copy link
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
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
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

@StormLiangMS StormLiangMS merged commit 267022f into sonic-net:202405 Jan 2, 2025
13 checks passed
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