Add platform test for kdump #10047

amnsinghal · 2023-09-15T18:18:48Z

Summary:
Add test_kdump.py to platform_tests.

Type of change

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

Back port request

201911
202012
202205

Approach

What is the motivation for this PR?

Add automation test for kdump functionality.

How did you do it?

Induce kernel panic thru sysrq-trigger.

How did you verify/test it?

For platform with kdump enabled, trigger kernel panic, and check for reboot-cause on subsequent bootup.
Validated on Cisco-8000 platforms thru internal sonic-mgmt test.

Any platform specific information?

N/A

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

Any platform with kdump enabled, else test will be skipped.

Signed-off-by: Aman Singhal <amans@cisco.com>

amnsinghal · 2023-09-15T18:21:00Z

@prgeor @lguohan @abdosi Pls help review this new testcase for kdump, thanks.

amnsinghal · 2023-09-16T02:10:22Z

@prgeor Can you pls help check this Elastictest failure, doesn't seem related to my change, thanks.

amnsinghal · 2023-09-25T18:23:37Z

@prgeor Can you pls help review and check this Elastictest failure, doesn't seem related to my change, thanks.

mssonicbld · 2023-10-02T23:51:05Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/platform_tests/test_kdump.py:21:1: E302 expected 2 blank lines, found 1 flake8...............................................(no files to check)Skipped check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

tests/platform_tests/test_kdump.py

bpar9 · 2023-10-03T00:20:13Z

@yxieca there is an old PR which has a superset of TCs then what is present in this PR. That PR has been reviewed but not merged for past couple of years- #3562 . Is there some reason why community has not merged the old PR

yxieca · 2023-10-03T15:49:30Z

@yxieca there is an old PR which has a superset of TCs then what is present in this PR. That PR has been reviewed but not merged for past couple of years- #3562 . Is there some reason why community has not merged the old PR

Good question. Thanks for looking up the history. I believe the old PR #3562 has unresolve review comments. That was the reason for leaving it open.

bpar9 · 2023-10-10T00:22:06Z

@yxieca there is an old PR which has a superset of TCs then what is present in this PR. That PR has been reviewed but not merged for past couple of years- #3562 . Is there some reason why community has not merged the old PR

Good question. Thanks for looking up the history. I believe the old PR #3562 has unresolve review comments. That was the reason for leaving it open.

thanks @yxieca for the reply. So what is your guidance on this PR-10047? it has a subset of the TCs of #3562 which are currently applicable for our platform. So do we wait for 3562 to be merged or can we go ahead and push this PR in. Please let us know, so that we can proceed accordingly. Thanks

yxieca · 2023-10-10T16:50:50Z

@yxieca there is an old PR which has a superset of TCs then what is present in this PR. That PR has been reviewed but not merged for past couple of years- #3562 . Is there some reason why community has not merged the old PR

Good question. Thanks for looking up the history. I believe the old PR #3562 has unresolve review comments. That was the reason for leaving it open.

thanks @yxieca for the reply. So what is your guidance on this PR-10047? it has a subset of the TCs of #3562 which are currently applicable for our platform. So do we wait for 3562 to be merged or can we go ahead and push this PR in. Please let us know, so that we can proceed accordingly. Thanks

We can work on this PR and get it merged. At this point. this PR contains blocking comments. Please resolve.

bpar9 · 2023-10-10T22:21:19Z

@yxieca there is an old PR which has a superset of TCs then what is present in this PR. That PR has been reviewed but not merged for past couple of years- #3562 . Is there some reason why community has not merged the old PR

Good question. Thanks for looking up the history. I believe the old PR #3562 has unresolve review comments. That was the reason for leaving it open.

thanks @yxieca for the reply. So what is your guidance on this PR-10047? it has a subset of the TCs of #3562 which are currently applicable for our platform. So do we wait for 3562 to be merged or can we go ahead and push this PR in. Please let us know, so that we can proceed accordingly. Thanks

We can work on this PR and get it merged. At this point. this PR contains blocking comments. Please resolve.

thanks @yxieca , Aman will work on addressing the comments and proceed.

amnsinghal · 2023-10-10T22:55:29Z

