diff --git a/nexus/db-queries/src/db/datastore/ipv4_nat_entry.rs b/nexus/db-queries/src/db/datastore/ipv4_nat_entry.rs index 274937b299..1caf5617bb 100644 --- a/nexus/db-queries/src/db/datastore/ipv4_nat_entry.rs +++ b/nexus/db-queries/src/db/datastore/ipv4_nat_entry.rs @@ -36,6 +36,7 @@ impl DataStore { .filter(dsl::sled_address.eq(nat_entry.sled_address)) .filter(dsl::vni.eq(nat_entry.vni)) .filter(dsl::mac.eq(nat_entry.mac)) + .filter(dsl::version_removed.is_null()) .select(( dsl::external_address, dsl::first_port, @@ -275,7 +276,7 @@ mod test { use crate::db::datastore::datastore_test; use chrono::Utc; - use nexus_db_model::{Ipv4NatValues, MacAddr, Vni}; + use nexus_db_model::{Ipv4NatEntry, Ipv4NatValues, MacAddr, Vni}; use nexus_test_utils::db::test_setup_database; use omicron_common::api::external; use omicron_test_utils::dev; @@ -427,7 +428,6 @@ mod test { datastore.ipv4_nat_list_since_version(&opctx, 0, 10).await.unwrap(); assert_eq!(nat_entries.len(), 1); - // version should be unchanged assert_eq!( datastore.ipv4_nat_current_version(&opctx).await.unwrap(), @@ -437,4 +437,150 @@ mod test { db.cleanup().await.unwrap(); logctx.cleanup_successful(); } + + #[tokio::test] + /// Table design and queries should only insert one active NAT entry for a given + /// set of properties, but allow multiple deleted nat entries for the same set + /// of properties. + async fn table_allows_unique_active_multiple_deleted() { + let logctx = dev::test_setup_log("test_nat_version_tracking"); + let mut db = test_setup_database(&logctx.log).await; + let (opctx, datastore) = datastore_test(&logctx, &db).await; + + // We should not have any NAT entries at this moment + let initial_state = + datastore.ipv4_nat_list_since_version(&opctx, 0, 10).await.unwrap(); + + assert!(initial_state.is_empty()); + assert_eq!( + datastore.ipv4_nat_current_version(&opctx).await.unwrap(), + 0 + ); + + // Each change (creation / deletion) to the NAT table should increment the + // version number of the row in the NAT table + let external_address = external::Ipv4Net( + ipnetwork::Ipv4Network::try_from("10.0.0.100").unwrap(), + ); + + let sled_address = external::Ipv6Net( + ipnetwork::Ipv6Network::try_from("fd00:1122:3344:104::1").unwrap(), + ); + + // Add a nat entry. + let nat1 = Ipv4NatValues { + external_address: external_address.into(), + first_port: 0.into(), + last_port: 999.into(), + sled_address: sled_address.into(), + vni: Vni(external::Vni::random()), + mac: MacAddr( + external::MacAddr::from_str("A8:40:25:F5:EB:2A").unwrap(), + ), + }; + + datastore.ensure_ipv4_nat_entry(&opctx, nat1.clone()).await.unwrap(); + + // Try to add it again. It should still only result in a single entry. + datastore.ensure_ipv4_nat_entry(&opctx, nat1.clone()).await.unwrap(); + let first_entry = datastore + .ipv4_nat_find_by_values(&opctx, nat1.clone()) + .await + .unwrap(); + + let nat_entries = + datastore.ipv4_nat_list_since_version(&opctx, 0, 10).await.unwrap(); + + // The NAT table has undergone one change. One entry has been added, + // none deleted, so we should be at version 1. + assert_eq!(nat_entries.len(), 1); + assert_eq!(nat_entries.last().unwrap().version_added, 1); + assert_eq!( + datastore.ipv4_nat_current_version(&opctx).await.unwrap(), + 1 + ); + + datastore.ipv4_nat_delete(&opctx, &first_entry).await.unwrap(); + + // The NAT table has undergone two changes. One entry has been added, + // then deleted, so we should be at version 2. + let nat_entries = datastore + .ipv4_nat_list_since_version(&opctx, 0, 10) + .await + .unwrap() + .into_iter(); + + let active: Vec = nat_entries + .clone() + .filter(|entry| entry.version_removed.is_none()) + .collect(); + + let inactive: Vec = nat_entries + .filter(|entry| entry.version_removed.is_some()) + .collect(); + + assert!(active.is_empty()); + assert_eq!(inactive.len(), 1); + assert_eq!( + datastore.ipv4_nat_current_version(&opctx).await.unwrap(), + 2 + ); + + // Add the same entry back. This simulates the behavior we will see + // when stopping and then restarting an instance. + datastore.ensure_ipv4_nat_entry(&opctx, nat1.clone()).await.unwrap(); + + // The NAT table has undergone three changes. + let nat_entries = datastore + .ipv4_nat_list_since_version(&opctx, 0, 10) + .await + .unwrap() + .into_iter(); + + let active: Vec = nat_entries + .clone() + .filter(|entry| entry.version_removed.is_none()) + .collect(); + + let inactive: Vec = nat_entries + .filter(|entry| entry.version_removed.is_some()) + .collect(); + + assert_eq!(active.len(), 1); + assert_eq!(inactive.len(), 1); + assert_eq!( + datastore.ipv4_nat_current_version(&opctx).await.unwrap(), + 3 + ); + + let second_entry = + datastore.ipv4_nat_find_by_values(&opctx, nat1).await.unwrap(); + datastore.ipv4_nat_delete(&opctx, &second_entry).await.unwrap(); + + // The NAT table has undergone four changes + let nat_entries = datastore + .ipv4_nat_list_since_version(&opctx, 0, 10) + .await + .unwrap() + .into_iter(); + + let active: Vec = nat_entries + .clone() + .filter(|entry| entry.version_removed.is_none()) + .collect(); + + let inactive: Vec = nat_entries + .filter(|entry| entry.version_removed.is_some()) + .collect(); + + assert_eq!(active.len(), 0); + assert_eq!(inactive.len(), 2); + assert_eq!( + datastore.ipv4_nat_current_version(&opctx).await.unwrap(), + 4 + ); + + db.cleanup().await.unwrap(); + logctx.cleanup_successful(); + } } diff --git a/nexus/src/app/sagas/instance_delete.rs b/nexus/src/app/sagas/instance_delete.rs index 1605465c74..e35b922c87 100644 --- a/nexus/src/app/sagas/instance_delete.rs +++ b/nexus/src/app/sagas/instance_delete.rs @@ -8,6 +8,7 @@ use super::ActionRegistry; use super::NexusActionContext; use super::NexusSaga; use crate::app::sagas::declare_saga_actions; +use nexus_db_queries::db::lookup::LookupPath; use nexus_db_queries::{authn, authz, db}; use omicron_common::api::external::{Error, ResourceType}; use omicron_common::api::internal::shared::SwitchLocation; @@ -39,6 +40,9 @@ declare_saga_actions! { DEALLOCATE_EXTERNAL_IP -> "no_result3" { + sid_deallocate_external_ip } + INSTANCE_DELETE_NAT -> "no_result4" { + + sid_delete_nat + } } // instance delete saga: definition @@ -57,6 +61,7 @@ impl NexusSaga for SagaInstanceDelete { _params: &Self::Params, mut builder: steno::DagBuilder, ) -> Result { + builder.append(instance_delete_nat_action()); builder.append(instance_delete_record_action()); builder.append(delete_network_interfaces_action()); builder.append(deallocate_external_ip_action()); @@ -110,6 +115,32 @@ async fn sid_delete_network_interfaces( Ok(()) } +async fn sid_delete_nat( + sagactx: NexusActionContext, +) -> Result<(), ActionError> { + let params = sagactx.saga_params::()?; + let instance_id = params.authz_instance.id(); + let osagactx = sagactx.user_data(); + let opctx = crate::context::op_context_for_saga_action( + &sagactx, + ¶ms.serialized_authn, + ); + + let (.., authz_instance) = LookupPath::new(&opctx, &osagactx.datastore()) + .instance_id(instance_id) + .lookup_for(authz::Action::Modify) + .await + .map_err(ActionError::action_failed)?; + + osagactx + .nexus() + .instance_delete_dpd_config(&opctx, &authz_instance) + .await + .map_err(ActionError::action_failed)?; + + Ok(()) +} + async fn sid_deallocate_external_ip( sagactx: NexusActionContext, ) -> Result<(), ActionError> { diff --git a/package-manifest.toml b/package-manifest.toml index 3bce4aafee..37ae1100f8 100644 --- a/package-manifest.toml +++ b/package-manifest.toml @@ -476,8 +476,8 @@ only_for_targets.image = "standard" # 2. Copy dendrite.tar.gz from dendrite/out to omicron/out source.type = "prebuilt" source.repo = "dendrite" -source.commit = "2af6adea85c62ac37e451148b84e5eb0ef005f36" -source.sha256 = "dc93b671cce54e83ed55faaa267f81ba9e65abcd6714aa559d68a8783d73b1c1" +source.commit = "1b15e62b04044ef2b15c82d8dcef03f6fc24b3d8" +source.sha256 = "06b5eeedaebf30e96a5c5e932e08034c90947af7a54e9bc04d57d6807013ade9" output.type = "zone" output.intermediate_only = true @@ -501,8 +501,8 @@ only_for_targets.image = "standard" # 2. Copy the output zone image from dendrite/out to omicron/out source.type = "prebuilt" source.repo = "dendrite" -source.commit = "2af6adea85c62ac37e451148b84e5eb0ef005f36" -source.sha256 = "c34b10d47fa3eb9f9f6b3655ea4ed8a726f93399ea177efea79f5c89f2ab5a1e" +source.commit = "1b15e62b04044ef2b15c82d8dcef03f6fc24b3d8" +source.sha256 = "51be0b0342bc7cdf927797af45af3bc82861bb8efb174d50958cb16b5620c51d" output.type = "zone" output.intermediate_only = true @@ -519,8 +519,8 @@ only_for_targets.image = "standard" # 2. Copy dendrite.tar.gz from dendrite/out to omicron/out/dendrite-softnpu.tar.gz source.type = "prebuilt" source.repo = "dendrite" -source.commit = "2af6adea85c62ac37e451148b84e5eb0ef005f36" -source.sha256 = "ce7065227c092ee82704f39a966b7441e3ae82d75eedb6eb281bd8b3e5873e32" +source.commit = "1b15e62b04044ef2b15c82d8dcef03f6fc24b3d8" +source.sha256 = "9afb24cdae27755eaf86a856268686bb641048b5d450dae858cf47b9daaa46ed" output.type = "zone" output.intermediate_only = true diff --git a/tools/dendrite_openapi_version b/tools/dendrite_openapi_version index c2dda4dbd0..b6dc45a8d0 100644 --- a/tools/dendrite_openapi_version +++ b/tools/dendrite_openapi_version @@ -1,2 +1,2 @@ -COMMIT="2af6adea85c62ac37e451148b84e5eb0ef005f36" +COMMIT="1b15e62b04044ef2b15c82d8dcef03f6fc24b3d8" SHA2="07d115bfa8498a8015ca2a8447efeeac32e24aeb25baf3d5e2313216e11293c0" diff --git a/tools/dendrite_stub_checksums b/tools/dendrite_stub_checksums index 77ee198fc5..95f04db9e8 100644 --- a/tools/dendrite_stub_checksums +++ b/tools/dendrite_stub_checksums @@ -1,3 +1,3 @@ -CIDL_SHA256_ILLUMOS="dc93b671cce54e83ed55faaa267f81ba9e65abcd6714aa559d68a8783d73b1c1" -CIDL_SHA256_LINUX_DPD="b13b391a085ba6bf16fdd99774f64c9d53cd7220ad518d5839c8558fb925c40c" -CIDL_SHA256_LINUX_SWADM="6bfa4e367eb2b0be89f1588ac458026a186314597a4feb9fee6cea60101c7ebe" +CIDL_SHA256_ILLUMOS="06b5eeedaebf30e96a5c5e932e08034c90947af7a54e9bc04d57d6807013ade9" +CIDL_SHA256_LINUX_DPD="99a800cbd5739245154831004892d47be5a871e37c536ec3009911ddb02fdb16" +CIDL_SHA256_LINUX_SWADM="e92bfc071f3944523a2e69b13ee877a4fd87cb8a9a78011b4aa8f40218347e25"