Skip to content

Commit c229f0b

Browse files
feat(api): poll power state for PowerShelf in Ready (#1878)
While a PowerShelf idles in `Ready`, perform a best-effort `GetPowerStateByDeviceList` RPC against RMS and persist the observed `pstate` to `power_shelves.status`. The lookup mirrors the `NodeSet`-with-inline-BMC-endpoint shape used by the `Maintenance` handler's `SetPowerStateByDeviceList` path, so `build_power_shelf_node_info` is bumped to `pub(super)` for reuse. Missing prerequisites (no RMS client, no rack association, no BMC details, no credentials) and transport / status failures are logged but never transition the controller out of `Ready`, so a transient RMS outage cannot bounce the shelf into `Error`. Also bumps `librms` from `v0.0.12-rc1` to `v0.0.12-rc4` (adds the `prost-types` dependency and the new `GetPowerStateByDeviceList`, firmware-object management, and `SetScaleUpFabricState` RPCs), and updates the in-tree mock `RmsApi` implementations to cover the new trait methods. ## Description <!-- Describe what this PR does --> ## Type of Change <!-- Check one that best describes this PR --> - [ ] **Add** - New feature or capability - [ ] **Change** - Changes in existing functionality - [ ] **Fix** - Bug fixes - [ ] **Remove** - Removed features or deprecated functionality - [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.) ## Related Issues (Optional) <!-- If applicable, provide GitHub Issue. --> ## Breaking Changes - [ ] This PR contains breaking changes <!-- If checked above, describe the breaking changes and migration steps --> ## Testing <!-- How was this tested? Check all that apply --> - [ ] Unit tests added/updated - [ ] Integration tests added/updated - [ ] Manual testing performed - [ ] No testing required (docs, internal refactor, etc.) ## Additional Notes <!-- Any additional context, deployment notes, or reviewer guidance -->
1 parent d287b3a commit c229f0b

6 files changed

Lines changed: 246 additions & 16 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ authors = ["NVIDIA Carbide Engineering <carbide-dev@exchange.nvidia.com>"]
2727
[workspace.dependencies]
2828
clap = { version = "4", features = ["derive", "env"] }
2929
libredfish = { git = "https://github.com/NVIDIA/libredfish.git", tag = "v0.44.4" }
30-
librms = { git = "https://github.com/NVIDIA/nv-rms-client.git", tag = "v0.0.12-rc3" }
30+
librms = { git = "https://github.com/NVIDIA/nv-rms-client.git", tag = "v0.0.12-rc4" }
3131
ansi-to-html = "0.2.2"
3232

3333
tokio = { version = "1", features = ["full", "tracing"] }

crates/api-test-helper/src/mock_rms.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ pub struct MockRmsApi {
6767
Mutex<VecDeque<Result<rms::GetPowerStateResponse, RackManagerError>>>,
6868
get_power_state_calls: Mutex<Vec<rms::GetPowerStateRequest>>,
6969

70+
get_power_state_by_device_list_responses:
71+
Mutex<VecDeque<Result<rms::GetPowerStateByDeviceListResponse, RackManagerError>>>,
72+
get_power_state_by_device_list_calls: Mutex<Vec<rms::GetPowerStateByDeviceListRequest>>,
73+
7074
sequence_rack_power_responses:
7175
Mutex<VecDeque<Result<rms::SequenceRackPowerResponse, RackManagerError>>>,
7276
sequence_rack_power_calls: Mutex<Vec<rms::SequenceRackPowerRequest>>,
@@ -244,6 +248,8 @@ impl MockRmsApi {
244248
set_power_state_calls: Default::default(),
245249
set_power_state_by_device_list_responses: Default::default(),
246250
set_power_state_by_device_list_calls: Default::default(),
251+
get_power_state_by_device_list_responses: Default::default(),
252+
get_power_state_by_device_list_calls: Default::default(),
247253
get_power_state_responses: Default::default(),
248254
get_power_state_calls: Default::default(),
249255
sequence_rack_power_responses: Default::default(),
@@ -809,6 +815,16 @@ fn pop_or_err<T>(
809815

810816
#[async_trait::async_trait]
811817
impl RmsApi for MockRmsApi {
818+
async fn get_power_state_by_device_list(
819+
&self,
820+
cmd: rms::GetPowerStateByDeviceListRequest,
821+
) -> Result<rms::GetPowerStateByDeviceListResponse, RackManagerError> {
822+
self.get_power_state_by_device_list_calls
823+
.lock()
824+
.await
825+
.push(cmd);
826+
pop_or_err(&mut self.get_power_state_by_device_list_responses.lock().await)
827+
}
812828
async fn set_power_state(
813829
&self,
814830
cmd: rms::SetPowerStateRequest,

crates/power-shelf-controller/src/maintenance.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -245,11 +245,12 @@ async fn invoke_rms_power_operation(
245245
}
246246

247247
/// Build the `rms::NewNodeInfo` describing this power shelf for inclusion
248-
/// in a `SetPowerStateByDeviceList` request. Resolves the BMC IP from the
249-
/// database and BMC credentials via the credential manager, since the
250-
/// caller-supplied variant of the RPC requires the BMC connection details
251-
/// inline rather than relying on RMS's inventory.
252-
async fn build_power_shelf_node_info(
248+
/// in any caller-supplied `NodeSet` request (`SetPowerStateByDeviceList`
249+
/// from `Maintenance`, `GetDeviceInfoByDeviceList` from `Ready`). Resolves
250+
/// the BMC IP from the database and BMC credentials via the credential
251+
/// manager, since these RPCs require the BMC connection details inline
252+
/// rather than relying on RMS's inventory.
253+
pub(super) async fn build_power_shelf_node_info(
253254
power_shelf_id: &PowerShelfId,
254255
state: &PowerShelf,
255256
rack_id: String,

crates/power-shelf-controller/src/ready.rs

Lines changed: 198 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,27 +17,33 @@
1717

1818
//! Handler for PowerShelfControllerState::Ready.
1919
20+
use carbide_rack::rack_manager_error;
2021
use carbide_uuid::power_shelf::PowerShelfId;
21-
use model::power_shelf::{PowerShelf, PowerShelfControllerState};
22+
use db::power_shelf as db_power_shelf;
23+
use librms::protos::rack_manager as rms;
24+
use model::power_shelf::{PowerShelf, PowerShelfControllerState, PowerShelfStatus};
25+
use sqlx::PgTransaction;
2226
use state_controller::state_handler::{
2327
StateHandlerContext, StateHandlerError, StateHandlerOutcome,
2428
};
2529

2630
use crate::context::PowerShelfStateHandlerContextObjects;
31+
use crate::maintenance::build_power_shelf_node_info;
2732

2833
/// Handles the Ready state for a power shelf.
2934
///
3035
/// If the power shelf is marked for deletion, transitions to `Deleting`.
3136
/// If a maintenance request has been posted via
3237
/// `power_shelf_maintenance_requested`, transitions to `Maintenance` with the
33-
/// requested operation (PowerOn / PowerOff). Otherwise idles.
38+
/// requested operation (PowerOn / PowerOff). Otherwise polls RMS for the
39+
/// current power state (best-effort observation) and idles.
3440
///
3541
/// TODO: Implement PowerShelf monitoring (health checks, status updates,
3642
/// power consumption / efficiency tracking).
3743
pub async fn handle_ready(
3844
power_shelf_id: &PowerShelfId,
3945
state: &mut PowerShelf,
40-
_ctx: &mut StateHandlerContext<'_, PowerShelfStateHandlerContextObjects>,
46+
ctx: &mut StateHandlerContext<'_, PowerShelfStateHandlerContextObjects>,
4147
) -> Result<StateHandlerOutcome<PowerShelfControllerState>, StateHandlerError> {
4248
if state.is_marked_as_deleted() {
4349
return Ok(StateHandlerOutcome::transition(
@@ -58,9 +64,193 @@ pub async fn handle_ready(
5864
));
5965
}
6066

61-
tracing::info!("PowerShelf {} is ready", power_shelf_id,);
62-
Ok(StateHandlerOutcome::wait(format!(
63-
"PowerShelf {} is ready",
64-
power_shelf_id
65-
)))
67+
let txn = poll_rms_power_state(power_shelf_id, state, ctx).await;
68+
69+
Ok(StateHandlerOutcome::do_nothing().with_txn_opt(txn))
70+
}
71+
///
72+
/// On a successful response, the observed `pstate` for this power shelf is
73+
/// persisted to the `power_shelves.status` column and the in-memory `state`
74+
/// is updated to match. The returned `PgTransaction` (if any) carries that
75+
/// status write so the caller can attach it to the `Ready` outcome and have
76+
/// the state-controller framework commit it alongside the usual outcome
77+
/// bookkeeping.
78+
async fn poll_rms_power_state(
79+
power_shelf_id: &PowerShelfId,
80+
state: &mut PowerShelf,
81+
ctx: &mut StateHandlerContext<'_, PowerShelfStateHandlerContextObjects>,
82+
) -> Option<PgTransaction<'static>> {
83+
let Some(rms_client) = ctx.services.rms_client.as_ref() else {
84+
tracing::debug!(
85+
power_shelf_id = %power_shelf_id,
86+
"PowerShelf Ready: skipping RMS GetPowerStateByDeviceList; RMS client not configured",
87+
);
88+
return None;
89+
};
90+
91+
let Some(rack_id) = state.rack_id.as_ref() else {
92+
tracing::debug!(
93+
power_shelf_id = %power_shelf_id,
94+
"PowerShelf Ready: skipping RMS GetPowerStateByDeviceList; power shelf has no rack association",
95+
);
96+
return None;
97+
};
98+
99+
let device = match build_power_shelf_node_info(
100+
power_shelf_id,
101+
state,
102+
rack_id.to_string(),
103+
&ctx.services.db_pool,
104+
ctx.services.credential_manager.as_ref(),
105+
)
106+
.await
107+
{
108+
Ok(device) => device,
109+
Err(cause) => {
110+
tracing::debug!(
111+
power_shelf_id = %power_shelf_id,
112+
rack_id = %rack_id,
113+
cause = %cause,
114+
"PowerShelf Ready: skipping RMS GetPowerStateByDeviceList; unable to build NodeSet",
115+
);
116+
return None;
117+
}
118+
};
119+
120+
let request = rms::GetPowerStateByDeviceListRequest {
121+
nodes: Some(rms::NodeSet {
122+
devices: vec![device],
123+
}),
124+
..Default::default()
125+
};
126+
127+
let rack_id_str = rack_id.to_string();
128+
let response = match rms_client.get_power_state_by_device_list(request).await {
129+
Ok(response) => response,
130+
Err(error) => {
131+
let error = rack_manager_error("get_power_state_by_device_list", error);
132+
tracing::warn!(
133+
power_shelf_id = %power_shelf_id,
134+
rack_id = %rack_id_str,
135+
error = %error,
136+
"RMS GetPowerStateByDeviceList transport error",
137+
);
138+
return None;
139+
}
140+
};
141+
142+
let batch = response.response.clone().unwrap_or_default();
143+
if !(batch.status == rms::ReturnCode::Success as i32 && batch.failed_nodes == 0) {
144+
tracing::warn!(
145+
power_shelf_id = %power_shelf_id,
146+
rack_id = %rack_id_str,
147+
batch_status = batch.status,
148+
successful_nodes = batch.successful_nodes,
149+
failed_nodes = batch.failed_nodes,
150+
message = %batch.message,
151+
"RMS GetPowerStateByDeviceList returned non-Success result",
152+
);
153+
return None;
154+
}
155+
156+
tracing::info!(
157+
power_shelf_id = %power_shelf_id,
158+
rack_id = %rack_id_str,
159+
successful_nodes = batch.successful_nodes,
160+
pstates = ?response
161+
.node_power_states
162+
.iter()
163+
.map(|node| (node.node_id.as_str(), node.pstate.as_str()))
164+
.collect::<Vec<_>>(),
165+
"RMS GetPowerStateByDeviceList succeeded",
166+
);
167+
168+
persist_observed_power_state(power_shelf_id, state, ctx, &response.node_power_states).await
169+
}
170+
171+
/// Look up the `NodePowerState` for this power shelf in the RMS response,
172+
/// stamp the value into `state.status`, and persist it via
173+
/// `db_power_shelf::update`. Returns the open `PgTransaction` so the caller
174+
/// can attach it to the `Ready` outcome.
175+
///
176+
/// Status persistence is best-effort: if RMS did not echo a result for this
177+
/// node, or if the DB write fails, the in-memory state is left untouched
178+
/// and `None` is returned — `Ready` must stay in `Ready` regardless.
179+
async fn persist_observed_power_state(
180+
power_shelf_id: &PowerShelfId,
181+
state: &mut PowerShelf,
182+
ctx: &mut StateHandlerContext<'_, PowerShelfStateHandlerContextObjects>,
183+
node_power_states: &[rms::NodePowerState],
184+
) -> Option<PgTransaction<'static>> {
185+
let node_id = power_shelf_id.to_string();
186+
let Some(observed) = node_power_states
187+
.iter()
188+
.find(|node| node.node_id == node_id)
189+
else {
190+
tracing::debug!(
191+
power_shelf_id = %power_shelf_id,
192+
"RMS GetPowerStateByDeviceList: no NodePowerState echoed for this power shelf; skipping status update",
193+
);
194+
return None;
195+
};
196+
197+
let new_power_state = observed.pstate.to_lowercase();
198+
let new_status = match state.status.as_ref() {
199+
Some(existing) => PowerShelfStatus {
200+
shelf_name: existing.shelf_name.clone(),
201+
power_state: new_power_state.clone(),
202+
health_status: existing.health_status.clone(),
203+
},
204+
None => PowerShelfStatus {
205+
shelf_name: state.config.name.clone(),
206+
power_state: new_power_state.clone(),
207+
health_status: String::new(),
208+
},
209+
};
210+
211+
if state
212+
.status
213+
.as_ref()
214+
.is_some_and(|s| s.power_state == new_status.power_state)
215+
{
216+
tracing::debug!(
217+
power_shelf_id = %power_shelf_id,
218+
power_state = %new_status.power_state,
219+
"PowerShelf status power_state unchanged; skipping DB write",
220+
);
221+
return None;
222+
}
223+
224+
let previous_status = state.status.replace(new_status);
225+
226+
let mut txn = match ctx.services.db_pool.begin().await {
227+
Ok(txn) => txn,
228+
Err(error) => {
229+
state.status = previous_status;
230+
tracing::warn!(
231+
power_shelf_id = %power_shelf_id,
232+
error = %error,
233+
"PowerShelf Ready: failed to begin txn while persisting observed power state",
234+
);
235+
return None;
236+
}
237+
};
238+
239+
if let Err(error) = db_power_shelf::update(state, &mut txn).await {
240+
state.status = previous_status;
241+
tracing::warn!(
242+
power_shelf_id = %power_shelf_id,
243+
error = %error,
244+
"PowerShelf Ready: failed to persist observed power state to DB",
245+
);
246+
return None;
247+
}
248+
249+
tracing::info!(
250+
power_shelf_id = %power_shelf_id,
251+
power_state = %new_power_state,
252+
"PowerShelf Ready: persisted observed power state from RMS",
253+
);
254+
255+
Some(txn)
66256
}

crates/rack/src/rms_client.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ pub mod test_support {
158158

159159
fn build_mock_client(&self) -> MockRmsClient {
160160
MockRmsClient {
161+
submitted_get_power_state_by_device_list_requests: Arc::new(Mutex::new(Vec::new())),
162+
queued_get_power_state_by_device_list_responses: Arc::new(Mutex::new(
163+
VecDeque::new(),
164+
)),
161165
fail_add_node: self.fail_add_node.clone(),
162166
fail_inventory_get: self.fail_inventory_get.clone(),
163167
registered_nodes: self.registered_nodes.clone(),
@@ -452,6 +456,10 @@ pub mod test_support {
452456
switch_system_image_job_statuses:
453457
Arc<Mutex<HashMap<String, rms::GetSwitchSystemImageJobStatusResponse>>>,
454458
switch_system_image_job_errors: Arc<Mutex<HashMap<String, String>>>,
459+
submitted_get_power_state_by_device_list_requests:
460+
Arc<Mutex<Vec<rms::GetPowerStateByDeviceListRequest>>>,
461+
queued_get_power_state_by_device_list_responses:
462+
Arc<Mutex<VecDeque<Result<rms::GetPowerStateByDeviceListResponse, RackManagerError>>>>,
455463
submitted_get_device_info_by_device_list_requests:
456464
Arc<Mutex<Vec<rms::GetDeviceInfoByDeviceListRequest>>>,
457465
queued_get_device_info_by_device_list_responses:
@@ -473,6 +481,21 @@ pub mod test_support {
473481

474482
#[async_trait::async_trait]
475483
impl RmsApi for MockRmsClient {
484+
async fn get_power_state_by_device_list(
485+
&self,
486+
cmd: rms::GetPowerStateByDeviceListRequest,
487+
) -> Result<rms::GetPowerStateByDeviceListResponse, RackManagerError> {
488+
self.submitted_get_power_state_by_device_list_requests
489+
.lock()
490+
.await
491+
.push(cmd);
492+
self.queued_get_power_state_by_device_list_responses
493+
.lock()
494+
.await
495+
.pop_front()
496+
.unwrap_or(Ok(rms::GetPowerStateByDeviceListResponse::default()))
497+
}
498+
476499
async fn get_device_info_by_device_list(
477500
&self,
478501
cmd: rms::GetDeviceInfoByDeviceListRequest,

0 commit comments

Comments
 (0)