-
Notifications
You must be signed in to change notification settings - Fork 650
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
[show platform summary] Add chassis hardware info to platform summary and version #1624
Conversation
This pull request introduces 3 alerts when merging 750259d into 00bd0ce - view on LGTM.com new alerts:
|
FYI Build failure is due to the dependency mentioned in the PR body. |
This pull request introduces 1 alert when merging 089ed37 into 00bd0ce - view on LGTM.com new alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but I believe you also need to update the command reference guide as commands' output was changed.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@robocoder99 can you please confirm that the below failure is because dependent PR is not yet merged? |
@liat-grozovik I can confirm that. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
… and version (sonic-net#1624) #### What I did I added chassis model number, serial number, and hardware revision to the commands `show platform summary` and `show version` #### How I did it I refactored and modified the existing `get_hw_info_dict()` function to make calls to STATE_DB and get the chassis information populated by sonic-net/sonic-platform-daemons#183 script. The new refactored versions of `get_hw_info_dict()` are added here sonic-net/sonic-buildimage#7652
if chassis_info.get(k, '') in failed_vals: | ||
if platform_chassis is None: | ||
import platform | ||
platform_chassis = sonic_platform.platform.Platform().get_chassis() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This get_chassis() trying to call Eeprom() to retrieve EEPROM details but failed due to permission error to access Eeprom details via sysfs by a non-root user for many platforms.
We can suppress this error by adding a root user check in Eeprom() to return "N/A" instead of throwing a permission error exception when non-root user trying to access Eeprom() API.
But consequence is that until PMON comes up to fill the DB with EEPROM details, show version
will display serial number, hwsku, etc as "None" since Eeprom() is not accessible by non-root users.
This consequence is a day 1 behavior, but not seen earlier because previously we retrieve the Eeprom details by deliberately calling "decode-syseeprom" with sudo
privileges even in non-root user environment in code.
… and version (sonic-net#1624) #### What I did I added chassis model number, serial number, and hardware revision to the commands `show platform summary` and `show version` #### How I did it I refactored and modified the existing `get_hw_info_dict()` function to make calls to STATE_DB and get the chassis information populated by sonic-net/sonic-platform-daemons#183 script. The new refactored versions of `get_hw_info_dict()` are added here sonic-net/sonic-buildimage#7652
@alexrallen can you create separate PR for 202012 branch? |
… and version (sonic-net#1624) I added chassis model number, serial number, and hardware revision to the commands `show platform summary` and `show version` I refactored and modified the existing `get_hw_info_dict()` function to make calls to STATE_DB and get the chassis information populated by sonic-net/sonic-platform-daemons#183 script. The new refactored versions of `get_hw_info_dict()` are added here sonic-net/sonic-buildimage#7652
202012 branch PR closed: #2259 alexrallen commented on Jul 8, 2022 |
DEPENDS ON sonic-net/sonic-buildimage#7652 DO NOT MERGE WITHOUT
What I did
I added chassis model number, serial number, and hardware revision to the commands
show platform summary
andshow version
How I did it
I refactored and modified the existing
get_hw_info_dict()
function to make calls to STATE_DB and get the chassis information populated by sonic-net/sonic-platform-daemons#183 script.The new refactored versions of
get_hw_info_dict()
are added here sonic-net/sonic-buildimage#7652How to verify it
Run
show platform summary
andshow version
and verify that following fields are populated:Previous command output (if the output of a command-line utility has changed)
Previous output for
show version
(example run on Mellanox MSN2700)Previous output for
show platform summary
(example run on Mellanox MSN2700)New command output (if the output of a command-line utility has changed)
New output for
show version
(example run on Mellanox MSN2700)New output for
show platform summary
(example run on Mellanox MSN2700)