Skip to content

feat: add Sushy emulator vendor implementation - #115

Merged
poroh merged 1 commit into
dsx-ai-factory:mainfrom
fabiendupont:feat/sushy-vendor-support
Aug 25, 2026
Merged

poroh merged 1 commit into
dsx-ai-factory:mainfrom
fabiendupont:feat/sushy-vendor-support

Conversation

@fabiendupont

Copy link
Copy Markdown

Add Sushy (sushy-tools) as a recognized Redfish vendor for development and
testing without physical BMC hardware. Sushy is an OpenStack Redfish emulator
backed by libvirt — it translates Redfish power/boot operations into libvirt
domain commands.

The Sushy Bmc wraps RedfishStandard and delegates read operations and
power/boot control (which Sushy handles via libvirt). Operations that Sushy
cannot support are stubbed:

  • Account management (create_user, delete_user, change_password,
    get_accounts): accept silently — Sushy has no AccountService endpoint
  • BIOS configuration (machine_setup, set_bios, reset_bios): no-op —
    VMs have no real BIOS
  • Lockdown: no-op
  • GPU sensors, secure boot certificates, attestation: NotSupported
  • Boot order / infinite boot: always report as set up

Vendor detection matches "sushy", "contoso", or "redvirt" in the
ServiceRoot Vendor field. "Contoso" is the fictional manufacturer name from
DMTF's Redfish mockup data that Sushy ships with in its default Chassis
templates. "RedVirt" is an alternate configuration alias.

Testing

  • cargo check
  • cargo clippy --all-targets -- -D warnings
  • End-to-end: NICo deployed on CRC with 3 Sushy VMs explored successfully

Copilot AI review requested due to automatic review settings July 28, 2026 07:37
@copy-pr-bot

copy-pr-bot Bot commented Jul 28, 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.

Pull request overview

This PR adds a new Redfish vendor implementation for the Sushy (sushy-tools) emulator so the library can be used for development/testing against libvirt-backed virtual BMCs without physical hardware.

Changes:

  • Introduces a new Sushy vendor (src/sushy.rs) that wraps RedfishStandard and stubs/overrides unsupported behaviors.
  • Extends vendor detection to recognize sushy, contoso, and redvirt in ServiceRoot.Vendor.
  • Wires the new vendor into the standard vendor-dispatch path.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/sushy.rs Adds the Sushy-specific Redfish implementation that delegates most calls to RedfishStandard and overrides emulator-incompatible operations.
src/standard.rs Adds vendor dispatch for RedfishVendor::Sushy to construct the new crate::sushy::Bmc.
src/model/service_root.rs Adds RedfishVendor::Sushy and maps "sushy", "contoso", and "redvirt" to it during vendor detection.
src/lib.rs Registers the new sushy module in the crate.
Comments suppressed due to low confidence (1)

src/sushy.rs:412

  • Per the PR description, BIOS reset should be a no-op for Sushy. Delegating reset_bios to RedfishStandard can return errors (or perform unsupported operations) on the emulator.
    fn reset_bios<'a>(&'a self) -> crate::RedfishFuture<'a, Result<(), RedfishError>> {
        Box::pin(async move { self.standard.reset_bios().await })
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib.rs Outdated
Comment thread src/sushy.rs
Comment thread src/sushy.rs
Comment thread src/sushy.rs
Comment thread src/sushy.rs
@s3rj1k

s3rj1k commented Jul 28, 2026

Copy link
Copy Markdown

@fabiendupont

Copy link
Copy Markdown
Author

@s3rj1k, absolutely agree that extending Sushy is the proper way, just slower :)
That's why I went with stubbing in Rust, so I would have a short term solution. And it's easy to remove the stubs as the MRs are merged.

@s3rj1k

s3rj1k commented Jul 29, 2026

Copy link
Copy Markdown

@s3rj1k, absolutely agree that extending Sushy is the proper way, just slower :) That's why I went with stubbing in Rust, so I would have a short term solution. And it's easy to remove the stubs as the MRs are merged.

