Skip to content

feat(rack): coordinate power shelf, compute , switch firmware upgrades across rack components #4089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vinodchitraliNVIDIA wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(rack): coordinate power shelf, compute , switch firmware upgrades across rack components #4089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/api-core/src/cfg/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ applicable.
| `ib_partition_state_controller` | `IbPartitionStateControllerConfig` | *(see below)* | `hardware` | IB partition state controller timing. |
| `dpa_interface_state_controller` | `DpaInterfaceStateControllerConfig` | *(see below)* | `networking` | DPA interface state controller timing. |
| `rack_state_controller` | `RackStateControllerConfig` | *(see below)* | `hardware` | Rack state controller timing. |
| `power_shelf_state_controller` | `PowerShelfStateControllerConfig` | *(see below)* | `hardware` | Power shelf state controller timing. |
| `power_shelf_state_controller` | `PowerShelfStateControllerConfig` | *(see below)* | `hardware` | Power shelf state controller timing and optional rack firmware reprovisioning. |
| `switch_state_controller` | `SwitchStateControllerConfig` | *(see below)* | `hardware` | Switch state controller timing. |
| `spdm_state_controller` | `SpdmStateControllerConfig` | *(see below)* | `security` | SPDM state controller timing. |
| `host_models` | `HashMap<String, Firmware>` | `{}` | `machines` | Maps host model identifiers to firmware definitions for BMC/UEFI/NIC upgrades. |
Expand Down
16 changes: 16 additions & 0 deletions crates/api-core/src/cfg/file.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2309,6 +2309,22 @@ pub struct PowerShelfStateControllerConfig {
/// Common state controller configs
#[serde(default = "StateControllerConfig::default")]
pub controller: StateControllerConfig,

/// When `true`, the power shelf Ready handler accepts rack-level
/// `power_shelf_reprovisioning_requested` and enters
/// `ReProvisioning::WaitingForRackFirmwareUpgrade`.
///
/// Defaults to `false` so power shelves stay out of rack firmware wait
/// unless explicitly enabled.
///
/// Configured in `nico-api-config.toml`:
///
/// ```toml
/// [power_shelf_state_controller]
/// rack_firmware_reprovisioning_enabled = true
/// ```
#[serde(default)]
pub rack_firmware_reprovisioning_enabled: bool,
}

/// RackStateController related config
Expand Down
193 changes: 146 additions & 47 deletions crates/api-core/src/handlers/component_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,13 +652,15 @@ struct RackFirmwareMaintenanceTarget {
rack_id: RackId,
machine_ids: Vec<String>,
switch_ids: Vec<String>,
power_shelf_ids: Vec<String>,
}

fn push_rack_firmware_target(
targets: &mut Vec<RackFirmwareMaintenanceTarget>,
rack_id: RackId,
machine_id: Option<String>,
switch_id: Option<String>,
power_shelf_id: Option<String>,
) {
let target = match targets.iter_mut().find(|target| target.rack_id == rack_id) {
Some(target) => target,
Expand All @@ -667,6 +669,7 @@ fn push_rack_firmware_target(
rack_id,
machine_ids: Vec::new(),
switch_ids: Vec::new(),
power_shelf_ids: Vec::new(),
});
targets.last_mut().expect("target was just pushed")
}
Expand All @@ -678,6 +681,9 @@ fn push_rack_firmware_target(
if let Some(switch_id) = switch_id {
target.switch_ids.push(switch_id);
}
if let Some(power_shelf_id) = power_shelf_id {
target.power_shelf_ids.push(power_shelf_id);
}
}

async fn group_machine_ids_by_rack(
Expand Down Expand Up @@ -706,7 +712,13 @@ async fn group_machine_ids_by_rack(
"machine {machine_id} is not associated with a rack"
))
})?;
push_rack_firmware_target(&mut targets, rack_id, Some(machine_id.to_string()), None);
push_rack_firmware_target(
&mut targets,
rack_id,
Some(machine_id.to_string()),
None,
None,
);
}

Ok(targets)
Expand Down Expand Up @@ -853,7 +865,57 @@ async fn group_switch_ids_by_rack(
let rack_id = switch.rack_id.clone().ok_or_else(|| {
Status::failed_precondition(format!("switch {switch_id} is not associated with a rack"))
})?;
push_rack_firmware_target(&mut targets, rack_id, None, Some(switch_id.to_string()));
push_rack_firmware_target(
&mut targets,
rack_id,
None,
Some(switch_id.to_string()),
None,
);
}

Ok(targets)
}

