Skip to content

[action] [PR:16264] [sanity][bgp] Skip default route missing recover for multi-asic#16267

Closed
mssonicbld wants to merge 1 commit intosonic-net:202405from
mssonicbld:cherry/202405/16264
Closed

[action] [PR:16264] [sanity][bgp] Skip default route missing recover for multi-asic#16267
mssonicbld wants to merge 1 commit intosonic-net:202405from
mssonicbld:cherry/202405/16264

Conversation

@mssonicbld
Copy link
Copy Markdown
Collaborator

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?

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

…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 Author

/azp run

@mssonicbld
Copy link
Copy Markdown
Collaborator Author

Original PR: #16264

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@yaqiangz
Copy link
Copy Markdown
Contributor

yaqiangz commented Jan 2, 2025

/azpw run Azure.sonic-mgmt

@yaqiangz
Copy link
Copy Markdown
Contributor

yaqiangz commented Jan 2, 2025

Close due to PR check failure, it has been manually merged
#16293

@yaqiangz yaqiangz closed this Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants