fix: support Chassis variants for lite-on power shelves#848
Conversation
| self.chassis | ||
| .iter() | ||
| .any(|c| c.id.to_lowercase().contains("powershelf")) | ||
| self.chassis.iter().any(|c| { |
There was a problem hiding this comment.
Question: why not query expected_powershelves for this information?
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-04-08 04:52:40 UTC | Commit: e88d331 |
poroh
left a comment
poroh
left a comment
There was a problem hiding this comment.
I double checked and chassis id is powershelf. Please don't merge before resolve.
@poroh Not on these. Lite-on power shelves in ytl-shard-1. Actively working on them. Had to use /redfish/v1/Chassis/chassis. What should I do? |
Because we have both approaches we need to decide which we need to support in bmc_mock (and keep tests update):
Right now you switched to chassis one. |
|
@poroh If you think it's supposed to be powershelf, let's keep it powershelf in bmc-mock, and I'll just keep this code doing what it's doing now -- it looks for powershelf and falls back to chassis + manufacturer(lite-on)? In general this seems like a firmware bug. I'll post back here to a bug reference once we open something. |
poroh
dismissed
their stale review
Title of PR was misleading. Actual code support both, so I don't see blockers to merge it. Follow up is to check what we should support in bmc mock and we can handle it in separate PR.
It seems that "powershelf" version belongs to more recent firmware. So, I would prefer to have powershelf in bmc-mock instead of "chassis". Everything else LGTM. |
|
Yeah to follow on @poroh's comment above, and for visibility: it turns out that To support old and new firmware in the interim (new firmware correctly enumerates it), and especially since all of the racks we're trying to bring up in labs are probably on older firmware, we're going to keep this PR as-is to support both variants (we'll try Eventually this should be able to go away. The alternative is we don't do it, and we need to manually update a bunch of power shelves. But we're instead preferring to put in the workaround to let Carbide ingest them, and subsequently do the firmware updates on them anyway. |
There must have been a typo when this code was first put together, and we didn't test it against an actual power shelf. For now we must maintain this fallback/hack for Lite-On power shelves, although I do think they're working on an update. In the meantime, it's `/Chassis/chassis`, and not `/Chassis/powershelf`. Just for completeless, I'm updating "*is power shelf*" checks to [continue to] look for `"powershelf"`, and if that's not found, then to look for `"chassis"` where the manufacturer contains `"lite-on`". That will make things a little less brittle, but the real fix is making sure the power shelf vendors give us vendor information in the serivce root, and use a common chassis ID. Confirmed on actual hardware, and also added some unit tests. On the plus side, our fallback credentials logic is working. This is just the next bit (we did the fall back, collected the vendor details, and the vendor details failed, because we were looking for `"powershelf"`). Signed-off-by: Chet Nichols III <chetn@nvidia.com>
chet
force-pushed
the
power_shelf_look_at_chassis
branch
from
e88d331 to
320b83e
Compare
chet
changed the title
chet
changed the title
chet
changed the title
3 participants
Description
It turns out that Lite-On power shelves support
/Chassis/chassisAND/Chassis/powershelf. In older firmware, only/Chassis/chassisis what is exposed in the Chassis Collection registry, meaning the code we have (which checks for"powershelf"in the registry, fails.I'm updating "is power shelf" checks to [continue to] look for
"powershelf", and if that's not found, then to look for"chassis"where the manufacturer contains"lite-on". The real fixes are:/Chassis/powershelfin the Chassis Collection (which is already fixed in newer FW).Confirmed on actual hardware, and also added some unit tests.
On the plus side, our credentials fallback logic from #842 is working. This is just the next bit (we did the fall back, collected the vendor details, and the vendor details failed, because we were looking for
"powershelf").Signed-off-by: Chet Nichols III chetn@nvidia.com
Type of Change
Related Issues (Optional)
Breaking Changes
Testing
Additional Notes