async fn group_power_shelf_ids_by_rack(
api: &Api,
power_shelf_ids: &[PowerShelfId],
) -> Result<Vec<RackFirmwareMaintenanceTarget>, Status> {
let mut txn = api
.database_connection
.begin()
.await
.map_err(|e| Status::internal(format!("failed to begin transaction: {e}")))?;
let power_shelves = db::power_shelf::find_by(
&mut txn,
db::ObjectColumnFilter::List(db::power_shelf::IdColumn, power_shelf_ids),
)
.await
.map_err(|e| Status::internal(format!("failed to look up power shelves: {e}")))?;
drop(txn);

let power_shelves_by_id: HashMap<_, _> = power_shelves
.into_iter()
.map(|power_shelf| (power_shelf.id, power_shelf))
.collect();

let mut targets = Vec::new();
for power_shelf_id in power_shelf_ids {
let power_shelf = power_shelves_by_id
.get(power_shelf_id)
.ok_or_else(|| Status::not_found(format!("power shelf {power_shelf_id} not found")))?;
let rack_id = power_shelf.rack_id.clone().ok_or_else(|| {
Status::failed_precondition(format!(
"power shelf {power_shelf_id} is not associated with a rack"
))
})?;
push_rack_firmware_target(
&mut targets,
rack_id,
None,
None,
Some(power_shelf_id.to_string()),
);
}

Ok(targets)
Expand Down Expand Up @@ -881,14 +943,15 @@ async fn submit_rack_firmware_maintenance_requests(
.machine_ids
.iter()
.chain(target.switch_ids.iter())
.chain(target.power_shelf_ids.iter())
.cloned()
.collect();
let maintenance_req = Request::new(rpc::RackMaintenanceOnDemandRequest {
rack_id: Some(target.rack_id),
scope: Some(rpc::RackMaintenanceScope {
machine_ids: target.machine_ids,
switch_ids: target.switch_ids,
power_shelf_ids: vec![],
power_shelf_ids: target.power_shelf_ids,
activities: activities.clone(),
}),
});
Expand Down Expand Up @@ -2345,54 +2408,70 @@ pub(crate) async fn update_component_firmware(
let route_through_state_controller =
cm.power_shelf_use_state_controller && !bypass_state_controller;
if route_through_state_controller {
// TODO: implement state controller path for power shelf firmware control
return Err(Status::unimplemented(
"power shelf firmware control through the state controller is not yet supported",
));
}

let options = if cm.power_shelf.supports_firmware_object_json() {
require_firmware_object_json_for_direct_rms(
let token = require_firmware_object_json_for_rack_maintenance(
"power shelf",
&access_token,
&req.target_version,
)?;
let components = map_power_shelf_components(&t.components)?;
let component_names = components
.iter()
.map(|component| match component {
PowerShelfComponent::Pmc => "pmc".to_string(),
PowerShelfComponent::Psu => "psu".to_string(),
})
.collect();
maintenance_activities = vec![firmware_upgrade_activity(
req.target_version.clone(),
component_names,
Some(token),
force_update,
)?
)];
rack_maintenance_targets = group_power_shelf_ids_by_rack(api, &list.ids).await?;
} else {
reject_power_shelf_firmware_object_json(&access_token)?;
FirmwareUpdateOptions {
force_update,
..FirmwareUpdateOptions::default()
}
};
let components = map_power_shelf_components(&t.components)?;
let endpoints = resolve_power_shelf_endpoints(api, &list.ids).await?;
let options = if cm.power_shelf.supports_firmware_object_json() {
require_firmware_object_json_for_direct_rms(
"power shelf",
&access_token,
&req.target_version,
force_update,
)?
} else {
reject_power_shelf_firmware_object_json(&access_token)?;
FirmwareUpdateOptions {
force_update,
..FirmwareUpdateOptions::default()
}
};
let components = map_power_shelf_components(&t.components)?;
let endpoints = resolve_power_shelf_endpoints(api, &list.ids).await?;

let mut results: Vec<_> = endpoints
.unresolved
.iter()
.map(|u| error_result(&u.id.to_string(), u.reason.clone()))
.collect();
let mut results: Vec<_> = endpoints
.unresolved
.iter()
.map(|u| error_result(&u.id.to_string(), u.reason.clone()))
.collect();

let backend_results = cm
.power_shelf
.update_firmware(
&endpoints.resolved.endpoints,
&req.target_version,
&components,
&options,
)
.await
.map_err(component_manager_error_to_status)?;
results.extend(backend_results.into_iter().map(|r| {
let id = ps_mac_to_id_str(&r.pmc_mac, &endpoints.resolved.mac_to_id);
if r.success {
success_result(&id)
} else {
error_result(&id, r.error.unwrap_or_default())
}
}));
power_shelf_results = Some(results);
let backend_results = cm
.power_shelf
.update_firmware(
&endpoints.resolved.endpoints,
&req.target_version,
&components,
&options,
)
.await
.map_err(component_manager_error_to_status)?;
results.extend(backend_results.into_iter().map(|r| {
let id = ps_mac_to_id_str(&r.pmc_mac, &endpoints.resolved.mac_to_id);
if r.success {
success_result(&id)
} else {
error_result(&id, r.error.unwrap_or_default())
}
}));
power_shelf_results = Some(results);
}
}
rpc::update_component_firmware_request::Target::Racks(t) => {
if bypass_state_controller {
Expand Down Expand Up @@ -2952,17 +3031,37 @@ mod tests {
let rack_b = RackId::new("rack-b".to_string());
let mut targets = Vec::new();

push_rack_firmware_target(&mut targets, rack_a.clone(), Some("machine-a".into()), None);
push_rack_firmware_target(&mut targets, rack_b.clone(), None, Some("switch-b".into()));
push_rack_firmware_target(&mut targets, rack_a.clone(), Some("machine-c".into()), None);
push_rack_firmware_target(
&mut targets,
rack_a.clone(),
Some("machine-a".into()),
None,
None,
);
push_rack_firmware_target(
&mut targets,
rack_b.clone(),
None,
Some("switch-b".into()),
None,
);
push_rack_firmware_target(
&mut targets,
rack_a.clone(),
Some("machine-c".into()),
None,
None,
);

assert_eq!(targets.len(), 2);
assert_eq!(targets[0].rack_id, rack_a);
assert_eq!(targets[0].machine_ids, vec!["machine-a", "machine-c"]);
assert!(targets[0].switch_ids.is_empty());
assert!(targets[0].power_shelf_ids.is_empty());
assert_eq!(targets[1].rack_id, rack_b);
assert_eq!(targets[1].switch_ids, vec!["switch-b"]);
assert!(targets[1].machine_ids.is_empty());
assert!(targets[1].power_shelf_ids.is_empty());
}

#[test]
Expand Down
3 changes: 3 additions & 0 deletions crates/api-core/src/setup.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1444,6 +1444,9 @@ async fn initialize_and_start_controllers<'a>(
component_manager: component_manager.clone().map(Arc::new),
credential_manager: credential_manager.clone(),
per_object_metrics_registry: per_object_metrics_registry.clone(),
rack_firmware_reprovisioning_enabled: carbide_config
.power_shelf_state_controller
.rack_firmware_reprovisioning_enabled,
}
.into(),
)
Expand Down
1 change: 1 addition & 0 deletions crates/api-core/src/test_support/default_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ pub fn get() -> CarbideConfig {
controller: StateControllerConfig::default(),
},
power_shelf_state_controller: PowerShelfStateControllerConfig {
rack_firmware_reprovisioning_enabled: false,
controller: StateControllerConfig::default(),
},
rack_state_controller: RackStateControllerConfig {
Expand Down
1 change: 1 addition & 0 deletions crates/api-core/src/tests/common/api_fixtures/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1655,6 +1655,7 @@ pub async fn create_test_env_with_overrides(
component_manager: test_component_manager.clone(),
credential_manager: credential_manager.clone(),
per_object_metrics_registry: per_object_metrics_registry.clone(),
rack_firmware_reprovisioning_enabled: false,

@coderabbitai coderabbitai Bot Jul 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Honor the test configuration override.

This fixture builds config from overrides.config, but the controller service hard-codes false, so callers cannot enable rack firmware reprovisioning through the test environment. Enabled-path integration tests will silently exercise the disabled behavior.

Proposed fix 
-                rack_firmware_reprovisioning_enabled: false,
+                rack_firmware_reprovisioning_enabled: config
+                    .power_shelf_state_controller
+                    .rack_firmware_reprovisioning_enabled,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
rack_firmware_reprovisioning_enabled: false,
rack_firmware_reprovisioning_enabled: config
.power_shelf_state_controller
.rack_firmware_reprovisioning_enabled,
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/api-core/src/tests/common/api_fixtures/mod.rs` at line 1658, Update
the controller service configuration in the fixture to use the rack firmware
reprovisioning value from overrides.config instead of hard-coding false.
Preserve the existing default when no override is provided, so tests can
explicitly enable the feature through their configuration.

}
.into(),
)
Expand Down
1 change: 1 addition & 0 deletions crates/api-core/src/tests/power_shelf_state_controller/error_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ async fn services(
component_manager,
credential_manager: Arc::new(TestCredentialManager::default()),
per_object_metrics_registry: env.per_object_metrics_registry(),
rack_firmware_reprovisioning_enabled: false,
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/api-core/src/tests/power_shelf_state_controller/maintenance.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ fn services_with_component_manager(
password: TEST_BMC_PASSWORD.into(),
})),
per_object_metrics_registry: env.per_object_metrics_registry(),
rack_firmware_reprovisioning_enabled: false,
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/api-core/src/tests/power_shelf_state_controller/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use crate::tests::common::api_fixtures::create_test_env;
mod error_state;
mod fixtures;
mod maintenance;
mod reprovisioning;
use carbide_secrets::test_support::credentials::TestCredentialManager;
use fixtures::power_shelf::{mark_power_shelf_as_deleted, set_power_shelf_controller_state};

Expand Down Expand Up @@ -130,6 +131,7 @@ async fn test_power_shelf_deletion_with_state_controller(
component_manager: None,
credential_manager: credential_manager.clone(),
per_object_metrics_registry: env.per_object_metrics_registry(),
rack_firmware_reprovisioning_enabled: false,
}
.into(),
)
Expand Down
Loading
Loading