Update the fwutil function to support define the component for specific dut. by nhe-NV · Pull Request #6756 · sonic-net/sonic-mgmt

nhe-NV · 2022-11-07T07:00:17Z

In the existing fwutil test implement, user can only define the component(BIOS, ONIE, CPLD) based on the platform type, if for the same platform, it require to define different components for different dut(such as some setup are respined), them the origin implementation dose not support. modify the script to support such scenario.
The fwutil test case should not be skipped, since the [test_fwutil]fixture 'fw_pkg_name' not found #6489 is not a real issue.
Fix some pep8 issue

Description of PR

Summary: Update the fwutil function to support define the component for specific dut.
Fixes # (issue) Update the fwutil function to support define the component for specific dut.

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?

Update the fwutil function to support define the component for specific dut.

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 · 2022-11-07T07:01:12Z

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/plugins/conditional_mark/tests_mark_conditions_platform_tests.yaml Fixing tests/platform_tests/fwutil/fwutil_common.py fix end of files.........................................................Passed check yaml...............................................................Failed - hook id: check-yaml - exit code: 1 while constructing a mapping in "tests/common/plugins/conditional_mark/tests_mark_conditions_platform_tests.yaml", line 4, column 1 found duplicate key "platform_tests/api/test_chassis.py::TestChassisApi::test_get_watchdog" with value "{}" (original value: "{}") in "tests/common/plugins/conditional_mark/tests_mark_conditions_platform_tests.yaml", line 68, column 1 To suppress this check see: http://yaml.readthedocs.io/en/latest/api.html#duplicate-keys ... [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>

roy-sror

Hi Nana,

I've published some comments, please have a look.

Roy

roy-sror · 2022-11-07T09:29:33Z

tests/platform_tests/fwutil/fwutil_common.py

Update the component content, in case there is a pre-definition for a specific host.
Sometimes, if there is some DUTs has specific component(for example a respined board which requires a different CPLD) - it can be defined in the firmware.json file

I suggest also to change the firmware.json to include such an instance, as an example

roy-sror · 2022-11-07T09:30:48Z

tests/platform_tests/fwutil/fwutil_common.py

how can you guarantee that defined_fw["host"] exists?

roy-sror · 2022-11-07T09:33:38Z

tests/platform_tests/fwutil/fwutil_common.py

why do we need to deep copy the sub-dictionary? can't we do it in a more pythonic way?

…ic setup Change-Id: I416d470b62f4b100fd23ad8468ada5b461470e5e

azure-pipelines · 2022-11-07T10:29:35Z

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/plugins/conditional_mark/tests_mark_conditions_platform_tests.yaml fix end of files.........................................................Passed check yaml...............................................................Failed - hook id: check-yaml - exit code: 1 while constructing a mapping in "tests/common/plugins/conditional_mark/tests_mark_conditions_platform_tests.yaml", line 4, column 1 found duplicate key "platform_tests/api/test_chassis.py::TestChassisApi::test_get_watchdog" with value "{}" (original value: "{}") in "tests/common/plugins/conditional_mark/tests_mark_conditions_platform_tests.yaml", line 68, column 1 To suppress this check see: http://yaml.readthedocs.io/en/latest/api.html#duplicate-keys ... [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>

nhe-NV · 2022-11-08T01:03:36Z

/azpw run

mssonicbld · 2022-11-08T01:03:37Z

/AzurePipelines run

azure-pipelines · 2022-11-08T01:03:46Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2022-11-08T01:04:37Z

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/plugins/conditional_mark/tests_mark_conditions_platform_tests.yaml fix end of files.........................................................Passed check yaml...............................................................Failed - hook id: check-yaml - exit code: 1 while constructing a mapping in "tests/common/plugins/conditional_mark/tests_mark_conditions_platform_tests.yaml", line 4, column 1 found duplicate key "platform_tests/api/test_chassis.py::TestChassisApi::test_get_watchdog" with value "{}" (original value: "{}") in "tests/common/plugins/conditional_mark/tests_mark_conditions_platform_tests.yaml", line 68, column 1 To suppress this check see: http://yaml.readthedocs.io/en/latest/api.html#duplicate-keys ... [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>

liat-grozovik · 2022-11-14T19:52:30Z

@yxieca could you help review/assign someone. It was tested on top of Nvidia platform and the test is now running properly. I believe we need this fix with prio and also in 202205.

yxieca · 2022-11-15T06:05:10Z

@prgeor, @sujinmkang please review this change. Thanks.

liat-grozovik · 2022-11-28T16:25:34Z

@prgeor , @sujinmkang kindly reminder.

Blueve · 2023-03-03T05:11:48Z

@nhe-NV can you help backport this fix to 202012 and 202205? The conditional mark was backported to 202012 branch but all these tests are failed due to the issue closed.

@liat-grozovik and @prgeor

nhe-NV · 2023-03-05T09:34:35Z

@Blueve I added the backport requst for the 202205, but seems no actions for it, I will now add the backport request for the 202012.

Blueve · 2023-03-09T06:57:24Z

Thanks @nhe-NV ! I added backport labels and expect they can been cherry-picked in recent days. If they cannot be clean merged, I will let you know.

