Skip to content

fix: support stock Sushy emulator without custom root.json - #137

Open
fabiendupont wants to merge 1 commit into
dsx-ai-factory:mainfrom
fabiendupont:fix/sushy-stock-compatibility
Open

fix: support stock Sushy emulator without custom root.json#137
fabiendupont wants to merge 1 commit into
dsx-ai-factory:mainfrom
fabiendupont:fix/sushy-stock-compatibility

Conversation

@fabiendupont

Copy link
Copy Markdown

Summary

Fixes compatibility with stock upstream sushy-tools (tested against stable 2.2.0 from PyPI). Follow-up to #115.

@s3rj1k reported two issues when running NiCO against vanilla Sushy master:

  1. HTTP 404 Not Found at /redfish/v1/UpdateService/FirmwareInventory (comment)
  2. Stuck in HOSTINITIALIZING/WAITINGFORLOCKDOWN regardless of admin-cli input (comment)

Investigation revealed a third issue: stock Sushy has no Vendor field in the service root, so vendor auto-detection fails entirely — libredfish falls back to the generic RedfishStandard client instead of the Sushy vendor implementation.

Root causes and fixes

1. Vendor auto-detection (service_root.rs)

Stock Sushy 2.2.0 has no Vendor or Oem fields in the service root JSON, but uses "Id": "RedvirtService" and "Name": "Redvirt Service". Added id and name fields to ServiceRoot and extended vendor_string() with a third fallback that detects "sushy" or "redvirt" in those fields.

Previously, without a custom root.json template that injects "Vendor": "Sushy", libredfish resolved the vendor as Unknown and used the generic standard client — none of the Sushy-specific stubs applied.

2. lockdown_status stuck in WAITINGFORLOCKDOWN (sushy.rs)

The Sushy vendor returned Ok(Status::Enabled) for lockdown_status, faking a successful lockdown. Callers then believed the machine was locked down and waited for a state transition that could never happen. Changed to return Err(RedfishError::NotSupported("lockdown_status")), matching the RedfishStandard default for unsupported operations.

3. FirmwareInventory 404 (sushy.rs)

Stock Sushy 2.2.0 advertises FirmwareInventory in the UpdateService JSON body but doesn't implement the route handler (added in unreleased dev builds). The standard get_software_inventories call hits a raw 404. Added an override that tries the standard path first and converts a 404 into NotSupported. When the route exists (e.g. Metal3 dev builds with the controller), it works normally.

Test plan

  • Unit tests: 3 new tests for Sushy vendor detection (explicit field, Id fallback, Name fallback)
  • Full test suite passes (144 tests, 0 failures)
  • Live tested against stock sushy-tools 2.2.0 (PyPI) in a container — no custom root.json, no patches
  • Live tested against Metal3 dev build (0.0.1.dev405) with custom root.json — no regressions

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings September 3, 2026 07:25
@copy-pr-bot

copy-pr-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new 404-handling guard compares reqwest::StatusCode to an integer literal (404), which will not compile and must be fixed before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR improves compatibility with upstream/stock sushy-tools by making libredfish’s Sushy vendor detection and a couple of vendor-specific behaviors work without requiring a custom root.json template.

Changes:

  • Extend ServiceRoot parsing and vendor detection to fall back on Id/Name patterns when Vendor/Oem are missing (stock Sushy behavior).
  • Make lockdown_status report NotSupported for Sushy, aligning with the standard client and avoiding stuck state transitions.
  • Add Sushy override for get_software_inventories to convert a FirmwareInventory 404 into NotSupported.
File summaries
File Description
src/sushy.rs Adjust Sushy vendor behavior for unsupported lockdown status and handle missing FirmwareInventory route gracefully.
src/model/service_root.rs Add Id/Name fields and extend vendor auto-detection logic + tests for stock Sushy roots.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/sushy.rs
Comment thread src/model/service_root.rs Outdated
Three issues prevented libredfish from working with stock upstream
sushy-tools (tested against stable 2.2.0):

1. Vendor auto-detection failed because stock Sushy has no "Vendor"
   field in the service root. Add Id and Name fields to ServiceRoot
   and fall back to detecting "sushy" or "redvirt" in those fields
   when Vendor and Oem are both absent.

2. lockdown_status returned a fake Ok(Enabled) status, causing
   callers to get stuck in WAITINGFORLOCKDOWN. Return NotSupported
   instead, matching the RedfishStandard default for unsupported
   operations.

3. get_software_inventories hit a raw 404 on stock Sushy (the
   FirmwareInventory route doesn't exist in stable releases).
   Convert the 404 to NotSupported so callers get a clean error.
   When the route exists (e.g. Metal3 dev builds), the standard
   path is used and works normally.

Tested against both the Metal3 dev build (0.0.1.dev405 with custom
root.json) and stock stable sushy-tools 2.2.0 from PyPI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Fabien Dupont <fdupont@redhat.com>

Signed-off-by: Fabien Dupont <fdupont@redhat.com>

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@fabiendupont
fabiendupont force-pushed the fix/sushy-stock-compatibility branch from 680ed74 to 4aaeeb2 Compare September 3, 2026 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants