diff --git a/crates/admin-cli/src/instance/allocate/args.rs b/crates/admin-cli/src/instance/allocate/args.rs index c78bdf10f5..6d2816687b 100644 --- a/crates/admin-cli/src/instance/allocate/args.rs +++ b/crates/admin-cli/src/instance/allocate/args.rs @@ -16,12 +16,12 @@ */ use carbide_uuid::machine::MachineId; -use carbide_uuid::vpc::VpcPrefixId; +use carbide_uuid::vpc::{VpcId, VpcPrefixId}; use clap::{ArgGroup, Parser}; use rpc::forge::{InstanceOperatingSystemConfig, InstanceSpxConfig}; #[derive(Parser, Debug)] -#[clap(group(ArgGroup::new("selector").required(true).args(&["subnet", "vpc_prefix_id"])))] +#[clap(group(ArgGroup::new("selector").required(true).multiple(true).args(&["subnet", "vpc_prefix_id", "flat_vpc_id"])))] #[command(after_long_help = "\ EXAMPLES: @@ -101,8 +101,11 @@ pub struct Args { #[clap(short, long, help = "The VPC prefix to assign to a PF")] pub vpc_prefix_id: Vec, - #[clap(long, help = "Allocate a zero-dpu host")] - pub zero_dpu: bool, + #[clap( + long, + help = "Create an instance in the given \"flat\" VPC, for machines without DPUs" + )] + pub flat_vpc_id: Option, #[clap(long, help = "The VPC prefix to assign to a VF")] pub vf_vpc_prefix_id: Vec, diff --git a/crates/admin-cli/src/instance/allocate/cmd.rs b/crates/admin-cli/src/instance/allocate/cmd.rs index fa299b17c8..952ad797c5 100644 --- a/crates/admin-cli/src/instance/allocate/cmd.rs +++ b/crates/admin-cli/src/instance/allocate/cmd.rs @@ -67,7 +67,7 @@ pub async fn allocate( api_client, &mut machine_ids, min_interface_count, - allocate_request.zero_dpu, + allocate_request.flat_vpc_id, ) .await else { @@ -109,7 +109,7 @@ pub async fn allocate( api_client, &mut machine_ids, min_interface_count, - allocate_request.zero_dpu, + allocate_request.flat_vpc_id, ) .await else { diff --git a/crates/admin-cli/src/instance/show/cmd.rs b/crates/admin-cli/src/instance/show/cmd.rs index 56b30ffe83..dd8507ba2e 100644 --- a/crates/admin-cli/src/instance/show/cmd.rs +++ b/crates/admin-cli/src/instance/show/cmd.rs @@ -178,12 +178,14 @@ async fn convert_instance_to_nice_format( let width = 25; writeln!(&mut lines, "INTERFACES:")?; - let if_configs = instance + let network_config = instance .config .as_ref() - .and_then(|config| config.network.as_ref()) + .and_then(|config| config.network.as_ref()); + let if_configs = network_config .map(|config| config.interfaces.as_slice()) .unwrap_or_default(); + let auto_network = network_config.is_some_and(|config| config.auto_config.is_some()); let if_status = instance .status .as_ref() @@ -191,86 +193,110 @@ async fn convert_instance_to_nice_format( .map(|status| status.interfaces.as_slice()) .unwrap_or_default(); - if if_configs.is_empty() || if_status.is_empty() { + if if_status.is_empty() { writeln!(&mut lines, "\tEMPTY")?; - } else if if_configs.len() != if_status.len() { + } else if !auto_network && if_configs.len() != if_status.len() { writeln!(&mut lines, "\tLENGTH MISMATCH")?; } else { - for (i, interface) in if_configs.iter().enumerate() { - let status = &if_status[i]; - - let vpc = if let Some(network_segment_id) = interface.network_segment_id { - get_vpc_for_interface_network_segment(api_client, network_segment_id).await? - } else { - None - }; - - let data: &[(&str, Cow)] = &[ - ( - "FUNCTION_TYPE", - forgerpc::InterfaceFunctionType::try_from(interface.function_type) - .ok() - .map(|ty| format!("{ty:?}").into()) - .unwrap_or_else(|| "INVALID".into()), - ), - ( - "VF ID", - status - .virtual_function_id - .map(|id| id.to_string().into()) - .unwrap_or_default(), - ), - ( - "SEGMENT ID", - interface - .network_segment_id - .unwrap_or_default() - .to_string() + let vpcs: Vec = if auto_network { + futures::future::join_all( + if_status + .iter() + .filter_map(|s| s.vpc_id) + .map(|vpc_id| get_vpc_by_id(api_client, vpc_id)), + ) + .await + .into_iter() + .collect::, _>>()? + .into_iter() + } else { + futures::future::join_all(if_configs.iter().filter_map(|c| c.network_segment_id).map( + |segment_id| async move { + get_vpc_for_interface_network_segment(api_client, segment_id).await + }, + )) + .await + .into_iter() + .collect::, _>>()? + .into_iter() + } + .flatten() + .collect(); + if !auto_network && if_configs.len() != if_status.len() { + writeln!(&mut lines, "\tLENGTH MISMATCH")?; + } else { + for (idx, status) in if_status.iter().enumerate() { + let vpc = vpcs.get(idx); + let if_config = if_configs.get(idx); + let data: &[(&str, Cow)] = &[ + ( + "FUNCTION_TYPE", + format!( + "{:?}", + match status.virtual_function_id { + Some(_) => forgerpc::InterfaceFunctionType::Virtual, + None => forgerpc::InterfaceFunctionType::Physical, + } + ) .into(), - ), - ( - "VPC PREFIX ID", - match &interface.network_details { - Some(forgerpc::instance_interface_config::NetworkDetails::SegmentId(_)) => { - "Segment Based Allocation".into() - } - Some(forgerpc::instance_interface_config::NetworkDetails::VpcPrefixId( - x, - )) => x.to_string().into(), - None => "NA".into(), - }, - ), - ( - "MAC ADDR", - status - .mac_address - .as_ref() - .map(|s| s.as_str().into()) - .unwrap_or_default(), - ), - ("ADDRESSES", status.addresses.as_slice().join(", ").into()), - ( - "VPC ID", - vpc.as_ref() - .map(|v| v.id.unwrap_or_default().to_string().into()) - .unwrap_or("".into()), - ), - ( - "VPC NAME", - vpc.as_ref() - .and_then(|v| v.metadata.as_ref()) - .map(|v| Cow::Borrowed(v.name.as_str())) - .unwrap_or("".into()), - ), - ]; - - for (key, value) in data { - writeln!(&mut lines, "\t{key: "Segment Based Allocation".into(), + Some( + forgerpc::instance_interface_config::NetworkDetails::VpcPrefixId(x), + ) => x.to_string().into(), + None => "NA".into(), + }, + ), + ( + "MAC ADDR", + status + .mac_address + .as_ref() + .map(|s| s.as_str().into()) + .unwrap_or_default(), + ), + ("ADDRESSES", status.addresses.as_slice().join(", ").into()), + ( + "VPC ID", + vpc.map(|v| v.id.unwrap_or_default().to_string().into()) + .unwrap_or("".into()), + ), + ( + "VPC NAME", + vpc.and_then(|v| v.metadata.as_ref()) + .map(|v| Cow::Borrowed(v.name.as_str())) + .unwrap_or("".into()), + ), + ]; + + for (key, value) in data { + writeln!(&mut lines, "\t{key: CarbideCliResu Ok(()) } -#[allow(deprecated)] async fn get_vpc_for_interface_network_segment( api_client: &ApiClient, network_segment_id: NetworkSegmentId, @@ -565,10 +590,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| 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| { + #[allow(deprecated)] + s.config.as_ref().and_then(|c| c.vpc_id).or(s.vpc_id) + }) { let vpc_ids: Vec = vec![vpc_id]; Ok(api_client @@ -582,3 +607,15 @@ async fn get_vpc_for_interface_network_segment( Ok(None) } } + +async fn get_vpc_by_id(api_client: &ApiClient, vpc_id: VpcId) -> CarbideCliResult> { + Ok(api_client + .0 + .find_vpcs_by_ids(VpcsByIdsRequest { + vpc_ids: vec![vpc_id], + }) + .await? + .vpcs + .into_iter() + .next()) +} diff --git a/crates/admin-cli/src/machine/show/cmd.rs b/crates/admin-cli/src/machine/show/cmd.rs index a883f553bd..b17248db8f 100644 --- a/crates/admin-cli/src/machine/show/cmd.rs +++ b/crates/admin-cli/src/machine/show/cmd.rs @@ -21,6 +21,7 @@ use std::fmt::Write; use ::rpc::admin_cli::OutputFormat; use ::rpc::forge as forgerpc; use carbide_uuid::machine::MachineId; +use carbide_uuid::vpc::VpcId; use prettytable::{Table, row}; use rpc::Machine; @@ -428,7 +429,7 @@ pub async fn get_next_free_machine( api_client: &ApiClient, machine_ids: &mut VecDeque, min_interface_count: usize, - zero_dpu: bool, + flat_vpc_id: Option, ) -> Option { while let Some(id) = machine_ids.pop_front() { tracing::debug!("Checking {}", id); @@ -437,7 +438,7 @@ pub async fn get_next_free_machine( tracing::debug!("Machine is not ready"); continue; } - if zero_dpu { + if flat_vpc_id.is_some() { return Some(machine); } if let Some(discovery_info) = &machine.discovery_info { diff --git a/crates/admin-cli/src/rpc.rs b/crates/admin-cli/src/rpc.rs index 3d32954a99..20eba55840 100644 --- a/crates/admin-cli/src/rpc.rs +++ b/crates/admin-cli/src/rpc.rs @@ -1261,7 +1261,42 @@ impl ApiClient { modified_by: Option, ) -> CarbideCliResult { let mut vf_function_id = 0; - let (interface_configs, tenant_org) = if !allocate_instance.subnet.is_empty() { + let (interface_configs, tenant_org, vpc_id) = if let Some(vpc_id) = + allocate_instance.flat_vpc_id + { + if !allocate_instance.vpc_prefix_id.is_empty() + || !allocate_instance.vf_vpc_prefix_id.is_empty() + || !allocate_instance.vf_subnet.is_empty() + { + return Err(CarbideCliError::GenericError( + "--flat-vpc-id does not support explicit VPC prefixes or VF subnets" + .to_string(), + )); + } + + let vpc = self + .0 + .find_vpcs_by_ids(VpcsByIdsRequest { + vpc_ids: vec![vpc_id], + }) + .await? + .vpcs + .into_iter() + .next() + .ok_or_else(|| { + CarbideCliError::GenericError(format!("VPC {vpc_id} was not found")) + })?; + + let VpcVirtualizationType::Flat = vpc.network_virtualization_type() else { + return Err(CarbideCliError::GenericError(format!( + "VPC {} is not a flat VPC, is of type {}", + vpc_id, + vpc.network_virtualization_type().as_str_name() + ))); + }; + + (Vec::new(), vpc.tenant_organization_id, Some(vpc_id)) + } else if !allocate_instance.subnet.is_empty() { if !allocate_instance.vf_vpc_prefix_id.is_empty() { return Err(CarbideCliError::GenericError( "Cannot use vf_vpc_prefix_id with subnet".to_string(), @@ -1299,9 +1334,7 @@ impl ApiClient { .pci_properties .as_ref() .map(|pci| &pci.vendor) - .is_some_and(|v| { - v.to_ascii_lowercase().contains("mellanox") || allocate_instance.zero_dpu - }) + .is_some_and(|v| v.to_ascii_lowercase().contains("mellanox")) }); let mut interface_config = Vec::default(); let mut vf_chunk_iter = vf_network_segment_ids.chunks(vfs_per_pf); @@ -1362,7 +1395,9 @@ impl ApiClient { allocate_instance .tenant_org .as_deref() - .unwrap_or("devenv_test_org"), + .unwrap_or("devenv_test_org") + .to_string(), + None, ) } else if !allocate_instance.vpc_prefix_id.is_empty() { let Some(discovery_info) = &machine.discovery_info else { @@ -1470,11 +1505,12 @@ impl ApiClient { ( interface_configs, - allocate_instance.tenant_org.as_deref().ok_or_else(|| { + allocate_instance.tenant_org.clone().ok_or_else(|| { CarbideCliError::GenericError( "Tenant org is mandatory in case of vpc_prefix.".to_string(), ) })?, + None, ) } else { return Err(CarbideCliError::GenericError( @@ -1482,11 +1518,12 @@ impl ApiClient { )); }; - if interface_configs.len() - != (allocate_instance.subnet.len() - + allocate_instance.vf_subnet.len() - + allocate_instance.vpc_prefix_id.len() - + allocate_instance.vf_vpc_prefix_id.len()) + if allocate_instance.flat_vpc_id.is_none() + && interface_configs.len() + != (allocate_instance.subnet.len() + + allocate_instance.vf_subnet.len() + + allocate_instance.vpc_prefix_id.len() + + allocate_instance.vf_vpc_prefix_id.len()) { return Err(CarbideCliError::GenericError( "Could not create the correct number of interface configs to satisfy request." @@ -1494,7 +1531,7 @@ impl ApiClient { )); } let tenant_config = rpc::TenantConfig { - tenant_organization_id: tenant_org.to_string(), + tenant_organization_id: tenant_org, tenant_keyset_ids: vec![], hostname: None, }; @@ -1503,12 +1540,12 @@ impl ApiClient { tenant: Some(tenant_config), os: allocate_instance.os.clone(), network: Some(rpc::InstanceNetworkConfig { - interfaces: if allocate_instance.zero_dpu { - vec![] - } else { - interface_configs - }, - auto: allocate_instance.zero_dpu, + interfaces: interface_configs, + auto_config: vpc_id.map(|vpc_id| rpc::InstanceNetworkAutoConfig { + vpc_id: Some(vpc_id), + }), + #[allow(deprecated)] + auto: allocate_instance.flat_vpc_id.is_some(), }), network_security_group_id: allocate_instance.network_security_group_id.clone(), infiniband: None, diff --git a/crates/agent/src/tests/full.rs b/crates/agent/src/tests/full.rs index 7225a232ec..973f0f0304 100644 --- a/crates/agent/src/tests/full.rs +++ b/crates/agent/src/tests/full.rs @@ -789,7 +789,9 @@ async fn handle_netconf(AxumState(state): AxumState>>) -> impl ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, network_security_group_id: None, @@ -810,8 +812,9 @@ async fn handle_netconf(AxumState(state): AxumState>>) -> impl addresses: vec!["10.217.104.146".to_string()], gateways: vec!["10.217.104.145/30".to_string()], prefixes: vec!["10.217.104.146/32".to_string()], - device: None, - device_instance: 0u32, + device: None, + device_instance: 0u32, + vpc_id: None, }], configs_synced: rpc::SyncState::Synced.into(), }), diff --git a/crates/api-core/src/cfg/file.rs b/crates/api-core/src/cfg/file.rs index cda2620437..443218ac3b 100644 --- a/crates/api-core/src/cfg/file.rs +++ b/crates/api-core/src/cfg/file.rs @@ -2394,6 +2394,7 @@ mod tests { use model::resource_pool; use super::*; + use crate::test_support::network_segment::FIXTURE_TENANT_ORG_ID; const TEST_DATA_DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/src/cfg/test_data"); @@ -3859,7 +3860,7 @@ firmware_url = "https://firmware.example.com/fw-b.bin" assert_eq!( vpcs.get("zero-dpu-vpc").unwrap(), &VpcDefinition { - organization_id: Some("2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string()), + organization_id: Some(FIXTURE_TENANT_ORG_ID.to_string()), network_virtualization_type: VpcVirtualizationType::Flat, routing_profile_type: None, vni: None, diff --git a/crates/api-core/src/ethernet_virtualization.rs b/crates/api-core/src/ethernet_virtualization.rs index 0c17dadc06..267c3381f0 100644 --- a/crates/api-core/src/ethernet_virtualization.rs +++ b/crates/api-core/src/ethernet_virtualization.rs @@ -149,7 +149,13 @@ pub(crate) async fn validate_instance_interface_routing_profiles( "instance interface routing_profile requires a network segment".to_string(), ) })?; - let vpc = db::vpc::find_by_segment(&mut *txn, segment_id).await?; + let vpc = db::vpc::find_by_segment(&mut *txn, segment_id) + .await? + .ok_or_else(|| { + CarbideError::InvalidArgument( + "network segment is not a member of a VPC".to_string(), + ) + })?; // Interface routing profiles are only valid on FNN VPC interfaces. if vpc.network_virtualization_type != VpcVirtualizationType::Fnn { diff --git a/crates/api-core/src/handlers/dpu.rs b/crates/api-core/src/handlers/dpu.rs index 25550e3ec7..52378bc0d2 100644 --- a/crates/api-core/src/handlers/dpu.rs +++ b/crates/api-core/src/handlers/dpu.rs @@ -255,8 +255,12 @@ pub(crate) async fn get_managed_host_network_config_inner( // network segment is empty, return error. return Err(CarbideError::NetworkSegmentNotAllocated.into()); }; - let vpc = db::vpc::find_by_segment(&mut txn, network_segment_id) - .await?; + let Some(vpc) = db::vpc::find_by_segment(&mut txn, network_segment_id) + .await? else { + return Err(CarbideError::InvalidArgument( + "network segment is not a member of a VPC".to_string(), + ).into()) + }; network_virtualization_type = vpc.network_virtualization_type; diff --git a/crates/api-core/src/handlers/instance.rs b/crates/api-core/src/handlers/instance.rs index 5dfc93fefc..3043e8cf8e 100644 --- a/crates/api-core/src/handlers/instance.rs +++ b/crates/api-core/src/handlers/instance.rs @@ -1363,10 +1363,10 @@ async fn update_instance_network_config( // Auto-ness can't change for an existing instance. If a tenant has created // an instance with auto, it must remain auto until it is released. Maybe // eventually this can change, but for now this is what we support. - if network.auto != instance.config.network.auto { + if network.auto_config != instance.config.network.auto_config { return Err(CarbideError::InvalidArgument(format!( - "cannot change `InstanceNetworkConfig.auto` on an existing instance (was {}, update requested {})", - instance.config.network.auto, network.auto, + "cannot change `InstanceNetworkConfig.auto_config` on an existing instance (was {:?}, update requested {:?})", + instance.config.network.auto_config, network.auto_config, ))); } @@ -1374,7 +1374,16 @@ async fn update_instance_network_config( // HostInband segments before any diff check. Same machine state == no-op // via the diff check below. Operator-added or removed HostInband segments // since allocation == reflected in the update. - if network.auto { + if let Some(requested_auto_config) = network.auto_config { + if let Some(current_auto_config) = instance.config.network.auto_config + && current_auto_config.vpc_id != requested_auto_config.vpc_id + { + return Err(CarbideError::InvalidArgument(format!( + "cannot change `InstanceNetworkConfig.vpc_id` on an existing auto-networked instance (was {}, update requested {})", + current_auto_config.vpc_id, requested_auto_config.vpc_id + ))); + } + // Just to make sure, an auto instance should still be on a zero-DPU // host. If this machine suddenly has DPUs, we should yell, at least // for now. I guess there's a world where we might have a primary NIC diff --git a/crates/api-core/src/instance/mod.rs b/crates/api-core/src/instance/mod.rs index 8c358f6bbb..a6d910fa50 100644 --- a/crates/api-core/src/instance/mod.rs +++ b/crates/api-core/src/instance/mod.rs @@ -26,8 +26,9 @@ use carbide_uuid::instance::InstanceId; use carbide_uuid::instance_type::InstanceTypeId; use carbide_uuid::machine::MachineId; use carbide_uuid::spx::SpxPartitionId; -use carbide_uuid::vpc::VpcPrefixId; +use carbide_uuid::vpc::{VpcId, VpcPrefixId}; use config_version::ConfigVersion; +use db::db_read::DbReader; use db::{ self, ObjectColumnFilter, ObjectFilter, compute_allocation, extension_service, ib_partition, network_security_group, @@ -85,6 +86,44 @@ fn build_requested_linknet_prefix( } use crate::{CarbideError, CarbideResult}; +async fn validate_zero_dpu_auto_vpc( + txn: impl DbReader<'_>, + vpc_id: VpcId, + tenant_organization_id: &TenantOrganizationId, +) -> Result { + let vpc = db::vpc::find_by(txn, ObjectColumnFilter::One(db::vpc::IdColumn, &vpc_id)) + .await? + .into_iter() + .next() + .ok_or_else(|| { + CarbideError::FailedPrecondition(format!("VPC `{vpc_id}` does not exist")) + })?; + + if vpc.tenant_organization_id != tenant_organization_id.to_string() { + return Err(CarbideError::FailedPrecondition(format!( + "VPC `{}` is not owned by Tenant `{}`", + vpc.id, tenant_organization_id + ))); + } + + if vpc.network_virtualization_type != VpcVirtualizationType::Flat { + return Err(CarbideError::FailedPrecondition(format!( + "zero-DPU auto allocation requires a Flat VPC; VPC {} uses {}", + vpc.id, vpc.network_virtualization_type + ))); + } + + let vpc_iface = vpc.network_virtualization_type.fabric_interface_type(); + if vpc_iface != FabricInterfaceType::Nic { + return Err(CarbideError::FailedPrecondition(format!( + "zero-DPU auto allocation requires a VPC whose fabric_interface_type is `nic`; VPC {} ({}) has `{vpc_iface}`", + vpc.id, vpc.network_virtualization_type + ))); + } + + Ok(vpc) +} + /// Validates that an operating system definition referenced by ID exists, is active, /// and has status READY. Returns `Ok(())` when the OS variant is not /// `OperatingSystemId` (inline iPXE / OS image variants need no lookup). @@ -898,12 +937,19 @@ pub async fn batch_allocate_instances( // which one(s) the host is on. Conversely, hosts with DPUs cannot use // `auto`, and are expected to enumerate their interfaces explicitly. if !mh_snapshot.has_managed_dpus() { - if !request.config.network.auto { + let Some(requested_auto_config) = request.config.network.auto_config else { return Err(CarbideError::InvalidArgument(format!( "zero-DPU host {} requires `InstanceNetworkConfig.auto = true`; cannot allocate an instance with explicitly-listed interfaces or with `auto = false`", mh_snapshot.host_snapshot.id, ))); - } + }; + + validate_zero_dpu_auto_vpc( + &mut txn, + requested_auto_config.vpc_id, + &request.config.tenant.tenant_organization_id, + ) + .await?; // ...and eeven though gRPC <-> model validation rejects // auto + non-empty interfaces, double-check here so a future @@ -935,35 +981,31 @@ pub async fn batch_allocate_instances( } } - // Each of the host's HostInband segments must be bound to a - // VPC whose fabric interface type matches a zero-DPU host's - // (i.e. `Nic`). HostInband segments are allowed to exist - // without a VPC at segment-create time (so operators can - // create them up front for DHCP routing during site-explorer - // ingestion); we require the binding here, when a tenant - // intent actually shows up to allocate an instance. + // HostInband segments may be unbound so multiple Flat VPCs can + // share the same physical segment. If a segment is still bound, + // it must not conflict with the VPC requested for this allocation. for segment_id in &allowed_segment_ids { - let vpc = db::vpc::find_by_segment(&mut txn, *segment_id) - .await - .map_err(|e| { - if e.is_not_found() { - CarbideError::FailedPrecondition(format!( - "zero-DPU host {} has HostInband segment {} that is not bound to a Flat VPC; instance allocation requires the segment to be in a Flat VPC", - mh_snapshot.host_snapshot.id, segment_id, - )) - } else { - CarbideError::from(e) - } - })?; - let vpc_iface = vpc.network_virtualization_type.fabric_interface_type(); - if vpc_iface != FabricInterfaceType::Nic { - return Err(CarbideError::FailedPrecondition(format!( - "zero-DPU host {} has HostInband segment {} bound to VPC {} ({}); zero-DPU hosts can only allocate into VPCs whose fabric_interface_type is `nic` (got `{vpc_iface}`)", - mh_snapshot.host_snapshot.id, - segment_id, - vpc.id, - vpc.network_virtualization_type, - ))); + if let Some(vpc) = db::vpc::find_by_segment(&mut txn, *segment_id).await? { + if vpc.id != requested_auto_config.vpc_id { + return Err(CarbideError::FailedPrecondition(format!( + "zero-DPU host {} has HostInband segment {} bound to VPC {}, but allocation requested VPC {}; shared Flat segments must be left unbound", + mh_snapshot.host_snapshot.id, + segment_id, + vpc.id, + requested_auto_config.vpc_id, + ))); + } + + let vpc_iface = vpc.network_virtualization_type.fabric_interface_type(); + if vpc_iface != FabricInterfaceType::Nic { + return Err(CarbideError::FailedPrecondition(format!( + "zero-DPU host {} has HostInband segment {} bound to VPC {} ({}); zero-DPU hosts can only allocate into VPCs whose fabric_interface_type is `nic` (got `{vpc_iface}`)", + mh_snapshot.host_snapshot.id, + segment_id, + vpc.id, + vpc.network_virtualization_type, + ))); + } } } @@ -979,7 +1021,7 @@ pub async fn batch_allocate_instances( } else { // `auto` is only valid on zero-DPU hosts; DPU-managed hosts must // list their interfaces explicitly. - if request.config.network.auto { + if request.config.network.auto_config.is_some() { return Err(CarbideError::InvalidArgument(format!( "host {} has DPUs; `InstanceNetworkConfig.auto` is only valid on zero-DPU hosts", mh_snapshot.host_snapshot.id, @@ -995,18 +1037,28 @@ pub async fn batch_allocate_instances( // rather than getting stuck somewhere downstream. for iface in &request.config.network.interfaces { if let Some(ns_id) = iface.network_segment_id { - let vpc = db::vpc::find_by_segment(&mut txn, ns_id) + match db::vpc::find_by_segment(&mut txn, ns_id) .await - .map_err(CarbideError::from)?; - let vpc_iface = vpc.network_virtualization_type.fabric_interface_type(); - if vpc_iface != FabricInterfaceType::Dpu { - return Err(CarbideError::FailedPrecondition(format!( - "DPU-managed host {} cannot allocate an instance into VPC {} ({}, via segment {}); DPU hosts can only allocate into VPCs whose fabric_interface_type is `dpu` (got `{vpc_iface}`)", - mh_snapshot.host_snapshot.id, - vpc.id, - vpc.network_virtualization_type, - ns_id, - ))); + .map_err(CarbideError::from)? + { + Some(vpc) => { + let vpc_iface = vpc.network_virtualization_type.fabric_interface_type(); + if vpc_iface != FabricInterfaceType::Dpu { + return Err(CarbideError::FailedPrecondition(format!( + "DPU-managed host {} cannot allocate an instance into VPC {} ({}, via segment {}); DPU hosts can only allocate into VPCs whose fabric_interface_type is `dpu` (got `{vpc_iface}`)", + mh_snapshot.host_snapshot.id, + vpc.id, + vpc.network_virtualization_type, + ns_id, + ))); + } + } + None => { + return Err(CarbideError::FailedPrecondition(format!( + "DPU-managed host {} cannot allocate an instance into network segment {}; DPU allocations require the segment to be attached to a VPC", + mh_snapshot.host_snapshot.id, ns_id, + ))); + } } } } diff --git a/crates/api-core/src/setup.rs b/crates/api-core/src/setup.rs index 474a926c2f..7f94a5fd0b 100644 --- a/crates/api-core/src/setup.rs +++ b/crates/api-core/src/setup.rs @@ -1528,6 +1528,7 @@ mod tests { use super::*; use crate::cfg::file::{CarbideConfig, InitialObjectsConfig}; + use crate::test_support::network_segment::FIXTURE_TENANT_ORG_ID; fn carbide_with_networks( networks: Option>, @@ -1981,7 +1982,7 @@ mod tests { fn initial_objects_vpcs_only_succeeds() { let cfg = carbide_with_vpcs(None); let def = vpc_definition( - Some("2829bbe3-c169-4cd9-8b2a-19a8b1618a93"), + Some(FIXTURE_TENANT_ORG_ID), VpcVirtualizationType::Flat, None, ); @@ -2012,7 +2013,7 @@ mod tests { fn disjoint_union_returns_all_vpcs() { let legacy_def = vpc_definition(None, VpcVirtualizationType::EthernetVirtualizer, None); let initial_def = vpc_definition( - Some("2829bbe3-c169-4cd9-8b2a-19a8b1618a93"), + Some(FIXTURE_TENANT_ORG_ID), VpcVirtualizationType::Flat, None, ); diff --git a/crates/api-core/src/test_support/network_segment.rs b/crates/api-core/src/test_support/network_segment.rs index c0989a6e5c..b58ea6ea4a 100644 --- a/crates/api-core/src/test_support/network_segment.rs +++ b/crates/api-core/src/test_support/network_segment.rs @@ -29,6 +29,8 @@ use lazy_static::lazy_static; use crate::api::Api; +pub const FIXTURE_TENANT_ORG_ID: &str = "2829bbe3-c169-4cd9-8b2a-19a8b1618a93"; + lazy_static! { pub static ref FIXTURE_UNDERLAY_NETWORK_SEGMENT_GATEWAY: IpNetwork = IpNetwork::new(IpAddr::V4(Ipv4Addr::new(192, 0, 1, 1)), 24).unwrap(); @@ -139,14 +141,6 @@ pub async fn create_host_inband_network_segment( .to_string(); let gateway = FIXTURE_HOST_INBAND_NETWORK_SEGMENT_GATEWAY.ip().to_string(); - // HostInband segments must live in Flat VPCs. If the caller did not - // supply a VPC, create a Flat VPC here so the fixture mirrors the - // production binding rather than landing in the default ETV VPC. - let vpc_id = match vpc_id { - Some(id) => Some(id), - None => Some(create_default_flat_vpc(api, "FIXTURE_HOST_INBAND_FLAT").await), - }; - create_network_segment( api, "HOST_INBAND", @@ -159,10 +153,11 @@ pub async fn create_host_inband_network_segment( .await } -/// Creates a Flat VPC for the default test tenant and returns its id. Used as -/// the implicit parent VPC for HostInband segment fixtures. +/// Creates a Flat VPC for the default test tenant and returns its id. Pass the +/// returned id to `create_host_inband_network_segment` when a test needs an +/// explicitly VPC-bound HostInband segment. pub async fn create_default_flat_vpc(api: &Api, name: &str) -> VpcId { - let request = rpc::forge::VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + let request = rpc::forge::VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(rpc::forge::Metadata { name: name.to_string(), ..Default::default() diff --git a/crates/api-core/src/tests/client_resolution.rs b/crates/api-core/src/tests/client_resolution.rs index 9f6460e885..fa6191b27d 100644 --- a/crates/api-core/src/tests/client_resolution.rs +++ b/crates/api-core/src/tests/client_resolution.rs @@ -28,6 +28,7 @@ use tonic::IntoRequest; use crate::CarbideError; use crate::handlers::client_resolution::resolve_machine_interface; use crate::test_support::fixture_config::ManagedHostConfigExt as _; +use crate::test_support::network_segment::{FIXTURE_TENANT_ORG_ID, create_default_flat_vpc}; use crate::tests::common; use crate::tests::common::api_fixtures::instance::{ default_os_config, default_tenant_config, single_interface_network_config, @@ -156,6 +157,7 @@ async fn test_zero_dpu_cloud_init_prefers_instance_when_ip_matches_host_interfac ) .await; create_host_inband_network_segment(&env.api, None).await; + let vpc_id = create_default_flat_vpc(&env.api, "flat-vpc").await; env.run_network_segment_controller_iteration().await; env.run_network_segment_controller_iteration().await; @@ -179,14 +181,22 @@ async fn test_zero_dpu_cloud_init_prefers_instance_when_ip_matches_host_interfac machine_id: Some(mh.host().id), instance_type_id: None, config: Some(rpc::InstanceConfig { - tenant: Some(default_tenant_config()), + tenant: Some(rpc::TenantConfig { + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), + tenant_keyset_ids: vec![], + hostname: None, + }), os: Some(rpc::forge::InstanceOperatingSystemConfig { user_data: Some(tenant_user_data.to_string()), ..default_os_config() }), network: Some(rpc::forge::InstanceNetworkConfig { interfaces: vec![], + #[allow(deprecated)] auto: true, + auto_config: Some(rpc::forge::InstanceNetworkAutoConfig { + vpc_id: Some(vpc_id), + }), }), infiniband: None, network_security_group_id: None, diff --git a/crates/api-core/src/tests/common/api_fixtures/instance.rs b/crates/api-core/src/tests/common/api_fixtures/instance.rs index 9198f80952..5e96cb592b 100644 --- a/crates/api-core/src/tests/common/api_fixtures/instance.rs +++ b/crates/api-core/src/tests/common/api_fixtures/instance.rs @@ -207,7 +207,9 @@ pub fn single_interface_network_config(segment_id: NetworkSegmentId) -> rpc::Ins ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, } } @@ -244,7 +246,9 @@ pub fn single_interface_network_config_with_vfs( rpc::InstanceNetworkConfig { interfaces, + #[allow(deprecated)] auto: false, + auto_config: None, } } @@ -269,7 +273,9 @@ pub fn interface_network_config_with_devices( .collect(); rpc::InstanceNetworkConfig { interfaces, + #[allow(deprecated)] auto: false, + auto_config: None, } } @@ -288,7 +294,9 @@ pub fn single_interface_network_config_with_vpc_prefix( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, } } diff --git a/crates/api-core/src/tests/common/api_fixtures/mod.rs b/crates/api-core/src/tests/common/api_fixtures/mod.rs index 0beb06c0bb..3e459ce91f 100644 --- a/crates/api-core/src/tests/common/api_fixtures/mod.rs +++ b/crates/api-core/src/tests/common/api_fixtures/mod.rs @@ -140,7 +140,7 @@ use crate::test_support::ib_fabric::ib_fabric_test_manager; pub use crate::test_support::network::{FIXTURE_DHCP_RELAY_ADDRESS, TEST_SITE_PREFIXES}; pub use crate::test_support::network_segment; use crate::test_support::network_segment::{ - FIXTURE_TENANT_NETWORK_SEGMENT_GATEWAYS, create_admin_network_segment, + FIXTURE_TENANT_NETWORK_SEGMENT_GATEWAYS, FIXTURE_TENANT_ORG_ID, create_admin_network_segment, create_static_assignments_segment, create_tenant_network_segment, create_underlay_network_segment, }; @@ -843,7 +843,7 @@ impl TestEnv { NetworkSegmentId, ) { self.create_vpc_and_peer_vpc_with_tenant_segments_for_tenants( - "2829bbe3-c169-4cd9-8b2a-19a8b1618a93", + FIXTURE_TENANT_ORG_ID, vtype1, "e65a9d69-39d2-4872-a53e-e5cb87c84e75", vtype2, @@ -932,7 +932,7 @@ impl TestEnv { pub async fn create_vpc_and_tenant_segment(&self) -> NetworkSegmentId { self.create_vpc_and_tenant_segment_with_vpc_details( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "test vpc 1".to_string(), ..Default::default() @@ -947,7 +947,7 @@ impl TestEnv { segment_count: usize, ) -> Vec { self.create_vpc_and_tenant_segments_with_vpc_details( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "test vpc 1".to_string(), ..Default::default() @@ -962,7 +962,7 @@ impl TestEnv { let vpc = self .api .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "test vpc 1".to_string(), ..Default::default() diff --git a/crates/api-core/src/tests/common/api_fixtures/test_managed_host.rs b/crates/api-core/src/tests/common/api_fixtures/test_managed_host.rs index d8952fc839..0977661064 100644 --- a/crates/api-core/src/tests/common/api_fixtures/test_managed_host.rs +++ b/crates/api-core/src/tests/common/api_fixtures/test_managed_host.rs @@ -52,6 +52,18 @@ impl From for (MachineId, MachineId) { type Txn<'a> = sqlx::Transaction<'a, sqlx::Postgres>; impl TestManagedHost { + pub fn from_rpc_machine(m: &rpc::Machine, api: Arc) -> Self { + TestManagedHost { + id: m.id.unwrap(), + dpu_ids: m + .interfaces + .iter() + .filter_map(|i| i.attached_dpu_machine_id) + .collect(), + api, + } + } + pub fn dpu(&self) -> TestMachine { TestMachine::new(self.dpu_ids[0], self.api.clone()) } diff --git a/crates/api-core/src/tests/common/network_segment.rs b/crates/api-core/src/tests/common/network_segment.rs index 838045bfca..1bcae5fc84 100644 --- a/crates/api-core/src/tests/common/network_segment.rs +++ b/crates/api-core/src/tests/common/network_segment.rs @@ -25,6 +25,7 @@ use tonic::Request; use super::api_fixtures::TestEnv; use crate::api::Api; +use crate::test_support::network_segment::FIXTURE_TENANT_ORG_ID; use crate::tests::common::rpc_builder::VpcCreationRequest; pub struct NetworkSegmentHelper { @@ -71,9 +72,7 @@ pub async fn create_network_segment_with_api( ) -> rpc::forge::NetworkSegment { let vpc_id = if use_vpc { env.api - .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93").tonic_request(), - ) + .create_vpc(VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID).tonic_request()) .await .unwrap() .into_inner() diff --git a/crates/api-core/src/tests/instance.rs b/crates/api-core/src/tests/instance.rs index 2f924110e2..97090d3fea 100644 --- a/crates/api-core/src/tests/instance.rs +++ b/crates/api-core/src/tests/instance.rs @@ -47,6 +47,7 @@ use db::instance_address::UsedOverlayNetworkIpResolver; use db::ip_allocator::UsedIpResolver; use db::network_segment::IdColumn; use db::{self, ObjectColumnFilter}; +use futures_util::future::join_all; use ipnetwork::{IpNetwork, Ipv4Network, Ipv6Network}; use itertools::Itertools; use mac_address::MacAddress; @@ -83,6 +84,7 @@ use crate::cfg::file::VmaasConfig; use crate::instance::{allocate_instance, allocate_network}; use crate::network_segment::allocate::PrefixAllocator; use crate::test_support::fixture_config::FixtureDefault as _; +use crate::test_support::network_segment::FIXTURE_TENANT_ORG_ID; use crate::tests::common; use crate::tests::common::api_fixtures::instance::{ advance_created_instance_into_state, single_interface_network_config_with_vfs, @@ -161,6 +163,20 @@ async fn test_allocate_and_release_instance_impl( let pool = PgPoolOptions::new().connect_with(options).await.unwrap(); let env = create_test_env(pool).await; let segment_ids = env.create_vpc_and_tenant_segments(dpu_count).await; + + let vpc_ids: Vec = join_all(segment_ids.iter().map(|id| { + let pool = env.pool.clone(); + async move { + db::vpc::find_by_segment(&pool, *id) + .await + .map(|vpc| vpc.expect("missing VPC for created segment").id) + } + })) + .await + .into_iter() + .collect::>() + .unwrap(); + let mh = create_managed_host_multi_dpu(&env, dpu_count).await; let (used_dpu_ids, _unused_dpu_ids) = mh.dpu_ids.split_at(instance_interface_count); @@ -243,7 +259,7 @@ async fn test_allocate_and_release_instance_impl( } assert_eq!( network_config_no_addresses, - InstanceNetworkConfig::for_segment_ids(&segment_ids, &device_locators,) + InstanceNetworkConfig::for_segment_ids(&segment_ids, &device_locators, &vpc_ids) ); assert!(!fetched_instance.observations.network.is_empty()); @@ -336,6 +352,11 @@ async fn test_measurement_assigned_ready_to_waiting_for_measurements_to_ca_faile let env = create_test_env_with_overrides(pool, overrides).await; let segment_id = env.create_vpc_and_tenant_segment().await; + let vpc_id = db::vpc::find_by_segment(&env.pool, segment_id) + .await + .unwrap() + .unwrap() + .id; // add CA cert to pass attestation process let add_ca_request = tonic::Request::new(TpmCaCert { ca_cert: CA_CERT_SERIALIZED.to_vec(), @@ -470,7 +491,7 @@ async fn test_measurement_assigned_ready_to_waiting_for_measurements_to_ca_faile } assert_eq!( network_config_no_addresses, - InstanceNetworkConfig::for_segment_ids(&[segment_id], &[device_locator],) + InstanceNetworkConfig::for_segment_ids(&[segment_id], &[device_locator], &[vpc_id]) ); assert!(!fetched_instance.observations.network.is_empty()); @@ -1009,7 +1030,9 @@ async fn test_instance_dns_resolution(_: PgPoolOptions, options: PgConnectOption routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; // Create instance with hostname @@ -1464,6 +1487,11 @@ async fn test_instance_network_status_sync(_: PgPoolOptions, options: PgConnectO let pool = PgPoolOptions::new().connect_with(options).await.unwrap(); let env = create_test_env(pool).await; let segment_id = env.create_vpc_and_tenant_segment().await; + let vpc_id = db::vpc::find_by_segment(&env.pool, segment_id) + .await + .unwrap() + .unwrap() + .id; let mh = create_managed_host(&env).await; // TODO: The test is broken from here. This method already moves the instance @@ -1549,6 +1577,7 @@ async fn test_instance_network_status_sync(_: PgPoolOptions, options: PgConnectO gateways: vec![pf_gw.clone()], device: None, device_instance: 0u32, + vpc_id: Some(vpc_id), }] ); @@ -1584,6 +1613,7 @@ async fn test_instance_network_status_sync(_: PgPoolOptions, options: PgConnectO gateways: vec![pf_gw.clone()], device: None, device_instance: 0u32, + vpc_id: Some(vpc_id), }] ); @@ -1625,6 +1655,7 @@ async fn test_instance_network_status_sync(_: PgPoolOptions, options: PgConnectO gateways: vec![], device: None, device_instance: 0u32, + vpc_id: Some(vpc_id), }] ); @@ -1677,6 +1708,7 @@ async fn test_instance_network_status_sync(_: PgPoolOptions, options: PgConnectO gateways: vec![pf_gw.clone()], device: None, device_instance: 0u32, + vpc_id: Some(vpc_id), }] ); @@ -1714,6 +1746,7 @@ async fn test_instance_network_status_sync(_: PgPoolOptions, options: PgConnectO gateways: vec![], device: None, device_instance: 0u32, + vpc_id: Some(vpc_id), }] ); @@ -1725,6 +1758,11 @@ async fn test_can_not_create_instance_for_dpu(_: PgPoolOptions, options: PgConne let pool = PgPoolOptions::new().connect_with(options).await.unwrap(); let env = create_test_env(pool).await; let segment_id = env.create_vpc_and_tenant_segment().await; + let vpc_id = db::vpc::find_by_segment(&env.pool, segment_id) + .await + .unwrap() + .unwrap() + .id; let host_config = env.managed_host_config(); let dpu_machine_id = dpu::create_dpu_machine(&env, &host_config).await; let request = crate::instance::InstanceAllocationRequest { @@ -1734,7 +1772,7 @@ async fn test_can_not_create_instance_for_dpu(_: PgPoolOptions, options: PgConne config: model::instance::config::InstanceConfig { os: default_os_config().try_into().unwrap(), tenant: default_tenant_config().try_into().unwrap(), - network: InstanceNetworkConfig::for_segment_ids(&[segment_id], &Vec::default()), + network: InstanceNetworkConfig::for_segment_ids(&[segment_id], &[], &[vpc_id]), infiniband: InstanceInfinibandConfig::default(), nvlink: InstanceNvLinkConfig::default(), spxconfig: InstanceSpxConfig::default(), @@ -1808,7 +1846,9 @@ async fn test_instance_address_creation(_: PgPoolOptions, options: PgConnectOpti routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; let tinstance = mh.instance_builer(&env).network(network).build().await; @@ -2297,6 +2337,11 @@ async fn test_allocate_instance_with_old_network_segemnt( let pool = PgPoolOptions::new().connect_with(options).await.unwrap(); let env = create_test_env(pool).await; let segment_id = env.create_vpc_and_tenant_segment().await; + let vpc_id = db::vpc::find_by_segment(&env.pool, segment_id) + .await + .unwrap() + .unwrap() + .id; let mh = create_managed_host(&env).await; let txn = env @@ -2362,7 +2407,7 @@ async fn test_allocate_instance_with_old_network_segemnt( assert_eq!( network_config_no_addresses, - InstanceNetworkConfig::for_segment_ids(&[segment_id], &[device_locator],) + InstanceNetworkConfig::for_segment_ids(&[segment_id], &[device_locator], &[vpc_id]) ); } @@ -2394,7 +2439,9 @@ async fn test_allocate_network_vpc_prefix_id(_: PgPoolOptions, options: PgConnec ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let config = rpc::InstanceConfig { @@ -2591,7 +2638,7 @@ async fn test_allocate_and_release_instance_vpc_prefix_id( } assert_eq!( network_config_no_addresses, - InstanceNetworkConfig::for_vpc_prefix_id(vpc_prefix_id, Some(mh.dpu().id)) + InstanceNetworkConfig::for_vpc_prefix_id(vpc_prefix_id, Some(vpc.id)) ); assert!(!fetched_instance.observations.network.is_empty()); @@ -2735,7 +2782,7 @@ async fn test_vpc_prefix_handling(pool: PgPool) { let vpc = env .api .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "test vpc 1".to_string(), ..Default::default() @@ -2980,7 +3027,9 @@ fn dual_physical_network_config_with_vpc_prefixes( rpc::InstanceNetworkConfig { interfaces, + #[allow(deprecated)] auto: false, + auto_config: None, } } @@ -3364,7 +3413,9 @@ async fn test_network_details_migration( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }) .rpc(), ) @@ -3452,7 +3503,9 @@ async fn test_network_details_migration( device_instance: 0, virtual_function_id: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, nvlink: None, @@ -3537,7 +3590,9 @@ async fn test_network_details_migration( device_instance: 0, virtual_function_id: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, nvlink: None, @@ -3691,7 +3746,9 @@ async fn test_instance_cannot_allocate_requested_ip_with_network_segment( device_instance: 0, virtual_function_id: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, network_security_group_id: None, @@ -3767,7 +3824,9 @@ async fn test_allocate_and_update_network_config_instance( device_instance: 0, virtual_function_id: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; // Now update to change network config. @@ -3903,7 +3962,9 @@ async fn test_allocate_and_update_network_config_instance_add_vf( routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; // Now update to change network config. @@ -4079,7 +4140,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_delete_vf( routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; let initial_config = rpc::InstanceConfig { @@ -4176,7 +4239,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_delete_vf( routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; let mut updated_config_1 = initial_config.clone(); updated_config_1.network = Some(network); @@ -4316,7 +4381,9 @@ async fn test_allocate_and_update_network_config_instance_state_machine( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; // Now update to change network config. @@ -4452,7 +4519,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_state_machine( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let initial_config = rpc::InstanceConfig { @@ -4517,7 +4586,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_state_machine( routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; let mut updated_config_1 = initial_config.clone(); updated_config_1.network = Some(network); @@ -4671,7 +4742,9 @@ async fn test_allocate_network_multi_dpu_vpc_prefix_id( routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; let config = rpc::InstanceConfig { @@ -4752,7 +4825,7 @@ async fn test_allocate_instance_with_multiple_fnn_vpc_prefixes( let first_vpc = env .api .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "fnn vpc 1".to_string(), ..Default::default() @@ -4768,7 +4841,7 @@ async fn test_allocate_instance_with_multiple_fnn_vpc_prefixes( let second_vpc = env .api .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "fnn vpc 2".to_string(), ..Default::default() @@ -4870,10 +4943,7 @@ async fn test_fnn_vrf_loopbacks_are_per_vpc_and_removed_on_network_update(pool: // Create FNN tenants matching the existing peer-VPC fixture organizations. for (organization_id, name) in [ - ( - "2829bbe3-c169-4cd9-8b2a-19a8b1618a93", - "fnn loopback tenant 1", - ), + (FIXTURE_TENANT_ORG_ID, "fnn loopback tenant 1"), ( "e65a9d69-39d2-4872-a53e-e5cb87c84e75", "fnn loopback tenant 2", @@ -5011,10 +5081,7 @@ async fn test_fnn_vrf_loopbacks_are_per_vpc_for_pf_and_vf_on_one_dpu(pool: sqlx: // Create FNN tenants matching the existing peer-VPC fixture organizations. for (organization_id, name) in [ - ( - "2829bbe3-c169-4cd9-8b2a-19a8b1618a93", - "fnn vf loopback tenant 1", - ), + (FIXTURE_TENANT_ORG_ID, "fnn vf loopback tenant 1"), ( "e65a9d69-39d2-4872-a53e-e5cb87c84e75", "fnn vf loopback tenant 2", @@ -5191,7 +5258,7 @@ async fn test_allocate_instance_rejects_dual_stack_prefixes_from_different_vpcs( let first_vpc = env .api .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "dual-stack fnn vpc 1".to_string(), ..Default::default() @@ -5207,7 +5274,7 @@ async fn test_allocate_instance_rejects_dual_stack_prefixes_from_different_vpcs( let second_vpc = env .api .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "dual-stack fnn vpc 2".to_string(), ..Default::default() @@ -5263,7 +5330,9 @@ async fn test_allocate_instance_rejects_dual_stack_prefixes_from_different_vpcs( }), routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }, )) .tonic_request(), diff --git a/crates/api-core/src/tests/instance_allocate.rs b/crates/api-core/src/tests/instance_allocate.rs index b0c3bc3b99..8d6d74f97f 100644 --- a/crates/api-core/src/tests/instance_allocate.rs +++ b/crates/api-core/src/tests/instance_allocate.rs @@ -19,6 +19,7 @@ use std::ops::DerefMut; use ::rpc::forge::ManagedHostNetworkConfigRequest; use carbide_redfish::libredfish::test_support::RedfishSimAction; +use carbide_uuid::vpc::VpcId; use forge::forge_server::Forge; use ipnetwork::IpNetwork; use itertools::Itertools; @@ -29,6 +30,7 @@ use rpc::{Metadata, forge}; use crate::cfg::file::{FnnConfig, FnnRoutingProfileConfig, PrefixFilterPolicyEntry}; use crate::test_support::fixture_config::{FixtureDefault as _, ManagedHostConfigExt as _}; use crate::test_support::mac_address_pool::HOST_NON_DPU_MAC_ADDRESS_POOL; +use crate::test_support::network_segment::{FIXTURE_TENANT_ORG_ID, create_default_flat_vpc}; use crate::tests::common; use crate::tests::common::api_fixtures; use crate::tests::common::api_fixtures::network_segment::{ @@ -103,7 +105,7 @@ async fn create_test_env_for_instance_allocation( let vpc_1 = env .api .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "test vpc 1".to_string(), ..Default::default() @@ -117,7 +119,7 @@ async fn create_test_env_for_instance_allocation( let vpc_2 = env .api .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "test vpc 2".to_string(), ..Default::default() @@ -128,9 +130,10 @@ async fn create_test_env_for_instance_allocation( .unwrap() .into_inner(); - // HostInband segments now require Flat VPCs. Create two so that the - // "different VPCs" test variant can put each HostInband segment in a - // distinct Flat VPC. + // Create Flat VPCs for zero-DPU allocation. In the normal path the + // HostInband segments are unbound and the instance address carries the + // logical VPC. The "different VPCs" variant deliberately binds segments + // to conflicting VPCs so allocation is rejected. let flat_vpc_1_id = common::api_fixtures::network_segment::create_default_flat_vpc(&env.api, "test flat vpc 1") .await; @@ -159,9 +162,15 @@ async fn create_test_env_for_instance_allocation( ) .await; - create_host_inband_network_segment(&env.api, Some(flat_vpc_1_id)).await; - // Second HostInband segment lives in the same Flat VPC, or a different - // Flat VPC if the test wants to assert allocation rejection. + create_host_inband_network_segment( + &env.api, + options + .host_inband_segments_in_different_vpcs + .then_some(flat_vpc_1_id), + ) + .await; + // Second HostInband segment is normally unbound too, or bound to a + // different Flat VPC if the test wants to assert allocation rejection. create_network_segment( &env.api, "HOST_INBAND_2", @@ -174,11 +183,9 @@ async fn create_test_env_for_instance_allocation( .ip() .to_string(), forge::NetworkSegmentType::HostInband, - Some(if options.host_inband_segments_in_different_vpcs { - flat_vpc_2_id - } else { - flat_vpc_1_id - }), + options + .host_inband_segments_in_different_vpcs + .then_some(flat_vpc_2_id), true, ) .await; @@ -190,6 +197,17 @@ async fn create_test_env_for_instance_allocation( env } +async fn vpc_id_by_name(env: &TestEnv, name: &str) -> VpcId { + let mut txn = env.db_txn().await; + let vpcs = db::vpc::find_by_name(txn.as_mut(), name).await.unwrap(); + assert_eq!(vpcs.len(), 1, "expected exactly one VPC named {name}"); + vpcs[0].id +} + +async fn default_flat_vpc_id(env: &TestEnv) -> VpcId { + vpc_id_by_name(env, "test flat vpc 1").await +} + #[crate::sqlx_test] async fn test_allocate_instance_rejects_interface_anycast_prefix_outside_vpc_profile( pool: sqlx::PgPool, @@ -316,7 +334,7 @@ async fn test_zero_dpu_instance_allocation_rejects_explicit_interfaces( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), // from sql fixture + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -343,7 +361,9 @@ async fn test_zero_dpu_instance_allocation_rejects_explicit_interfaces( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, dpu_extension_services: None, @@ -370,8 +390,8 @@ async fn test_zero_dpu_instance_allocation_rejects_explicit_interfaces( /// The `auto: true` path: a zero-DPU host with one HostInband segment, allocated /// with empty interfaces and `auto: true`. NICo resolves the segment from the /// host snapshot and stores the resolved interface internally. What the caller -/// sees on the wire is stripped back to `{ auto: true, interfaces: [] }`, while -/// `instance.status.network.interfaces` reflects the resolved details. +/// sees on the wire is stripped back to `{ auto: true, vpc_id, interfaces: [] }`, +/// while `instance.status.network.interfaces` reflects the resolved details. #[crate::sqlx_test] async fn test_zero_dpu_instance_allocation_auto( pool: sqlx::PgPool, @@ -381,6 +401,7 @@ async fn test_zero_dpu_instance_allocation_auto( // Ingest zero DPU host let zero_dpu_host = api_fixtures::site_explorer::new_host(&env, config).await?; + let flat_vpc_id = default_flat_vpc_id(&env).await; let host_inband_segment = db::network_segment::find_by_name(env.pool.begin().await?.deref_mut(), "HOST_INBAND") @@ -393,7 +414,7 @@ async fn test_zero_dpu_instance_allocation_auto( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -410,7 +431,11 @@ async fn test_zero_dpu_instance_allocation_auto( }), network: Some(forge::InstanceNetworkConfig { interfaces: vec![], + #[allow(deprecated)] auto: true, + auto_config: Some(forge::InstanceNetworkAutoConfig { + vpc_id: Some(flat_vpc_id), + }), }), infiniband: None, dpu_extension_services: None, @@ -425,6 +450,7 @@ async fn test_zero_dpu_instance_allocation_auto( .await .expect("zero-DPU instance allocation with auto: true should succeed") .into_inner(); + let instance_id = instance.id.expect("allocated instance should have an id"); // Make sure getting the Machine over RPC has the correct instance network restrictions. While // not strictly testing instance allocation, it's very related, because cloud-api will be using @@ -462,9 +488,12 @@ async fn test_zero_dpu_instance_allocation_auto( // interface lives in status, not config, which takes place as // part of `into_external_view()`. let network = instance.config.unwrap().network.unwrap(); - assert!( - network.auto, - "auto must round-trip back to the caller as true" + #[allow(deprecated)] + let auto = network.auto; + assert!(auto, "auto must round-trip back to the caller as true"); + assert_eq!( + network.auto_config.as_ref().unwrap().vpc_id, + Some(flat_vpc_id) ); assert!( network.interfaces.is_empty(), @@ -478,6 +507,17 @@ async fn test_zero_dpu_instance_allocation_auto( 1, "status should reflect one resolved interface for the single HostInband segment" ); + assert_eq!(status_interfaces[0].vpc_id, Some(flat_vpc_id)); + + let mut txn = env.db_txn().await; + let address = db::instance_address::find_by_instance_id_and_segment_id( + txn.as_mut(), + &instance_id, + &host_inband_segment.id, + ) + .await? + .expect("zero-DPU allocation should persist an instance address"); + assert_eq!(address.vpc_id, Some(flat_vpc_id)); Ok(()) } @@ -501,7 +541,7 @@ async fn test_zero_dpu_instance_allocation_rejects_missing_auto( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -535,6 +575,130 @@ async fn test_zero_dpu_instance_allocation_rejects_missing_auto( Ok(()) } +/// `auto: true` on a zero-DPU host also needs a logical Flat VPC ID. The +/// HostInband segment may be unbound, so the instance address needs this +/// request-level VPC to preserve tenant intent. +#[crate::sqlx_test] +async fn test_zero_dpu_instance_allocation_rejects_missing_vpc_id( + pool: sqlx::PgPool, +) -> Result<(), Box> { + let env = create_test_env_for_instance_allocation(pool.clone(), None).await; + let config = ManagedHostConfig::zero_dpu(); + + let zero_dpu_host = api_fixtures::site_explorer::new_host(&env, config).await?; + + let result = crate::handlers::instance::allocate( + env.api.as_ref(), + tonic::Request::new(forge::InstanceAllocationRequest { + machine_id: Some(zero_dpu_host.host_snapshot.id), + instance_type_id: None, + config: Some(forge::InstanceConfig { + tenant: Some(forge::TenantConfig { + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), + hostname: None, + tenant_keyset_ids: vec![], + }), + os: Some(forge::InstanceOperatingSystemConfig { + phone_home_enabled: false, + run_provisioning_instructions_on_every_boot: false, + user_data: None, + variant: Some(forge::instance_operating_system_config::Variant::Ipxe( + forge::InlineIpxe { + ipxe_script: "exit".to_string(), + }, + )), + }), + network: Some(forge::InstanceNetworkConfig { + interfaces: vec![], + #[allow(deprecated)] + auto: true, + auto_config: Some(forge::InstanceNetworkAutoConfig { vpc_id: None }), + }), + infiniband: None, + nvlink: None, + spxconfig: None, + network_security_group_id: None, + dpu_extension_services: None, + }), + instance_id: None, + metadata: None, + allow_unhealthy_machine: false, + }), + ) + .await; + + let err = result.expect_err("zero-DPU auto allocation without VPC ID must be rejected"); + assert_eq!(err.code(), tonic::Code::InvalidArgument, "got: {err}"); + assert!( + err.message().contains("vpc_id"), + "error should mention vpc_id, got: {}", + err.message() + ); + Ok(()) +} + +#[crate::sqlx_test] +async fn test_zero_dpu_instance_allocation_rejects_non_flat_vpc_id( + pool: sqlx::PgPool, +) -> Result<(), Box> { + let env = create_test_env_for_instance_allocation(pool.clone(), None).await; + let config = ManagedHostConfig::zero_dpu(); + + let zero_dpu_host = api_fixtures::site_explorer::new_host(&env, config).await?; + let etv_vpc_id = vpc_id_by_name(&env, "test vpc 1").await; + + let result = crate::handlers::instance::allocate( + env.api.as_ref(), + tonic::Request::new(forge::InstanceAllocationRequest { + machine_id: Some(zero_dpu_host.host_snapshot.id), + instance_type_id: None, + config: Some(forge::InstanceConfig { + tenant: Some(forge::TenantConfig { + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), + hostname: None, + tenant_keyset_ids: vec![], + }), + os: Some(forge::InstanceOperatingSystemConfig { + phone_home_enabled: false, + run_provisioning_instructions_on_every_boot: false, + user_data: None, + variant: Some(forge::instance_operating_system_config::Variant::Ipxe( + forge::InlineIpxe { + ipxe_script: "exit".to_string(), + }, + )), + }), + network: Some(forge::InstanceNetworkConfig { + interfaces: vec![], + #[allow(deprecated)] + auto: true, + auto_config: Some(forge::InstanceNetworkAutoConfig { + vpc_id: Some(etv_vpc_id), + }), + }), + infiniband: None, + nvlink: None, + spxconfig: None, + network_security_group_id: None, + dpu_extension_services: None, + }), + instance_id: None, + metadata: None, + allow_unhealthy_machine: false, + }), + ) + .await; + + let err = result.expect_err("zero-DPU auto allocation into non-Flat VPC must be rejected"); + assert_eq!(err.code(), tonic::Code::FailedPrecondition, "got: {err}"); + assert!( + err.message().contains("Flat"), + "error should mention Flat VPC requirement, got: {}", + err.message() + ); + Ok(()) +} + /// `auto: true` on a multi-NIC zero-DPU host must resolve to one resolved /// interface per HostInband segment, with each interface inheriting the /// host's already-assigned IP for that segment. @@ -581,6 +745,7 @@ async fn test_zero_dpu_instance_allocation_auto_multi_segment( ) }) .await?; + let flat_vpc_id = default_flat_vpc_id(&env).await; let instance = crate::handlers::instance::allocate( env.api.as_ref(), @@ -589,7 +754,7 @@ async fn test_zero_dpu_instance_allocation_auto_multi_segment( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), // from sql fixture + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -605,7 +770,11 @@ async fn test_zero_dpu_instance_allocation_auto_multi_segment( }), network: Some(forge::InstanceNetworkConfig { interfaces: vec![], + #[allow(deprecated)] auto: true, + auto_config: Some(forge::InstanceNetworkAutoConfig { + vpc_id: Some(flat_vpc_id), + }), }), infiniband: None, nvlink: None, @@ -633,7 +802,13 @@ async fn test_zero_dpu_instance_allocation_auto_multi_segment( // HostInband segments resolved. The resolved-per-interface details // surface in status, not config. let rpc_network = instance.config.unwrap().network.unwrap(); - assert!(rpc_network.auto, "auto must round-trip back as true"); + #[allow(deprecated)] + let auto = rpc_network.auto; + assert!(auto, "auto must round-trip back as true"); + assert_eq!( + rpc_network.auto_config.as_ref().unwrap().vpc_id, + Some(flat_vpc_id) + ); assert!( rpc_network.interfaces.is_empty(), "external view of an auto config must have empty interfaces, got: {:?}", @@ -646,6 +821,12 @@ async fn test_zero_dpu_instance_allocation_auto_multi_segment( 2, "status should reflect both resolved HostInband interfaces" ); + assert!( + status_interfaces + .iter() + .all(|interface| interface.vpc_id == Some(flat_vpc_id)), + "all resolved HostInband status interfaces should expose the requested Flat VPC" + ); // Internal model: the persisted config has the fully-resolved interfaces. let host_snapshot_after_allocate = db::managed_host::load_snapshot( @@ -662,7 +843,7 @@ async fn test_zero_dpu_instance_allocation_auto_multi_segment( .expect("zero-dpu host snapshot should have an assigned instance"); assert!( - instance_snapshot.config.network.auto, + instance_snapshot.config.network.auto_config.is_some(), "internal model must preserve auto: true through resolution" ); assert_eq!( @@ -670,6 +851,10 @@ async fn test_zero_dpu_instance_allocation_auto_multi_segment( 2, "internal model must hold the fully-resolved interfaces, not just the wire-stripped view" ); + assert_eq!( + instance_snapshot.config.network.auto_config.unwrap().vpc_id, + flat_vpc_id + ); let interface_in_segment_1 = instance_snapshot .config @@ -690,10 +875,12 @@ async fn test_zero_dpu_instance_allocation_auto_multi_segment( !interface_in_segment_1.ip_addrs.is_empty(), "Instance interface in segment 1 should have IP addresses assigned" ); + assert_eq!(interface_in_segment_1.vpc_id, Some(flat_vpc_id)); assert!( !interface_in_segment_2.ip_addrs.is_empty(), "Instance interface in segment 2 should have IP addresses assigned" ); + assert_eq!(interface_in_segment_2.vpc_id, Some(flat_vpc_id)); assert!( interface_in_segment_1 @@ -715,6 +902,18 @@ async fn test_zero_dpu_instance_allocation_auto_multi_segment( ) ); + let mut txn = env.db_txn().await; + for segment_id in [host_inband_segment_1.id, host_inband_segment_2.id] { + let address = db::instance_address::find_by_instance_id_and_segment_id( + txn.as_mut(), + &instance_snapshot.id, + &segment_id, + ) + .await? + .expect("zero-DPU allocation should persist an instance address per HostInband segment"); + assert_eq!(address.vpc_id, Some(flat_vpc_id)); + } + Ok(()) } @@ -736,7 +935,7 @@ async fn test_reject_single_dpu_instance_allocation_no_network_config( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), // from sql fixture + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -797,7 +996,7 @@ async fn test_reject_single_dpu_instance_allocation_host_inband_network_config( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), // from sql fixture + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -823,7 +1022,9 @@ async fn test_reject_single_dpu_instance_allocation_host_inband_network_config( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }), network_security_group_id: None, dpu_extension_services: None, @@ -948,8 +1149,11 @@ async fn test_reject_zero_dpu_instance_allocation_multiple_vpcs( "Machine that was just ingested should have instance network restrictions showing host_inband_2_segment {}", host_inband_2_segment.id, ); + let flat_vpc_id = default_flat_vpc_id(&env).await; - // Allocate an instance without specifying a network config + // Allocate an auto-networked instance into the first Flat VPC. The second + // HostInband segment is deliberately bound to a different Flat VPC, so + // the shared-segment allocation path must reject the conflict. let result = crate::handlers::instance::allocate( env.api.as_ref(), tonic::Request::new(forge::InstanceAllocationRequest { @@ -958,7 +1162,7 @@ async fn test_reject_zero_dpu_instance_allocation_multiple_vpcs( config: Some(forge::InstanceConfig { network_security_group_id: None, tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), // from sql fixture + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -972,7 +1176,14 @@ async fn test_reject_zero_dpu_instance_allocation_multiple_vpcs( }, )), }), - network: None, + network: Some(forge::InstanceNetworkConfig { + interfaces: vec![], + #[allow(deprecated)] + auto: true, + auto_config: Some(forge::InstanceNetworkAutoConfig { + vpc_id: Some(flat_vpc_id), + }), + }), infiniband: None, dpu_extension_services: None, nvlink: None, @@ -986,7 +1197,7 @@ async fn test_reject_zero_dpu_instance_allocation_multiple_vpcs( .await; match result { - Err(e) if e.code() == tonic::Code::InvalidArgument => {} + Err(e) if e.code() == tonic::Code::FailedPrecondition => {} _ => panic!( "Creating an instance on a zero-dpu host that is a member of multiple VPC's should fail, got {result:?}" ), @@ -1019,7 +1230,7 @@ async fn test_single_dpu_instance_allocation( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), // from sql fixture + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -1045,7 +1256,9 @@ async fn test_single_dpu_instance_allocation( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, nvlink: None, @@ -1165,7 +1378,7 @@ async fn test_reject_zero_dpu_instance_with_tenant_network_segment( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -1192,7 +1405,9 @@ async fn test_reject_zero_dpu_instance_with_tenant_network_segment( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, dpu_extension_services: None, @@ -1237,6 +1452,7 @@ async fn test_zero_dpu_instance_surfaces_underlay_ip_in_status( let env = create_test_env_for_instance_allocation(pool.clone(), None).await; let config = ManagedHostConfig::zero_dpu(); let zero_dpu_host = api_fixtures::site_explorer::new_host(&env, config).await?; + let flat_vpc_id = default_flat_vpc_id(&env).await; crate::handlers::instance::allocate( env.api.as_ref(), @@ -1245,7 +1461,7 @@ async fn test_zero_dpu_instance_surfaces_underlay_ip_in_status( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -1264,7 +1480,11 @@ async fn test_zero_dpu_instance_surfaces_underlay_ip_in_status( // resolves the HostInband segment from the host snapshot. network: Some(forge::InstanceNetworkConfig { interfaces: vec![], + #[allow(deprecated)] auto: true, + auto_config: Some(forge::InstanceNetworkAutoConfig { + vpc_id: Some(flat_vpc_id), + }), }), infiniband: None, dpu_extension_services: None, @@ -1310,6 +1530,7 @@ async fn test_zero_dpu_instance_surfaces_underlay_ip_in_status( 1, "expected one synthesized interface mirroring the auto-filled HostInband config entry", ); + assert_eq!(net_status.interfaces[0].vpc_id, Some(flat_vpc_id)); let iface = &net_status.interfaces[0]; assert!( !iface.addresses.is_empty(), @@ -1340,9 +1561,12 @@ async fn test_zero_dpu_instance_surfaces_underlay_ip_in_status( .as_ref() .and_then(|c| c.network.as_ref()) .expect("instance.config.network should be set"); - assert!( - cfg_network.auto, - "auto must round-trip back to the caller as true", + #[allow(deprecated)] + let auto = cfg_network.auto; + assert!(auto, "auto must round-trip back to the caller as true"); + assert_eq!( + cfg_network.auto_config.as_ref().unwrap().vpc_id, + Some(flat_vpc_id) ); assert!( cfg_network.interfaces.is_empty(), @@ -1373,7 +1597,7 @@ async fn test_reject_zero_dpu_instance_with_extension_services( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -1423,6 +1647,7 @@ async fn test_instance_allocation_rejects_auto_with_explicit_interfaces( pool: sqlx::PgPool, ) -> Result<(), Box> { let env = create_test_env_for_instance_allocation(pool.clone(), None).await; + let vpc_id = create_default_flat_vpc(&env.api, "flat-vpc").await; let config = ManagedHostConfig::zero_dpu(); let zero_dpu_host = api_fixtures::site_explorer::new_host(&env, config).await?; @@ -1437,7 +1662,7 @@ async fn test_instance_allocation_rejects_auto_with_explicit_interfaces( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -1464,7 +1689,11 @@ async fn test_instance_allocation_rejects_auto_with_explicit_interfaces( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: true, + auto_config: Some(forge::InstanceNetworkAutoConfig { + vpc_id: Some(vpc_id), + }), }), infiniband: None, dpu_extension_services: None, @@ -1496,6 +1725,7 @@ async fn test_instance_allocation_rejects_auto_on_dpu_host( pool: sqlx::PgPool, ) -> Result<(), Box> { let env = create_test_env_for_instance_allocation(pool.clone(), None).await; + let vpc_id = create_default_flat_vpc(&env.api, "flat-vpc").await; // Default ManagedHostConfig has one DPU. let config = ManagedHostConfig::default().with_dpu_count(1); @@ -1508,7 +1738,7 @@ async fn test_instance_allocation_rejects_auto_on_dpu_host( instance_type_id: None, config: Some(forge::InstanceConfig { tenant: Some(forge::TenantConfig { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), hostname: None, tenant_keyset_ids: vec![], }), @@ -1525,7 +1755,11 @@ async fn test_instance_allocation_rejects_auto_on_dpu_host( }), network: Some(forge::InstanceNetworkConfig { interfaces: vec![], + #[allow(deprecated)] auto: true, + auto_config: Some(forge::InstanceNetworkAutoConfig { + vpc_id: Some(vpc_id), + }), }), infiniband: None, dpu_extension_services: None, diff --git a/crates/api-core/src/tests/instance_config_update.rs b/crates/api-core/src/tests/instance_config_update.rs index 0c588b341a..e86264c146 100644 --- a/crates/api-core/src/tests/instance_config_update.rs +++ b/crates/api-core/src/tests/instance_config_update.rs @@ -898,7 +898,9 @@ async fn test_update_instance_config_vpc_prefix_network_update( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let initial_config = rpc::InstanceConfig { @@ -964,7 +966,9 @@ async fn test_update_instance_config_vpc_prefix_network_update( routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; let mut updated_config_1 = initial_config.clone(); updated_config_1.network = Some(network); @@ -1021,7 +1025,9 @@ async fn test_update_instance_config_vpc_prefix_network_update( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let mut updated_config_1 = initial_config.clone(); updated_config_1.network = Some(network); @@ -1106,7 +1112,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_post_instance_del ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let initial_config = rpc::InstanceConfig { @@ -1177,7 +1185,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_post_instance_del routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; let mut updated_config_1 = initial_config.clone(); updated_config_1.network = Some(network); @@ -1263,7 +1273,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_multidpu( ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let initial_config = rpc::InstanceConfig { @@ -1329,7 +1341,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_multidpu( routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; let mut updated_config_1 = initial_config.clone(); updated_config_1.network = Some(network); @@ -1458,7 +1472,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_multidpu_differen ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let initial_config = rpc::InstanceConfig { @@ -1524,7 +1540,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_multidpu_differen routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; let mut updated_config_1 = initial_config.clone(); updated_config_1.network = Some(network); @@ -1639,7 +1657,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_different_prefix_ ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, network_security_group_id: None, @@ -1679,7 +1699,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_different_prefix_ ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, network_security_group_id: None, @@ -1721,7 +1743,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_different_prefix_ ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, network_security_group_id: None, @@ -1857,7 +1881,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_different_prefix_ routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, network_security_group_id: None, @@ -1915,7 +1941,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_different_prefix_ routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, network_security_group_id: None, @@ -1973,7 +2001,9 @@ async fn test_update_instance_config_vpc_prefix_network_update_different_prefix_ routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }), infiniband: None, network_security_group_id: None, diff --git a/crates/api-core/src/tests/machine_dhcp.rs b/crates/api-core/src/tests/machine_dhcp.rs index 36529d492d..dba2378659 100644 --- a/crates/api-core/src/tests/machine_dhcp.rs +++ b/crates/api-core/src/tests/machine_dhcp.rs @@ -361,7 +361,9 @@ async fn test_machine_dhcp_with_api_for_instance_physical_virtual( routing_profile: None, }, ], + #[allow(deprecated)] auto: false, + auto_config: None, }; mh.instance_builer(&env).network(network).build().await; diff --git a/crates/api-core/src/tests/machine_network.rs b/crates/api-core/src/tests/machine_network.rs index c9afcb8139..ef4cd1bf46 100644 --- a/crates/api-core/src/tests/machine_network.rs +++ b/crates/api-core/src/tests/machine_network.rs @@ -510,9 +510,11 @@ async fn test_managed_host_network_config_includes_per_vpc_routing_profiles(pool let mut txn = env.db_txn().await; let internal_vpc = db::vpc::find_by_segment(txn.as_mut(), internal_segment_id) .await + .unwrap() .unwrap(); let external_vpc = db::vpc::find_by_segment(txn.as_mut(), external_segment_id) .await + .unwrap() .unwrap(); let internal_vni = internal_vpc.status.unwrap().vni.unwrap() as u32; let external_vni = external_vpc.status.unwrap().vni.unwrap() as u32; @@ -621,6 +623,7 @@ async fn test_managed_host_network_config_omits_fnn_vrf_loopback_by_default(pool let mut txn = env.db_txn().await; let vpc = db::vpc::find_by_segment(txn.as_mut(), segment_id) .await + .unwrap() .unwrap(); let loopback = db::vpc_dpu_loopback::find(txn.as_mut(), &dpu_machine_id, &vpc.id) .await @@ -694,6 +697,7 @@ async fn test_managed_host_network_config_includes_fnn_vrf_loopback_when_enabled let mut txn = env.db_txn().await; let vpc = db::vpc::find_by_segment(txn.as_mut(), segment_id) .await + .unwrap() .unwrap(); let loopback = db::vpc_dpu_loopback::find(txn.as_mut(), &dpu_machine_id, &vpc.id) .await @@ -1102,7 +1106,9 @@ async fn test_managed_host_network_status(pool: sqlx::PgPool) { ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; mh.instance_builer(&env) @@ -1204,7 +1210,9 @@ async fn test_managed_host_network_config_with_extension_services(pool: sqlx::Pg ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let default_tenant_org = "best_org"; diff --git a/crates/api-core/src/tests/network_security_group.rs b/crates/api-core/src/tests/network_security_group.rs index 2579470316..6f164dd723 100644 --- a/crates/api-core/src/tests/network_security_group.rs +++ b/crates/api-core/src/tests/network_security_group.rs @@ -36,6 +36,7 @@ use crate::tests::common::api_fixtures::instance::{ default_os_config, default_tenant_config, interface_network_config_with_devices, single_interface_network_config, }; +use crate::tests::common::api_fixtures::test_managed_host::TestManagedHost; use crate::tests::common::api_fixtures::{ create_test_env, populate_network_security_groups, site_explorer, }; @@ -1760,7 +1761,6 @@ async fn test_network_security_group_get_attachments( let good_network_security_group_id = "fd3ab096-d811-11ef-8fe9-7be4b2483448"; let vpc_id: VpcId = uuid!("2ff5ba26-da6a-11ef-9c48-5b78e547a5e7").into(); - let instance_id: InstanceId = uuid!("46c555e0-da6a-11ef-b86d-db132142d068").into(); // Check attachments before doing anything else. // There should be no objects with any attached NSG. @@ -1806,32 +1806,23 @@ async fn test_network_security_group_get_attachments( .await .unwrap(); + let test_managed_host = + TestManagedHost::from_rpc_machine(&mh.host_snapshot.clone().into(), env.api.clone()); // Create an Instance - let _ = env - .api - .allocate_instance(tonic::Request::new(rpc::forge::InstanceAllocationRequest { - machine_id: mh.host_snapshot.id.into(), - config: Some(rpc::InstanceConfig { - tenant: Some(default_tenant_config()), - os: Some(default_os_config()), - network: Some(single_interface_network_config(segment_id)), - infiniband: None, - nvlink: None, - spxconfig: None, - network_security_group_id: Some(good_network_security_group_id.to_string()), - dpu_extension_services: None, - }), - instance_id: Some(instance_id), - instance_type_id: None, - metadata: Some(rpc::forge::Metadata { - name: "newinstance".to_string(), - description: "desc".to_string(), - labels: vec![], - }), - allow_unhealthy_machine: false, - })) - .await - .unwrap(); + let instance = test_managed_host + .instance_builer(&env) + .config(rpc::InstanceConfig { + tenant: Some(default_tenant_config()), + os: Some(default_os_config()), + network: Some(single_interface_network_config(segment_id)), + infiniband: None, + nvlink: None, + spxconfig: None, + network_security_group_id: Some(good_network_security_group_id.to_string()), + dpu_extension_services: None, + }) + .build() + .await; // Check attachments let prop_status = env @@ -1849,21 +1840,15 @@ async fn test_network_security_group_get_attachments( attachments: vec![rpc::forge::NetworkSecurityGroupAttachments { network_security_group_id: good_network_security_group_id.to_string(), vpc_ids: vec![vpc_id.to_string()], - instance_ids: vec![instance_id.to_string()], + instance_ids: vec![instance.id.to_string()], }], }; assert_eq!(prop_status, expected_results); - // Delete the instance - env.api - .release_instance(tonic::Request::new(rpc::forge::InstanceReleaseRequest { - id: Some(instance_id), - issue: None, - is_repair_tenant: None, - })) - .await - .unwrap(); + // Delete the instance and wait for it to be fully gone + test_managed_host.delete_instance(&env, instance.id).await; + // Delete the VPC env.api .delete_vpc(tonic::Request::new(rpc::forge::VpcDeletionRequest { diff --git a/crates/api-core/src/tests/network_segment.rs b/crates/api-core/src/tests/network_segment.rs index 258bc00c60..7be8c59311 100644 --- a/crates/api-core/src/tests/network_segment.rs +++ b/crates/api-core/src/tests/network_segment.rs @@ -49,6 +49,7 @@ use rpc::forge::forge_server::Forge; use tonic::Request; use crate::db_init; +use crate::test_support::network_segment::FIXTURE_TENANT_ORG_ID; use crate::tests::common; use crate::tests::common::api_fixtures::network_segment::FIXTURE_TENANT_NETWORK_SEGMENT_GATEWAYS; use crate::tests::common::api_fixtures::{ @@ -68,7 +69,7 @@ async fn test_advance_network_prefix_state( let vpc = env .api .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(rpc::forge::Metadata { name: "test vpc 1".to_string(), ..Default::default() @@ -592,7 +593,7 @@ pub async fn test_create_initial_vpc_and_attached_network( let vpcs = HashMap::from([( "zero-dpu-vpc".to_string(), VpcDefinition { - organization_id: Some("2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string()), + organization_id: Some(FIXTURE_TENANT_ORG_ID.to_string()), network_virtualization_type: VpcVirtualizationType::Flat, routing_profile_type: None, vni: None, @@ -623,10 +624,7 @@ pub async fn test_create_initial_vpc_and_attached_network( let seeded_vpcs = db::vpc::find_by_name(txn.as_mut(), "zero-dpu-vpc").await?; assert_eq!(seeded_vpcs.len(), 1); let seeded_vpc = &seeded_vpcs[0]; - assert_eq!( - seeded_vpc.tenant_organization_id, - "2829bbe3-c169-4cd9-8b2a-19a8b1618a93" - ); + assert_eq!(seeded_vpc.tenant_organization_id, FIXTURE_TENANT_ORG_ID); assert_eq!( seeded_vpc.network_virtualization_type, VpcVirtualizationType::Flat @@ -1323,7 +1321,7 @@ async fn test_create_dual_stack_tenant_segment(pool: sqlx::PgPool) -> Result<(), let vpc = env .api .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "dual-stack vpc".to_string(), ..Default::default() @@ -1408,7 +1406,7 @@ async fn test_ipv6_tenant_prefix_rejected_when_not_in_site_fabric( let vpc = env .api .create_vpc( - VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(Metadata { name: "uncontained-ipv6-vpc".to_string(), description: "".to_string(), @@ -1585,7 +1583,7 @@ async fn flat_vpc_accepts_host_inband_segment( let (_vpc_id, vpc) = common::api_fixtures::vpc::create_flat_vpc( &env, "flat".to_string(), - Some("2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string()), + Some(FIXTURE_TENANT_ORG_ID.to_string()), ) .await; @@ -1629,7 +1627,7 @@ async fn flat_vpc_rejects_tenant_segment( let (_vpc_id, vpc) = common::api_fixtures::vpc::create_flat_vpc( &env, "flat".to_string(), - Some("2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string()), + Some(FIXTURE_TENANT_ORG_ID.to_string()), ) .await; @@ -1679,7 +1677,7 @@ async fn etv_vpc_rejects_host_inband_segment( let (_vpc_id, vpc) = common::api_fixtures::vpc::create_vpc( &env, "etv".to_string(), - Some("2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string()), + Some(FIXTURE_TENANT_ORG_ID.to_string()), None, ) .await; diff --git a/crates/api-core/src/tests/vpc.rs b/crates/api-core/src/tests/vpc.rs index 395327beb6..61dd6fec2f 100644 --- a/crates/api-core/src/tests/vpc.rs +++ b/crates/api-core/src/tests/vpc.rs @@ -27,6 +27,7 @@ use model::metadata::Metadata; use model::vpc::{UpdateVpc, UpdateVpcVirtualization}; use rpc::forge::forge_server::Forge; +use crate::test_support::network_segment::FIXTURE_TENANT_ORG_ID; use crate::tests::common; use crate::tests::common::api_fixtures::{TestEnvOverrides, create_test_env_with_overrides}; use crate::tests::common::rpc_builder::{VpcCreationRequest, VpcDeletionRequest, VpcUpdateRequest}; @@ -748,8 +749,8 @@ async fn find_vpc_by_id(pool: sqlx::PgPool) -> Result<(), Box Result<(), Box Result<(), Box> { + let instance_ids = futures_util::future::join_all(vpc_ids.iter().map(|vpc_id| { + async move { + env.api + .find_instance_ids( + rpc::forge::InstanceSearchFilter { + vpc_id: Some(vpc_id.to_string()), + ..Default::default() + } + .into_request(), + ) + .map_ok(|r| r.into_inner().instance_ids) + .await + .unwrap() + } + .boxed() + })) + .await + .into_iter() + .flatten() + .collect::>(); + + if instance_ids.is_empty() { + return Ok(()); + } + + let instances = env + .api + .find_instances_by_ids(rpc::forge::InstancesByIdsRequest { instance_ids }.into_request()) + .await + .expect("searching for instances should succeed") + .into_inner() + .instances; + + let mut machines: HashMap = env + .api + .find_machines_by_ids( + rpc::forge::MachinesByIdsRequest { + machine_ids: instances.iter().filter_map(|i| i.machine_id).collect(), + include_history: false, + } + .into_request(), + ) + .await + .expect("Finding machines should succeed") + .into_inner() + .machines + .into_iter() + .map(|m| (m.id.unwrap(), m)) + .collect(); + + futures_util::future::join_all(instances.into_iter().map(|i| { + let machine = machines + .remove(&i.machine_id.unwrap()) + .expect("Should have found machine for instance"); + async move { + TestManagedHost::from_rpc_machine(&machine, env.api.clone()) + .delete_instance(env, i.id.unwrap()) + .await + } + .boxed() + })) + .await; + + Ok(()) +} + async fn find_vpc_id_by_name( env: &TestEnv, vpc_name: &str, @@ -283,6 +358,8 @@ async fn test_vpc_peering_full(pool: PgPool) -> Result<(), Box Result<(), Box Result<(), Box { if vni == 0 { tracing::warn!("Did not expect DPA VNI to be zero"); diff --git a/crates/api-db/src/instance.rs b/crates/api-db/src/instance.rs index 3ed1fffc62..7104a8c47f 100644 --- a/crates/api-db/src/instance.rs +++ b/crates/api-db/src/instance.rs @@ -121,9 +121,7 @@ pub async fn find_ids( builder.push( "SELECT instances.id FROM instances INNER JOIN instance_addresses ON instance_addresses.instance_id = instances.id -INNER JOIN network_segments ON instance_addresses.segment_id = network_segments.id -INNER JOIN vpcs ON network_segments.vpc_id = vpcs.id -WHERE vpc_id = ", +WHERE instance_addresses.vpc_id = ", ); builder.push_bind(vpc_id); builder.push(")"); diff --git a/crates/api-db/src/instance_address.rs b/crates/api-db/src/instance_address.rs index 983e03d183..e7f279dee6 100644 --- a/crates/api-db/src/instance_address.rs +++ b/crates/api-db/src/instance_address.rs @@ -21,6 +21,7 @@ use std::ops::DerefMut; use carbide_network::virtualization::{VpcVirtualizationType, get_host_ip}; use carbide_uuid::instance::InstanceId; use carbide_uuid::network::{NetworkPrefixId, NetworkSegmentId}; +use carbide_uuid::vpc::VpcId; use ipnetwork::IpNetwork; use itertools::Itertools; use model::ConfigValidationError; @@ -131,8 +132,18 @@ pub async fn delete_addresses( Ok(()) } +fn interface_vpc_id(iface: &InstanceInterfaceConfig, segments: &[NetworkSegment]) -> Option { + iface.vpc_id.or_else(|| { + let segment_id = iface.network_segment_id?; + segments + .iter() + .find(|segment| segment.id == segment_id) + .and_then(|segment| segment.config.vpc_id) + }) +} + fn validate( - segments: &Vec, + segments: &[NetworkSegment], instance_network: &InstanceNetworkConfig, segment_ids_using_vpc_prefix: &[NetworkSegmentId], all_fnn: bool, @@ -163,15 +174,20 @@ fn validate( } } } + } - match segment.config.vpc_id { - Some(x) => { - vpc_ids.insert(x); + for iface in &instance_network.interfaces { + match interface_vpc_id(iface, segments) { + Some(vpc_id) => { + vpc_ids.insert(vpc_id); } None => { - return Err(ConfigValidationError::VpcNotAttachedToSegment(segment.id).into()); + let segment_id = iface + .network_segment_id + .ok_or(DatabaseError::NetworkSegmentNotAllocated)?; + return Err(ConfigValidationError::VpcNotAttachedToSegment(segment_id).into()); } - }; + } } if vpc_ids.len() != 1 && !all_fnn { @@ -244,9 +260,10 @@ pub async fn allocate( .await?; // Multi-VPC instance interfaces are supported only when every referenced VPC is FNN. - let vpc_ids = segments + let vpc_ids = updated_config + .interfaces .iter() - .filter_map(|segment| segment.config.vpc_id) + .filter_map(|iface| interface_vpc_id(iface, &segments)) .collect::>() .into_iter() .collect_vec(); @@ -371,8 +388,13 @@ pub async fn allocate( iface.assign_ips_from(ip_allocator)? }; - let query = "INSERT INTO instance_addresses (instance_id, address, segment_id, prefix) - VALUES ($1::uuid, $2, $3::uuid, $4::cidr)"; + let vpc_id = interface_vpc_id(iface, &segments) + .ok_or(ConfigValidationError::VpcNotAttachedToSegment(segment.id))?; + iface.vpc_id = Some(vpc_id); + + let query = + "INSERT INTO instance_addresses (instance_id, address, segment_id, prefix, vpc_id) + VALUES ($1::uuid, $2, $3::uuid, $4::cidr, $5::uuid)"; for address in addresses { sqlx::query(query) @@ -382,6 +404,7 @@ pub async fn allocate( // eg. 10.3.2.0/30 .bind(segment.id) .bind(IpNetwork::new(address.network(), address.prefix())?) + .bind(vpc_id) .fetch_all(inner_txn.as_pgconn()) .await .map_err(|e| DatabaseError::query(query, e))?; @@ -652,13 +675,14 @@ mod tests { let network_segments: Vec = InterfaceFunctionId::iter_all() .enumerate() .map(|(idx, _function_id)| { - let id = format!("91609f10-c91d-470d-a260-6293ea0c00{idx:02}"); + let id: NetworkSegmentId = + Uuid::from_u128(BASE_SEGMENT_ID.as_u128() + idx as u128).into(); let version = ConfigVersion::initial(); NetworkSegment { - id: NetworkSegmentId::from_str(&id).unwrap(), + id, version, config: NetworkSegmentConfig { - name: id, + name: id.to_string(), subdomain_id: None, vpc_id: Some(vpc_id), mtu: 1500, @@ -711,13 +735,14 @@ mod tests { host_inband_mac_address: None, device_locator: None, internal_uuid: uuid::Uuid::new_v4(), + vpc_id: None, } }) .collect(); InstanceNetworkConfig { interfaces, - auto: false, + auto_config: None, } } diff --git a/crates/api-db/src/instance_network_config.rs b/crates/api-db/src/instance_network_config.rs index d3c93e5930..2d0c9074c1 100644 --- a/crates/api-db/src/instance_network_config.rs +++ b/crates/api-db/src/instance_network_config.rs @@ -55,7 +55,7 @@ pub async fn batch_get_inband_segments_by_machine_ids( /// Add inband interfaces to a network config based on segment IDs. /// This is a pure function that can be used after batch querying. /// -/// This only injects when `auto` is true. If we get a non-auto +/// This only injects when `auto_config` is set with a VpcId. If we get a non-auto /// config, just leave as-is and return it unchanged (as in, there /// are no inband interfaces to add). /// @@ -67,9 +67,9 @@ pub fn add_inband_interfaces_to_config( mut network_config: InstanceNetworkConfig, host_inband_segment_ids: &[NetworkSegmentId], ) -> DatabaseResult { - if !network_config.auto { + let Some(vpc_id) = network_config.auto_config.map(|c| c.vpc_id) else { return Ok(network_config); - } + }; if !network_config.interfaces.is_empty() { return Err(DatabaseError::InvalidArgument(format!( @@ -94,6 +94,7 @@ pub fn add_inband_interfaces_to_config( requested_ip_addr: None, ipv6_interface_config: None, routing_profile: None, + vpc_id: Some(vpc_id), }); } diff --git a/crates/api-db/src/network_security_group.rs b/crates/api-db/src/network_security_group.rs index 9969b60a09..0834917e4f 100644 --- a/crates/api-db/src/network_security_group.rs +++ b/crates/api-db/src/network_security_group.rs @@ -241,8 +241,8 @@ pub async fn get_propagation_status( let mut vpc_query_builder = sqlx::QueryBuilder::new( // Querying for VPC status is slightly more complicated because // instance records don't have a vpc_id column, so we need to - // start on instances and then trace the VPC through the network - // segment of each interface. + // start on instances and then trace the VPC through the allocated + // address of each interface. // This does seem like it might have the upside of accounting for // a future case where a machine has multiple DPUs within separate // VPCs. @@ -279,8 +279,12 @@ pub async fn get_propagation_status( JOIN machines dpu ON dpu.id = mi.attached_dpu_machine_id /* network_status_observation is stored in dpu now. */ LEFT OUTER JOIN jsonb_array_elements(dpu.network_status_observation #>'{instance_network_observation,interfaces}') ifco on ifco->>'internal_uuid' = ifc->>'internal_uuid' - JOIN network_segments ns on ns.id=(ifc->>'network_segment_id')::uuid - JOIN vpcs v on v.id=ns.vpc_id + JOIN ( + SELECT DISTINCT instance_id, segment_id, vpc_id + FROM instance_addresses + WHERE vpc_id IS NOT NULL + ) ia ON ia.instance_id = i.id AND ia.segment_id = (ifc->>'network_segment_id')::uuid + JOIN vpcs v on v.id=ia.vpc_id JOIN network_security_groups nsg on nsg.id=v.network_security_group_id WHERE i.network_security_group_id IS NULL AND i.deleted IS NULL" diff --git a/crates/api-db/src/vpc.rs b/crates/api-db/src/vpc.rs index ff39ab2855..a04447d7a0 100644 --- a/crates/api-db/src/vpc.rs +++ b/crates/api-db/src/vpc.rs @@ -240,7 +240,7 @@ pub async fn find_by_name(txn: impl DbReader<'_>, name: &str) -> Result pub async fn find_by_segment( txn: impl DbReader<'_>, segment_id: NetworkSegmentId, -) -> Result { +) -> Result, DatabaseError> { let mut query = FilterableQueryBuilder::new( "SELECT v.* from vpcs v INNER JOIN network_segments s ON v.id = s.vpc_id", ) @@ -252,7 +252,7 @@ pub async fn find_by_segment( query .build_query_as() - .fetch_one(txn) + .fetch_optional(txn) .await .map_err(|e| DatabaseError::query(query.sql(), e)) } @@ -272,7 +272,7 @@ pub async fn try_delete(txn: &mut PgConnection, id: VpcId) -> Result let vpc_prefix_count_query = "SELECT count(*) FROM network_vpc_prefixes WHERE vpc_id=$1 AND EXISTS (SELECT 1 FROM vpcs WHERE id=$1 AND deleted IS NULL)"; - let (vpc_prefix_count,): (i64,) = sqlx::query_as(vpc_prefix_count_query) + let vpc_prefix_count: i64 = sqlx::query_scalar(vpc_prefix_count_query) .bind(id) .fetch_one(&mut *txn) .await @@ -283,6 +283,20 @@ pub async fn try_delete(txn: &mut PgConnection, id: VpcId) -> Result ))); } + let instance_address_count_query = "SELECT count(*) FROM instance_addresses + WHERE vpc_id=$1 + AND EXISTS (SELECT 1 FROM vpcs WHERE id=$1 AND deleted IS NULL)"; + let instance_address_count: i64 = sqlx::query_scalar(instance_address_count_query) + .bind(id) + .fetch_one(&mut *txn) + .await + .map_err(|e| DatabaseError::query(instance_address_count_query, e))?; + if instance_address_count > 0 { + return Err(DatabaseError::FailedPrecondition(format!( + "VPC {id} cannot be deleted while {instance_address_count} instance addresses still reference it" + ))); + } + let query = "UPDATE vpcs SET updated=NOW(), deleted=NOW() WHERE id=$1 AND deleted IS NULL RETURNING *"; match sqlx::query_as(query).bind(id).fetch_one(txn).await { diff --git a/crates/api-model/src/instance/config/network.rs b/crates/api-model/src/instance/config/network.rs index bdfdf8b1ec..d015561914 100644 --- a/crates/api-model/src/instance/config/network.rs +++ b/crates/api-model/src/instance/config/network.rs @@ -21,7 +21,7 @@ use std::net::IpAddr; use carbide_uuid::machine::MachineId; use carbide_uuid::network::{NetworkPrefixId, NetworkSegmentId}; -use carbide_uuid::vpc::VpcPrefixId; +use carbide_uuid::vpc::{VpcId, VpcPrefixId}; use ipnetwork::IpNetwork; use mac_address::MacAddress; use serde::ser::SerializeMap; @@ -110,7 +110,7 @@ pub struct InstanceNetworkConfig { /// hosts (well, no DPU, *or* DPU in NIC mode). /// /// It is also important to note that on the wire (request AND response), - /// `auto: true` only travels with `interfaces: []`, but internally some + /// `auto_config: {...}` only travels with `interfaces: []`, but internally some /// other things are happening. /// /// On allocation/update, NICo resolves the empty interfaces: [] into @@ -120,12 +120,20 @@ pub struct InstanceNetworkConfig { /// /// Then, at the model <-> RPC boundary, the resolved interfaces are /// stripped off to `[]`, so callers reading the instance config back - /// simply see what they originally sent (`auto: true` with no interfaces). + /// simply see what they originally sent (`auto: {...}` with no interfaces). /// /// The resolved per-interface details (IP, MAC, gateway, prefix) appear in /// `Instance.status.network.interfaces` like usual. - #[serde(default)] - pub auto: bool, + pub auto_config: Option, +} + +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct InstanceNetworkAutoConfig { + /// The logical VPC to allocate into when `auto` networking is used. + /// + /// Auto networking resolves the concrete HostInband segment from the host, + /// so the segment itself does not have to carry VPC ownership. + pub vpc_id: VpcId, } /// Struct to store instance network config updated request with current config. @@ -147,6 +155,7 @@ impl InstanceNetworkConfig { pub fn for_segment_ids( network_segment_ids: &[NetworkSegmentId], device_locators: &[DeviceLocator], + vpc_ids: &[VpcId], ) -> Self { if device_locators.is_empty() { Self { @@ -165,8 +174,9 @@ impl InstanceNetworkConfig { host_inband_mac_address: None, device_locator: None, internal_uuid: uuid::Uuid::nil(), + vpc_id: vpc_ids.first().copied(), }], - auto: false, + auto_config: None, } } else { Self { @@ -188,18 +198,16 @@ impl InstanceNetworkConfig { host_inband_mac_address: None, device_locator: Some(dl.clone()), internal_uuid: uuid::Uuid::nil(), + vpc_id: vpc_ids.get(dl_index).copied(), }) .collect(), - auto: false, + auto_config: None, } } } /// Returns a network configuration for a single physical interface - pub fn for_vpc_prefix_id( - vpc_prefix_id: VpcPrefixId, - _dpu_machine_id: Option, - ) -> Self { + pub fn for_vpc_prefix_id(vpc_prefix_id: VpcPrefixId, vpc_id: Option) -> Self { Self { interfaces: vec![InstanceInterfaceConfig { function_id: InterfaceFunctionId::Physical {}, @@ -214,8 +222,9 @@ impl InstanceNetworkConfig { host_inband_mac_address: None, device_locator: None, internal_uuid: uuid::Uuid::nil(), + vpc_id, }], - auto: false, + auto_config: None, } } @@ -230,10 +239,10 @@ impl InstanceNetworkConfig { /// back to them as they sent it, and mask any internal interface /// resolution that happened as a result of `auto`. pub fn into_external_view(self) -> Self { - if self.auto { + if self.auto_config.is_some() { Self { interfaces: vec![], - auto: true, + auto_config: self.auto_config, } } else { self @@ -388,6 +397,7 @@ impl InstanceNetworkConfig { iface.network_segment_gateways.clear(); iface.host_inband_mac_address = None; iface.internal_uuid = uuid::Uuid::nil(); + iface.vpc_id = None; // It is possible that cloud sends network_segment_id with network_details as well. if iface.network_details.is_some() { @@ -401,6 +411,7 @@ impl InstanceNetworkConfig { iface.network_segment_id = None; } iface.internal_uuid = uuid::Uuid::nil(); + iface.vpc_id = None; } current != new_config @@ -460,6 +471,7 @@ impl InstanceNetworkConfig { if interface.network_details.is_some() { interface.network_segment_id = existing_interface.network_segment_id; } + interface.vpc_id = existing_interface.vpc_id; common_function_ids.push(existing_interface); } } @@ -659,6 +671,13 @@ pub struct InstanceInterfaceConfig { /// An internal ID used to associate an interface status with the interface config pub internal_uuid: uuid::Uuid, + + /// Logical VPC ownership for this resolved interface. + /// + /// For legacy segment-bound allocations this is derived from the segment. + /// For Flat zero-DPU auto allocations this is copied from + /// [`InstanceNetworkConfig::vpc_id`] after HostInband segment resolution. + pub vpc_id: Option, } impl InstanceInterfaceConfig { @@ -811,11 +830,12 @@ mod tests { network_details: None, device_locator: None, internal_uuid, + vpc_id: None, }; let serialized = serde_json::to_string(&interface).unwrap(); assert_eq!( serialized, - r#"{"function_id":{"type":"physical"},"network_details":null,"network_segment_id":"91609f10-c91d-470d-a260-6293ea0c1200","ip_addrs":{"91609f10-c91d-470d-a260-6293ea0c1201":"192.168.1.2"},"requested_ip_addr":"192.168.1.2","ipv6":null,"routing_profile":null,"interface_prefixes":{"91609f10-c91d-470d-a260-6293ea0c1201":"192.168.1.2/32"},"network_segment_gateways":{},"host_inband_mac_address":null,"device_locator":null,"internal_uuid":"37c3dc65-9aef-4439-b7ca-d532a0a41d7f"}"# + r#"{"function_id":{"type":"physical"},"network_details":null,"network_segment_id":"91609f10-c91d-470d-a260-6293ea0c1200","ip_addrs":{"91609f10-c91d-470d-a260-6293ea0c1201":"192.168.1.2"},"requested_ip_addr":"192.168.1.2","ipv6":null,"routing_profile":null,"interface_prefixes":{"91609f10-c91d-470d-a260-6293ea0c1201":"192.168.1.2/32"},"network_segment_gateways":{},"host_inband_mac_address":null,"device_locator":null,"internal_uuid":"37c3dc65-9aef-4439-b7ca-d532a0a41d7f","vpc_id":null}"# ); assert_eq!( @@ -849,16 +869,41 @@ mod tests { network_details: None, device_locator: None, internal_uuid: uuid::Uuid::new_v4(), + vpc_id: None, } }) .collect(); InstanceNetworkConfig { interfaces, - auto: false, + auto_config: None, } } + #[test] + fn network_update_detection_ignores_derived_vpc_id() { + let mut current = create_valid_network_config(); + let mut requested = current.clone(); + + current.interfaces[0].vpc_id = Some(VpcId::new()); + requested.interfaces[0].vpc_id = None; + requested.interfaces[0].internal_uuid = uuid::Uuid::new_v4(); + + assert!(!current.is_network_config_update_requested(&requested)); + } + + #[test] + fn copy_existing_resources_preserves_derived_vpc_id() { + let vpc_id = VpcId::new(); + let mut current = create_valid_network_config(); + current.interfaces[0].vpc_id = Some(vpc_id); + + let mut requested = create_valid_network_config(); + requested.copy_existing_resources(¤t); + + assert_eq!(requested.interfaces[0].vpc_id, Some(vpc_id)); + } + // InstanceNetworkConfig::validate over a base valid config mutated per row. // Input is (config, allow_instance_vf). ConfigValidationError is not // PartialEq, so rejections assert only that validation errs (Fails); the diff --git a/crates/api-model/src/instance/status/network.rs b/crates/api-model/src/instance/status/network.rs index e3c1b9b929..945667d887 100644 --- a/crates/api-model/src/instance/status/network.rs +++ b/crates/api-model/src/instance/status/network.rs @@ -20,6 +20,7 @@ use std::convert::Into; use std::net::IpAddr; use carbide_uuid::machine::MachineId; +use carbide_uuid::vpc::VpcId; use chrono::{DateTime, Utc}; use config_version::{ConfigVersion, Versioned}; use ipnetwork::IpNetwork; @@ -147,6 +148,7 @@ impl InstanceNetworkStatus { addresses: obs_iface.addresses.clone(), prefixes: obs_iface.prefixes.clone(), gateways: obs_iface.gateways.clone(), + vpc_id: config_iface.vpc_id, device: config_iface .device_locator .as_ref() @@ -174,6 +176,7 @@ impl InstanceNetworkStatus { addresses: Vec::new(), prefixes: Vec::new(), gateways: Vec::new(), + vpc_id: config_iface.vpc_id, device: config_iface .device_locator .as_ref() @@ -195,6 +198,7 @@ impl InstanceNetworkStatus { addresses: Vec::new(), prefixes: Vec::new(), gateways: Vec::new(), + vpc_id: config_iface.vpc_id, device: config_iface .device_locator .as_ref() @@ -241,6 +245,7 @@ impl InstanceNetworkStatus { addresses: intf_obs.addresses.clone(), prefixes: intf_obs.prefixes.clone(), gateways: intf_obs.gateways.clone(), + vpc_id: config_iface.vpc_id, device: config_iface .device_locator .as_ref() @@ -267,6 +272,7 @@ impl InstanceNetworkStatus { addresses: Vec::new(), prefixes: Vec::new(), gateways: Vec::new(), + vpc_id: config_iface.vpc_id, device: config_iface .device_locator .as_ref() @@ -313,6 +319,7 @@ impl InstanceNetworkStatus { addresses: Vec::new(), prefixes: Vec::new(), gateways: Vec::new(), + vpc_id: iface.vpc_id, device: iface.device_locator.as_ref().map(|dl| dl.device.clone()), device_instance: iface .device_locator @@ -367,6 +374,9 @@ pub struct InstanceInterfaceStatus { /// The list of gateways, in CIDR notation, one for each address in `addresses`. pub gateways: Vec, + /// The logical VPC this interface belongs to. + pub vpc_id: Option, + pub device: Option, pub device_instance: usize, } @@ -408,6 +418,7 @@ impl InstanceInterfaceStatus { addresses, prefixes, gateways, + vpc_id: value.vpc_id, device: None, device_instance: 0, } @@ -651,6 +662,7 @@ mod tests { network_details: None, device_locator: None, internal_uuid: uuid::Uuid::new_v4(), + vpc_id: None, }, InstanceInterfaceConfig { function_id: InterfaceFunctionId::Virtual { id: 1 }, @@ -674,6 +686,7 @@ mod tests { network_details: None, device_locator: None, internal_uuid: uuid::Uuid::new_v4(), + vpc_id: None, }, InstanceInterfaceConfig { function_id: InterfaceFunctionId::Virtual { id: 2 }, @@ -697,9 +710,10 @@ mod tests { network_details: None, device_locator: None, internal_uuid: uuid::Uuid::new_v4(), + vpc_id: None, }, ], - auto: false, + auto_config: None, } } @@ -733,6 +747,7 @@ mod tests { network_details: None, device_locator: None, internal_uuid: internal_uuid1, + vpc_id: None, }, InstanceInterfaceConfig { function_id: InterfaceFunctionId::Virtual { id: 1 }, @@ -756,6 +771,7 @@ mod tests { network_details: None, device_locator: None, internal_uuid: internal_uuid2, + vpc_id: None, }, InstanceInterfaceConfig { function_id: InterfaceFunctionId::Virtual { id: 2 }, @@ -779,9 +795,10 @@ mod tests { network_details: None, device_locator: None, internal_uuid: internal_uuid3, + vpc_id: None, }, ], - auto: false, + auto_config: None, } } @@ -842,6 +859,7 @@ mod tests { addresses: Vec::new(), prefixes: Vec::new(), gateways: Vec::new(), + vpc_id: None, device: None, device_instance: 0, }, @@ -851,6 +869,7 @@ mod tests { addresses: Vec::new(), prefixes: Vec::new(), gateways: Vec::new(), + vpc_id: None, device: None, device_instance: 0, }, @@ -860,6 +879,7 @@ mod tests { addresses: Vec::new(), prefixes: Vec::new(), gateways: Vec::new(), + vpc_id: None, device: None, device_instance: 0, }, @@ -880,6 +900,7 @@ mod tests { addresses: iface.ip_addrs.values().copied().collect(), prefixes: iface.interface_prefixes.values().copied().collect(), gateways: iface.network_segment_gateways.values().copied().collect(), + vpc_id: iface.vpc_id, device: iface.device_locator.as_ref().map(|dl| dl.device.clone()), device_instance: iface .device_locator @@ -895,6 +916,7 @@ mod tests { addresses: iface.ip_addrs.values().copied().collect(), prefixes: iface.interface_prefixes.values().copied().collect(), gateways: iface.network_segment_gateways.values().copied().collect(), + vpc_id: iface.vpc_id, device: iface.device_locator.as_ref().map(|dl| dl.device.clone()), device_instance: iface .device_locator @@ -911,6 +933,7 @@ mod tests { addresses: iface.ip_addrs.values().copied().collect(), prefixes: iface.interface_prefixes.values().copied().collect(), gateways: iface.network_segment_gateways.values().copied().collect(), + vpc_id: iface.vpc_id, device: iface.device_locator.as_ref().map(|dl| dl.device.clone()), device_instance: iface .device_locator @@ -934,6 +957,7 @@ mod tests { addresses: vec!["127.0.1.2".parse().unwrap()], prefixes: vec!["127.0.1.0/24".parse().unwrap()], gateways: vec!["127.0.1.1/24".parse().unwrap()], + vpc_id: None, device: None, device_instance: 0, }, @@ -943,6 +967,7 @@ mod tests { addresses: vec!["127.0.2.2".parse().unwrap()], prefixes: vec!["127.0.2.0/24".parse().unwrap()], gateways: vec!["127.0.2.1/24".parse().unwrap()], + vpc_id: None, device: None, device_instance: 0, }, @@ -952,6 +977,7 @@ mod tests { addresses: vec!["127.0.3.2".parse().unwrap()], prefixes: vec!["127.0.3.0/24".parse().unwrap()], gateways: vec!["127.0.3.1/24".parse().unwrap()], + vpc_id: None, device: None, device_instance: 0, }, diff --git a/crates/api-model/src/instance_address.rs b/crates/api-model/src/instance_address.rs index 123bc061c3..ce5d8151f9 100644 --- a/crates/api-model/src/instance_address.rs +++ b/crates/api-model/src/instance_address.rs @@ -16,12 +16,14 @@ */ use carbide_uuid::instance::InstanceId; use carbide_uuid::network::NetworkSegmentId; +use carbide_uuid::vpc::VpcId; use sqlx::FromRow; #[derive(Debug, FromRow, Clone)] pub struct InstanceAddress { pub instance_id: InstanceId, pub segment_id: NetworkSegmentId, + pub vpc_id: Option, // pub id: Uuid, // unused pub address: std::net::IpAddr, // pub prefix: IpNetwork, // unused diff --git a/crates/machine-a-tron/src/api_client.rs b/crates/machine-a-tron/src/api_client.rs index 3d5fb255a3..487fefa1f4 100644 --- a/crates/machine-a-tron/src/api_client.rs +++ b/crates/machine-a-tron/src/api_client.rs @@ -288,7 +288,9 @@ impl ApiClient { }), network: Some(rpc::InstanceNetworkConfig { interfaces: vec![interface_config], + #[allow(deprecated)] auto: false, + auto_config: None, }), network_security_group_id: None, infiniband: None, diff --git a/crates/network/src/virtualization.rs b/crates/network/src/virtualization.rs index 2302c727e9..3770d2fd28 100644 --- a/crates/network/src/virtualization.rs +++ b/crates/network/src/virtualization.rs @@ -232,11 +232,7 @@ mod sqlx_db_tests { impl fmt::Display for VpcVirtualizationType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::EthernetVirtualizer | Self::EthernetVirtualizerWithNvue => write!(f, "etv"), - Self::Fnn => write!(f, "fnn"), - Self::Flat => write!(f, "flat"), - } + f.write_str(self.as_str()) } } diff --git a/crates/rpc/build.rs b/crates/rpc/build.rs index aad53fb881..79fd25e5b1 100644 --- a/crates/rpc/build.rs +++ b/crates/rpc/build.rs @@ -928,6 +928,10 @@ fn main() -> Result<(), Box> { .type_attribute("forge.CreateComputeAllocationRequest", derive_prost_builder) .type_attribute("forge.UpdateComputeAllocationRequest", derive_prost_builder) .type_attribute("forge.DeleteComputeAllocationRequest", derive_prost_builder) + .type_attribute( + "forge.InstanceNetworkAutoConfig", + "#[derive(serde::Serialize)]", + ) .build_server(true) .build_client(true) .protoc_arg("--experimental_allow_proto3_optional") diff --git a/crates/rpc/proto/forge.proto b/crates/rpc/proto/forge.proto index 6c33dd7235..564023ea2f 100644 --- a/crates/rpc/proto/forge.proto +++ b/crates/rpc/proto/forge.proto @@ -2751,22 +2751,32 @@ message InstanceNetworkConfig { // interface configuration the caller wants applied. repeated InstanceInterfaceConfig interfaces = 1; - // When true, NICO (or potentially some pluggable SDN backend) will + // Deprecated. Set auto_config.vpc_id to the requested VPC ID instead. + bool auto = 2 [deprecated = true]; + + // When set, NICO (or potentially some pluggable SDN backend) will // auto-resolve the instance's network interfaces from the host's // HostInband network segments. Only valid for instances on zero-DPU // hosts (well, no DPU, *or* DPU in NIC mode). When set, `interfaces` // MUST be empty on the request. // - // Callers reading the instance back also see `auto = true` with an empty + // Callers reading the instance back also see `auto_config` set, with an empty // `interfaces` list (what they sent is preserved verbatim). The // resolved per-interface details -- IP, MAC, gateway, prefix -- surface // in `Instance.status.network.interfaces` instead. // - // On update, sending `auto: true` re-resolves interfaces from the host's + // On update, setting `auto_config` re-resolves interfaces from the host's // current HostInband segments. If nothing has changed, this is a no-op. // If the host's segments have been added/removed by the operator, the // update reflects that change. - bool auto = 2; + optional InstanceNetworkAutoConfig auto_config = 3; +} + +message InstanceNetworkAutoConfig { + // For Flat zero-DPU deployments, multiple VPCs may share the same + // HostInband network segment, so VPC ownership is carried by the + // instance address rather than by the segment. + optional common.VpcId vpc_id = 1; } // Desired infiniband configuration for an instance @@ -3193,6 +3203,9 @@ message InstanceInterfaceStatus { optional string device = 6; uint32 device_instance = 7; + + // The logical VPC this interface belongs to. + optional common.VpcId vpc_id = 8; } // The actual status of a single IB interface of an instance diff --git a/crates/rpc/src/model/instance/config/network.rs b/crates/rpc/src/model/instance/config/network.rs index 65e28dacce..8877c7f63e 100644 --- a/crates/rpc/src/model/instance/config/network.rs +++ b/crates/rpc/src/model/instance/config/network.rs @@ -20,12 +20,14 @@ use std::net::IpAddr; use itertools::Itertools; use model::instance::config::network::{ - DeviceLocator, InstanceInterfaceConfig, InstanceInterfaceRoutingProfile, InstanceNetworkConfig, - InterfaceFunctionId, InterfaceFunctionType, Ipv6InterfaceConfig, NetworkDetails, + DeviceLocator, InstanceInterfaceConfig, InstanceInterfaceRoutingProfile, + InstanceNetworkAutoConfig, InstanceNetworkConfig, InterfaceFunctionId, InterfaceFunctionType, + Ipv6InterfaceConfig, NetworkDetails, }; use crate as rpc; use crate::errors::RpcDataConversionError; +use crate::forge; impl TryFrom for InterfaceFunctionType { type Error = RpcDataConversionError; @@ -135,7 +137,7 @@ impl TryFrom for InstanceNetworkConfig { fn try_from(config: rpc::InstanceNetworkConfig) -> Result { // try_from for interfaces: - let auto = config.auto; + let auto = config.auto_config.is_some(); if auto && !config.interfaces.is_empty() { return Err(RpcDataConversionError::InvalidArgument( @@ -306,10 +308,26 @@ impl TryFrom for InstanceNetworkConfig { host_inband_mac_address: None, device_locator, internal_uuid: uuid::Uuid::new_v4(), + vpc_id: None, }); } - Ok(Self { interfaces, auto }) + Ok(Self { + interfaces, + auto_config: config.auto_config.map(TryInto::try_into).transpose()?, + }) + } +} + +impl TryFrom for InstanceNetworkAutoConfig { + type Error = RpcDataConversionError; + + fn try_from(value: forge::InstanceNetworkAutoConfig) -> Result { + Ok(Self { + vpc_id: value + .vpc_id + .ok_or(RpcDataConversionError::MissingArgument("vpc_id"))?, + }) } } @@ -321,7 +339,6 @@ impl TryFrom for rpc::InstanceNetworkConfig { // stripping resolved interfaces in the case of an auto config, // but leaving them untouched otherwise. let config = config.into_external_view(); - let auto = config.auto; let mut interfaces = Vec::with_capacity(config.interfaces.len()); for iface in config.interfaces.into_iter() { let function_type = iface.function_id.function_type(); @@ -369,7 +386,20 @@ impl TryFrom for rpc::InstanceNetworkConfig { }); } - Ok(rpc::InstanceNetworkConfig { interfaces, auto }) + Ok(rpc::InstanceNetworkConfig { + interfaces, + auto_config: config.auto_config.map(Into::into), + #[allow(deprecated)] + auto: config.auto_config.is_some(), + }) + } +} + +impl From for forge::InstanceNetworkAutoConfig { + fn from(value: InstanceNetworkAutoConfig) -> Self { + Self { + vpc_id: Some(value.vpc_id), + } } } @@ -422,7 +452,7 @@ mod tests { use carbide_test_support::Outcome::*; use carbide_test_support::{scenarios, value_scenarios}; use carbide_uuid::network::NetworkSegmentId; - use carbide_uuid::vpc::VpcPrefixId; + use carbide_uuid::vpc::{VpcId, VpcPrefixId}; use model::instance::config::network::{INTERFACE_VFID_MAX, INTERFACE_VFID_MIN}; use super::*; @@ -448,7 +478,9 @@ mod tests { ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let netconfig: InstanceNetworkConfig = config.try_into().unwrap(); @@ -467,6 +499,7 @@ mod tests { network_details: Some(NetworkDetails::NetworkSegment(BASE_SEGMENT_ID.into()),), device_locator: None, internal_uuid: netconfig.interfaces.first().unwrap().internal_uuid, + vpc_id: None, }] ); } @@ -500,7 +533,9 @@ mod tests { let config = rpc::InstanceNetworkConfig { interfaces, + #[allow(deprecated)] auto: false, + auto_config: None, }; let netconfig: InstanceNetworkConfig = config.try_into().unwrap(); let mut netconf_interfaces_iter = netconfig.interfaces.iter(); @@ -518,6 +553,7 @@ mod tests { network_details: Some(NetworkDetails::NetworkSegment(BASE_SEGMENT_ID.into())), device_locator: None, internal_uuid: netconf_interfaces_iter.next().unwrap().internal_uuid, + vpc_id: None, }]; for vfid in INTERFACE_VFID_MIN..=INTERFACE_VFID_MAX { @@ -535,6 +571,7 @@ mod tests { network_details: Some(NetworkDetails::NetworkSegment(segment_id)), device_locator: None, internal_uuid: netconf_interfaces_iter.next().unwrap().internal_uuid, + vpc_id: None, }); } assert_eq!(netconfig.interfaces, &expected_interfaces[..]); @@ -629,7 +666,8 @@ mod tests { run = |interfaces| { let network_config = rpc::InstanceNetworkConfig { interfaces, - auto: false, + #[allow(deprecated)] auto: false, + auto_config: None, }; let network_config = InstanceNetworkConfig::try_from(network_config).map_err(drop)?; @@ -720,8 +758,9 @@ mod tests { host_inband_mac_address: None, device_locator: None, internal_uuid: uuid::Uuid::new_v4(), + vpc_id: None, }], - auto: false, + auto_config: None, }; // Model -> RPC @@ -780,7 +819,9 @@ mod tests { }], }), }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let model: InstanceNetworkConfig = rpc_config.try_into().unwrap(); @@ -829,7 +870,9 @@ mod tests { }], }), }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let result: Result = rpc_config.try_into(); @@ -907,7 +950,8 @@ mod tests { run = |iface| { let rpc_config = rpc::InstanceNetworkConfig { interfaces: vec![iface], - auto: false, + #[allow(deprecated)] auto: false, + auto_config: None, }; InstanceNetworkConfig::try_from(rpc_config).is_ok() }; @@ -943,7 +987,9 @@ mod tests { ipv6_interface_config: None, routing_profile: None, }], + #[allow(deprecated)] auto: false, + auto_config: None, }; let model: InstanceNetworkConfig = rpc_config.try_into().unwrap(); assert_eq!( @@ -969,7 +1015,9 @@ mod tests { ipv6_interface_config: None, routing_profile: None, }], - auto: true, + #[allow(deprecated)] + auto: false, + auto_config: None, }; let result: Result = rpc_config.try_into(); let err = result.expect_err("auto + non-empty interfaces should be rejected"); @@ -982,15 +1030,20 @@ mod tests { #[test] fn test_auto_allows_empty_interfaces() { + let vpc_id = VpcId::new(); // Verify "auto" requests work. let rpc_config = rpc::InstanceNetworkConfig { interfaces: vec![], - auto: true, + #[allow(deprecated)] + auto: false, + auto_config: Some(forge::InstanceNetworkAutoConfig { + vpc_id: Some(vpc_id), + }), }; let model: InstanceNetworkConfig = rpc_config .try_into() .expect("auto + empty should round-trip"); - assert!(model.auto); assert!(model.interfaces.is_empty()); + assert_eq!(model.auto_config.unwrap().vpc_id, vpc_id); } } diff --git a/crates/rpc/src/model/instance/status/network.rs b/crates/rpc/src/model/instance/status/network.rs index 809430fa87..22cdef0920 100644 --- a/crates/rpc/src/model/instance/status/network.rs +++ b/crates/rpc/src/model/instance/status/network.rs @@ -71,6 +71,7 @@ impl TryFrom for rpc::InstanceInterfaceStatus { .collect(), device: status.device, device_instance: status.device_instance as u32, + vpc_id: status.vpc_id, }) } } diff --git a/crates/test-harness/src/network/controller.rs b/crates/test-harness/src/network/controller.rs index 5f5af6975c..76f61d91c6 100644 --- a/crates/test-harness/src/network/controller.rs +++ b/crates/test-harness/src/network/controller.rs @@ -19,7 +19,8 @@ use std::sync::Arc; use carbide_api_core::test_support::network_segment::{ FIXTURE_ADMIN_NETWORK_SEGMENT_GATEWAY, FIXTURE_HOST_INBAND_NETWORK_SEGMENT_GATEWAY, - FIXTURE_TENANT_NETWORK_SEGMENT_GATEWAYS, FIXTURE_UNDERLAY_NETWORK_SEGMENT_GATEWAY, + FIXTURE_TENANT_NETWORK_SEGMENT_GATEWAYS, FIXTURE_TENANT_ORG_ID, + FIXTURE_UNDERLAY_NETWORK_SEGMENT_GATEWAY, }; use carbide_api_core::test_support::rpc::forge::forge_server::Forge; use carbide_network_segment_controller::context::NetworkSegmentStateHandlerServices; @@ -167,7 +168,7 @@ impl TestNetworkController { let vpc = self .api .create_vpc(tonic::Request::new(rpc::forge::VpcCreationRequest { - tenant_organization_id: "2829bbe3-c169-4cd9-8b2a-19a8b1618a93".to_string(), + tenant_organization_id: FIXTURE_TENANT_ORG_ID.to_string(), network_virtualization_type: Some(rpc::forge::VpcVirtualizationType::Flat.into()), metadata: Some(rpc::forge::Metadata { name: "HOST_INBAND_FLAT".to_string(), @@ -218,7 +219,7 @@ impl TestNetworkController { let vpc = self .api .create_vpc( - rpc::forge::VpcCreationRequest::builder("2829bbe3-c169-4cd9-8b2a-19a8b1618a93") + rpc::forge::VpcCreationRequest::builder(FIXTURE_TENANT_ORG_ID) .metadata(rpc::forge::Metadata { name: name.to_string(), ..Default::default()