@yxieca there is an old PR which has a superset of TCs then what is present in this PR. That PR has been reviewed but not merged for past couple of years- #3562 . Is there some reason why community has not merged the old PR

Good question. Thanks for looking up the history. I believe the old PR #3562 has unresolve review comments. That was the reason for leaving it open.

thanks @yxieca for the reply. So what is your guidance on this PR-10047? it has a subset of the TCs of #3562 which are currently applicable for our platform. So do we wait for 3562 to be merged or can we go ahead and push this PR in. Please let us know, so that we can proceed accordingly. Thanks

We can work on this PR and get it merged. At this point. this PR contains blocking comments. Please resolve.

thanks @yxieca , Aman will work on addressing the comments and proceed.

Thanks for confirming @yxieca and @bpar9, I have resolved the comments and update the PR, pls review and approve/merge.

amnsinghal · 2023-10-11T17:55:04Z

@yxieca All checks have passed, except Semgrep is still waiting for status to be reported, can you pls check this. Thanks!

mssonicbld · 2023-10-12T02:55:14Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/platform_tests/test_kdump.py:61:9: F841 local variable 'dut_ip' is assigned to but never used tests/platform_tests/test_kdump.py:63:9: F841 local variable 'dut_datetime' is assigned to but never used flake8...............................................(no files to check)Skipped check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

yxieca · 2023-10-12T15:48:23Z

@prgeor can you review this test?

amnsinghal · 2023-10-13T15:32:41Z

@prgeor Can you pls review and approve/merge, if no further suggestions, thanks!

prgeor · 2023-10-20T03:46:45Z

tests/platform_tests/test_kdump.py

+    """
+    This test case is used to verify that DUT will load kdump crashkernel on kernel panic.
+    """
+    def wait_lc_healthy_if_sup(self, duthost, duthosts, localhost, conn_graph_facts, xcvr_skip_list):


@amnsinghal is this test supposed to run only on supervisor? Why?

