From 5f38986fbf6c3cc6d930b7c07d247f82c57cd9df Mon Sep 17 00:00:00 2001 From: Laura Gray Date: Fri, 10 Jul 2026 11:32:42 +0100 Subject: [PATCH] feat(admin-cli): add --host_nics to expected-machine patch `expected-machine patch` could not update a machine's host NICs -- the merge path preserved the existing list (per an old TODO). Expose `--host_nics` on patch, mirroring `expected-machine add`: a JSON array of ExpectedHostNic that full-replaces the machine's host NIC list. Omitting the flag preserves the existing list. Also preserve sku_id on partial patches: the merge passed the raw `--sku-id` param straight through, and the DB update binds sku_id directly (not COALESCE), so any patch without `--sku-id` (now including a host-NIC-only patch) silently cleared an existing SKU. Fall back to the stored value with `.or(expected_machine.sku_id)`, matching the other preserved fields. File-based `expected-machine update` keeps its prior behavior (host_nics untouched); its JSON cannot distinguish an omitted list from a cleared one, so wiring that through is left as a separate change. --- .../src/expected_machines/patch/args.rs | 12 ++++++++- .../src/expected_machines/patch/mod.rs | 1 + .../admin-cli/src/expected_machines/tests.rs | 25 +++++++++++++++++++ .../src/expected_machines/update/mod.rs | 3 +++ crates/admin-cli/src/rpc.rs | 9 ++++--- 5 files changed, 46 insertions(+), 4 deletions(-) diff --git a/crates/admin-cli/src/expected_machines/patch/args.rs b/crates/admin-cli/src/expected_machines/patch/args.rs index 3f9d8f57b1..e97272fbe4 100644 --- a/crates/admin-cli/src/expected_machines/patch/args.rs +++ b/crates/admin-cli/src/expected_machines/patch/args.rs @@ -50,6 +50,7 @@ use crate::errors::CarbideCliError; "dpu_mode", "bmc_ip_allocation", "dpf_enabled", +"host_nics", ])))] #[command(after_long_help = "\ EXAMPLES: @@ -201,6 +202,14 @@ pub struct Args { )] pub bmc_ip_allocation: Option, + #[clap( + long = "host_nics", + value_name = "HOST_NICS", + group = "group", + help = "Host NICs as a JSON array of ExpectedHostNic objects (fields: mac_address, network_segment_type, fixed_ip, fixed_mask, fixed_gateway, primary; legacy: nic_type). Replaces the machine's full host NIC list." + )] + pub host_nics: Option, + #[clap( long = "disable-lockdown", value_name = "DISABLE_LOCKDOWN", @@ -234,8 +243,9 @@ impl Args { && self.bmc_ip_address.is_none() && self.dpu_mode.is_none() && self.bmc_ip_allocation.is_none() + && self.host_nics.is_none() { - return Err(CarbideCliError::GenericError("One of the following options must be specified: bmc-username and bmc-password or chassis-serial-number or fallback-dpu-serial-number or bmc-ip-address or dpu-mode or bmc-ip-allocation or dpf-enabled".to_string())); + return Err(CarbideCliError::GenericError("One of the following options must be specified: bmc-username and bmc-password or chassis-serial-number or fallback-dpu-serial-number or bmc-ip-address or dpu-mode or bmc-ip-allocation or dpf-enabled or host_nics".to_string())); } if self .fallback_dpu_serial_numbers diff --git a/crates/admin-cli/src/expected_machines/patch/mod.rs b/crates/admin-cli/src/expected_machines/patch/mod.rs index beec8645a3..6dd47b671e 100644 --- a/crates/admin-cli/src/expected_machines/patch/mod.rs +++ b/crates/admin-cli/src/expected_machines/patch/mod.rs @@ -56,6 +56,7 @@ impl Run for Args { .map(|dl| ::rpc::forge::HostLifecycleProfile { disable_lockdown: Some(dl), }), + self.host_nics, ) .await?; Ok(()) diff --git a/crates/admin-cli/src/expected_machines/tests.rs b/crates/admin-cli/src/expected_machines/tests.rs index 741b187455..ca052dc387 100644 --- a/crates/admin-cli/src/expected_machines/tests.rs +++ b/crates/admin-cli/src/expected_machines/tests.rs @@ -819,3 +819,28 @@ fn validate_patch_with_bmc_ip_allocation_only() { _ => panic!("expected Patch variant"), } } + +// `patch --host_nics '[...]'` alone (no other patchable fields) must satisfy +// clap's ArgGroup and `Args::validate()`'s "at least one field" check. +#[test] +fn validate_patch_with_host_nics_only() { + let cmd = Cmd::try_parse_from([ + "expected-machine", + "patch", + "--bmc-mac-address", + "00:00:00:00:00:00", + "--host_nics", + r#"[{"mac_address":"00:11:22:33:44:55","primary":true}]"#, + ]) + .expect("patch --host_nics alone should parse (ArgGroup)"); + + match cmd { + Cmd::Patch(args) => { + assert!( + args.validate().is_ok(), + "patch --host_nics alone should validate" + ); + } + _ => panic!("expected Patch variant"), + } +} diff --git a/crates/admin-cli/src/expected_machines/update/mod.rs b/crates/admin-cli/src/expected_machines/update/mod.rs index e3485a30dc..508b9ccf8c 100644 --- a/crates/admin-cli/src/expected_machines/update/mod.rs +++ b/crates/admin-cli/src/expected_machines/update/mod.rs @@ -75,6 +75,9 @@ impl Run for Args { disable_lockdown: hlp.disable_lockdown, } }), + // TODO: file-based update preserves existing host_nics; wire in + // expected_machine.host_nics to honor the file's list + None, ) .await?; Ok(()) diff --git a/crates/admin-cli/src/rpc.rs b/crates/admin-cli/src/rpc.rs index 71b060246a..43a3fb7d52 100644 --- a/crates/admin-cli/src/rpc.rs +++ b/crates/admin-cli/src/rpc.rs @@ -831,6 +831,7 @@ impl ApiClient { dpu_mode: Option<::rpc::forge::DpuMode>, bmc_ip_allocation: Option<::rpc::forge::BmcIpAllocationType>, host_lifecycle_profile: Option<::rpc::forge::HostLifecycleProfile>, + host_nics: Option, ) -> Result<(), CarbideCliError> { let get_req = match (bmc_mac_address, id) { (Some(_), Some(_)) => { @@ -900,10 +901,12 @@ impl ApiClient { fallback_dpu_serial_numbers: fallback_dpu_serial_numbers .unwrap_or(expected_machine.fallback_dpu_serial_numbers), metadata: merged_metadata, - sku_id, + sku_id: sku_id.or(expected_machine.sku_id), id: expected_machine.id, - // TODO(chet): Add support for patching host_nics at some point. - host_nics: expected_machine.host_nics, + host_nics: host_nics + .map(|s| serde_json::from_str::>(&s)) + .transpose()? + .unwrap_or(expected_machine.host_nics), rack_id: rack_id.or(expected_machine.rack_id), default_pause_ingestion_and_poweron, #[allow(deprecated)]