…ic setup (#6756) In the existing fwutil test implement, user can only define the component(BIOS, ONIE, CPLD) based on the platform type, if for the same platform, it require to define different components for different dut(such as some setup are respined), them the origin implementation dose not support. modify the script to support such scenario. The fwutil test case should not be skipped, since the [test_fwutil]fixture 'fw_pkg_name' not found #6489 is not a real issue. Fix some pep8 issue

wangxin · 2023-03-10T01:24:52Z

@nhe-NV Cherry-picking this PR to 202012 branch got many conflicts. Can you create a separate PR to 202012 branch to include this change?

…ic setup (sonic-net#6756) In the existing fwutil test implement, user can only define the component(BIOS, ONIE, CPLD) based on the platform type, if for the same platform, it require to define different components for different dut(such as some setup are respined), them the origin implementation dose not support. modify the script to support such scenario. The fwutil test case should not be skipped, since the [test_fwutil]fixture 'fw_pkg_name' not found sonic-net#6489 is not a real issue. Fix some pep8 issue

nhe-NV · 2023-03-13T03:52:17Z

@nhe-NV Cherry-picking this PR to 202012 branch got many conflicts. Can you create a separate PR to 202012 branch to include this change?

created the new PR for 202012

…ic dut.(202012) (#6756) (#7726) In the existing fwutil test implement, user can only define the component(BIOS, ONIE, CPLD) based on the platform type, if for the same platform, it require to define different components for different dut(such as some setup are respined), them the origin implementation dose not support. modify the script to support such scenario. The fwutil test case should not be skipped, since the [test_fwutil]fixture 'fw_pkg_name' not found #6489 is not a real issue. Fix some pep8 issue

ZhaohuiS · 2023-03-21T05:45:40Z

@nhe-NV May I ask which platforms are supported for test_fwutil.py?
Does is support Broadcom? The reason I asked this is because cases of test_fwutil.py failed on Broadcom platforms.
Or, it only supports Mellanox?

ZhaohuiS · 2023-03-21T06:08:43Z

@nhe-NV I skipped this script for other platforms, only run it on mellanox, please review #7802. Thanks.

nhe-NV · 2023-03-21T07:02:12Z

Hi，I only test on the mellanox platform, but I think on other platform it should work if they also support using fwtutil command to install or upgrade onie/cpld/bois.

Thanks

Skip fwutil tests in Celestica-e1031 nightly pipeline until below PR backport to internal-202012: * sonic-net#6756

nhe-NV requested a review from sujinmkang as a code owner November 7, 2022 07:00

roy-sror reviewed Nov 7, 2022

View reviewed changes

Update the fwutil function to support define the component for specif…

f79edbb

…ic setup Change-Id: I416d470b62f4b100fd23ad8468ada5b461470e5e

nhe-NV force-pushed the update_fwutil branch from 26d7939 to f79edbb Compare November 7, 2022 10:28

roy-sror approved these changes Nov 10, 2022

View reviewed changes

liat-grozovik approved these changes Nov 28, 2022

View reviewed changes

liat-grozovik merged commit aacdf8e into sonic-net:master Dec 1, 2022

Blueve added Request for 202012 branch Request for 202205 branch labels Mar 9, 2023

wangxin added Included in 202205 branch Cherry Pick Conflict_202012 labels Mar 10, 2023

This was referenced Mar 13, 2023

Update the fwutil function to support define the component for specific dut (202012) #7725

Closed

Update the fwutil function to support define the component for specific dut.(202012) #7726

Merged

wangxin added Included in 202012 branch and removed Cherry Pick Conflict_202012 labels Mar 15, 2023

ZhaohuiS pushed a commit to ZhaohuiS/sonic-mgmt that referenced this pull request Aug 18, 2023

Merged PR 2000: Skip fwutil tests in Celestica-e1031 nightly pipeline

48cbe53

Skip fwutil tests in Celestica-e1031 nightly pipeline until below PR backport to internal-202012: * sonic-net#6756

Conversation

nhe-NV commented Nov 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

azure-pipelines bot commented Nov 7, 2022

Uh oh!

roy-sror left a comment

Choose a reason for hiding this comment

Uh oh!

roy-sror Nov 7, 2022

Choose a reason for hiding this comment

Uh oh!

roy-sror Nov 7, 2022

Choose a reason for hiding this comment

Uh oh!

nhe-NV Nov 10, 2022

Choose a reason for hiding this comment

Uh oh!

roy-sror Nov 7, 2022

Choose a reason for hiding this comment

Uh oh!

nhe-NV Nov 10, 2022

Choose a reason for hiding this comment

Uh oh!

roy-sror Nov 7, 2022

Choose a reason for hiding this comment

Uh oh!

azure-pipelines bot commented Nov 7, 2022

Uh oh!

nhe-NV commented Nov 8, 2022

Uh oh!

mssonicbld commented Nov 8, 2022

Uh oh!

azure-pipelines bot commented Nov 8, 2022

Uh oh!

azure-pipelines bot commented Nov 8, 2022

Uh oh!

liat-grozovik commented Nov 14, 2022

Uh oh!

yxieca commented Nov 15, 2022

Uh oh!

liat-grozovik commented Nov 28, 2022

Uh oh!

Blueve commented Mar 3, 2023

Uh oh!

nhe-NV commented Mar 5, 2023

Uh oh!

Blueve commented Mar 9, 2023

Uh oh!

wangxin commented Mar 10, 2023

Uh oh!

nhe-NV commented Mar 13, 2023

Uh oh!

ZhaohuiS commented Mar 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZhaohuiS commented Mar 21, 2023

Uh oh!

nhe-NV commented Mar 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

nhe-NV commented Nov 7, 2022 •

edited

Loading

ZhaohuiS commented Mar 21, 2023 •

edited

Loading