I would be nice to ping OpenStack people and facilitate reviews of MRs TBH.

@dtantsur

Copy link
Copy Markdown

I would be nice to ping OpenStack people and facilitate reviews of MRs TBH.

I'm aware of them but please note that extending sushy-tools is very far from being a priority for any of us. If it's blocking this PR, please go ahead with stubs.

@kfelternv

Copy link
Copy Markdown

@fabiendupont We want to get this merged in, could you resolve conflicts and mark any review comments as resolved?

@fabiendupont

Copy link
Copy Markdown
Author

@kfelternv, I'm on PTO without my computer right now. I should be able to do it next week. Is that okay time wise?

@fabiendupont
fabiendupont force-pushed the feat/sushy-vendor-support branch from db90952 to 4ba6c5c Compare August 24, 2026 07:26
@fabiendupont

Copy link
Copy Markdown
Author

@kfelternv, back to work :)
I marked all comments as resolved and rebased.

@poroh

poroh commented Aug 25, 2026

Copy link
Copy Markdown

/ok to test 4ba6c5c

@poroh

poroh commented Aug 25, 2026

Copy link
Copy Markdown

@fabiendupont cargo build failed:

error[E0050]: method `bmc_reset` has 1 parameter but the declaration in trait `Redfish::bmc_reset` has 2
   --> src/sushy.rs:182:22
    |
182 |       fn bmc_reset<'a>(&'a self) -> crate::RedfishFuture<'a, Result<(), RedfishError>> {
    |                        ^^^^^^^^ expected 2 parameters, found 1
    |
   ::: src/lib.rs:200:9
    |
200 | /         &'a self,
201 | |         reset_type: Option<ManagerResetType>,
    | |____________________________________________- trait requires 2 parameters

error[E0061]: this method takes 1 argument but 0 arguments were supplied
   --> src/sushy.rs:183:45
    |
183 |         Box::pin(async move { self.standard.bmc_reset().await })
    |                                             ^^^^^^^^^-- argument #1 of type `std::option::Option<manager::ManagerResetType>` is missing
    |
note: method defined here

@poroh poroh 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.

@fabiendupont I wanted to merge you PR first and then #132 but because your PR had build error #132 was merged first. Could you please remove methods that delegates calls to standard implmenetation. Now it happens by default, you don't need to provide this implementation.

Example of function from you PR that can be removed:

    fn update_firmware<'a>(
        &'a self,
        firmware: tokio::fs::File,
    ) -> crate::RedfishFuture<'a, Result<crate::model::task::Task, RedfishError>> {
        Box::pin(async move { self.standard.update_firmware(firmware).await })
    }

@fabiendupont
fabiendupont force-pushed the feat/sushy-vendor-support branch from 4ba6c5c to b19881f Compare August 25, 2026 08:20
@fabiendupont

Copy link
Copy Markdown
Author

@poroh, #132 is really nice and allowed me to remove a good chunck of code.
Locally, the build and tests pass.

poroh
poroh previously approved these changes Aug 25, 2026
@poroh

poroh commented Aug 25, 2026

Copy link
Copy Markdown

/ok to test b19881f

@poroh

poroh commented Aug 25, 2026

Copy link
Copy Markdown

@fabiendupont Next one is formatting:

Diff in /__w/libredfish/libredfish/src/sushy.rs:43:
 use crate::model::task::Task;
 use crate::model::update_service::ComponentType;
 use crate::{
-    standard::RedfishStandard,
-    BiosProfileType, MachineSetupStatus, Redfish, RedfishError, RoleId,
+    standard::RedfishStandard, BiosProfileType, MachineSetupStatus, Redfish, RedfishError, RoleId,
 };
 
 pub struct Bmc {
Diff in /__w/libredfish/libredfish/src/sushy.rs:184:
     ) -> crate::RedfishFuture<'a, Result<crate::Status, RedfishError>> {
         Box::pin(async move {
             info!("Sushy: reporting lockdown as enabled (emulator)");
-            Ok(crate::Status { status: crate::StatusInternal::Enabled, message: "Sushy emulator".to_string() })
+            Ok(crate::Status {
+                status: crate::StatusInternal::Enabled,
+                message: "Sushy emulator".to_string(),
+            })
         })
     }

