From 70f8b396fc4dc74b21bfdf82f67be59aa9b18091 Mon Sep 17 00:00:00 2001 From: Zeeshan Lakhani Date: Sat, 7 Mar 2026 01:44:42 +0000 Subject: [PATCH 1/2] drop mvlan from multicast group API --- Cargo.lock | 6 +- Cargo.toml | 4 +- nexus/db-model/src/multicast_group.rs | 33 +---- nexus/db-model/src/schema_versions.rs | 3 +- .../src/db/datastore/multicast/groups.rs | 75 +++------- .../src/db/datastore/multicast/members.rs | 1 - .../src/db/pub_test_utils/multicast.rs | 1 - .../db/queries/external_multicast_group.rs | 14 +- nexus/db-schema/src/schema.rs | 1 - nexus/external-api/src/lib.rs | 64 ++++++++- .../app/background/tasks/multicast/groups.rs | 17 +-- nexus/src/app/multicast/dataplane.rs | 36 ++--- nexus/src/app/multicast/mod.rs | 33 +++-- .../app/sagas/multicast_group_dpd_ensure.rs | 1 - .../tests/integration_tests/multicast/api.rs | 80 ++++++----- nexus/types/src/multicast.rs | 7 - nexus/types/versions/src/impls/multicast.rs | 71 +--------- nexus/types/versions/src/latest.rs | 8 +- nexus/types/versions/src/lib.rs | 2 + .../versions/src/multicast_drop_mvlan/mod.rs | 12 ++ .../src/multicast_drop_mvlan/multicast.rs | 132 ++++++++++++++++++ .../nexus-2026030600.0.0-19c639.json.gitstub | 1 + ....json => nexus-2026031000.0.0-0e9242.json} | 16 +-- openapi/nexus/nexus-latest.json | 2 +- schema/crdb/dbinit.sql | 13 +- schema/crdb/multicast-drop-mvlan/up01.sql | 3 + schema/crdb/multicast-drop-mvlan/up02.sql | 2 + 27 files changed, 348 insertions(+), 290 deletions(-) create mode 100644 nexus/types/versions/src/multicast_drop_mvlan/mod.rs create mode 100644 nexus/types/versions/src/multicast_drop_mvlan/multicast.rs create mode 100644 openapi/nexus/nexus-2026030600.0.0-19c639.json.gitstub rename openapi/nexus/{nexus-2026030600.0.0-19c639.json => nexus-2026031000.0.0-0e9242.json} (99%) create mode 100644 schema/crdb/multicast-drop-mvlan/up01.sql create mode 100644 schema/crdb/multicast-drop-mvlan/up02.sql diff --git a/Cargo.lock b/Cargo.lock index 31e144f33ec..66e590ba60e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3160,8 +3160,7 @@ dependencies = [ [[package]] name = "dropshot-api-manager" version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c38a99616a680fafd8b7a03cc300aab80b0bbf0fb17c6baac53431d9d12b64b5" +source = "git+https://github.com/oxidecomputer/dropshot-api-manager?branch=sunshowers%2Fspr%2Fdropshot-api-manager-add-support-for-non-colocated-jj-repos#4324b26963c07991f265aa899f1b98f4bfbacb07" dependencies = [ "anyhow", "atomicwrites", @@ -3193,8 +3192,7 @@ dependencies = [ [[package]] name = "dropshot-api-manager-types" version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f5116e3d69b975bb89824a721d0326707b8643e4fd3ba69ca880de1aada768cf" +source = "git+https://github.com/oxidecomputer/dropshot-api-manager?branch=sunshowers%2Fspr%2Fdropshot-api-manager-add-support-for-non-colocated-jj-repos#4324b26963c07991f265aa899f1b98f4bfbacb07" dependencies = [ "anyhow", "camino", diff --git a/Cargo.toml b/Cargo.toml index 17e9dd6acf4..a4d5305fdc9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1040,8 +1040,8 @@ opt-level = 3 # drift = { path = "../drift" } # dropshot = { path = "../dropshot/dropshot" } # dropshot_endpoint = { path = "../dropshot/dropshot_endpoint" } -# dropshot-api-manager = { path = "../dropshot-api-manager/crates/dropshot-api-manager" } -# dropshot-api-manager-types = { path = "../dropshot-api-manager/crates/dropshot-api-manager-types" } +dropshot-api-manager = { git = "https://github.com/oxidecomputer/dropshot-api-manager", branch = "sunshowers/spr/dropshot-api-manager-add-support-for-non-colocated-jj-repos" } +dropshot-api-manager-types = { git = "https://github.com/oxidecomputer/dropshot-api-manager", branch = "sunshowers/spr/dropshot-api-manager-add-support-for-non-colocated-jj-repos" } # progenitor = { path = "../progenitor/progenitor" } # progenitor-client = { path = "../progenitor/progenitor-client" } # steno = { path = "../steno" } diff --git a/nexus/db-model/src/multicast_group.rs b/nexus/db-model/src/multicast_group.rs index a6bd415138a..ed23b3a650b 100644 --- a/nexus/db-model/src/multicast_group.rs +++ b/nexus/db-model/src/multicast_group.rs @@ -178,28 +178,6 @@ pub struct ExternalMulticastGroup { pub ip_pool_range_id: Uuid, /// Primary multicast IP address (overlay/external). pub multicast_ip: IpNetwork, - /// Multicast VLAN (MVLAN) for egress multicast traffic to upstream networks. - /// - /// When specified, this VLAN ID is passed to switches (via DPD) as part of - /// the `ExternalForwarding` configuration to tag multicast packets leaving - /// the rack. This enables multicast traffic to traverse VLAN-segmented - /// upstream networks (e.g., peering with external multicast sources/receivers - /// on specific VLANs). - /// - /// The MVLAN value is sent to switches during group creation/updates and - /// controls VLAN tagging for egress traffic only; it does not affect ingress - /// multicast traffic received by the rack. Switch port selection for egress - /// traffic remains pending (see TODOs in `nexus/src/app/multicast/dataplane.rs`). - /// - /// Valid range when specified: 2-4094 (IEEE 802.1Q; Dendrite requires >= 2). - /// - /// Database Type: i16 (INT2) - this field uses `i16` (INT2) for storage - /// efficiency, unlike other VLAN columns in the schema which use `SqlU16` - /// (forcing INT4). Direct `i16` is appropriate here since VLANs fit in - /// INT2's range. - /// - /// TODO(multicast): Remove mvlan field - being deprecated from multicast groups - pub mvlan: Option, /// Associated underlay group for NAT. /// Initially None in ["Creating"](MulticastGroupState::Creating) state, /// populated by reconciler when group becomes ["Active"](MulticastGroupState::Active). @@ -248,11 +226,11 @@ pub struct MulticastGroupMemberValues { pub parent_id: Uuid, pub sled_id: Option>, pub state: MulticastGroupMemberState, - // version_added and version_removed are omitted - database assigns these - // via DEFAULT nextval() pub multicast_ip: IpNetwork, /// Source IPs for source-filtered multicast (optional for ASM, required for SSM). pub source_ips: Vec, + // version_added and version_removed are omitted - database assigns these + // via DEFAULT nextval() } /// A member of a multicast group (instance that receives multicast traffic). @@ -329,9 +307,7 @@ impl TryFrom for multicast_types::MulticastGroupMember { /// An incomplete external multicast group, used to store state required for /// issuing the database query that selects an available multicast IP and stores -/// the resulting record. -/// -/// Note: tag is computed in SQL as `{uuid}:{multicast_ip}`. +/// the resulting record. Tag is computed in SQL as `{uuid}:{multicast_ip}`. #[derive(Clone, Debug, PartialEq, Eq)] pub struct IncompleteExternalMulticastGroup { pub id: Uuid, @@ -341,7 +317,6 @@ pub struct IncompleteExternalMulticastGroup { pub ip_pool_id: Uuid, /// Optional address requesting a specific multicast IP be allocated. pub explicit_address: Option, - pub mvlan: Option, pub vni: Vni, } @@ -353,7 +328,6 @@ pub struct IncompleteExternalMulticastGroupParams { pub description: String, pub ip_pool_id: Uuid, pub explicit_address: Option, - pub mvlan: Option, pub vni: Vni, } @@ -367,7 +341,6 @@ impl IncompleteExternalMulticastGroup { time_created: Utc::now(), ip_pool_id: params.ip_pool_id, explicit_address: params.explicit_address.map(|ip| ip.into()), - mvlan: params.mvlan, vni: params.vni, } } diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 28d548d9ca2..5e7e47c655f 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock}; /// /// This must be updated when you change the database schema. Refer to /// schema/crdb/README.adoc in the root of this repository for details. -pub const SCHEMA_VERSION: Version = Version::new(237, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(238, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock> = LazyLock::new(|| { // | leaving the first copy as an example for the next person. // v // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"), + KnownVersion::new(238, "multicast-drop-mvlan"), KnownVersion::new(237, "switch-slot-enum"), KnownVersion::new( 236, diff --git a/nexus/db-queries/src/db/datastore/multicast/groups.rs b/nexus/db-queries/src/db/datastore/multicast/groups.rs index b2ea63aadcd..0fb1b6e1e2b 100644 --- a/nexus/db-queries/src/db/datastore/multicast/groups.rs +++ b/nexus/db-queries/src/db/datastore/multicast/groups.rs @@ -38,7 +38,6 @@ use omicron_common::api::external::{ IdentityMetadataCreateParams, IpVersion, ListResultVec, LookupResult, LookupType, ResourceType, UpdateResult, }; -use omicron_common::vlan::VlanID; use omicron_uuid_kinds::{GenericUuid, MulticastGroupUuid}; use super::EnsureUnderlayResult; @@ -54,44 +53,24 @@ use crate::db::pagination::paginated; use crate::db::queries::external_multicast_group::NextExternalMulticastGroup; use crate::db::update_and_check::{UpdateAndCheck, UpdateStatus}; -/// External multicast group with computed source IPs from members. -/// -/// The `source_ips` field contains the union of all member source IPs, -/// computed via a separate query. This struct enables a clean `TryFrom` -/// conversion to the API view. -/// -// TODO(multicast): Remove mvlan field, being deprecated from multicast groups +/// External multicast group with computed source filter state from members. #[derive(Clone, Debug)] pub struct ExternalMulticastGroupWithSources { pub group: ExternalMulticastGroup, pub source_ips: Vec, + pub has_any_source_member: bool, } -impl TryFrom for views::MulticastGroup { - type Error = external::Error; - - fn try_from( - value: ExternalMulticastGroupWithSources, - ) -> Result { - let mvlan = value - .group - .mvlan - .map(|vlan| VlanID::new(vlan as u16)) - .transpose() - .map_err(|e| { - external::Error::internal_error(&format!( - "invalid VLAN ID: {e:#}" - )) - })?; - - Ok(views::MulticastGroup { +impl From for views::MulticastGroup { + fn from(value: ExternalMulticastGroupWithSources) -> Self { + views::MulticastGroup { identity: value.group.identity(), multicast_ip: value.group.multicast_ip.ip(), source_ips: value.source_ips, - mvlan, + has_any_source_member: value.has_any_source_member, ip_pool_id: value.group.ip_pool_id, state: value.group.state.to_string(), - }) + } } } @@ -104,7 +83,6 @@ pub(crate) struct MulticastGroupAllocationParams { pub identity: IdentityMetadataCreateParams, /// How to allocate the multicast IP address. pub ip_allocation: MulticastIpAllocation, - pub mvlan: Option, /// Derived for whether the joining member has source IPs. /// Used for default pool selection -> if true, prefer SSM pool first. pub has_sources: bool, @@ -283,7 +261,6 @@ impl DataStore { MulticastGroupAllocationParams { identity: params.identity.clone(), ip_allocation, - mvlan: params.mvlan, has_sources: params.has_sources, }, ) @@ -344,12 +321,21 @@ impl DataStore { let filter_state_map = self .multicast_groups_source_filter_state(opctx, &[group_id]) .await?; - let source_ips = filter_state_map + let (source_ips, has_any_source_member) = filter_state_map .get(&group_id.into_untyped_uuid()) - .map(|state| state.specific_sources.iter().copied().collect()) + .map(|state| { + ( + state.specific_sources.iter().copied().collect(), + state.has_any_source_member, + ) + }) .unwrap_or_default(); - Ok(ExternalMulticastGroupWithSources { group, source_ips }) + Ok(ExternalMulticastGroupWithSources { + group, + source_ips, + has_any_source_member, + }) } /// Lookup an external multicast group by IP address. @@ -643,7 +629,6 @@ impl DataStore { description: params.identity.description.clone(), ip_pool_id: authz_pool.id(), explicit_address: explicit_ip, - mvlan: params.mvlan.map(|vlan_id| u16::from(vlan_id) as i16), vni, }, ); @@ -1013,7 +998,6 @@ mod tests { description: "First group".to_string(), }, multicast_ip: None, - mvlan: None, has_sources: false, ip_version: None, }; @@ -1031,7 +1015,6 @@ mod tests { description: "Second group".to_string(), }, multicast_ip: None, - mvlan: None, has_sources: false, ip_version: None, }; @@ -1049,7 +1032,6 @@ mod tests { description: "Should fail".to_string(), }, multicast_ip: None, - mvlan: None, has_sources: false, ip_version: None, }; @@ -1078,7 +1060,6 @@ mod tests { description: "Should reuse freed IP".to_string(), }, multicast_ip: None, - mvlan: None, has_sources: false, ip_version: None, }; @@ -1161,7 +1142,6 @@ mod tests { description: "Group using default pool".to_string(), }, multicast_ip: None, - mvlan: None, has_sources: false, ip_version: None, }; @@ -1187,7 +1167,6 @@ mod tests { description: "Second group from default pool".to_string(), }, multicast_ip: None, - mvlan: None, has_sources: false, ip_version: None, }; @@ -1315,7 +1294,6 @@ mod tests { description: "Comprehensive test group".to_string(), }, multicast_ip: Some("224.1.3.3".parse().unwrap()), - mvlan: None, has_sources: false, ip_version: None, }; @@ -1416,7 +1394,6 @@ mod tests { description: "Group for IP reuse test".to_string(), }, multicast_ip: Some(target_ip), - mvlan: None, has_sources: false, ip_version: None, }; @@ -1444,7 +1421,6 @@ mod tests { description: "Second group reusing same IP".to_string(), }, multicast_ip: Some(target_ip), - mvlan: None, has_sources: false, ip_version: None, }; @@ -1525,7 +1501,6 @@ mod tests { description: "Group for deallocation testing".to_string(), }, multicast_ip: None, - mvlan: None, has_sources: false, ip_version: None, }; @@ -1650,7 +1625,6 @@ mod tests { description: "Test group for fetch operations".to_string(), }, multicast_ip: Some("224.100.10.5".parse().unwrap()), - mvlan: None, has_sources: false, ip_version: None, }; @@ -1757,7 +1731,6 @@ mod tests { description: "Fleet-wide group 1".to_string(), }, multicast_ip: Some("224.100.20.10".parse().unwrap()), - mvlan: None, has_sources: false, ip_version: None, }; @@ -1768,7 +1741,6 @@ mod tests { description: "Fleet-wide group 2".to_string(), }, multicast_ip: Some("224.100.20.11".parse().unwrap()), - mvlan: None, has_sources: false, ip_version: None, }; @@ -1779,7 +1751,6 @@ mod tests { description: "Fleet-wide group 3".to_string(), }, multicast_ip: Some("224.100.20.12".parse().unwrap()), - mvlan: None, has_sources: false, ip_version: None, }; @@ -1888,7 +1859,6 @@ mod tests { description: "Test group for state transitions".to_string(), }, multicast_ip: Some("224.100.30.5".parse().unwrap()), - mvlan: None, has_sources: false, ip_version: None, }; @@ -2225,7 +2195,6 @@ mod tests { description: "Group using ASM pool".to_string(), }, multicast_ip: None, // No explicit IP -> triggers pool auto-selection - mvlan: None, has_sources: false, ip_version: None, }; @@ -2316,7 +2285,6 @@ mod tests { description: "Should fall back to ASM when no SSM".to_string(), }, multicast_ip: None, - mvlan: None, has_sources: true, ip_version: None, }; @@ -2389,7 +2357,6 @@ mod tests { description: "Should prefer SSM over ASM".to_string(), }, multicast_ip: None, - mvlan: None, has_sources: true, ip_version: None, }; @@ -2413,7 +2380,6 @@ mod tests { description: "has_sources=false should use ASM".to_string(), }, multicast_ip: None, - mvlan: None, has_sources: false, ip_version: None, }; @@ -2493,7 +2459,6 @@ mod tests { description: "First group for collision test".to_string(), }, multicast_ip: Some("224.10.1.1".parse().unwrap()), - mvlan: None, has_sources: false, ip_version: None, }; @@ -2509,7 +2474,6 @@ mod tests { description: "Second group for collision test".to_string(), }, multicast_ip: Some("224.10.1.2".parse().unwrap()), - mvlan: None, has_sources: false, ip_version: None, }; @@ -2633,7 +2597,6 @@ mod tests { description: "Group for salt testing".to_string(), }, multicast_ip: Some("224.20.1.1".parse().unwrap()), - mvlan: None, has_sources: false, ip_version: None, }; diff --git a/nexus/db-queries/src/db/datastore/multicast/members.rs b/nexus/db-queries/src/db/datastore/multicast/members.rs index a9ecce508e4..1c2d25a703b 100644 --- a/nexus/db-queries/src/db/datastore/multicast/members.rs +++ b/nexus/db-queries/src/db/datastore/multicast/members.rs @@ -929,7 +929,6 @@ mod tests { description: "Creating test group".to_string(), }, multicast_ip: Some("224.10.1.6".parse().unwrap()), - mvlan: None, has_sources: false, ip_version: None, }; diff --git a/nexus/db-queries/src/db/pub_test_utils/multicast.rs b/nexus/db-queries/src/db/pub_test_utils/multicast.rs index 968a4daeb0a..ba576bb23cd 100644 --- a/nexus/db-queries/src/db/pub_test_utils/multicast.rs +++ b/nexus/db-queries/src/db/pub_test_utils/multicast.rs @@ -196,7 +196,6 @@ pub async fn create_test_group_with_state( description: format!("Test group: {group_name}"), }, multicast_ip: Some(multicast_ip.parse().unwrap()), - mvlan: None, has_sources: false, ip_version: None, }; diff --git a/nexus/db-queries/src/db/queries/external_multicast_group.rs b/nexus/db-queries/src/db/queries/external_multicast_group.rs index 2053e9de93a..1202fab4cec 100644 --- a/nexus/db-queries/src/db/queries/external_multicast_group.rs +++ b/nexus/db-queries/src/db/queries/external_multicast_group.rs @@ -106,10 +106,6 @@ impl NextExternalMulticastGroup { // The multicast IP comes from the candidates subquery out.push_sql("candidate_ip AS multicast_ip, "); - // MVLAN for external uplink forwarding - out.push_bind_param::, Option>(&self.group.mvlan)?; - out.push_sql(" AS mvlan, "); - out.push_bind_param::, Option>(&None)?; out.push_sql(" AS underlay_group_id, "); @@ -268,10 +264,10 @@ impl QueryFragment for NextExternalMulticastGroup { out.push_sql("INSERT INTO "); schema::multicast_group::table.walk_ast(out.reborrow())?; out.push_sql( - " (id, name, description, time_created, time_modified, time_deleted, vni, ip_pool_id, ip_pool_range_id, multicast_ip, mvlan, underlay_group_id, tag, state, version_added, version_removed, underlay_salt) - SELECT id, name, description, time_created, time_modified, time_deleted, vni, ip_pool_id, ip_pool_range_id, multicast_ip, mvlan, underlay_group_id, tag, state, version_added, version_removed, underlay_salt FROM next_external_multicast_group + " (id, name, description, time_created, time_modified, time_deleted, vni, ip_pool_id, ip_pool_range_id, multicast_ip, underlay_group_id, tag, state, version_added, version_removed, underlay_salt) + SELECT id, name, description, time_created, time_modified, time_deleted, vni, ip_pool_id, ip_pool_range_id, multicast_ip, underlay_group_id, tag, state, version_added, version_removed, underlay_salt FROM next_external_multicast_group WHERE NOT EXISTS (SELECT 1 FROM previously_allocated_group) - RETURNING id, name, description, time_created, time_modified, time_deleted, vni, ip_pool_id, ip_pool_range_id, multicast_ip, mvlan, underlay_group_id, tag, state, version_added, version_removed, underlay_salt", + RETURNING id, name, description, time_created, time_modified, time_deleted, vni, ip_pool_id, ip_pool_range_id, multicast_ip, underlay_group_id, tag, state, version_added, version_removed, underlay_salt", ); out.push_sql("), "); @@ -282,9 +278,9 @@ impl QueryFragment for NextExternalMulticastGroup { // Return either the newly inserted or previously allocated group out.push_sql( - "SELECT id, name, description, time_created, time_modified, time_deleted, vni, ip_pool_id, ip_pool_range_id, multicast_ip, mvlan, underlay_group_id, tag, state, version_added, version_removed, underlay_salt FROM previously_allocated_group + "SELECT id, name, description, time_created, time_modified, time_deleted, vni, ip_pool_id, ip_pool_range_id, multicast_ip, underlay_group_id, tag, state, version_added, version_removed, underlay_salt FROM previously_allocated_group UNION ALL - SELECT id, name, description, time_created, time_modified, time_deleted, vni, ip_pool_id, ip_pool_range_id, multicast_ip, mvlan, underlay_group_id, tag, state, version_added, version_removed, underlay_salt FROM multicast_group", + SELECT id, name, description, time_created, time_modified, time_deleted, vni, ip_pool_id, ip_pool_range_id, multicast_ip, underlay_group_id, tag, state, version_added, version_removed, underlay_salt FROM multicast_group", ); Ok(()) diff --git a/nexus/db-schema/src/schema.rs b/nexus/db-schema/src/schema.rs index 623eb6e4624..2157e7669e6 100644 --- a/nexus/db-schema/src/schema.rs +++ b/nexus/db-schema/src/schema.rs @@ -2903,7 +2903,6 @@ table! { ip_pool_id -> Uuid, ip_pool_range_id -> Uuid, multicast_ip -> Inet, - mvlan -> Nullable, underlay_group_id -> Nullable, tag -> Nullable, state -> crate::enums::MulticastGroupStateEnum, diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index 0d1a5595094..4749d9a9ad1 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -78,6 +78,7 @@ api_versions!([ // | date-based version should be at the top of the list. // v // (next_yyyy_mm_dd_nn, IDENT), + (2026_03_10_00, MULTICAST_DROP_MVLAN), (2026_03_06_00, RENAME_SWITCH_LOCATION_TO_SWITCH_SLOT), (2026_03_02_00, ADD_TIME_FIELDS_TO_USERS), (2026_02_25_00, SET_TARGET_RELEASE_UPDATE_RECOVERY), @@ -2894,7 +2895,7 @@ pub trait NexusExternalApi { method = GET, path = "/v1/multicast-groups", tags = ["experimental"], - versions = VERSION_MULTICAST_IMPLICIT_LIFECYCLE_UPDATES.., + versions = VERSION_MULTICAST_DROP_MVLAN.., }] async fn multicast_group_list( rqctx: RequestContext, @@ -2905,18 +2906,20 @@ pub trait NexusExternalApi { >; /// List multicast groups + /// + /// Response includes the mvlan field (always null) #[endpoint { method = GET, path = "/v1/multicast-groups", tags = ["experimental"], operation_id = "multicast_group_list", - versions = ..VERSION_MULTICAST_IMPLICIT_LIFECYCLE_UPDATES, + versions = VERSION_MULTICAST_IMPLICIT_LIFECYCLE_UPDATES..VERSION_MULTICAST_DROP_MVLAN, }] - async fn multicast_group_list_v2025_11_20_00( + async fn multicast_group_list_v2026_01_08_00( rqctx: RequestContext, query_params: Query, ) -> Result< - HttpResponseOk>, + HttpResponseOk>, HttpError, > { Self::multicast_group_list(rqctx, query_params).await.map( @@ -2929,6 +2932,31 @@ pub trait NexusExternalApi { ) } + /// List multicast groups + #[endpoint { + method = GET, + path = "/v1/multicast-groups", + tags = ["experimental"], + operation_id = "multicast_group_list", + versions = ..VERSION_MULTICAST_IMPLICIT_LIFECYCLE_UPDATES, + }] + async fn multicast_group_list_v2025_11_20_00( + rqctx: RequestContext, + query_params: Query, + ) -> Result< + HttpResponseOk>, + HttpError, + > { + Self::multicast_group_list_v2026_01_08_00(rqctx, query_params) + .await + .map(|HttpResponseOk(page)| { + HttpResponseOk(ResultsPage { + items: page.items.into_iter().map(Into::into).collect(), + next_page: page.next_page, + }) + }) + } + /// Create a multicast group /// /// Deprecated: Groups are created implicitly when adding members in newer @@ -2965,13 +2993,35 @@ pub trait NexusExternalApi { method = GET, path = "/v1/multicast-groups/{multicast_group}", tags = ["experimental"], - versions = VERSION_MULTICAST_IMPLICIT_LIFECYCLE_UPDATES.., + versions = VERSION_MULTICAST_DROP_MVLAN.., }] async fn multicast_group_view( rqctx: RequestContext, path_params: Path, ) -> Result, HttpError>; + /// Fetch multicast group + /// + /// Response includes the mvlan field (always null) + #[endpoint { + method = GET, + path = "/v1/multicast-groups/{multicast_group}", + tags = ["experimental"], + operation_id = "multicast_group_view", + versions = VERSION_MULTICAST_IMPLICIT_LIFECYCLE_UPDATES..VERSION_MULTICAST_DROP_MVLAN, + }] + async fn multicast_group_view_v2026_01_08_00( + rqctx: RequestContext, + path_params: Path, + ) -> Result< + HttpResponseOk, + HttpError, + > { + Self::multicast_group_view(rqctx, path_params) + .await + .map(|resp| resp.map(Into::into)) + } + /// Fetch a multicast group /// /// The group can be specified by name or UUID. @@ -2990,7 +3040,7 @@ pub trait NexusExternalApi { HttpError, > { let path = path_params.map(Into::into); - Self::multicast_group_view(rqctx, path) + Self::multicast_group_view_v2026_01_08_00(rqctx, path) .await .map(|resp| resp.map(Into::into)) } @@ -3172,7 +3222,7 @@ pub trait NexusExternalApi { HttpError, > { let path = path_params.map(Into::into); - Self::multicast_group_view(rqctx, path) + Self::multicast_group_view_v2026_01_08_00(rqctx, path) .await .map(|resp| resp.map(Into::into)) } diff --git a/nexus/src/app/background/tasks/multicast/groups.rs b/nexus/src/app/background/tasks/multicast/groups.rs index 933acc91242..db2c51938a5 100644 --- a/nexus/src/app/background/tasks/multicast/groups.rs +++ b/nexus/src/app/background/tasks/multicast/groups.rs @@ -163,15 +163,6 @@ fn dpd_state_matches_sources( } } -/// Check if DPD vlan_id matches database mvlan. -fn dpd_state_matches_mvlan( - dpd_group: &dpd_client::types::MulticastGroupExternalResponse, - db_group: &MulticastGroup, -) -> bool { - let db_mvlan = db_group.mvlan.map(|v| v as u16); - dpd_group.external_forwarding.vlan_id == db_mvlan -} - /// Trait for processing different types of multicast groups trait GroupStateProcessor { /// Process a group in "Creating" state. @@ -685,10 +676,8 @@ impl MulticastGroupReconciler { &source_filter, group, ); - let mvlan_matches = dpd_state_matches_mvlan(&dpd_group, group); - let needs_update = - !tag_matches || !sources_match || !mvlan_matches; + let needs_update = !tag_matches || !sources_match; if needs_update { debug!( @@ -696,8 +685,7 @@ impl MulticastGroupReconciler { "detected DPD state mismatch for active group"; "group_id" => %group.id(), "tag_matches" => tag_matches, - "sources_match" => sources_match, - "mvlan_matches" => mvlan_matches + "sources_match" => sources_match ); } @@ -969,7 +957,6 @@ mod tests { ip_pool_range_id: Uuid::new_v4(), vni: Vni(omicron_common::api::external::Vni::DEFAULT_MULTICAST_VNI), multicast_ip: multicast_ip.parse().unwrap(), - mvlan: None, underlay_group_id: None, underlay_salt: None, tag: Some("test-tag".to_string()), diff --git a/nexus/src/app/multicast/dataplane.rs b/nexus/src/app/multicast/dataplane.rs index 4dc55ba5e11..8c56690c1eb 100644 --- a/nexus/src/app/multicast/dataplane.rs +++ b/nexus/src/app/multicast/dataplane.rs @@ -61,7 +61,6 @@ use nexus_db_queries::db::datastore::multicast::members::SourceFilterState; use nexus_types::identity::Resource; use omicron_common::address::is_ssm_address; use omicron_common::api::external::Error; -use omicron_common::vlan::VlanID; use sled_agent_types::early_networking::SwitchSlot; use crate::app::dpd_clients; @@ -398,15 +397,6 @@ impl MulticastDataplaneClient { Error::internal_error("multicast group missing tag") })?; - // Convert MVLAN to u16 for DPD, validating through VlanID - let vlan_id = external_group - .mvlan - .map(|v| VlanID::new(v as u16)) - .transpose() - .map_err(|e| { - Error::internal_error(&format!("invalid VLAN ID: {e:#}")) - })? - .map(u16::from); let underlay_ip_admin = underlay_group.multicast_ip.ip().into_underlay_multicast()?; let underlay_ipv6 = match underlay_group.multicast_ip.ip() { @@ -468,9 +458,14 @@ impl MulticastDataplaneClient { ) .await?; + // TODO: `vlan_id` is `None` because egress VLAN tagging is not + // yet supported. When egress support is added, this should + // be populated from group configuration. let external_entry = MulticastGroupCreateExternalEntry { group_ip: external_group_ip, - external_forwarding: ExternalForwarding { vlan_id }, + external_forwarding: ExternalForwarding { + vlan_id: None, + }, internal_forwarding: InternalForwarding { nat_target: Some(nat_target), }, @@ -556,16 +551,6 @@ impl MulticastDataplaneClient { let dpd_clients = &self.dpd_clients; // Pre-compute shared data once - // Convert MVLAN to u16 for DPD, validating through VlanID - let vlan_id = params - .external_group - .mvlan - .map(|v| VlanID::new(v as u16)) - .transpose() - .map_err(|e| { - Error::internal_error(&format!("invalid VLAN ID: {e:#}")) - })? - .map(u16::from); let underlay_ip_admin = params .underlay_group .multicast_ip @@ -698,8 +683,13 @@ impl MulticastDataplaneClient { Error::internal_error("failed to update underlay") })?; - // Prepare external update/create entries with pre-computed data - let external_forwarding = ExternalForwarding { vlan_id }; + // Prepare external update/create entries with pre-computed data. + // + // TODO: `vlan_id` is `None` because egress VLAN tagging is not + // yet supported. When egress support is added, this should + // be populated from group configuration. + let external_forwarding = + ExternalForwarding { vlan_id: None }; let internal_forwarding = InternalForwarding { nat_target: Some(nat_target) }; diff --git a/nexus/src/app/multicast/mod.rs b/nexus/src/app/multicast/mod.rs index c5f6a066839..629d1253c89 100644 --- a/nexus/src/app/multicast/mod.rs +++ b/nexus/src/app/multicast/mod.rs @@ -232,12 +232,22 @@ impl super::Nexus { .db_datastore .multicast_groups_source_filter_state(opctx, &[group_id]) .await?; - let source_ips = filter_state_map + let (source_ips, has_any_source_member) = filter_state_map .get(&group.identity.id) - .map(|state| state.specific_sources.iter().copied().collect()) + .map(|state| { + ( + state.specific_sources.iter().copied().collect(), + state.has_any_source_member, + ) + }) .unwrap_or_default(); - ExternalMulticastGroupWithSources { group, source_ips }.try_into() + Ok(ExternalMulticastGroupWithSources { + group, + source_ips, + has_any_source_member, + } + .into()) } /// List all multicast groups with full view. @@ -268,14 +278,21 @@ impl super::Nexus { groups .into_iter() .map(|group| { - let source_ips = filter_state_map + let (source_ips, has_any_source_member) = filter_state_map .get(&group.identity.id) .map(|state| { - state.specific_sources.iter().copied().collect() + ( + state.specific_sources.iter().copied().collect(), + state.has_any_source_member, + ) }) .unwrap_or_default(); - ExternalMulticastGroupWithSources { group, source_ips } - .try_into() + Ok(ExternalMulticastGroupWithSources { + group, + source_ips, + has_any_source_member, + } + .into()) }) .collect() } @@ -419,7 +436,6 @@ impl super::Nexus { ), }, multicast_ip: Some(ip), - mvlan: None, has_sources, // IP version is determined by the multicast IP address itself ip_version: None, @@ -500,7 +516,6 @@ impl super::Nexus { .to_string(), }, multicast_ip: None, - mvlan: None, has_sources, ip_version, }; diff --git a/nexus/src/app/sagas/multicast_group_dpd_ensure.rs b/nexus/src/app/sagas/multicast_group_dpd_ensure.rs index 4956f6379f4..17c1fc2b3a1 100644 --- a/nexus/src/app/sagas/multicast_group_dpd_ensure.rs +++ b/nexus/src/app/sagas/multicast_group_dpd_ensure.rs @@ -502,7 +502,6 @@ mod test { description: "Test saga state validation".to_string(), }, multicast_ip: Some(IpAddr::V4(Ipv4Addr::new(224, 70, 0, 100))), - mvlan: None, has_sources: false, ip_version: None, }; diff --git a/nexus/tests/integration_tests/multicast/api.rs b/nexus/tests/integration_tests/multicast/api.rs index 1e96b7bbf18..7c5a4193cf6 100644 --- a/nexus/tests/integration_tests/multicast/api.rs +++ b/nexus/tests/integration_tests/multicast/api.rs @@ -411,6 +411,10 @@ async fn test_join_by_ip_asm(cptestctx: &ControlPlaneTestContext) { assert_eq!(group.multicast_ip.to_string(), asm_ip); assert_eq!(group.ip_pool_id, asm_pool.identity.id); assert!(group.source_ips.is_empty()); + assert!( + group.has_any_source_member, + "ASM group with any-source member should have has_any_source_member=true" + ); } // Test SSM join-by-IP (sources required) @@ -434,6 +438,10 @@ async fn test_join_by_ip_asm(cptestctx: &ControlPlaneTestContext) { let group = wait_for_group_active(client, &ssm_group_name).await; assert_eq!(group.ip_pool_id, ssm_pool.identity.id); assert_eq!(group.source_ips, vec![source_ip]); + assert!( + !group.has_any_source_member, + "SSM group should have has_any_source_member=false (sources required)" + ); } // Cleanup both @@ -845,7 +853,10 @@ async fn test_join_by_ip_existing_group(cptestctx: &ControlPlaneTestContext) { /// Test that ASM groups can optionally have source IPs (IGMPv3/MLDv2 filtering). /// /// Unlike SSM where sources are required, ASM addresses allow optional source -/// filtering. The group's `source_ips` field shows the union of all member sources. +/// filtering. This test verifies: +/// - First member joins with sources, has_any_source_member=false +/// - Second member joins without sources, has_any_source_member=true +/// - Group source_ips shows union of all member sources #[nexus_test] async fn test_join_by_ip_asm_with_sources_succeeds( cptestctx: &ControlPlaneTestContext, @@ -877,61 +888,64 @@ async fn test_join_by_ip_asm_with_sources_succeeds( let expected_group_name = format!("mcast-{}", explicit_ip.to_string().replace('.', "-")); - // First instance joins ASM group without sources + // First instance joins ASM group with sources + let source1: IpAddr = "10.99.99.1".parse().unwrap(); + let source2: IpAddr = "10.99.99.2".parse().unwrap(); let join_url1 = format!( "/v1/instances/{}/multicast-groups/{explicit_ip}?project={project_name}", instance1.identity.name ); - put_upsert::<_, MulticastGroupMember>( - client, - &join_url1, - &InstanceMulticastGroupJoin::default(), - ) - .await; + let join_body1 = InstanceMulticastGroupJoin { + source_ips: Some(vec![source1, source2]), + ip_version: None, + }; + put_upsert::<_, MulticastGroupMember>(client, &join_url1, &join_body1) + .await; - wait_for_group_active(client, &expected_group_name).await; + let group = wait_for_group_active(client, &expected_group_name).await; - // Verify group has no source_ips initially - let group: MulticastGroup = object_get( - client, - &format!("/v1/multicast-groups/{expected_group_name}"), - ) - .await; + // All members have sources, so has_any_source_member=false assert!( - group.source_ips.is_empty(), - "ASM group with no-source member should have empty source_ips" + !group.has_any_source_member, + "ASM group where all members specify sources should have has_any_source_member=false" ); + let mut actual_sources = group.source_ips.clone(); + actual_sources.sort(); + let mut expected_sources = vec![source1, source2]; + expected_sources.sort(); + assert_eq!(actual_sources, expected_sources); - // Second instance joins the same ASM group with sources (valid for ASM) + // Second instance joins without sources (any-source subscription) let join_url2 = format!( "/v1/instances/{}/multicast-groups/{explicit_ip}?project={project_name}", instance2.identity.name ); - let source1: IpAddr = "10.99.99.1".parse().unwrap(); - let source2: IpAddr = "10.99.99.2".parse().unwrap(); - let join_body_2 = InstanceMulticastGroupJoin { - source_ips: Some(vec![source1, source2]), - ip_version: None, - }; - put_upsert::<_, MulticastGroupMember>(client, &join_url2, &join_body_2) - .await; + put_upsert::<_, MulticastGroupMember>( + client, + &join_url2, + &InstanceMulticastGroupJoin::default(), + ) + .await; - // Verify group source_ips is union of all member sources + // Now has any-source member, so has_any_source_member=true let group: MulticastGroup = object_get( client, &format!("/v1/multicast-groups/{expected_group_name}"), ) .await; + assert!( + group.has_any_source_member, + "ASM group with any-source member should have has_any_source_member=true" + ); + // source_ips should still show the first member's sources let mut actual_sources = group.source_ips.clone(); actual_sources.sort(); - let mut expected_sources = vec![source1, source2]; - expected_sources.sort(); assert_eq!( actual_sources, expected_sources, "ASM group source_ips should be union of member sources" ); - // Also verify list endpoint returns the same source_ips + // Also verify list endpoint returns the same values let groups: Collection = NexusRequest::iter_collection_authn( client, @@ -949,10 +963,8 @@ async fn test_join_by_ip_asm_with_sources_succeeds( .expect("ASM group should appear in list"); let mut listed_sources = listed_group.source_ips.clone(); listed_sources.sort(); - assert_eq!( - listed_sources, expected_sources, - "List endpoint should also show source_ips union for ASM group" - ); + assert_eq!(listed_sources, expected_sources); + assert!(listed_group.has_any_source_member); cleanup_instances( cptestctx, diff --git a/nexus/types/src/multicast.rs b/nexus/types/src/multicast.rs index 333199a161d..550e8daff4c 100644 --- a/nexus/types/src/multicast.rs +++ b/nexus/types/src/multicast.rs @@ -5,7 +5,6 @@ //! Internal multicast types used by Nexus. use omicron_common::api::external::{IdentityMetadataCreateParams, IpVersion}; -use omicron_common::vlan::VlanID; use std::net::IpAddr; /// Internal parameters for creating a multicast group. @@ -22,12 +21,6 @@ pub struct MulticastGroupCreate { /// /// If `None`, one will be allocated from the default pool. pub multicast_ip: Option, - /// Multicast VLAN (MVLAN) for egress multicast traffic to upstream networks. - /// Tags packets leaving the rack to traverse VLAN-segmented upstream networks. - /// - /// Valid range: 2-4094 (VLAN IDs 0-1 are reserved by IEEE 802.1Q standard). - // TODO(multicast): Remove mvlan field - being deprecated from multicast groups - pub mvlan: Option, /// Whether the joining member has source IPs. /// /// Used for default pool selection when `multicast_ip` is `None`: diff --git a/nexus/types/versions/src/impls/multicast.rs b/nexus/types/versions/src/impls/multicast.rs index ab50c8f894d..7b9be4fc420 100644 --- a/nexus/types/versions/src/impls/multicast.rs +++ b/nexus/types/versions/src/impls/multicast.rs @@ -136,8 +136,6 @@ mod tests { use crate::latest::multicast::{ MulticastGroupCreate, MulticastGroupUpdate, }; - use omicron_common::api::external::Nullable; - use omicron_common::vlan::VlanID; #[test] fn test_validate_multicast_ip_v4() { @@ -322,8 +320,7 @@ mod tests { "description": "Test multicast group", "multicast_ip": "224.1.2.3", "source_ips": ["10.0.0.1", "10.0.0.2"], - "pool": "default", - "mvlan": 10 + "pool": "default" }"#; let result: Result = @@ -346,7 +343,7 @@ mod tests { #[test] fn test_multicast_group_create_deserialization_without_optional_fields() { - // This is the critical test - multicast_ip, source_ips, pool, and mvlan are all optional + // This is the critical test: multicast_ip, source_ips, and pool are all optional let json = r#"{ "name": "test-group", "description": "Test multicast group" @@ -364,7 +361,6 @@ mod tests { assert_eq!(params.multicast_ip, None); assert_eq!(params.source_ips, None); assert_eq!(params.pool, None); - assert_eq!(params.mvlan, None); } #[test] @@ -454,15 +450,14 @@ mod tests { #[test] fn test_multicast_group_create_deserialization_explicit_null_fields() { - // Test with explicit null values for optional fields - // This is what the CLI sends when fields are not provided + // Test with explicit null values for optional fields. + // This is what the CLI sends when fields are not provided. let json = r#"{ "name": "test-group", "description": "Test multicast group", "multicast_ip": null, "source_ips": null, - "pool": null, - "mvlan": null + "pool": null }"#; let result: Result = @@ -476,7 +471,6 @@ mod tests { assert_eq!(params.multicast_ip, None); assert_eq!(params.source_ips, None); assert_eq!(params.pool, None); - assert_eq!(params.mvlan, None); } #[test] @@ -487,8 +481,7 @@ mod tests { "description": "Test multicast group", "multicast_ip": "224.1.2.3", "source_ips": [], - "pool": null, - "mvlan": 30 + "pool": null }"#; let result: Result = @@ -501,7 +494,6 @@ mod tests { ); assert_eq!(params.source_ips, Some(vec![])); assert_eq!(params.pool, None); - assert_eq!(params.mvlan, Some(VlanID::new(30).unwrap())); } #[test] @@ -520,44 +512,6 @@ mod tests { ); let params = result.unwrap(); assert_eq!(params.source_ips, None); - assert_eq!(params.mvlan, None); - } - - #[test] - fn test_multicast_group_update_deserialization_explicit_null_mvlan() { - // When mvlan is explicitly null, it should be Some(Nullable(None)) (clearing the field) - let json = r#"{ - "name": "test-group", - "mvlan": null - }"#; - - let result: Result = - serde_json::from_str(json); - assert!( - result.is_ok(), - "Failed to deserialize update with null mvlan: {:?}", - result.err() - ); - let params = result.unwrap(); - assert_eq!(params.mvlan, Some(Nullable(None))); - } - - #[test] - fn test_multicast_group_update_deserialization_set_mvlan() { - // When mvlan has a value, it should be Some(Nullable(Some(value))) - let json = r#"{ - "name": "test-group", - "mvlan": 100 - }"#; - - let result: Result = - serde_json::from_str(json); - assert!(result.is_ok()); - let params = result.unwrap(); - assert_eq!( - params.mvlan, - Some(Nullable(Some(VlanID::new(100).unwrap()))) - ); } #[test] @@ -595,17 +549,4 @@ mod tests { let params = result.unwrap(); assert_eq!(params.source_ips, Some(vec![])); } - - #[test] - fn test_multicast_group_update_deserialization_invalid_mvlan() { - // VLAN ID 1 should be rejected (reserved) - let json = r#"{ - "name": "test-group", - "mvlan": 1 - }"#; - - let result: Result = - serde_json::from_str(json); - assert!(result.is_err(), "Should reject reserved VLAN ID 1"); - } } diff --git a/nexus/types/versions/src/latest.rs b/nexus/types/versions/src/latest.rs index 956511b6f06..43e0275dd15 100644 --- a/nexus/types/versions/src/latest.rs +++ b/nexus/types/versions/src/latest.rs @@ -220,14 +220,11 @@ pub mod metrics { } pub mod multicast { - pub use crate::v2025_11_20_00::multicast::MulticastGroupCreate; pub use crate::v2025_11_20_00::multicast::MulticastGroupIpLookupPath; pub use crate::v2025_11_20_00::multicast::MulticastGroupMemberRemove; - pub use crate::v2025_11_20_00::multicast::MulticastGroupUpdate; pub use crate::v2026_01_08_00::multicast::InstanceMulticastGroupJoin; pub use crate::v2026_01_08_00::multicast::InstanceMulticastGroupPath; - pub use crate::v2026_01_08_00::multicast::MulticastGroup; pub use crate::v2026_01_08_00::multicast::MulticastGroupIdentifier; pub use crate::v2026_01_08_00::multicast::MulticastGroupJoinSpec; pub use crate::v2026_01_08_00::multicast::MulticastGroupMember; @@ -236,6 +233,11 @@ pub mod multicast { pub use crate::v2026_01_08_00::multicast::MulticastGroupPath; pub use crate::v2026_01_08_00::multicast::MulticastGroupSelector; + // Types from MULTICAST_DROP_MVLAN (mvlan field removed). + pub use crate::v2026_03_10_00::multicast::MulticastGroup; + pub use crate::v2026_03_10_00::multicast::MulticastGroupCreate; + pub use crate::v2026_03_10_00::multicast::MulticastGroupUpdate; + pub use crate::impls::multicast::validate_multicast_ip; pub use crate::impls::multicast::validate_source_ip; } diff --git a/nexus/types/versions/src/lib.rs b/nexus/types/versions/src/lib.rs index de901e3514c..825227ed783 100644 --- a/nexus/types/versions/src/lib.rs +++ b/nexus/types/versions/src/lib.rs @@ -69,3 +69,5 @@ pub mod v2026_01_31_00; pub mod v2026_02_13_01; #[path = "add_time_fields_to_users/mod.rs"] pub mod v2026_03_02_00; +#[path = "multicast_drop_mvlan/mod.rs"] +pub mod v2026_03_10_00; diff --git a/nexus/types/versions/src/multicast_drop_mvlan/mod.rs b/nexus/types/versions/src/multicast_drop_mvlan/mod.rs new file mode 100644 index 00000000000..77aaa59f7f2 --- /dev/null +++ b/nexus/types/versions/src/multicast_drop_mvlan/mod.rs @@ -0,0 +1,12 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Types from API version `MULTICAST_IMPLICIT_LIFECYCLE_UPDATES` that changed +//! in version `MULTICAST_DROP_MVLAN`. +//! +//! Removes the `mvlan` field from `MulticastGroup`, `MulticastGroupCreate`, +//! and `MulticastGroupUpdate`. Adds `has_any_source_member` to +//! `MulticastGroup`. + +pub mod multicast; diff --git a/nexus/types/versions/src/multicast_drop_mvlan/multicast.rs b/nexus/types/versions/src/multicast_drop_mvlan/multicast.rs new file mode 100644 index 00000000000..44bc49b87c4 --- /dev/null +++ b/nexus/types/versions/src/multicast_drop_mvlan/multicast.rs @@ -0,0 +1,132 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Multicast group types from API version `MULTICAST_IMPLICIT_LIFECYCLE_UPDATES` +//! that changed in version `MULTICAST_DROP_MVLAN`. +//! +//! Removes the `mvlan` field from `MulticastGroup`, `MulticastGroupCreate`, +//! and `MulticastGroupUpdate`. Adds `has_any_source_member` to +//! `MulticastGroup`. + +use api_identity::ObjectIdentity; +use omicron_common::api::external::{ + IdentityMetadata, IdentityMetadataCreateParams, + IdentityMetadataUpdateParams, NameOrId, ObjectIdentity, +}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use std::net::IpAddr; +use uuid::Uuid; + +use crate::v2025_11_20_00::multicast::{ + validate_multicast_ip_param, validate_source_ips_param, +}; + +/// View of a Multicast Group +#[derive( + ObjectIdentity, Debug, PartialEq, Clone, Deserialize, Serialize, JsonSchema, +)] +pub struct MulticastGroup { + #[serde(flatten)] + pub identity: IdentityMetadata, + /// The multicast IP address held by this resource. + pub multicast_ip: IpAddr, + /// Deduplicated union of source IPs specified by members. + /// + /// Contains only sources from members that joined with explicit `source_ips`. + /// Members using any-source multicast (empty `source_ips`) do not contribute, + /// so a non-empty value does not imply all members use source filtering. + /// For SSM addresses (232/8, ff3x::/32), this is always non-empty. + pub source_ips: Vec, + /// True if any member joined without specifying source IPs (any-source). + /// + /// When true, at least one member receives traffic from any source rather + /// than filtering to specific sources. + pub has_any_source_member: bool, + /// The ID of the IP pool this resource belongs to. + pub ip_pool_id: Uuid, + /// Current state of the multicast group. + pub state: String, +} + +/// Create-time parameters for a multicast group. +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct MulticastGroupCreate { + #[serde(flatten)] + pub identity: IdentityMetadataCreateParams, + /// The multicast IP address to allocate. If None, one will be allocated + /// from the default pool. + #[serde(default, deserialize_with = "validate_multicast_ip_param")] + pub multicast_ip: Option, + /// Source IP addresses for Source-Specific Multicast (SSM). + /// + /// None uses default behavior (Any-Source Multicast). + /// Empty list explicitly allows any source (Any-Source Multicast). + /// Non-empty list restricts to specific sources (SSM). + #[serde(default, deserialize_with = "validate_source_ips_param")] + pub source_ips: Option>, + /// Name or ID of the IP pool to allocate from. If None, uses the default + /// multicast pool. + #[serde(default)] + pub pool: Option, +} + +/// Update-time parameters for a multicast group. +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct MulticastGroupUpdate { + #[serde(flatten)] + pub identity: IdentityMetadataUpdateParams, + #[serde( + default, + deserialize_with = "validate_source_ips_param", + skip_serializing_if = "Option::is_none" + )] + pub source_ips: Option>, +} + +// Downward conversion: MULTICAST_DROP_MVLAN -> MULTICAST_IMPLICIT_LIFECYCLE_UPDATES +// +// Response types convert downward through consecutive versions. + +impl From for crate::v2026_01_08_00::multicast::MulticastGroup { + fn from(new: MulticastGroup) -> Self { + Self { + identity: new.identity, + multicast_ip: new.multicast_ip, + source_ips: new.source_ips, + mvlan: None, + ip_pool_id: new.ip_pool_id, + state: new.state, + } + } +} + +// Upward conversions: INITIAL -> MULTICAST_DROP_MVLAN +// +// Request types convert upward, dropping the mvlan field. + +impl From + for MulticastGroupCreate +{ + fn from( + old: crate::v2025_11_20_00::multicast::MulticastGroupCreate, + ) -> Self { + Self { + identity: old.identity, + multicast_ip: old.multicast_ip, + source_ips: old.source_ips, + pool: old.pool, + } + } +} + +impl From + for MulticastGroupUpdate +{ + fn from( + old: crate::v2025_11_20_00::multicast::MulticastGroupUpdate, + ) -> Self { + Self { identity: old.identity, source_ips: old.source_ips } + } +} diff --git a/openapi/nexus/nexus-2026030600.0.0-19c639.json.gitstub b/openapi/nexus/nexus-2026030600.0.0-19c639.json.gitstub new file mode 100644 index 00000000000..07419d3ee28 --- /dev/null +++ b/openapi/nexus/nexus-2026030600.0.0-19c639.json.gitstub @@ -0,0 +1 @@ +df724fae54459e6ffa609f932547c643fb52e3f7:openapi/nexus/nexus-2026030600.0.0-19c639.json diff --git a/openapi/nexus/nexus-2026030600.0.0-19c639.json b/openapi/nexus/nexus-2026031000.0.0-0e9242.json similarity index 99% rename from openapi/nexus/nexus-2026030600.0.0-19c639.json rename to openapi/nexus/nexus-2026031000.0.0-0e9242.json index 05ae21c24af..14e33ccfe1e 100644 --- a/openapi/nexus/nexus-2026030600.0.0-19c639.json +++ b/openapi/nexus/nexus-2026031000.0.0-0e9242.json @@ -7,7 +7,7 @@ "url": "https://oxide.computer", "email": "api@oxide.computer" }, - "version": "2026030600.0.0" + "version": "2026031000.0.0" }, "paths": { "/device/auth": { @@ -24679,6 +24679,10 @@ "description": "human-readable free-form text about a resource", "type": "string" }, + "has_any_source_member": { + "description": "True if any member joined without specifying source IPs (any-source).\n\nWhen true, at least one member receives traffic from any source rather than filtering to specific sources.", + "type": "boolean" + }, "id": { "description": "unique, immutable, system-controlled identifier for each resource", "type": "string", @@ -24694,13 +24698,6 @@ "type": "string", "format": "ip" }, - "mvlan": { - "nullable": true, - "description": "Multicast VLAN (MVLAN) for egress multicast traffic to upstream networks. None means no VLAN tagging on egress.", - "type": "integer", - "format": "uint16", - "minimum": 0 - }, "name": { "description": "unique, mutable, user-controlled identifier for each resource", "allOf": [ @@ -24710,7 +24707,7 @@ ] }, "source_ips": { - "description": "Union of all member source IP addresses (computed, read-only).\n\nThis field shows the combined source IPs across all group members. Individual members may subscribe to different sources; this union reflects all sources that any member is subscribed to. Empty array means no members have source filtering enabled.", + "description": "Deduplicated union of source IPs specified by members.\n\nContains only sources from members that joined with explicit `source_ips`. Members using any-source multicast (empty `source_ips`) do not contribute, so a non-empty value does not imply all members use source filtering. For SSM addresses (232/8, ff3x::/32), this is always non-empty.", "type": "array", "items": { "type": "string", @@ -24734,6 +24731,7 @@ }, "required": [ "description", + "has_any_source_member", "id", "ip_pool_id", "multicast_ip", diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index 339cc36df58..a4d9595b0e3 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026030600.0.0-19c639.json \ No newline at end of file +nexus-2026031000.0.0-0e9242.json \ No newline at end of file diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 7c04d3e8522..c4c6f85a2d9 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -7695,11 +7695,6 @@ CREATE TABLE IF NOT EXISTS omicron.public.multicast_group ( ip_pool_range_id UUID NOT NULL, multicast_ip INET NOT NULL, - /* Multicast VLAN (MVLAN) for egress to upstream networks */ - /* Tags packets leaving the rack to traverse VLAN-segmented upstream networks */ - /* Internal rack traffic uses VNI-based underlay forwarding */ - mvlan INT2, - /* Associated underlay group for NAT */ /* We fill this as part of the RPW */ underlay_group_id UUID, @@ -7740,12 +7735,8 @@ CREATE TABLE IF NOT EXISTS omicron.public.multicast_group ( NOT multicast_ip << 'ff01::/16' AND -- Interface-local scope NOT multicast_ip << 'ff02::/16' -- Link-local scope ) - ), - - -- MVLAN validation (Dendrite requires >= 2) - CONSTRAINT mvlan_valid_range CHECK ( - mvlan IS NULL OR (mvlan >= 2 AND mvlan <= 4094) ) + ); /* @@ -8237,7 +8228,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '237.0.0', NULL) + (TRUE, NOW(), NOW(), '238.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/multicast-drop-mvlan/up01.sql b/schema/crdb/multicast-drop-mvlan/up01.sql new file mode 100644 index 00000000000..129c473e7b1 --- /dev/null +++ b/schema/crdb/multicast-drop-mvlan/up01.sql @@ -0,0 +1,3 @@ +-- Drop mvlan constraint from multicast_group +-- This field was for egress multicast which is not in MVP scope +ALTER TABLE omicron.public.multicast_group DROP CONSTRAINT IF EXISTS mvlan_valid_range; diff --git a/schema/crdb/multicast-drop-mvlan/up02.sql b/schema/crdb/multicast-drop-mvlan/up02.sql new file mode 100644 index 00000000000..38299928185 --- /dev/null +++ b/schema/crdb/multicast-drop-mvlan/up02.sql @@ -0,0 +1,2 @@ +-- Drop mvlan column from multicast_group +ALTER TABLE omicron.public.multicast_group DROP COLUMN IF EXISTS mvlan; From 6f12edd0ca0f8a94837748b17dc08d53c21e0a39 Mon Sep 17 00:00:00 2001 From: Zeeshan Lakhani Date: Tue, 17 Mar 2026 18:10:51 +0000 Subject: [PATCH 2/2] [review] add expectorate tests for multicast group allocation CTE queries + minor cleanup --- .../db/queries/external_multicast_group.rs | 58 +++++++ ...ext_external_multicast_group_automatic.sql | 153 +++++++++++++++++ ...next_external_multicast_group_explicit.sql | 157 ++++++++++++++++++ nexus/types/versions/src/latest.rs | 1 - 4 files changed, 368 insertions(+), 1 deletion(-) create mode 100644 nexus/db-queries/tests/output/next_external_multicast_group_automatic.sql create mode 100644 nexus/db-queries/tests/output/next_external_multicast_group_explicit.sql diff --git a/nexus/db-queries/src/db/queries/external_multicast_group.rs b/nexus/db-queries/src/db/queries/external_multicast_group.rs index 1202fab4cec..697a952929a 100644 --- a/nexus/db-queries/src/db/queries/external_multicast_group.rs +++ b/nexus/db-queries/src/db/queries/external_multicast_group.rs @@ -298,3 +298,61 @@ impl Query for NextExternalMulticastGroup { } impl RunQueryDsl for NextExternalMulticastGroup {} + +#[cfg(test)] +mod tests { + use super::NextExternalMulticastGroup; + + use omicron_common::api::external; + + use crate::db::model::{ + IncompleteExternalMulticastGroup, + IncompleteExternalMulticastGroupParams, Name, Vni, + }; + use crate::db::raw_query_builder::expectorate_query_contents; + + const GROUP_ID: uuid::Uuid = + uuid::uuid!("45633e04-3087-47eb-9d1e-8436fb090108"); + const POOL_ID: uuid::Uuid = + uuid::uuid!("789fdb15-9790-4df1-9d06-c1ecd72c94ae"); + + #[tokio::test] + async fn expectorate_next_external_multicast_group_automatic() { + let group = IncompleteExternalMulticastGroup::new( + IncompleteExternalMulticastGroupParams { + id: GROUP_ID, + name: Name("mcast-test".parse().unwrap()), + description: String::from("test multicast group"), + ip_pool_id: POOL_ID, + explicit_address: None, + vni: Vni(external::Vni::DEFAULT_MULTICAST_VNI), + }, + ); + let query = NextExternalMulticastGroup::new(group); + expectorate_query_contents( + query, + "tests/output/next_external_multicast_group_automatic.sql", + ) + .await; + } + + #[tokio::test] + async fn expectorate_next_external_multicast_group_explicit() { + let group = IncompleteExternalMulticastGroup::new( + IncompleteExternalMulticastGroupParams { + id: GROUP_ID, + name: Name("mcast-test".parse().unwrap()), + description: String::from("test multicast group"), + ip_pool_id: POOL_ID, + explicit_address: Some("239.1.1.1".parse().unwrap()), + vni: Vni(external::Vni::DEFAULT_MULTICAST_VNI), + }, + ); + let query = NextExternalMulticastGroup::new(group); + expectorate_query_contents( + query, + "tests/output/next_external_multicast_group_explicit.sql", + ) + .await; + } +} diff --git a/nexus/db-queries/tests/output/next_external_multicast_group_automatic.sql b/nexus/db-queries/tests/output/next_external_multicast_group_automatic.sql new file mode 100644 index 00000000000..fc058586967 --- /dev/null +++ b/nexus/db-queries/tests/output/next_external_multicast_group_automatic.sql @@ -0,0 +1,153 @@ +WITH + next_external_multicast_group + AS ( + SELECT + $1 AS id, + $2 AS name, + $3 AS description, + $4 AS time_created, + $5 AS time_modified, + $6 AS time_deleted, + $7 AS vni, + ip_pool_id, + ip_pool_range_id, + candidate_ip AS multicast_ip, + $8 AS underlay_group_id, + $9::STRING || ':' || candidate_ip::STRING AS tag, + $10 AS state, + nextval('omicron.public.multicast_group_version') AS version_added, + $11 AS version_removed, + $12 AS underlay_salt + FROM + ( + SELECT + ip_pool_id, + id AS ip_pool_range_id, + first_address + generate_series(0, last_address - first_address) AS candidate_ip + FROM + ip_pool_range + WHERE + ip_pool_id = $13 + AND time_deleted IS NULL + AND (first_address << '224.0.0.0/4'::INET OR first_address << 'ff00::/8'::INET) + ) + LEFT JOIN multicast_group ON multicast_ip = candidate_ip AND time_deleted IS NULL + WHERE + candidate_ip IS NOT NULL AND multicast_ip IS NULL + LIMIT + 1 + ), + previously_allocated_group + AS (SELECT * FROM multicast_group WHERE id = $14 AND time_deleted IS NULL), + multicast_group + AS ( + INSERT + INTO + multicast_group + ( + id, + name, + description, + time_created, + time_modified, + time_deleted, + vni, + ip_pool_id, + ip_pool_range_id, + multicast_ip, + underlay_group_id, + tag, + state, + version_added, + version_removed, + underlay_salt + ) + SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + vni, + ip_pool_id, + ip_pool_range_id, + multicast_ip, + underlay_group_id, + tag, + state, + version_added, + version_removed, + underlay_salt + FROM + next_external_multicast_group + WHERE + NOT EXISTS(SELECT 1 FROM previously_allocated_group) + RETURNING + id, + name, + description, + time_created, + time_modified, + time_deleted, + vni, + ip_pool_id, + ip_pool_range_id, + multicast_ip, + underlay_group_id, + tag, + state, + version_added, + version_removed, + underlay_salt + ), + updated_pool_range + AS ( + UPDATE + ip_pool_range + SET + time_modified = $15, rcgen = rcgen + 1 + WHERE + id = (SELECT ip_pool_range_id FROM next_external_multicast_group) AND time_deleted IS NULL + RETURNING + id + ) +SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + vni, + ip_pool_id, + ip_pool_range_id, + multicast_ip, + underlay_group_id, + tag, + state, + version_added, + version_removed, + underlay_salt +FROM + previously_allocated_group +UNION ALL + SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + vni, + ip_pool_id, + ip_pool_range_id, + multicast_ip, + underlay_group_id, + tag, + state, + version_added, + version_removed, + underlay_salt + FROM + multicast_group diff --git a/nexus/db-queries/tests/output/next_external_multicast_group_explicit.sql b/nexus/db-queries/tests/output/next_external_multicast_group_explicit.sql new file mode 100644 index 00000000000..b857f5f20a0 --- /dev/null +++ b/nexus/db-queries/tests/output/next_external_multicast_group_explicit.sql @@ -0,0 +1,157 @@ +WITH + next_external_multicast_group + AS ( + SELECT + $1 AS id, + $2 AS name, + $3 AS description, + $4 AS time_created, + $5 AS time_modified, + $6 AS time_deleted, + $7 AS vni, + ip_pool_id, + ip_pool_range_id, + candidate_ip AS multicast_ip, + $8 AS underlay_group_id, + $9::STRING || ':' || candidate_ip::STRING AS tag, + $10 AS state, + nextval('omicron.public.multicast_group_version') AS version_added, + $11 AS version_removed, + $12 AS underlay_salt + FROM + ( + SELECT + ip_pool_id, + id AS ip_pool_range_id, + CASE first_address <= $13 AND $14 <= last_address + WHEN true THEN $15 + ELSE NULL + END + AS candidate_ip + FROM + ip_pool_range + WHERE + ip_pool_id = $16 + AND time_deleted IS NULL + AND (first_address << '224.0.0.0/4'::INET OR first_address << 'ff00::/8'::INET) + ) + LEFT JOIN multicast_group ON multicast_ip = candidate_ip AND time_deleted IS NULL + WHERE + candidate_ip IS NOT NULL AND multicast_ip IS NULL + LIMIT + 1 + ), + previously_allocated_group + AS (SELECT * FROM multicast_group WHERE id = $17 AND time_deleted IS NULL), + multicast_group + AS ( + INSERT + INTO + multicast_group + ( + id, + name, + description, + time_created, + time_modified, + time_deleted, + vni, + ip_pool_id, + ip_pool_range_id, + multicast_ip, + underlay_group_id, + tag, + state, + version_added, + version_removed, + underlay_salt + ) + SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + vni, + ip_pool_id, + ip_pool_range_id, + multicast_ip, + underlay_group_id, + tag, + state, + version_added, + version_removed, + underlay_salt + FROM + next_external_multicast_group + WHERE + NOT EXISTS(SELECT 1 FROM previously_allocated_group) + RETURNING + id, + name, + description, + time_created, + time_modified, + time_deleted, + vni, + ip_pool_id, + ip_pool_range_id, + multicast_ip, + underlay_group_id, + tag, + state, + version_added, + version_removed, + underlay_salt + ), + updated_pool_range + AS ( + UPDATE + ip_pool_range + SET + time_modified = $18, rcgen = rcgen + 1 + WHERE + id = (SELECT ip_pool_range_id FROM next_external_multicast_group) AND time_deleted IS NULL + RETURNING + id + ) +SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + vni, + ip_pool_id, + ip_pool_range_id, + multicast_ip, + underlay_group_id, + tag, + state, + version_added, + version_removed, + underlay_salt +FROM + previously_allocated_group +UNION ALL + SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + vni, + ip_pool_id, + ip_pool_range_id, + multicast_ip, + underlay_group_id, + tag, + state, + version_added, + version_removed, + underlay_salt + FROM + multicast_group diff --git a/nexus/types/versions/src/latest.rs b/nexus/types/versions/src/latest.rs index e2d1ab38ddd..d0e809c328e 100644 --- a/nexus/types/versions/src/latest.rs +++ b/nexus/types/versions/src/latest.rs @@ -233,7 +233,6 @@ pub mod multicast { pub use crate::v2026_01_08_00::multicast::MulticastGroupPath; pub use crate::v2026_01_08_00::multicast::MulticastGroupSelector; - // Types from MULTICAST_DROP_MVLAN (mvlan field removed). pub use crate::v2026_03_14_00::multicast::MulticastGroup; pub use crate::v2026_03_14_00::multicast::MulticastGroupCreate; pub use crate::v2026_03_14_00::multicast::MulticastGroupUpdate;