Skip to content

chore: Remove deprecated flat fields from NetworkSegment object #4450

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

Draft
nvlitagaki wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

chore: Remove deprecated flat fields from NetworkSegment object #4450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b0a9dce
refactor(api): remove deprecated NetworkSegment fields
nvlitagaki Jul 30, 2026
dc80eaf
Merge branch 'main' into chore/remove-deprecated-subnet-fields
nvlitagaki Jul 31, 2026
eced758
Merge branch 'NVIDIA:main' into chore/remove-deprecated-subnet-fields
nvlitagaki Jul 31, 2026
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
9 changes: 2 additions & 7 deletions crates/admin-cli/src/devenv/config/apply/cmd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,17 @@ async fn handle_overlay_segment_creation(
.get_all_segments(None, Some(name.clone()), 2)
.await?;

#[allow(deprecated)]
if let Some(ns) = network_segment.network_segments.first() {
let ns_name = ns
.metadata
.as_ref()
.map(|m| m.name.as_str())
.unwrap_or(ns.name.as_str());
.unwrap_or_default();

let prefix_str = ns
.config
.as_ref()
.and_then(|c| c.prefixes.first())
.or_else(|| ns.prefixes.first())
.map_or("unknown", |p| p.prefix.as_str());

println!(
Expand All @@ -118,19 +116,16 @@ async fn handle_overlay_segment_creation(
)
.await?;

#[allow(deprecated)]
let ns_name = ns
.metadata
.as_ref()
.map(|m| m.name.as_str())
.unwrap_or(ns.name.as_str());
.unwrap_or_default();

#[allow(deprecated)]
let prefix_str = ns
.config
.as_ref()
.and_then(|c| c.prefixes.first())
.or_else(|| ns.prefixes.first())
.map_or("unknown", |p| p.prefix.as_str());

println!(
Expand Down
8 changes: 4 additions & 4 deletions crates/admin-cli/src/instance/show/cmd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,10 +637,10 @@ async fn get_vpc_for_interface_network_segment(
.await?;

if !network_segments.network_segments.is_empty()
&& let Some(vpc_id) = network_segments.network_segments.first().and_then(|s| {
#[allow(deprecated)]
s.config.as_ref().and_then(|c| c.vpc_id).or(s.vpc_id)
})
&& let Some(vpc_id) = network_segments
.network_segments
.first()
.and_then(|s| s.config.as_ref().and_then(|c| c.vpc_id))
{
let vpc_ids: Vec<VpcId> = vec![vpc_id];
Ok(api_client
Expand Down
255 changes: 26 additions & 229 deletions crates/admin-cli/src/network_segment/show/cmd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,6 @@ fn format_free_ip_count(legacy_count: u32, count_v2: Option<u64>, saturated: boo
}
}

#[allow(deprecated)]
fn convert_old_history(
history: &[forgerpc::NetworkSegmentStateHistory],
) -> Vec<forgerpc::StateHistoryRecord> {
history
.iter()
.map(|h| forgerpc::StateHistoryRecord {
state: h.state.clone(),
version: h.version.clone(),
time: h.time,
})
.collect()
}

#[allow(deprecated)]
pub async fn convert_network_to_nice_format(
segment: forgerpc::NetworkSegment,
history: Option<Vec<forgerpc::StateHistoryRecord>>,
Expand All @@ -65,32 +50,19 @@ pub async fn convert_network_to_nice_format(
.metadata
.as_ref()
.map(|m| m.name.clone())
.unwrap_or_else(|| segment.name.clone());
.unwrap_or_default();

let config = if let Some(config) = segment.config {
config
} else {
// Old server: construct from deprecated flat fields.
forgerpc::NetworkSegmentConfig {
vpc_id: segment.vpc_id,
subdomain_id: segment.subdomain_id,
mtu: segment.mtu,
segment_type: segment.segment_type,
prefixes: segment.prefixes.clone(),
}
};
let config = segment.config.unwrap_or_default();

let state = if let Some(lc) = segment.status.and_then(|s| s.lifecycle) {
serde_json::from_str::<NetworkState>(&lc.state)
.map(|ns| ns.state)
.unwrap_or_else(|_| lc.state)
} else {
// Old server: format the deprecated enum field as a string.
format!(
"{:?}",
forgerpc::TenantState::try_from(segment.state).unwrap_or_default()
)
};
let state = segment
.status
.and_then(|status| status.lifecycle)
.map(|lifecycle| {
serde_json::from_str::<NetworkState>(&lifecycle.state)
.map(|ns| ns.state)
.unwrap_or(lifecycle.state)
})
.unwrap_or_default();

let width = 10;
let mut lines = String::new();
Expand Down Expand Up @@ -218,7 +190,6 @@ async fn get_domain_name(domain_id: Option<DomainId>, api_client: &ApiClient) ->
}
}

#[allow(deprecated)]
fn convert_network_to_nice_table(
segments: forgerpc::NetworkSegmentList,
) -> CarbideCliResult<Box<Table>> {
Expand All @@ -234,35 +205,19 @@ fn convert_network_to_nice_table(
.metadata
.as_ref()
.map(|m| m.name.as_str())
.unwrap_or(segment.name.as_str())
.unwrap_or_default()
.to_string();

let config = if let Some(config) = segment.config {
config
} else {
// Old server: construct from deprecated flat fields.
forgerpc::NetworkSegmentConfig {
vpc_id: segment.vpc_id,
subdomain_id: segment.subdomain_id,
mtu: segment.mtu,
segment_type: segment.segment_type,
prefixes: segment.prefixes.clone(),
}
};
let config = segment.config.unwrap_or_default();

let (state, version) = if let Some(lc) = segment.status.and_then(|s| s.lifecycle) {
let state = serde_json::from_str::<NetworkState>(&lc.state)
.map(|ns| ns.state)
.unwrap_or_else(|_| lc.state.clone());
(state, lc.version)
} else {
// Old server: format deprecated enum and version fields.
let state = format!(
"{:?}",
forgerpc::TenantState::try_from(segment.state).unwrap_or_default()
);
(state, segment.version.clone())
};
let lifecycle = segment
.status
.and_then(|status| status.lifecycle)
.unwrap_or_default();
let state = serde_json::from_str::<NetworkState>(&lifecycle.state)
.map(|ns| ns.state)
.unwrap_or_else(|_| lifecycle.state.clone());
let version = lifecycle.version;

let Some(first_prefix) = config.prefixes.first() else {
continue;
Expand Down Expand Up @@ -333,15 +288,7 @@ async fn show_network_information(
return Err(CarbideCliError::SegmentNotFound);
};

// Try the `FindNetworkSegmentStateHistories` RPC first; fall back to the inline
// history field populated by old servers that lack the new RPC.
#[allow(deprecated)]
let history = api_client
.get_segment_state_history(segment_id)
.await
.ok()
.filter(|r| !r.is_empty())
.unwrap_or_else(|| convert_old_history(&segment.history));
let history = api_client.get_segment_state_history(segment_id).await?;

if json {
println!("{}", serde_json::to_string_pretty(&segment)?);
Expand Down Expand Up @@ -381,7 +328,7 @@ mod tests {
use ::rpc::forge as forgerpc;
use carbide_test_support::{Check, check_values};

use super::{convert_network_to_nice_table, convert_old_history, format_free_ip_count};
use super::{convert_network_to_nice_table, format_free_ip_count};

fn prefix(cidr: &str) -> forgerpc::NetworkPrefix {
forgerpc::NetworkPrefix {
Expand Down Expand Up @@ -447,47 +394,7 @@ mod tests {
);
}

fn old_history_record(state: &str, version: &str) -> forgerpc::NetworkSegmentStateHistory {
#[allow(deprecated)]
forgerpc::NetworkSegmentStateHistory {
state: state.to_string(),
version: version.to_string(),
time: None,
}
}

// ---------- convert_old_history ----------
//
// `show_network_information` calls `get_segment_state_history` (new RPC) and falls back to
// `convert_old_history(&segment.history)` when that RPC is unavailable or returns empty.
// That full path cannot be unit-tested here because it requires an `ApiClient`. The test
// below verifies the pure conversion function that the fallback delegates to.

#[test]
fn convert_old_history_preserves_fields() {
let records = vec![
old_history_record("Ready", "v1"),
old_history_record("Provisioning", "v2"),
];
let result = convert_old_history(&records);
assert_eq!(result.len(), 2);
assert_eq!(result[0].state, "Ready");
assert_eq!(result[0].version, "v1");
assert_eq!(result[1].state, "Provisioning");
assert_eq!(result[1].version, "v2");
}

// ---------- convert_network_to_nice_table ----------

// Simulates a response from a new server: new fields set, deprecated flat fields also set
// (the server intentionally populates both for external client compatibility).
fn new_format_segment(
name: &str,
vpc_uuid: &str,
cidr: &str,
state: &str,
) -> forgerpc::NetworkSegment {
#[allow(deprecated)]
fn segment(name: &str, vpc_uuid: &str, cidr: &str, state: &str) -> forgerpc::NetworkSegment {
forgerpc::NetworkSegment {
id: None,
config: Some(forgerpc::NetworkSegmentConfig {
Expand All @@ -512,50 +419,9 @@ mod tests {
description: String::new(),
labels: vec![],
}),
// Deprecated flat fields still populated by the new server for backward compat.
// These intentionally carry DIFFERENT values from the new fields so tests can
// confirm the new fields take priority.
name: format!("{name}-stale"),
vpc_id: Some("11111111-1111-1111-1111-111111111111".parse().unwrap()),
subdomain_id: None,
mtu: Some(1500),
prefixes: vec![prefix("192.0.2.0/24")],
segment_type: forgerpc::NetworkSegmentType::Admin as i32,
version: "old-version".to_string(),
state: forgerpc::TenantState::Provisioning as i32,
history: vec![],
created: None,
updated: None,
deleted: None,
flags: vec![],
state_reason: None,
state_sla: None,
}
}

fn old_format_segment(name: &str, vpc_uuid: &str, cidr: &str) -> forgerpc::NetworkSegment {
// Simulates a response from an old server: new fields absent, deprecated flat fields set.
#[allow(deprecated)]
forgerpc::NetworkSegment {
id: None,
config: None,
status: None,
metadata: None,
name: name.to_string(),
vpc_id: Some(vpc_uuid.parse().unwrap()),
subdomain_id: None,
mtu: Some(1500),
prefixes: vec![prefix(cidr)],
segment_type: forgerpc::NetworkSegmentType::Tenant as i32,
version: "1".to_string(),
state: forgerpc::TenantState::Ready as i32,
history: vec![],
created: None,
updated: None,
deleted: None,
flags: vec![],
state_reason: None,
state_sla: None,
}
}

Expand All @@ -567,88 +433,19 @@ mod tests {
format!("{table}")
}

// When both new fields and deprecated flat fields are present (as a new server always sends),
// the new fields must take priority.
#[test]
fn table_new_format_uses_new_fields_not_deprecated() {
let out = table_string(vec![new_format_segment(
fn table_uses_structured_fields() {
let out = table_string(vec![segment(
"seg-new",
"00000000-0000-0000-0000-000000000001",
"10.0.0.0/24",
"Ready",
)]);
// New name from metadata, not the stale "-stale" name in the flat field.
assert!(out.contains("seg-new"), "name from metadata missing: {out}");
assert!(
!out.contains("seg-new-stale"),
"stale deprecated name must not appear: {out}"
);
// State from status.lifecycle, not the deprecated TenantState::Provisioning.
assert!(out.contains("Ready"), "state from lifecycle missing: {out}");
assert!(
!out.contains("Provisioning"),
"stale deprecated state must not appear: {out}"
);
// VPC from config, not the stale flat field UUID.
assert!(
out.contains("00000000-0000-0000-0000-000000000001"),
"vpc_id from config missing: {out}"
);
assert!(
!out.contains("11111111-1111-1111-1111-111111111111"),
"stale deprecated vpc_id must not appear: {out}"
);
}

#[test]
fn table_old_format_shows_name_and_state() {
let out = table_string(vec![old_format_segment(
"seg-old",
"00000000-0000-0000-0000-000000000002",
"10.1.0.0/24",
)]);
assert!(out.contains("seg-old"), "name missing: {out}");
assert!(out.contains("Ready"), "state missing: {out}");
}

#[test]
fn table_old_format_shows_vpc_id() {
let vpc = "00000000-0000-0000-0000-000000000002";
let out = table_string(vec![old_format_segment("s", vpc, "10.1.0.0/24")]);
assert!(out.contains(vpc), "vpc_id missing: {out}");
}

#[test]
fn table_old_and_new_format_both_shown() {
let out = table_string(vec![
new_format_segment(
"seg-new",
"00000000-0000-0000-0000-000000000001",
"10.0.0.0/24",
"Ready",
),
old_format_segment(
"seg-old",
"00000000-0000-0000-0000-000000000002",
"10.1.0.0/24",
),
]);
assert!(out.contains("seg-new"), "new segment missing: {out}");
assert!(out.contains("seg-old"), "old segment missing: {out}");
}

// Coverage gaps (require a live ApiClient and cannot be unit-tested here):
//
// - `convert_network_to_nice_format` with old-format segments: same old->new fallback logic
// as `convert_network_to_nice_table` but also looks up the domain name via the API.
//
// - `show_network_information` history fallback: when `get_segment_state_history` (new RPC)
// fails or returns empty, the code falls back to `convert_old_history(&segment.history)`.
// The conversion itself is tested above; the integration path needs an ApiClient mock.
//
// - `get_vpc_for_interface_network_segment` (instance/show): the `.or(s.vpc_id)` fallback
// for the deprecated flat vpc_id field also requires ApiClient.
//
// - `handle_overlay_segment_creation` (devenv/config/apply): the name and prefix fallbacks
// are inline in an async function that requires ApiClient.
}
Loading
Loading