Add Sushy (sushy-tools) as a recognized Redfish vendor. Sushy is an
OpenStack Redfish emulator backed by libvirt, used for development and
testing without physical BMC hardware.

The Sushy Bmc wraps RedfishStandard and delegates most operations,
stubbing out operations the emulator cannot support:
- Account management: accept silently (no AccountService endpoint)
- Password management: accept silently (no real credentials)
- BIOS/boot config: skipped (no BIOS in a VM)
- Lockdown: no-op
- GPU sensors, secure boot certs, attestation: NotSupported
- Power control and boot operations: delegated (Sushy controls libvirt)

Vendor detection matches "sushy", "contoso", or "redvirt" in the
ServiceRoot Vendor field.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@fabiendupont

fabiendupont commented Aug 25, 2026

Copy link
Copy Markdown
Author

Sorry @poroh. Went too fast. Updated.

@poroh

poroh commented Aug 25, 2026

Copy link
Copy Markdown

/ok to test 6fa3844

@poroh
poroh merged commit bcb0c2c into dsx-ai-factory:main Aug 25, 2026
2 checks passed
@poroh

poroh commented Aug 25, 2026

Copy link
Copy Markdown

krish-nvidia added a commit to dsx-ai-factory/infra-controller that referenced this pull request Aug 26, 2026
This PR bumps libredfish to `v0.47.1` which brings in the following
changes:

- feat: network transport mtls support by @yoks in
dsx-ai-factory/libredfish#131
- refactor: delegate Redfish defaults to RedfishStandard by @poroh in
dsx-ai-factory/libredfish#132
- feat: add Sushy emulator vendor implementation by @fabiendupont in
dsx-ai-factory/libredfish#115
- chore: add SECURITY.md by @krish-nvidia in
dsx-ai-factory/libredfish#133
- fix: distinguish Supermicro and DGX GB300 BIOS behavior by
@martinraumann in dsx-ai-factory/libredfish#129

## Related issues
#5215

## Type of Change
- [ ] **Add** - New feature or capability
- [ ] **Change** - Changes in existing functionality
- [x] **Fix** - Bug fixes
- [ ] **Remove** - Removed features or deprecated functionality
- [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.)

## Breaking Changes
- [ ] **This PR contains breaking changes**

## Testing
- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [x] Manual testing performed
- [ ] No testing required (docs, internal refactor, etc.)

## Additional Notes

---------

Signed-off-by: Krish Dandiwala <kdandiwala@nvidia.com>
@s3rj1k

s3rj1k commented Aug 28, 2026

Copy link
Copy Markdown

@fabiendupont so I've run latest master of NiCO that already contains this change against vanilla master of Sushy and it does not work.

HTTP 404 Not Found at https://10.42.71.103:8000/redfish/v1/UpdateService/FirmwareInventory

Please share your experience.

@s3rj1k

s3rj1k commented Aug 28, 2026

Copy link
Copy Markdown

So, after fixing above issue directly in Sushy with some of previously proposed changes to Sushy itself, I get stuck in HOSTINITIALIZING/WAITINGFORLOCKDOWN independent of what I suppy from admin-cli (might be NiCO bug)

  With the vendor pinned to Sushy, there is no configuration that gets past handler.rs:7337. It needs the one-line change in libredfish's sushy.rs:181-192:

  Err(RedfishError::NotSupported("lockdown_status".to_string()))

@fabiendupont

Copy link
Copy Markdown
Author

@s3rj1k, it's my fault. I was not using a vanilla Sushy, but a version from Metal3 with a custom vendor.json. I switch to vanilla and created #137 to fix the issues.

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.

6 participants