Enhance Mellanox reboot cause test case #6916

echuawu · 2022-11-29T11:11:16Z

Add new reboot cause script for additional bios cpu and asic causes

Description of PR

Summary:
Add new script to cover following reboot cause scenarios:

BIOS - In case the BIOS upgrade process ended with failure and cause the switch to reset.
CPU - Reset is initiated by SW on the CPU. it could be that SW encountered some catastrophic situation like a memory leak, eventually, the kernel reset the whole switch.
Reset from ASIC - Reset which is caused by ASIC.

Fixes # (issue)

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 test for sonic-net/sonic-platform-common#277

How did you do it?

How did you verify/test it?

Add test script for enhance reboot cause

Any platform specific information?

Mellanox platforms, except for SPC1 and SIMX

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

Any topology

Documentation

Add new reboot cause script for bios cpu and asic causes

azure-pipelines · 2022-11-29T11:12:07Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/common/reboot.py 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/common/reboot.py:211:20: E221 multiple spaces before operator tests/common/reboot.py:246:121: E501 line too long (125 > 120 characters) tests/common/reboot.py:256:10: E221 multiple spaces before operator tests/common/reboot.py:306:9: F841 local variable 'e' is assigned to but never used tests/common/reboot.py:373:121: E501 line too long (121 > 120 characters) ... [truncated extra lines, please run pre-commit locally to view full check results]

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>

Add new reboot cause script for bios cpu and asic causes

azure-pipelines · 2022-11-30T06:32:38Z

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

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/mellanox/mellanox_thermal_control_test_helper.py:11:1: F401 'tests.common.utilities.check_skip_release' imported but unused

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>

lgtm-com · 2022-11-30T06:38:42Z

This pull request introduces 1 alert and fixes 2 when merging 3638b32 into 82b074e - view on LGTM.com

new alerts:

1 for Unused import

fixed alerts:

1 for Unused local variable
1 for 'import *' may pollute namespace

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

Update platform_tests/mellanox/mellanox_thermal_control_test_helper.py

lgtm-com · 2022-11-30T07:59:00Z

This pull request fixes 2 alerts when merging 0552d88 into 4b26186 - view on LGTM.com

fixed alerts:

1 for Unused local variable
1 for 'import *' may pollute namespace

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

echuawu · 2022-11-30T10:33:37Z

/azpw run

echuawu · 2022-11-30T10:35:05Z

/azpw run

mssonicbld · 2022-11-30T10:35:08Z

/AzurePipelines run

azure-pipelines · 2022-11-30T10:35:18Z

Azure Pipelines successfully started running 1 pipeline(s).

liat-grozovik

LGTM see minor comments

liat-grozovik · 2022-11-30T17:57:42Z

tests/platform_tests/mellanox/mellanox_thermal_control_test_helper.py

@@ -138,7 +147,7 @@ def __init__(self, dut):
        :param dut: DUT object representing a SONiC switch under test.
        """
        self.dut = dut
-        #self.unlink_file_list = {}
+        # self.unlink_file_list = {}


can you clean this code?

This modification was the update of pre-commit check. My code update does not contain this part, but it failed at pre-commit process.

All the codes I need to update was in the first commit. And other code changes are all for passing through the pre-commit check. There are many old codes were not checked by pre-commit then, but when I modified some parts of the old code, pre-commit would check the old codes too, so I edit them in order to pass the check.

liat-grozovik · 2022-11-30T17:58:33Z

tests/platform_tests/mellanox/mellanox_thermal_control_test_helper.py

@@ -913,6 +923,12 @@ def mock_data(self):
        platform_data = get_platform_data(self.mock_helper.dut)
        thermal_dict = platform_data["thermals"]
        for category, content in thermal_dict.items():
+            # TODO: temp local solution, since the "sodimm" is only supported in master,


why not checking on branch instead of todo?

This is also the original code which I did not modified. It was removed for some reason when I am handling the updates according to pre-commit check result in the seconds commit. I readd it again.

echuawu · 2022-12-02T01:51:23Z

This PR is dropped and reopens a new one: #6944

Add reboot cause script

99dc183

Add new reboot cause script for bios cpu and asic causes

nhe-NV previously approved these changes Nov 30, 2022

View reviewed changes

Add reboot cause script

3638b32

Add new reboot cause script for bios cpu and asic causes

echuawu dismissed nhe-NV’s stale review via 3638b32 November 30, 2022 06:15

echuawu changed the title ~~Add reboot cause script~~ Enhance Mellanox reboot cause test case Nov 30, 2022

keboliu previously approved these changes Nov 30, 2022

View reviewed changes

Update mellanox_thermal_control_test_helper.py

0552d88

Update platform_tests/mellanox/mellanox_thermal_control_test_helper.py

echuawu dismissed keboliu’s stale review via 0552d88 November 30, 2022 07:28

liat-grozovik reviewed Nov 30, 2022

View reviewed changes

echuawu closed this Dec 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance Mellanox reboot cause test case #6916

Enhance Mellanox reboot cause test case #6916

echuawu commented Nov 29, 2022

azure-pipelines bot commented Nov 29, 2022

azure-pipelines bot commented Nov 30, 2022

lgtm-com bot commented Nov 30, 2022

lgtm-com bot commented Nov 30, 2022

echuawu commented Nov 30, 2022

echuawu commented Nov 30, 2022

mssonicbld commented Nov 30, 2022

azure-pipelines bot commented Nov 30, 2022

liat-grozovik left a comment

liat-grozovik Nov 30, 2022

echuawu Dec 1, 2022

echuawu Dec 1, 2022

liat-grozovik Nov 30, 2022

echuawu Dec 1, 2022

echuawu commented Dec 2, 2022

Enhance Mellanox reboot cause test case #6916

Enhance Mellanox reboot cause test case #6916

Conversation

echuawu commented Nov 29, 2022

Description of PR

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?

Documentation

azure-pipelines bot commented Nov 29, 2022

azure-pipelines bot commented Nov 30, 2022

lgtm-com bot commented Nov 30, 2022

lgtm-com bot commented Nov 30, 2022

echuawu commented Nov 30, 2022

echuawu commented Nov 30, 2022

mssonicbld commented Nov 30, 2022

azure-pipelines bot commented Nov 30, 2022

liat-grozovik left a comment

Choose a reason for hiding this comment

liat-grozovik Nov 30, 2022

Choose a reason for hiding this comment

echuawu Dec 1, 2022

Choose a reason for hiding this comment

echuawu Dec 1, 2022

Choose a reason for hiding this comment

liat-grozovik Nov 30, 2022

Choose a reason for hiding this comment

echuawu Dec 1, 2022

Choose a reason for hiding this comment

echuawu commented Dec 2, 2022