Skip to content

refactor(vpc): separate Vpc fields into config/status #2613

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
ianderson-nvidia wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(vpc): separate Vpc fields into config/status #2613

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
71 changes: 52 additions & 19 deletions crates/admin-cli/src/vpc/show/cmd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,40 @@ async fn show_vpc_details(
Ok(())
}

#[allow(deprecated)]
fn vpc_config(vpc: &forgerpc::Vpc) -> forgerpc::VpcConfig {
if let Some(config) = vpc.config.clone() {
config
} else {
forgerpc::VpcConfig {
tenant_organization_id: vpc.tenant_organization_id.clone(),
tenant_keyset_id: vpc.tenant_keyset_id.clone(),
network_virtualization_type: vpc.network_virtualization_type,
network_security_group_id: vpc.network_security_group_id.clone(),
default_nvlink_logical_partition_id: vpc.default_nvlink_logical_partition_id,
vni: vpc.vni,
routing_profile_type: vpc.routing_profile_type.clone(),
}
}
}

#[allow(deprecated)]
fn vpc_allocated_vni(vpc: &forgerpc::Vpc) -> u32 {
vpc.status
.as_ref()
.and_then(|status| status.vni)
.or(vpc.deprecated_vni)
.unwrap_or_default()
}
Comment thread
ianderson-nvidia marked this conversation as resolved.

#[allow(deprecated)]
fn vpc_virt_type(vpc: &forgerpc::Vpc) -> i32 {
vpc_config(vpc)
.network_virtualization_type
.or(vpc.network_virtualization_type)
.unwrap_or_default()
}

