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

Add reboot-cause history into reboot tests #4209

Closed
wants to merge 1 commit into from

Conversation

JibinBao
Copy link
Contributor

@JibinBao JibinBao commented Sep 8, 2021

Description of PR

Summary:
Fixes # (issue)
Since adding the feature(sonic-net/SONiC#669) of show reboot-cause history, so integrate "show reboot-cause history" into reboot tests.
e.g.

show reboot-cause history
Name                 Cause                                                                Time                             User    Comment
-------------------  -------------------------------------------------------------------  -------------------------------  ------  ---------
2021_09_03_00_46_20  Hardware - Other (Reset caused by hotswap or halt)                   N/A                              N/A     N/A
2021_09_03_00_11_03  fast-reboot                                                          Fri 03 Sep 2021 12:09:55 AM UTC  admin   N/A
2021_09_02_23_03_32  Hardware - Other (Reset caused by hotswap or halt)                   N/A                              N/A     N/A
2021_09_02_22_53_35  Hardware - Other (Reset caused by hotswap or halt)                   N/A                              N/A     N/A
2021_09_02_22_34_32  fast-reboot                                                          Thu 02 Sep 2021 10:33:23 PM UTC  admin   N/A
2021_09_02_22_16_12  Hardware - Other (Reset caused by hotswap or halt)                   N/A                              N/A     N/A
2021_09_02_21_10_29  Hardware - Other (Reset caused by hotswap or halt)                   N/A                              N/A     N/A
2021_09_02_20_19_28  Hardware - Other (Reset caused by hotswap or halt)                   N/A                              N/A     N/A
2021_09_02_20_16_36  Unknown (First boot of SONiC version 202012.153-4d2acead7_Internal)  N/A                              N/A     N/A

Type of change

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

Back port request

  • 201911

Approach

What is the motivation for this PR?

In order to check reboot-cause history function, integrate "show reboot-cause history" into reboot tests.

How did you do it?

Add one function of check_reboot_cause_history_after_reboot in ansible/roles/test/files/ptftests/advanced-reboot.py.
After doing reboot test, and then check the reboot cause history.

How did you verify/test it?

run fast-reboot or warm-reboot test case with ansible

Any platform specific information?

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

Documentation

sonic-net/SONiC#669

@JibinBao JibinBao requested a review from a team as a code owner September 8, 2021 10:28
@lgtm-com
Copy link

lgtm-com bot commented Sep 8, 2021

This pull request fixes 1 alert when merging 0854333 into 8c8d3aa - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@vaibhavhd
Copy link
Contributor

I think this reboot-history check is better suited for test_reboot.py and not test_advanced_reboot. The former is for checking causes, and platform status, and the latter is for I/O related verification in dataplane and controlplane tests.

The test_reboot cases are located here: https://github.com/Azure/sonic-mgmt/blob/master/tests/platform_tests/test_reboot.py

For example, this present check can be enhanced to check reboot cause history:
https://github.com/Azure/sonic-mgmt/blob/2de0b95f906afcd7a1fd6a847910becad0710067/tests/platform_tests/test_reboot.py#L76

@vaibhavhd
Copy link
Contributor

Also these 3 checks in the new function are all verifying cosmetic appearance of the output. For example, if the output contains specific number of dotted lines in line 2. I think it would be more useful:

  1. if the real reboot cause can be verified as the latest in the reboot-cause history output.
  2. If instead of verifying raw output's appearance, verify json version of the same. I think we get json output too.

r"Hardware .*|Power Loss .*)[\s]{2,}(N\/A|(Mon|Tue|Wed|Thu|Fri|Sat|Sun) .*)"
r"[\s]{2,}(.*)[\s]{2,}(.*)"
}
stdout, stderr, return_code = self.dut_connection.execCommand("show reboot-cause history")
Copy link
Collaborator

Choose a reason for hiding this comment

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

The above command output makes it suitable for use show_and_parse: https://github.com/Azure/sonic-mgmt/blob/master/tests/common/devices/sonic.py#L1139

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a PTF script. The show_and_parse method is not available here.

Copy link
Contributor Author

@JibinBao JibinBao Sep 9, 2021

Choose a reason for hiding this comment

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

Thank you.
I will close the PR, and then add the function into sonic-mgmt/tests/platform_tests/test_reboot.py

@yxieca
Copy link
Collaborator

yxieca commented Sep 8, 2021

It would be better to add a separate test and reboot 3 times with different reboot types (could be randomly selected from available list) and check reboot history step by step.

@yozhao101
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JibinBao
Copy link
Contributor Author

JibinBao commented Sep 9, 2021

thank you.
I will close the PR, and then add the function into sonic-mgmt/tests/platform_tests/test_reboot.py

@JibinBao JibinBao closed this Sep 9, 2021
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.

5 participants