No, the test runs on both LCs and RPs. For RPs, there is an additional check to ensure that LCs come up healthy after RP goes down and come back up. (In SONiC distributed chassis, LCs go down as well if RP goes down - Ref: sonic-net/SONiC#945 (comment))

prgeor · 2023-10-20T03:52:27Z

tests/platform_tests/test_kdump.py

+        if "Enabled" not in out["stdout"]:
+            pytest.skip('DUT {}: Skip test since kdump is not enabled'.format(hostname))
+
+        reboot(duthost, localhost, reboot_type=REBOOT_TYPE_KERNEL_PANIC)


@amnsinghal why not trigger panic via echo c > /proc/sysrq-trigger

That is indeed what is being done with "REBOOT_TYPE_KERNEL_PANIC". See changes in reboot.py above.

amulyan7 · 2023-12-18T23:39:50Z

@prgeor if no further comments, can you please approve/merge, thanks!

bpar9 · 2023-12-21T00:25:15Z

@prgeor , @yxieca , as we addressed all the comments, can we go ahead and merge this? thanks

shivuv · 2023-12-21T22:27:05Z

@prgeor @yxieca @gechiang @abdosi : Please help to merge this PR into master and cherry pick this into 202205, 202305 and 202311 branches.

abdosi · 2023-12-29T20:35:13Z

@wangxin please help with cherry-pick into 202205/202305.

What is the motivation for this PR? Add automation test for kdump functionality. How did you do it? Induce kernel panic thru sysrq-trigger. How did you verify/test it? For platform with kdump enabled, trigger kernel panic, and check for reboot-cause on subsequent bootup.

mssonicbld · 2024-02-01T05:17:54Z

Cherry-pick PR to 202205: #11485

mssonicbld · 2024-02-01T05:17:55Z

Cherry-pick PR to 202305: #11486

What is the motivation for this PR? Add automation test for kdump functionality. How did you do it? Induce kernel panic thru sysrq-trigger. How did you verify/test it? For platform with kdump enabled, trigger kernel panic, and check for reboot-cause on subsequent bootup.

Add platform_test for kdump

9f253a0

Signed-off-by: Aman Singhal <amans@cisco.com>

amnsinghal requested a review from prgeor as a code owner September 15, 2023 18:18

Updated test_kdump to check for LC reboot-cause.

6f1cdea

Update test_kdump.py

fc266ca

bpar9 assigned bpar9 and unassigned bpar9 Oct 3, 2023

bpar9 self-requested a review October 3, 2023 00:14

bpar9 requested changes Oct 3, 2023

View reviewed changes

tests/platform_tests/test_kdump.py Show resolved Hide resolved

tests/platform_tests/test_kdump.py Outdated Show resolved Hide resolved

Added REBOOT_TYPE_KERNEL_PANIC in reboot_ctrl_dict

ea550a7

amnsinghal requested a review from bpar9 October 10, 2023 17:37

Resolved PR comments.

b8c85d6

Update test_kdump.py

b34d896

bpar9 approved these changes Oct 12, 2023

View reviewed changes

prgeor reviewed Oct 20, 2023

View reviewed changes

abdosi added chassis Request for 202205 branch Request for 202305 branch labels Dec 29, 2023

abdosi merged commit 7b1d43a into sonic-net:master Dec 29, 2023
16 checks passed

wangxin added Approved for 202205 branch Approved for 202305 branch labels Feb 1, 2024

mssonicbld added the Created PR to 202205 branch label Feb 1, 2024

mssonicbld added the Created PR to 202305 branch label Feb 1, 2024

This was referenced Feb 1, 2024

[action] [PR:10047] Add platform test for kdump #11485

Merged

[action] [PR:10047] Add platform test for kdump #11486

Merged

mssonicbld added Included in 202305 branch Included in 202205 branch and removed Created PR to 202305 branch Created PR to 202205 branch labels Feb 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add platform test for kdump #10047

Add platform test for kdump #10047

amnsinghal commented Sep 15, 2023

amnsinghal commented Sep 15, 2023

amnsinghal commented Sep 16, 2023

amnsinghal commented Sep 25, 2023

mssonicbld commented Oct 2, 2023

bpar9 commented Oct 3, 2023

yxieca commented Oct 3, 2023

bpar9 commented Oct 10, 2023 •

edited

Loading

yxieca commented Oct 10, 2023

bpar9 commented Oct 10, 2023

amnsinghal commented Oct 10, 2023

amnsinghal commented Oct 11, 2023

mssonicbld commented Oct 12, 2023

yxieca commented Oct 12, 2023

amnsinghal commented Oct 13, 2023

prgeor Oct 20, 2023 •

edited

Loading

bmridul Oct 26, 2023

prgeor Oct 20, 2023

bmridul Oct 26, 2023

amulyan7 commented Dec 18, 2023

bpar9 commented Dec 21, 2023

shivuv commented Dec 21, 2023 •

edited

Loading

abdosi commented Dec 29, 2023

mssonicbld commented Feb 1, 2024

mssonicbld commented Feb 1, 2024

Add platform test for kdump #10047

Add platform test for kdump #10047

Conversation

amnsinghal commented Sep 15, 2023

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

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

amnsinghal commented Sep 15, 2023

amnsinghal commented Sep 16, 2023

amnsinghal commented Sep 25, 2023

mssonicbld commented Oct 2, 2023

bpar9 commented Oct 3, 2023

yxieca commented Oct 3, 2023

bpar9 commented Oct 10, 2023 • edited Loading

yxieca commented Oct 10, 2023

bpar9 commented Oct 10, 2023

amnsinghal commented Oct 10, 2023

amnsinghal commented Oct 11, 2023

mssonicbld commented Oct 12, 2023

yxieca commented Oct 12, 2023

amnsinghal commented Oct 13, 2023

prgeor Oct 20, 2023 • edited Loading

Choose a reason for hiding this comment

bmridul Oct 26, 2023

Choose a reason for hiding this comment

prgeor Oct 20, 2023

Choose a reason for hiding this comment

bmridul Oct 26, 2023

Choose a reason for hiding this comment

amulyan7 commented Dec 18, 2023

bpar9 commented Dec 21, 2023

shivuv commented Dec 21, 2023 • edited Loading

abdosi commented Dec 29, 2023

mssonicbld commented Feb 1, 2024

mssonicbld commented Feb 1, 2024

bpar9 commented Oct 10, 2023 •

edited

Loading

prgeor Oct 20, 2023 •

edited

Loading

shivuv commented Dec 21, 2023 •

edited

Loading