-
Notifications
You must be signed in to change notification settings - Fork 186
feat(admin-cli): add health-history commands for switch, power-shelf, rack, and machine #5509
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,8 @@ | |
|
|
||
| use ::rpc::admin_cli::OutputFormat; | ||
| use ::rpc::forge::{self as forgerpc}; | ||
| use prettytable::{Table, row}; | ||
| use prettytable::{Cell, Row, Table, row}; | ||
| use serde::Serialize; | ||
|
|
||
| use crate::errors::{CarbideCliError, CarbideCliResult}; | ||
| use crate::machine::{HealthReportTemplates, get_empty_template, get_health_report}; | ||
|
|
@@ -67,6 +68,82 @@ pub(crate) fn display_health_reports( | |
| Ok(()) | ||
| } | ||
|
|
||
| /// One row of an object's health history, summarized for tabular display. | ||
| #[derive(Serialize)] | ||
| struct HealthHistoryRecordView { | ||
| time: String, | ||
| source: String, | ||
| status: String, | ||
| alerts: Vec<String>, | ||
| } | ||
|
|
||
| impl From<&forgerpc::HealthHistoryRecord> for HealthHistoryRecordView { | ||
| fn from(record: &forgerpc::HealthHistoryRecord) -> Self { | ||
| let health = record.health.as_ref(); | ||
| let source = health.map(|h| h.source.clone()).unwrap_or_default(); | ||
| let alerts: Vec<String> = health | ||
| .map(|h| h.alerts.iter().map(|alert| alert.id.clone()).collect()) | ||
| .unwrap_or_default(); | ||
| let status = if alerts.is_empty() { | ||
| "Healthy" | ||
| } else { | ||
| "Alerting" | ||
| } | ||
| .to_string(); | ||
| Self { | ||
| time: record.time.map(|time| time.to_string()).unwrap_or_default(), | ||
| source, | ||
| status, | ||
| alerts, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn health_history_table(records: &[HealthHistoryRecordView]) -> Table { | ||
| let mut table = Table::new(); | ||
| table.set_titles(Row::new( | ||
| ["Time", "Source", "Status", "Alerts"] | ||
| .into_iter() | ||
| .map(Cell::new) | ||
| .collect(), | ||
| )); | ||
| for record in records { | ||
| let alerts = if record.alerts.is_empty() { | ||
| "-".to_string() | ||
| } else { | ||
| record.alerts.join(", ") | ||
| }; | ||
| table.add_row(row![record.time, record.source, record.status, alerts]); | ||
| } | ||
| table | ||
| } | ||
|
|
||
| /// Display an object's health history in the requested output format. Shared by | ||
| /// the per-resource `health-history` subcommands, whose only difference is the | ||
| /// RPC that produced `history`. | ||
| pub(crate) fn display_health_history( | ||
| object_id: &str, | ||
| history: Vec<forgerpc::HealthHistoryRecord>, | ||
| output_format: OutputFormat, | ||
| ) -> CarbideCliResult<()> { | ||
| if history.is_empty() { | ||
| println!("No health history found for {object_id}"); | ||
| return Ok(()); | ||
|
Comment on lines
+129
to
+131
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a resource has no recorded history, which is normal for newly discovered resources, this return runs before the format switch. Consequently, Useful? React with 👍 / 👎. |
||
| } | ||
|
Comment on lines
+129
to
+132
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Preserve the requested output format for empty histories.
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
|
|
||
| let records: Vec<HealthHistoryRecordView> = | ||
| history.iter().map(HealthHistoryRecordView::from).collect(); | ||
| match output_format { | ||
| OutputFormat::Json => println!("{}", serde_json::to_string_pretty(&records)?), | ||
| OutputFormat::Yaml => println!("{}", serde_yaml::to_string(&records)?), | ||
| OutputFormat::Csv => { | ||
| health_history_table(&records).to_csv(std::io::stdout())?; | ||
| } | ||
| OutputFormat::AsciiTable => health_history_table(&records).printstd(), | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Resolve a health report from either a template or raw JSON. | ||
| pub(crate) fn resolve_health_report( | ||
| template: Option<HealthReportTemplates>, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| use carbide_uuid::machine::MachineId; | ||
| use clap::Parser; | ||
|
|
||
| use crate::cfg::run::Run; | ||
| use crate::cfg::runtime::RuntimeContext; | ||
| use crate::errors::CarbideCliResult; | ||
| use crate::health_utils::display_health_history; | ||
|
|
||
| #[derive(Parser, Debug)] | ||
| #[command(after_long_help = "\ | ||
| EXAMPLES: | ||
|
|
||
| Show health history for a machine: | ||
| $ nico-admin-cli machine health-history fm100ht038bg3qsho433vkg684heguv282qaggmrsh2ugn1qk096n2c6hcg | ||
|
|
||
| ")] | ||
| pub(crate) struct Args { | ||
| #[clap(help = "Machine ID to show health history for")] | ||
| machine_id: MachineId, | ||
| } | ||
|
Comment on lines
+34
to
+37
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 2 'health_history::Args' crates/admin-cli/srcRepository: NVIDIA/infra-controller Length of output: 1322 🏁 Script executed: #!/bin/bash
set -euo pipefail
for file in \
crates/admin-cli/src/machine/health_history.rs \
crates/admin-cli/src/power_shelf/health_history.rs \
crates/admin-cli/src/switch/health_history.rs \
crates/admin-cli/src/rack/health_history.rs
do
echo "=== $file ==="
sed -n '1,100p' "$file"
done
echo "=== health_history::Args references ==="
rg -n -C 2 'health_history::Args' crates/admin-cli/srcRepository: NVIDIA/infra-controller Length of output: 7782 🏁 Script executed: #!/bin/bash
set -euo pipefail
for file in \
crates/admin-cli/src/machine/mod.rs \
crates/admin-cli/src/power_shelf/mod.rs \
crates/admin-cli/src/switch/mod.rs \
crates/admin-cli/src/rack/mod.rs
do
echo "=== $file ==="
sed -n '1,110p' "$file"
done
echo "=== all admin-cli references ==="
rg -n 'health_history::Args|pub(crate) struct Args|pub\(super\) struct Args' crates/admin-cliRepository: NVIDIA/infra-controller Length of output: 9446 Restrict all health-history Change 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| impl Run for Args { | ||
| async fn run(self, ctx: &mut RuntimeContext) -> CarbideCliResult<()> { | ||
| let history = ctx | ||
| .api_client | ||
| .get_machine_health_history(self.machine_id) | ||
| .await?; | ||
| display_health_history(&self.machine_id.to_string(), history, ctx.config.format) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When any new health-history command is invoked with the root Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| use carbide_uuid::machine::MachineId; | ||
| use clap::CommandFactory; | ||
|
|
||
| use super::*; | ||
| use crate::test_support::parse_leaf; | ||
|
|
||
| const SAMPLE_MACHINE_ID: &str = "fm100ht038bg3qsho433vkg684heguv282qaggmrsh2ugn1qk096n2c6hcg"; | ||
|
|
||
| #[test] | ||
| fn verify_cmd_structure() { | ||
| Cmd::command().debug_assert(); | ||
| } | ||
|
|
||
| #[test] | ||
| fn parse_health_history_command() { | ||
| let matches = parse_leaf::<Cmd>( | ||
| &["machine", "health-history", SAMPLE_MACHINE_ID], | ||
| &["health-history"], | ||
| ) | ||
| .expect("health-history should parse"); | ||
|
Comment on lines
+33
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new machine, rack, switch, and power-shelf tests stop after parsing the command and never invoke a public command or assert the shared history table's headers, populated cells, and empty cells. This leaves the newly introduced user-visible output contract—including malformed-health rendering and format behavior—unprotected despite the repository's explicit requirement for public-command coverage of CLI table changes. AGENTS.md reference: AGENTS.md:L140-L142 Useful? React with 👍 / 👎. |
||
| let machine_id = matches | ||
| .get_one::<MachineId>("machine_id") | ||
| .expect("machine ID is required"); | ||
| assert_eq!(machine_id, &SAMPLE_MACHINE_ID.parse::<MachineId>().unwrap()); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| use carbide_uuid::power_shelf::PowerShelfId; | ||
| use clap::Parser; | ||
|
|
||
| use crate::cfg::run::Run; | ||
| use crate::cfg::runtime::RuntimeContext; | ||
| use crate::errors::CarbideCliResult; | ||
| use crate::health_utils::display_health_history; | ||
|
|
||
| #[derive(Parser, Debug)] | ||
| #[command(after_long_help = "\ | ||
| EXAMPLES: | ||
|
|
||
| Show health history for a power shelf: | ||
| $ nico-admin-cli power-shelf health-history ps100ht038bg3qsho433vkg684heguv282qaggmrsh2ugn1qk096n2c6hcg | ||
|
|
||
| ")] | ||
| pub(crate) struct Args { | ||
| #[clap(help = "Power shelf ID to show health history for")] | ||
| power_shelf_id: PowerShelfId, | ||
| } | ||
|
|
||
| impl Run for Args { | ||
| async fn run(self, ctx: &mut RuntimeContext) -> CarbideCliResult<()> { | ||
| let history = ctx | ||
| .api_client | ||
| .get_power_shelf_health_history(self.power_shelf_id) | ||
| .await?; | ||
| display_health_history(&self.power_shelf_id.to_string(), history, ctx.config.format) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| use carbide_uuid::rack::RackId; | ||
| use clap::Parser; | ||
|
|
||
| use crate::cfg::run::Run; | ||
| use crate::cfg::runtime::RuntimeContext; | ||
| use crate::errors::CarbideCliResult; | ||
| use crate::health_utils::display_health_history; | ||
|
|
||
| #[derive(Parser, Debug)] | ||
| #[command(after_long_help = "\ | ||
| EXAMPLES: | ||
|
|
||
| Show health history for a rack: | ||
| $ nico-admin-cli rack health-history ipp6-b03-gb-nvl-124-mini2 | ||
|
|
||
| ")] | ||
| pub(crate) struct Args { | ||
| #[clap(help = "Rack ID to show health history for")] | ||
| rack_id: RackId, | ||
| } | ||
|
|
||
| impl Run for Args { | ||
| async fn run(self, ctx: &mut RuntimeContext) -> CarbideCliResult<()> { | ||
| let history = ctx | ||
| .api_client | ||
| .get_rack_health_history(self.rack_id.clone()) | ||
| .await?; | ||
| display_health_history(self.rack_id.as_ref(), history, ctx.config.format) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If an API response contains a history record without its optional protobuf
healthmessage, this conversion produces an empty alert list and labels the recordHealthy. That turns an incomplete or malformed server response into a false all-clear; the existing API-web conversion treats this case as a missing-report alert, so this renderer should likewise synthesize missing health or return an error before deriving status.Useful? React with 👍 / 👎.