[BMC] Align is_bmc_supported check for the new infra#4584
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Ben Levi <belevi@nvidia.com>
benle7
force-pushed
the
host_bmc_communication
branch
from
3e448be to
c6cd2c3
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@judyjoseph Can you please help to merge? |
yxieca
left a comment
yxieca
left a comment
There was a problem hiding this comment.
Reviewed by AI agent on behalf of Ying.
Decision: Approve — clean, correctly-scoped alignment to the new host/BMC infra. CI green, and the change is fail-safe: any error (missing/unparseable /etc/sonic/bmc.json, import failure) degrades to "BMC not supported," which both call sites handle as a graceful skip. The return 0/1 contract of is_bmc_supported() is preserved exactly.
One non-blocking suggestion (clarity, no code change required):
The behavior of is_bmc_supported() changed but its header comment still reads only "Check BMC presence / 0 if BMC is supported, 1 otherwise." Consider documenting the new criteria so the next reader understands the trigger conditions moved, e.g.:
Host-side BMC support: returns 0 only when running on the Switch-Host (
is_switch_host()) and runtime BMC data (/etc/sonic/bmc.json) is available. The Switch-BMC-side unit intentionally reports "not supported" for this host-side collection path.
This makes clear that a platform which merely ships a static device/$platform/bmc.json (but isn't the switch-host with runtime data) now reports unsupported — and that the BMC-side exclusion is by design, not a regression.
Verified no platform regresses: the platforms that previously matched the old file-existence check are still covered (their switch-host role populates /etc/sonic/bmc.json at boot), and the only behavior change is correctly excluding the BMC-side unit. Does not block merge.
yxieca
added
Request for msft-202608 Branch
Approved for msft-202608 Branch
and removed
Request for 202605 Branch
labels
|
Cherry-pick PR to msft-202608: Azure/sonic-utilities.msft#385 |
mssonicbld
added
Included in msft-202608 Branch
and removed
Created PR to msft-202608 Branch
labels
What: Updates is_bmc_supported() in scripts/generate_dump to detect BMC support via device_info.is_switch_host() + device_info.get_bmc_data() instead of checking for a platform-local bmc.json file. Why: Aligns the generate_dump BMC check with the new infrastructure flow (sonic-buildimage#26544), validating host-side role and runtime BMC data from the canonical source rather than legacy platform-folder assumptions. How: Replaces the static /usr/share/sonic/device/$platform/bmc.json existence check with a runtime query against /etc/sonic/bmc.json APIs; preserves the 0/1 return contract and existing flow behavior. Testing: CI green (Azure, Analyze, Semgrep, DCO, EasyCLA all pass). Verified on switch-host w/ BMC data (path runs) and non-switch-host / missing-data (path skipped). Signed-off-by: Ben Levi <belevi@nvidia.com> Signed-off-by: Yogapriya Mohankumar <ymohanku@cisco.com>
5 participants
What I did
The BMC support check in
generate_dumpwas aligned with the new infrastructure flow(sonic-net/sonic-buildimage#26544).
Instead of checking for platform-local
bmc.jsonassumptions, the logic now validates host-side role and runtime BMC data from the canonical source used by the system.How I did it
generate_dumpBMC detection to usedevice_info.is_switch_host()together withdevice_info.get_bmc_data().bmc.jsonexistence behavior for this path.generate_dumpflow behavior intact while aligning detection to runtime/etc/sonic/bmc.json-based APIs.How to verify it
show techsupport(orgenerate_dump) on:Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)