Provide BMC defaults for system status LED methods in ChassisBase - #731
Open
shreyansh-nexthop wants to merge 2 commits into
Open
Provide BMC defaults for system status LED methods in ChassisBase#731shreyansh-nexthop wants to merge 2 commits into
shreyansh-nexthop wants to merge 2 commits into
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
|
Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks! ---Powered by SONiC BuildBot
|
Contributor
|
@shreyansh-nexthop is this ready for review, please set once you complete tests - thx |
shreyansh-nexthop
force-pushed
the
shreyansh.fix_system_health_cli
branch
from
August 7, 2026 06:35
854092b to
b94028c
Compare
Collaborator
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
shreyansh-nexthop
force-pushed
the
shreyansh.fix_system_health_cli
branch
from
August 7, 2026 06:41
b94028c to
8ebd14a
Compare
Collaborator
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
`show system-health` (sonic-utilities) calls initizalize_system_led(), set_status_led() and get_status_led() unconditionally on the chassis, but BMC platforms have no controllable system status LED. ChassisBase left set/get as NotImplementedError and had no initizalize_system_led at all, so the CLI failed on BMC images. Provide BMC defaults in ChassisBase, gated on is_switch_bmc(): initizalize_system_led() returns True, set_status_led() returns False, and get_status_led() returns "N/A". Non-BMC platforms are unaffected: they fall through to NotImplementedError as before, or override these methods. Signed-off-by: shreyansh-nexthop <shreyansh@nexthop.ai>
Cover both branches of initizalize_system_led(), set_status_led() and get_status_led(): the BMC defaults when is_switch_bmc() is true, and the NotImplementedError path for non-BMC platforms. Signed-off-by: shreyansh-nexthop <shreyansh@nexthop.ai>
shreyansh-nexthop
force-pushed
the
shreyansh.fix_system_health_cli
branch
from
August 7, 2026 06:45
8ebd14a to
ca02be5
Compare
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
shreyansh-nexthop
marked this pull request as ready for review
August 7, 2026 06:46
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Author
@judyjoseph, the PR is now ready for review. Thanks! |
Contributor
|
@oleksandrivantsiv @vivekrnv for a quick review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add BMC defaults for the system status LED methods on
ChassisBase, gated onis_switch_bmc():initizalize_system_led()returnsTrueset_status_led()returnsFalseget_status_led()returns"N/A"Non-BMC platforms are unaffected: they still
raise NotImplementedError(or use their own platform override).device_infois imported lazily inside each method, matching the existing pattern inmodule_base.pyandbmc_base.py.Motivation and Context
show system-health(sonic-utilities) callschassis.initizalize_system_led(),set_status_led()andget_status_led()unconditionally. BMC platforms have no controllable system status LED, andChassisBaseleftset/getasNotImplementedErrorwith noinitizalize_system_led()at all, so the CLI failed on BMC images. Handling this once in the common base, instead of duplicating no-op stubs in each vendor'schassis.py, keeps the behavior consistent across all BMC platforms.The method name
initizalize_system_ledintentionally keeps the existing (misspelled) name used by theshow system-healthcaller and current platform implementations.How Has This Been Tested?
Non-BMC behavior is unchanged (still
NotImplementedError, sinceis_switch_bmc()is false there). On a BMC image the expected result is thatshow system-health summaryno longer errors and reportsSystem status LED N/A.Additional Information (Optional)