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

Increase timeout for wait_bgp_sessions for T2 duts #16438

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Javier-Tan
Copy link
Contributor

@Javier-Tan Javier-Tan commented Jan 10, 2025

Description of PR

Summary:
Fixes #16436, caused by added BGP check in #15936, which doesn't account for T2 BGP time to come up

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
    • Add ownership here(Microsft required only)
  • Test case improvement

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405
  • 202411

Approach

What is the motivation for this PR?

Function wait_bgp_sessions timeout is too short for T2, fails in test_mgmt_ipv6_only test suite causing a fixture to error wrongly and not teardown properly leaving TB in bad state without ipv4 mgmt ip

How did you do it?

Increase timeout to 900s from 120s if duthost it is checking is supervisor

How did you verify/test it?

Run locally on T2

See for passing test:

17/01/2025 07:34:39 utilities.wait_until                     L0153 DEBUG  | check_bgp_session_state_all_asics is False, wait 10 seconds and check again
17/01/2025 07:34:49 utilities.wait_until                     L0135 DEBUG  | Time elapsed: 164.073392 seconds

Confirming it needs more than 120 seconds

Any platform specific information?

N/A

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

N/A

Documentation

N/A

…ions for supervisor duts

* Add check to increase timeout to 900 if duthost is a supervisor

Signed-off-by: Javier Tan [email protected]
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Javier-Tan Javier-Tan self-assigned this Jan 10, 2025
arlakshm
arlakshm previously approved these changes Jan 10, 2025
@@ -624,6 +624,10 @@ def wait_bgp_sessions(duthost, timeout=120):
A helper function to wait bgp sessions on DUT
"""
bgp_neighbors = duthost.get_bgp_neighbors_per_asic(state="all")
if duthost.is_supervisor_node():
Copy link
Contributor

Choose a reason for hiding this comment

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

on voq chassis supervisor has no bgp sessions, can we modify the caller to send the needed timeout when wait_bgp_sessions is called for Linecards

…to T2

* Change increase for timeout for supervisor nodes to increase for all T2 duts

Signed-off-by: Javier Tan [email protected]
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Javier-Tan Javier-Tan changed the title Increase timeout for wait_bgp_sessions for supervisor duts Increase timeout for wait_bgp_sessions for T2 duts Jan 17, 2025
@Javier-Tan Javier-Tan requested a review from arlakshm January 17, 2025 06:53
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.

[Bug][T2]: BGP check is too short in wait_bgp_sessions in duthost_utils
4 participants