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

[platform_tests/test_reload_config]: Respect overridden timeout #16031

Merged

Conversation

liamkearney-msft
Copy link
Contributor

@liamkearney-msft liamkearney-msft commented Dec 12, 2024

  • Respect overridden timeout in inventory for reboot in reload_configuration_checks_test
  • Replace plt_override option with wait_for_processes - will bail early after SSH is up if not set
  • Don't check all containers for t2 / modular chassis
  • Increase timeouts

Description of PR

Summary:
Fixes # (issue)

Type of change

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

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • [ x] 202405

Approach

What is the motivation for this PR?

There are various failures seen on nokia 7250 / arista 7800 / modular chassis DUTs for test_reload_config tests. Generally, they can be attributed to two things: too short of a timeout for t2 chassis, and the new docker status check.

How did you do it?

This PR refactors some logic in the reboot command, such that it will now respect overwritten values for the reboot timeout in the inventory file (so that SKU specific tweaks can be properly specified), and also repurposes the plt_reboot_ctrl_overwrite parameter to return_after_reconnect, which if set will cause the reboot function to return early after SSH comes back up, which is more in the spirit of how the test functions.

Additionally, some timeouts have been increased for this testcase, as t2 requires more time for services to come up.
Finally, the docker status check has been skipped for modular_chassis DUTs.

How did you verify/test it?

Ran on Nokia 7250 and Arista 7800 chassis

Any platform specific information?

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

Documentation

* Respect overridden timeout for reboot in
  reload_configuration_checks_test
* Replace plt_override option with wait_for_processes - will bail early
  after SSH is up if not set
* Don't check all containers for t2 / modular chassis
* Increase timeouts

Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liamkearney-msft liamkearney-msft requested review from auspham, arlakshm and wenyiz2021 and removed request for prgeor December 12, 2024 01:36
@liamkearney-msft liamkearney-msft self-assigned this Dec 12, 2024
Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
yejianquan
yejianquan previously approved these changes Dec 12, 2024
Copy link
Collaborator

@yejianquan yejianquan left a comment

Choose a reason for hiding this comment

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

LGTM

arlakshm
arlakshm previously approved these changes Dec 13, 2024
@arlakshm
Copy link
Contributor

/Azp run Azure.sonic-mgmt

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bingwang-ms
Copy link
Collaborator

Please share the test result for non-chassis platforms. The other PR #15951 caused regression on non-chassis platform

@liamkearney-msft
Copy link
Contributor Author

Hi @bingwang-ms, please see attached pass run on t1:
https://elastictest.org/scheduler/testplan/6762282fb4abf968292cbf94

@liamkearney-msft liamkearney-msft dismissed stale reviews from arlakshm and yejianquan via 10fda92 December 23, 2024 23:06
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@arlakshm arlakshm merged commit 696f580 into sonic-net:master Jan 2, 2025
17 checks passed
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.

6 participants