From 208de5892611531c6885fa21443fec7d248f8ed0 Mon Sep 17 00:00:00 2001 From: Ken Simon Date: Mon, 15 Jun 2026 16:38:05 -0400 Subject: [PATCH] feat(zero-dpu): Allow flat VPC's to not belong to a network segment In an environment without DPU's, we don't get dynamic network isolation, and the expectation is that there is a single, flat network address space, modeled as a single network segment. In this case, we still want to model VPC's for things like NVLink partitions, where tenants will have the understanding that the network is not isolated between VPC's. To make this work, a network segment can no longer be the "link" that links an instance to a VPC. Before, an instance interface maps to a machine interface, which maps to a network segment, which maps to a VPC. But if multiple VPC's are all sharing a network segment, this link doesn't exist (namely, network_segments.vpc_id will be null), and so we need the instance address itself to store the VPC ID on it. This change adds a vpc_id column to instance_addresses, so that we know the VPC of any given address without having to consult the network_segments table. It also changes the AllocateInstance API so that you can pass a `auto_config` object which allows specifying a VPC ID as part of the allocation request. (This replaces the `auto: bool` field, such that auto is implied if `auto_config` is set.) --- .../admin-cli/src/instance/allocate/args.rs | 11 +- crates/admin-cli/src/instance/allocate/cmd.rs | 4 +- crates/admin-cli/src/instance/show/cmd.rs | 201 ++++++----- crates/admin-cli/src/machine/show/cmd.rs | 5 +- crates/admin-cli/src/rpc.rs | 73 +++- crates/agent/src/tests/full.rs | 7 +- crates/api-core/src/cfg/file.rs | 3 +- .../api-core/src/ethernet_virtualization.rs | 8 +- crates/api-core/src/handlers/dpu.rs | 8 +- crates/api-core/src/handlers/instance.rs | 17 +- crates/api-core/src/instance/mod.rs | 138 +++++--- crates/api-core/src/setup.rs | 5 +- .../src/test_support/network_segment.rs | 17 +- .../api-core/src/tests/client_resolution.rs | 12 +- .../src/tests/common/api_fixtures/instance.rs | 8 + .../src/tests/common/api_fixtures/mod.rs | 10 +- .../common/api_fixtures/test_managed_host.rs | 12 + .../src/tests/common/network_segment.rs | 5 +- crates/api-core/src/tests/instance.rs | 105 +++++- .../api-core/src/tests/instance_allocate.rs | 312 +++++++++++++++--- .../src/tests/instance_config_update.rs | 30 ++ crates/api-core/src/tests/machine_dhcp.rs | 2 + crates/api-core/src/tests/machine_network.rs | 8 + .../src/tests/network_security_group.rs | 57 ++-- crates/api-core/src/tests/network_segment.rs | 20 +- crates/api-core/src/tests/vpc.rs | 7 +- crates/api-core/src/tests/vpc_find.rs | 6 +- crates/api-core/src/tests/vpc_peering.rs | 97 +++++- crates/api-core/src/tests/vpc_prefix.rs | 5 +- ...260615120000_instance_addresses_vpc_id.sql | 11 + crates/api-db/src/dpa_interface.rs | 6 +- crates/api-db/src/instance.rs | 4 +- crates/api-db/src/instance_address.rs | 53 ++- crates/api-db/src/instance_network_config.rs | 7 +- crates/api-db/src/network_security_group.rs | 12 +- crates/api-db/src/vpc.rs | 20 +- .../api-model/src/instance/config/network.rs | 77 ++++- .../api-model/src/instance/status/network.rs | 30 +- crates/api-model/src/instance_address.rs | 2 + crates/machine-a-tron/src/api_client.rs | 2 + crates/network/src/virtualization.rs | 6 +- crates/rpc/build.rs | 4 + crates/rpc/proto/forge.proto | 21 +- .../rpc/src/model/instance/config/network.rs | 79 ++++- .../rpc/src/model/instance/status/network.rs | 1 + crates/test-harness/src/network/controller.rs | 7 +- 46 files changed, 1162 insertions(+), 373 deletions(-) create mode 100644 crates/api-db/migrations/20260615120000_instance_addresses_vpc_id.sql 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()