fn convert_vpcs_to_nice_table(vpcs: forgerpc::VpcList) -> Box<Table> {
let mut table = Table::new();

Expand All @@ -115,18 +149,17 @@ fn convert_vpcs_to_nice_table(vpcs: forgerpc::VpcList) -> Box<Table> {

for vpc in vpcs.vpcs {
let metadata = vpc.metadata.as_ref().unwrap_or(&default_metadata);
let virt_type = forgerpc::VpcVirtualizationType::try_from(
vpc.network_virtualization_type.unwrap_or_default(),
)
.unwrap_or_default()
.as_str_name()
.to_string();
let config = vpc_config(&vpc);
let virt_type = forgerpc::VpcVirtualizationType::try_from(vpc_virt_type(&vpc))
.unwrap_or_default()
.as_str_name()
.to_string();

table.add_row(row![
vpc.id.unwrap_or_default(),
metadata.name,
vpc.tenant_organization_id,
vpc.network_security_group_id.unwrap_or_default(),
config.tenant_organization_id,
config.network_security_group_id.unwrap_or_default(),
vpc.version,
vpc.created.unwrap_or_default(),
virt_type,
Expand All @@ -146,9 +179,11 @@ fn convert_vpcs_to_nice_table(vpcs: forgerpc::VpcList) -> Box<Table> {
table.into()
}

#[allow(deprecated)]
fn convert_vpc_to_nice_format(vpc: &forgerpc::Vpc) -> CarbideCliResult<String> {
let width = 25;
let mut lines = String::new();
let config = vpc_config(vpc);

let vpc_name = vpc
.metadata
Expand All @@ -159,10 +194,10 @@ fn convert_vpc_to_nice_format(vpc: &forgerpc::Vpc) -> CarbideCliResult<String> {
let data: Vec<(&'static str, Cow<str>)> = vec![
("ID", vpc.id.unwrap_or_default().to_string().into()),
("NAME", vpc_name),
("TENANT ORG", vpc.tenant_organization_id.as_str().into()),
("TENANT ORG", config.tenant_organization_id.as_str().into()),
(
"NETWORK SECURITY GROUP",
vpc.network_security_group_id().into(),
config.network_security_group_id.unwrap_or_default().into(),
),
("VERSION", vpc.version.as_str().into()),
(
Expand All @@ -180,19 +215,17 @@ fn convert_vpc_to_nice_format(vpc: &forgerpc::Vpc) -> CarbideCliResult<String> {
None => "".into(),
},
),
("TENANT KEYSET", vpc.tenant_keyset_id().into()),
(
"VNI",
format!("{}", vpc.status.and_then(|s| s.vni).unwrap_or_default()).into(),
"TENANT KEYSET",
config.tenant_keyset_id.unwrap_or_default().into(),
),
("VNI", format!("{}", vpc_allocated_vni(vpc)).into()),
(
"NW VIRTUALIZATION",
forgerpc::VpcVirtualizationType::try_from(
vpc.network_virtualization_type.unwrap_or_default(),
)
.unwrap_or_default()
.as_str_name()
.into(),
forgerpc::VpcVirtualizationType::try_from(vpc_virt_type(vpc))
.unwrap_or_default()
.as_str_name()
.into(),
),
];

Expand Down
9 changes: 5 additions & 4 deletions crates/api-core/src/db_init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ pub async fn create_initial_networks(
ns.vpc_id = if let Some(vpc_name) = &def.vpc_name {
match db::vpc::find_by_name(&mut txn, vpc_name).await?.as_slice() {
[vpc] => {
vpc.network_virtualization_type
vpc.config
.network_virtualization_type
.ensure_supports_segment(&ns)?;
Some(vpc.id)
}
Expand Down Expand Up @@ -283,7 +284,7 @@ pub async fn update_network_segments_svi_ip(db_pool: &Pool<Postgres>) -> Result<
};

// SVI IP is needed only for FNN.
if vpc.network_virtualization_type != VpcVirtualizationType::Fnn {
if vpc.config.network_virtualization_type != VpcVirtualizationType::Fnn {
continue;
}

Expand Down Expand Up @@ -400,8 +401,8 @@ pub(crate) async fn create_admin_vpc(
};

if let Some(mut existing_vpc) = existing_vpc {
let existing_vni = existing_vpc.status.as_ref().and_then(|status| status.vni);
if existing_vni != Some(configured_vni) || existing_vpc.vni != Some(configured_vni) {
let existing_vni = existing_vpc.status.vni;
if existing_vni != Some(configured_vni) || existing_vpc.config.vni != Some(configured_vni) {
if let Some(conflicting_vpc) = db::vpc::find_by_vni(&mut txn, configured_vni)
.await?
.into_iter()
Expand Down
92 changes: 47 additions & 45 deletions crates/api-core/src/ethernet_virtualization.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ pub(crate) async fn validate_instance_interface_routing_profiles(
let vpc = db::vpc::find_by_segment(&mut *txn, segment_id).await?;

// Interface routing profiles are only valid on FNN VPC interfaces.
if vpc.network_virtualization_type != VpcVirtualizationType::Fnn {
if vpc.config.network_virtualization_type != VpcVirtualizationType::Fnn {
return Err(CarbideError::InvalidArgument(
"instance interface routing_profile is only supported for FNN VPC interfaces"
.to_string(),
Expand All @@ -165,7 +165,8 @@ pub(crate) async fn validate_instance_interface_routing_profiles(
)
})?;
let profile_type =
vpc.routing_profile_type
vpc.config
.routing_profile_type
.as_ref()
.ok_or_else(|| CarbideError::Internal {
message: "tenant routing profile type not found in VPC record".to_string(),
Expand Down Expand Up @@ -440,7 +441,7 @@ pub async fn admin_network(
return Err(CarbideError::FindOneReturnedNoResultsError(vpc_id.into()).into());
}
let vpc = vpcs.remove(0);
match vpc.status.and_then(|v| v.vni) {
match vpc.status.vni {
Some(vpc_vni) => {
let tenant_loopback_ip = if use_vpc_vrf_loopback {
Some(
Expand Down Expand Up @@ -643,50 +644,51 @@ pub async fn tenant_network(
None => None,
};

let vpc_vni = vpc
.as_ref()
.and_then(|v| v.status.as_ref().and_then(|vs| vs.vni))
.unwrap_or_default() as u32;
let vpc_vni = vpc.as_ref().and_then(|v| v.status.vni).unwrap_or_default() as u32;

// Resolve the routing profile from the VPC attached to this interface.
let (vpc_routing_profile, interface_routing_profile) = match (vpc.as_ref(), fnn_config) {
(Some(vpc), Some(fnn)) if vpc.network_virtualization_type == VpcVirtualizationType::Fnn => {
let profile_type =
vpc.routing_profile_type
.as_ref()
.ok_or_else(|| CarbideError::Internal {
let (vpc_routing_profile, interface_routing_profile) =
match (vpc.as_ref(), fnn_config) {
(Some(vpc), Some(fnn))
if vpc.config.network_virtualization_type == VpcVirtualizationType::Fnn =>
{
let profile_type = vpc.config.routing_profile_type.as_ref().ok_or_else(|| {
CarbideError::Internal {
message: "tenant routing profile type not found in VPC record".to_string(),
})?;
let profile = fnn.routing_profiles.get(profile_type).ok_or_else(|| {
CarbideError::NotFoundError {
kind: "routing_profile_type",
id: profile_type.to_string(),
}
})?;
}
})?;
let profile = fnn.routing_profiles.get(profile_type).ok_or_else(|| {
CarbideError::NotFoundError {
kind: "routing_profile_type",
id: profile_type.to_string(),
}
})?;

(
Some(rpc::RoutingProfile::from(profile)),
iface
.routing_profile
.as_ref()
.map(rpc::FlatInterfaceRoutingProfile::from),
)
}
(Some(vpc), None) if vpc.network_virtualization_type == VpcVirtualizationType::Fnn => {
return Err(CarbideError::Internal {
message: "FNN VPC found but no FNN config found".to_string(),
(
Some(rpc::RoutingProfile::from(profile)),
iface
.routing_profile
.as_ref()
.map(rpc::FlatInterfaceRoutingProfile::from),
)
}
.into());
}
_ if iface.routing_profile.is_some() => {
return Err(CarbideError::InvalidArgument(
"instance interface routing_profile is only supported for FNN VPC interfaces"
.to_string(),
)
.into());
}
_ => (None, None),
};
(Some(vpc), None)
if vpc.config.network_virtualization_type == VpcVirtualizationType::Fnn =>
{
return Err(CarbideError::Internal {
message: "FNN VPC found but no FNN config found".to_string(),
}
.into());
}
_ if iface.routing_profile.is_some() => {
return Err(CarbideError::InvalidArgument(
"instance interface routing_profile is only supported for FNN VPC interfaces"
.to_string(),
)
.into());
}
_ => (None, None),
};

let rpc_ft: rpc::InterfaceFunctionType = iface.function_id.function_type().into();
let (svi_ip, svi_ip_v6) = ds.svi_ips(network_virtualization_type, is_l2_segment)?;
Expand All @@ -701,14 +703,14 @@ pub async fn tenant_network(
// see if there's an associated VPC (there should be),
// and see if the VPC has an NSG attached.
(false, None, Some(v)) => {
match v.network_security_group_id.as_ref() {
match v.config.network_security_group_id.as_ref() {
None => None,
Some(vpc_nsg_id) => {
// Make our DB query for the IDs to get our NetworkSecurityGroup
let network_security_group = network_security_group::find_by_ids(
txn,
&[vpc_nsg_id.to_owned()],
Some(&v.tenant_organization_id.parse().map_err(|_| {
Some(&v.config.tenant_organization_id.parse().map_err(|_| {
CarbideError::Internal {
message: "invalid tenant org in VPC data".to_string(),
}
Expand All @@ -719,7 +721,7 @@ pub async fn tenant_network(
.pop()
.ok_or(CarbideError::NotFoundError {
kind: "NetworkSecurityGroup",
id: v.tenant_organization_id.clone(),
id: vpc_nsg_id.to_string(),
})?;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Some((
Expand Down
4 changes: 2 additions & 2 deletions crates/api-core/src/handlers/dpu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,9 @@ pub(crate) async fn get_managed_host_network_config_inner(
let vpc = db::vpc::find_by_segment(&mut txn, network_segment_id)
.await?;

network_virtualization_type = vpc.network_virtualization_type;
network_virtualization_type = vpc.config.network_virtualization_type;

vpc_vni = vpc.status.as_ref().and_then(|v| v.vni.map(|x|x as u32));
vpc_vni = vpc.status.vni.map(|x| x as u32);

let suppress_tenant_security_groups = match &snapshot.managed_state {
ManagedHostState::Assigned { instance_state } => {
Expand Down
5 changes: 3 additions & 2 deletions crates/api-core/src/handlers/network_segment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub(crate) async fn create(
.first()
.ok_or_else(|| CarbideError::internal(format!("VPC ID: {vpc_id} not found.")))?;

let virtualization_type = vpc.network_virtualization_type;
let virtualization_type = vpc.config.network_virtualization_type;

// Segment compatibility (segment-type binding + IPv6 support)
// and SVI allocation are both expressed as capability checks
Expand Down Expand Up @@ -214,7 +214,8 @@ pub(crate) async fn attach_to_vpc(
.into());
}

vpc.network_virtualization_type
vpc.config
.network_virtualization_type
.ensure_supports_segment(&segment)
.map_err(CarbideError::from)?;

Expand Down
12 changes: 8 additions & 4 deletions crates/api-core/src/handlers/vpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ pub(crate) async fn update(
&mut txn,
std::slice::from_ref(&id),
Some(
&vpc.tenant_organization_id
&vpc.config
.tenant_organization_id
.parse()
.map_err(|e: InvalidTenantOrg| {
CarbideError::from(RpcDataConversionError::InvalidTenantOrg(e.to_string()))
Expand All @@ -203,7 +204,7 @@ pub(crate) async fn update(
{
return Err(CarbideError::FailedPrecondition(format!(
"NetworkSecurityGroup `{}` does not exist or is not owned by Tenant `{}`",
id, vpc.tenant_organization_id
id, vpc.config.tenant_organization_id
))
.into());
}
Expand Down Expand Up @@ -299,13 +300,16 @@ pub(crate) async fn delete(
}
};

if let Some(vni) = vpc.status.as_ref().and_then(|s| s.vni) {
if let Some(vni) = vpc.status.vni {
// We can just keep deriving int/ext from the routing profile
// because a VPC is not allowed to change its profile after
// creation. VPC types that don't carry a routing profile
// (ETV, Flat) land in the internal pool on create -- mirror
// that here so the VNI is released back to the same pool.
let internal = match (api.runtime_config.fnn.as_ref(), vpc.routing_profile_type) {
let internal = match (
api.runtime_config.fnn.as_ref(),
vpc.config.routing_profile_type,
) {
(None, _) | (Some(_), None) => true,
(Some(f), Some(profile_type)) => {
let Some(profile) = f.routing_profiles.get(&profile_type) else {
Expand Down
5 changes: 3 additions & 2 deletions crates/api-core/src/handlers/vpc_peering.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ pub async fn create(
// Make sure the VPCs are allowed to peer based on their
// virtualization types. Their capabilities will determine
// if they are allowed or not.
vpc1.network_virtualization_type
.ensure_can_peer_with(vpc2.network_virtualization_type)
vpc1.config
.network_virtualization_type
.ensure_can_peer_with(vpc2.config.network_virtualization_type)
.map_err(CarbideError::from)?;
}
Some(VpcPeeringPolicy::Mixed) => {
Expand Down
3 changes: 2 additions & 1 deletion crates/api-core/src/handlers/vpc_prefix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ pub async fn create(
})?;

if new_prefix.config.prefix.is_ipv6() {
vpc.network_virtualization_type
vpc.config
.network_virtualization_type
.ensure_supports_ipv6_prefix()
.map_err(CarbideError::from)?;
}
Expand Down
Loading
Loading