From 8f1d37ceae8373cef2e2b815e26c556655f07b32 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Thu, 30 Jan 2025 12:25:54 -0800 Subject: [PATCH 1/6] [nexus] Add CRUD implementations for Affinity/Anti-Affinity Groups --- nexus/auth/src/authz/api_resources.rs | 16 + nexus/auth/src/authz/oso_generic.rs | 2 + nexus/db-queries/src/db/datastore/affinity.rs | 2701 +++++++++++++++++ nexus/db-queries/src/db/datastore/instance.rs | 7 + nexus/db-queries/src/db/datastore/mod.rs | 1 + nexus/db-queries/src/db/datastore/project.rs | 5 + nexus/db-queries/src/db/lookup.rs | 30 +- 7 files changed, 2761 insertions(+), 1 deletion(-) create mode 100644 nexus/db-queries/src/db/datastore/affinity.rs diff --git a/nexus/auth/src/authz/api_resources.rs b/nexus/auth/src/authz/api_resources.rs index 745a699cf2..4d588036b7 100644 --- a/nexus/auth/src/authz/api_resources.rs +++ b/nexus/auth/src/authz/api_resources.rs @@ -713,6 +713,22 @@ authz_resource! { polar_snippet = InProject, } +authz_resource! { + name = "AffinityGroup", + parent = "Project", + primary_key = Uuid, + roles_allowed = false, + polar_snippet = InProject, +} + +authz_resource! { + name = "AntiAffinityGroup", + parent = "Project", + primary_key = Uuid, + roles_allowed = false, + polar_snippet = InProject, +} + authz_resource! { name = "InstanceNetworkInterface", parent = "Instance", diff --git a/nexus/auth/src/authz/oso_generic.rs b/nexus/auth/src/authz/oso_generic.rs index 321bb98b1c..32b3dbd1f8 100644 --- a/nexus/auth/src/authz/oso_generic.rs +++ b/nexus/auth/src/authz/oso_generic.rs @@ -125,6 +125,8 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result { Disk::init(), Snapshot::init(), ProjectImage::init(), + AffinityGroup::init(), + AntiAffinityGroup::init(), Instance::init(), IpPool::init(), InstanceNetworkInterface::init(), diff --git a/nexus/db-queries/src/db/datastore/affinity.rs b/nexus/db-queries/src/db/datastore/affinity.rs new file mode 100644 index 0000000000..e09a0e4340 --- /dev/null +++ b/nexus/db-queries/src/db/datastore/affinity.rs @@ -0,0 +1,2701 @@ +// 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/. + +//! [`DataStore`] methods on Affinity Groups + +use super::DataStore; +use crate::authz; +use crate::authz::ApiResource; +use crate::db; +use crate::db::collection_insert::AsyncInsertError; +use crate::db::collection_insert::DatastoreCollection; +use crate::db::datastore::OpContext; +use crate::db::error::public_error_from_diesel; +use crate::db::error::ErrorHandler; +use crate::db::identity::Resource; +use crate::db::model::AffinityGroup; +use crate::db::model::AffinityGroupInstanceMembership; +use crate::db::model::AffinityGroupUpdate; +use crate::db::model::AntiAffinityGroup; +use crate::db::model::AntiAffinityGroupInstanceMembership; +use crate::db::model::AntiAffinityGroupUpdate; +use crate::db::model::InstanceState; +use crate::db::model::Name; +use crate::db::model::Project; +use crate::db::pagination::paginated; +use crate::transaction_retry::OptionalError; +use async_bb8_diesel::AsyncRunQueryDsl; +use chrono::Utc; +use diesel::prelude::*; +use omicron_common::api::external; +use omicron_common::api::external::http_pagination::PaginatedBy; +use omicron_common::api::external::CreateResult; +use omicron_common::api::external::DeleteResult; +use omicron_common::api::external::Error; +use omicron_common::api::external::ListResultVec; +use omicron_common::api::external::LookupType; +use omicron_common::api::external::ResourceType; +use omicron_common::api::external::UpdateResult; +use omicron_uuid_kinds::AffinityGroupUuid; +use omicron_uuid_kinds::AntiAffinityGroupUuid; +use omicron_uuid_kinds::GenericUuid; +use omicron_uuid_kinds::InstanceUuid; +use ref_cast::RefCast; + +impl DataStore { + pub async fn affinity_group_list( + &self, + opctx: &OpContext, + authz_project: &authz::Project, + pagparams: &PaginatedBy<'_>, + ) -> ListResultVec { + use db::schema::affinity_group::dsl; + + opctx.authorize(authz::Action::ListChildren, authz_project).await?; + + match pagparams { + PaginatedBy::Id(pagparams) => { + paginated(dsl::affinity_group, dsl::id, &pagparams) + } + PaginatedBy::Name(pagparams) => paginated( + dsl::affinity_group, + dsl::name, + &pagparams.map_name(|n| Name::ref_cast(n)), + ), + } + .filter(dsl::project_id.eq(authz_project.id())) + .filter(dsl::time_deleted.is_null()) + .select(AffinityGroup::as_select()) + .get_results_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + + pub async fn anti_affinity_group_list( + &self, + opctx: &OpContext, + authz_project: &authz::Project, + pagparams: &PaginatedBy<'_>, + ) -> ListResultVec { + use db::schema::anti_affinity_group::dsl; + + opctx.authorize(authz::Action::ListChildren, authz_project).await?; + + match pagparams { + PaginatedBy::Id(pagparams) => { + paginated(dsl::anti_affinity_group, dsl::id, &pagparams) + } + PaginatedBy::Name(pagparams) => paginated( + dsl::anti_affinity_group, + dsl::name, + &pagparams.map_name(|n| Name::ref_cast(n)), + ), + } + .filter(dsl::project_id.eq(authz_project.id())) + .filter(dsl::time_deleted.is_null()) + .select(AntiAffinityGroup::as_select()) + .get_results_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + + pub async fn affinity_group_create( + &self, + opctx: &OpContext, + authz_project: &authz::Project, + group: AffinityGroup, + ) -> CreateResult { + use db::schema::affinity_group::dsl; + + opctx.authorize(authz::Action::CreateChild, authz_project).await?; + + let conn = self.pool_connection_authorized(opctx).await?; + let name = group.name().as_str().to_string(); + + let affinity_group: AffinityGroup = Project::insert_resource( + authz_project.id(), + diesel::insert_into(dsl::affinity_group).values(group), + ) + .insert_and_get_result_async(&conn) + .await + .map_err(|e| match e { + AsyncInsertError::CollectionNotFound => authz_project.not_found(), + AsyncInsertError::DatabaseError(diesel_error) => { + public_error_from_diesel( + diesel_error, + ErrorHandler::Conflict(ResourceType::AffinityGroup, &name), + ) + } + })?; + Ok(affinity_group) + } + + pub async fn anti_affinity_group_create( + &self, + opctx: &OpContext, + authz_project: &authz::Project, + group: AntiAffinityGroup, + ) -> CreateResult { + use db::schema::anti_affinity_group::dsl; + + opctx.authorize(authz::Action::CreateChild, authz_project).await?; + + let conn = self.pool_connection_authorized(opctx).await?; + let name = group.name().as_str().to_string(); + + let anti_affinity_group: AntiAffinityGroup = Project::insert_resource( + authz_project.id(), + diesel::insert_into(dsl::anti_affinity_group).values(group), + ) + .insert_and_get_result_async(&conn) + .await + .map_err(|e| match e { + AsyncInsertError::CollectionNotFound => authz_project.not_found(), + AsyncInsertError::DatabaseError(diesel_error) => { + public_error_from_diesel( + diesel_error, + ErrorHandler::Conflict( + ResourceType::AntiAffinityGroup, + &name, + ), + ) + } + })?; + Ok(anti_affinity_group) + } + + pub async fn affinity_group_update( + &self, + opctx: &OpContext, + authz_affinity_group: &authz::AffinityGroup, + updates: AffinityGroupUpdate, + ) -> UpdateResult { + opctx.authorize(authz::Action::Modify, authz_affinity_group).await?; + + use db::schema::affinity_group::dsl; + diesel::update(dsl::affinity_group) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::id.eq(authz_affinity_group.id())) + .set(updates) + .returning(AffinityGroup::as_returning()) + .get_result_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource(authz_affinity_group), + ) + }) + } + + pub async fn affinity_group_delete( + &self, + opctx: &OpContext, + authz_affinity_group: &authz::AffinityGroup, + ) -> DeleteResult { + opctx.authorize(authz::Action::Delete, authz_affinity_group).await?; + + let err = OptionalError::new(); + let conn = self.pool_connection_authorized(opctx).await?; + self.transaction_retry_wrapper("affinity_group_delete") + .transaction(&conn, |conn| { + let err = err.clone(); + async move { + use db::schema::affinity_group::dsl as group_dsl; + let now = Utc::now(); + + // Delete the Affinity Group + diesel::update(group_dsl::affinity_group) + .filter(group_dsl::time_deleted.is_null()) + .filter(group_dsl::id.eq(authz_affinity_group.id())) + .set(group_dsl::time_deleted.eq(now)) + .returning(AffinityGroup::as_returning()) + .execute_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource( + authz_affinity_group, + ), + ) + }) + })?; + + // Ensure all memberships in the affinity group are deleted + use db::schema::affinity_group_instance_membership::dsl as member_dsl; + diesel::delete(member_dsl::affinity_group_instance_membership) + .filter(member_dsl::group_id.eq(authz_affinity_group.id())) + .execute_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel(e, ErrorHandler::Server) + }) + })?; + + Ok(()) + } + }) + .await + .map_err(|e| { + if let Some(err) = err.take() { + return err; + } + public_error_from_diesel(e, ErrorHandler::Server) + })?; + Ok(()) + } + + pub async fn anti_affinity_group_update( + &self, + opctx: &OpContext, + authz_anti_affinity_group: &authz::AntiAffinityGroup, + updates: AntiAffinityGroupUpdate, + ) -> UpdateResult { + opctx + .authorize(authz::Action::Modify, authz_anti_affinity_group) + .await?; + + use db::schema::anti_affinity_group::dsl; + diesel::update(dsl::anti_affinity_group) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::id.eq(authz_anti_affinity_group.id())) + .set(updates) + .returning(AntiAffinityGroup::as_returning()) + .get_result_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource(authz_anti_affinity_group), + ) + }) + } + + pub async fn anti_affinity_group_delete( + &self, + opctx: &OpContext, + authz_anti_affinity_group: &authz::AntiAffinityGroup, + ) -> DeleteResult { + opctx + .authorize(authz::Action::Delete, authz_anti_affinity_group) + .await?; + + let err = OptionalError::new(); + let conn = self.pool_connection_authorized(opctx).await?; + self.transaction_retry_wrapper("anti_affinity_group_delete") + .transaction(&conn, |conn| { + let err = err.clone(); + async move { + use db::schema::anti_affinity_group::dsl as group_dsl; + let now = Utc::now(); + + // Delete the Anti Affinity Group + diesel::update(group_dsl::anti_affinity_group) + .filter(group_dsl::time_deleted.is_null()) + .filter(group_dsl::id.eq(authz_anti_affinity_group.id())) + .set(group_dsl::time_deleted.eq(now)) + .returning(AntiAffinityGroup::as_returning()) + .execute_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource( + authz_anti_affinity_group, + ), + ) + }) + })?; + + // Ensure all memberships in the anti affinity group are deleted + use db::schema::anti_affinity_group_instance_membership::dsl as member_dsl; + diesel::delete(member_dsl::anti_affinity_group_instance_membership) + .filter(member_dsl::group_id.eq(authz_anti_affinity_group.id())) + .execute_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel(e, ErrorHandler::Server) + }) + })?; + + Ok(()) + } + }) + .await + .map_err(|e| { + if let Some(err) = err.take() { + return err; + } + public_error_from_diesel(e, ErrorHandler::Server) + })?; + Ok(()) + } + + pub async fn affinity_group_member_list( + &self, + opctx: &OpContext, + authz_affinity_group: &authz::AffinityGroup, + pagparams: &PaginatedBy<'_>, + ) -> ListResultVec { + opctx.authorize(authz::Action::Read, authz_affinity_group).await?; + + use db::schema::affinity_group_instance_membership::dsl; + match pagparams { + PaginatedBy::Id(pagparams) => paginated( + dsl::affinity_group_instance_membership, + dsl::instance_id, + &pagparams, + ), + PaginatedBy::Name(_) => { + return Err(Error::invalid_request( + "Cannot paginate group members by name", + )); + } + } + .filter(dsl::group_id.eq(authz_affinity_group.id())) + .select(AffinityGroupInstanceMembership::as_select()) + .load_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + + pub async fn anti_affinity_group_member_list( + &self, + opctx: &OpContext, + authz_anti_affinity_group: &authz::AntiAffinityGroup, + pagparams: &PaginatedBy<'_>, + ) -> ListResultVec { + opctx.authorize(authz::Action::Read, authz_anti_affinity_group).await?; + + use db::schema::anti_affinity_group_instance_membership::dsl; + match pagparams { + PaginatedBy::Id(pagparams) => paginated( + dsl::anti_affinity_group_instance_membership, + dsl::instance_id, + &pagparams, + ), + PaginatedBy::Name(_) => { + return Err(Error::invalid_request( + "Cannot paginate group members by name", + )); + } + } + .filter(dsl::group_id.eq(authz_anti_affinity_group.id())) + .select(AntiAffinityGroupInstanceMembership::as_select()) + .load_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + + pub async fn affinity_group_member_view( + &self, + opctx: &OpContext, + authz_affinity_group: &authz::AffinityGroup, + member: external::AffinityGroupMember, + ) -> Result { + opctx.authorize(authz::Action::Read, authz_affinity_group).await?; + let conn = self.pool_connection_authorized(opctx).await?; + + let instance_id = match member { + external::AffinityGroupMember::Instance(id) => id, + }; + + use db::schema::affinity_group_instance_membership::dsl; + dsl::affinity_group_instance_membership + .filter(dsl::group_id.eq(authz_affinity_group.id())) + .filter(dsl::instance_id.eq(instance_id)) + .select(AffinityGroupInstanceMembership::as_select()) + .get_result_async(&*conn) + .await + .map(|m| m.into()) + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::AffinityGroupMember, + LookupType::by_id(instance_id), + ), + ) + }) + } + + pub async fn anti_affinity_group_member_view( + &self, + opctx: &OpContext, + authz_anti_affinity_group: &authz::AntiAffinityGroup, + member: external::AntiAffinityGroupMember, + ) -> Result { + opctx.authorize(authz::Action::Read, authz_anti_affinity_group).await?; + let conn = self.pool_connection_authorized(opctx).await?; + + let instance_id = match member { + external::AntiAffinityGroupMember::Instance(id) => id, + }; + + use db::schema::anti_affinity_group_instance_membership::dsl; + dsl::anti_affinity_group_instance_membership + .filter(dsl::group_id.eq(authz_anti_affinity_group.id())) + .filter(dsl::instance_id.eq(instance_id)) + .select(AntiAffinityGroupInstanceMembership::as_select()) + .get_result_async(&*conn) + .await + .map(|m| m.into()) + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::AntiAffinityGroupMember, + LookupType::by_id(instance_id), + ), + ) + }) + } + + pub async fn affinity_group_member_add( + &self, + opctx: &OpContext, + authz_affinity_group: &authz::AffinityGroup, + member: external::AffinityGroupMember, + ) -> Result<(), Error> { + opctx.authorize(authz::Action::Modify, authz_affinity_group).await?; + + let instance_id = match member { + external::AffinityGroupMember::Instance(id) => id, + }; + + let err = OptionalError::new(); + let conn = self.pool_connection_authorized(opctx).await?; + self.transaction_retry_wrapper("affinity_group_member_add") + .transaction(&conn, |conn| { + let err = err.clone(); + use db::schema::affinity_group::dsl as group_dsl; + use db::schema::affinity_group_instance_membership::dsl as membership_dsl; + use db::schema::instance::dsl as instance_dsl; + + async move { + // Check that the group exists + group_dsl::affinity_group + .filter(group_dsl::time_deleted.is_null()) + .filter(group_dsl::id.eq(authz_affinity_group.id())) + .select(group_dsl::id) + .first_async::(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource( + authz_affinity_group, + ), + ) + }) + })?; + + // Check that the instance exists, and has no VMM + // + // NOTE: I'd prefer to use the "LookupPath" infrastructure + // to look up the path, but that API does not give the + // option to use the transaction's database connection. + // + // Looking up the instance on a different database + // connection than the transaction risks several concurrency + // issues, so we do the lookup manually. + let instance_state = instance_dsl::instance + .filter(instance_dsl::time_deleted.is_null()) + .filter(instance_dsl::id.eq(instance_id)) + .select(instance_dsl::state) + .first_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Instance, + LookupType::ById(instance_id) + ), + ) + }) + })?; + + // NOTE: It may be possible to add non-stopped instances to + // affinity groups, depending on where they have already + // been placed. However, only operating on "stopped" + // instances is much easier to work with, as it does not + // require any understanding of the group policy. + match instance_state { + InstanceState::NoVmm => (), + other => { + return Err(err.bail(Error::invalid_request( + format!( + "Instance cannot be added to affinity group in state: {other}" + ) + ))); + }, + } + + diesel::insert_into(membership_dsl::affinity_group_instance_membership) + .values(AffinityGroupInstanceMembership::new( + AffinityGroupUuid::from_untyped_uuid(authz_affinity_group.id()), + InstanceUuid::from_untyped_uuid(instance_id), + )) + .execute_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::Conflict( + ResourceType::AffinityGroupMember, + &instance_id.to_string(), + ), + ) + }) + })?; + Ok(()) + } + }) + .await + .map_err(|e| { + if let Some(err) = err.take() { + return err; + } + public_error_from_diesel(e, ErrorHandler::Server) + })?; + Ok(()) + } + + pub async fn anti_affinity_group_member_add( + &self, + opctx: &OpContext, + authz_anti_affinity_group: &authz::AntiAffinityGroup, + member: external::AntiAffinityGroupMember, + ) -> Result<(), Error> { + opctx + .authorize(authz::Action::Modify, authz_anti_affinity_group) + .await?; + + let instance_id = match member { + external::AntiAffinityGroupMember::Instance(id) => id, + }; + + let err = OptionalError::new(); + let conn = self.pool_connection_authorized(opctx).await?; + self.transaction_retry_wrapper("anti_affinity_group_member_add") + .transaction(&conn, |conn| { + let err = err.clone(); + use db::schema::anti_affinity_group::dsl as group_dsl; + use db::schema::anti_affinity_group_instance_membership::dsl as membership_dsl; + use db::schema::instance::dsl as instance_dsl; + + async move { + // Check that the group exists + group_dsl::anti_affinity_group + .filter(group_dsl::time_deleted.is_null()) + .filter(group_dsl::id.eq(authz_anti_affinity_group.id())) + .select(group_dsl::id) + .first_async::(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource( + authz_anti_affinity_group, + ), + ) + }) + })?; + + // Check that the instance exists, and has no VMM + let instance_state = instance_dsl::instance + .filter(instance_dsl::time_deleted.is_null()) + .filter(instance_dsl::id.eq(instance_id)) + .select(instance_dsl::state) + .first_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Instance, + LookupType::ById(instance_id) + ), + ) + }) + })?; + + // NOTE: It may be possible to add non-stopped instances to + // anti affinity groups, depending on where they have already + // been placed. However, only operating on "stopped" + // instances is much easier to work with, as it does not + // require any understanding of the group policy. + match instance_state { + InstanceState::NoVmm => (), + other => { + return Err(err.bail(Error::invalid_request( + format!( + "Instance cannot be added to anti-affinity group in state: {other}" + ) + ))); + }, + } + + diesel::insert_into(membership_dsl::anti_affinity_group_instance_membership) + .values(AntiAffinityGroupInstanceMembership::new( + AntiAffinityGroupUuid::from_untyped_uuid(authz_anti_affinity_group.id()), + InstanceUuid::from_untyped_uuid(instance_id), + )) + .execute_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::Conflict( + ResourceType::AntiAffinityGroupMember, + &instance_id.to_string(), + ), + ) + }) + })?; + Ok(()) + } + }) + .await + .map_err(|e| { + if let Some(err) = err.take() { + return err; + } + public_error_from_diesel(e, ErrorHandler::Server) + })?; + Ok(()) + } + + pub async fn instance_affinity_group_memberships_delete( + &self, + opctx: &OpContext, + instance_id: InstanceUuid, + ) -> Result<(), Error> { + use db::schema::affinity_group_instance_membership::dsl; + + diesel::delete(dsl::affinity_group_instance_membership) + .filter(dsl::instance_id.eq(instance_id.into_untyped_uuid())) + .execute_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + Ok(()) + } + + pub async fn instance_anti_affinity_group_memberships_delete( + &self, + opctx: &OpContext, + instance_id: InstanceUuid, + ) -> Result<(), Error> { + use db::schema::anti_affinity_group_instance_membership::dsl; + + diesel::delete(dsl::anti_affinity_group_instance_membership) + .filter(dsl::instance_id.eq(instance_id.into_untyped_uuid())) + .execute_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; + Ok(()) + } + + pub async fn affinity_group_member_delete( + &self, + opctx: &OpContext, + authz_affinity_group: &authz::AffinityGroup, + member: external::AffinityGroupMember, + ) -> Result<(), Error> { + opctx.authorize(authz::Action::Modify, authz_affinity_group).await?; + + let instance_id = match member { + external::AffinityGroupMember::Instance(id) => id, + }; + + let err = OptionalError::new(); + let conn = self.pool_connection_authorized(opctx).await?; + self.transaction_retry_wrapper("affinity_group_member_delete") + .transaction(&conn, |conn| { + let err = err.clone(); + use db::schema::affinity_group::dsl as group_dsl; + use db::schema::affinity_group_instance_membership::dsl as membership_dsl; + use db::schema::instance::dsl as instance_dsl; + + async move { + // Check that the group exists + group_dsl::affinity_group + .filter(group_dsl::time_deleted.is_null()) + .filter(group_dsl::id.eq(authz_affinity_group.id())) + .select(group_dsl::id) + .first_async::(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource( + authz_affinity_group, + ), + ) + }) + })?; + + // Check that the instance exists + instance_dsl::instance + .filter(instance_dsl::time_deleted.is_null()) + .filter(instance_dsl::id.eq(instance_id)) + .select(instance_dsl::id) + .first_async::(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Instance, + LookupType::ById(instance_id) + ), + ) + }) + })?; + + let rows = diesel::delete(membership_dsl::affinity_group_instance_membership) + .filter(membership_dsl::group_id.eq(authz_affinity_group.id())) + .filter(membership_dsl::instance_id.eq(instance_id)) + .execute_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel(e, ErrorHandler::Server) + }) + })?; + if rows == 0 { + return Err(err.bail(LookupType::ById(instance_id).into_not_found( + ResourceType::AffinityGroupMember, + ))); + } + Ok(()) + } + }) + .await + .map_err(|e| { + if let Some(err) = err.take() { + return err; + } + public_error_from_diesel(e, ErrorHandler::Server) + })?; + Ok(()) + } + + pub async fn anti_affinity_group_member_delete( + &self, + opctx: &OpContext, + authz_anti_affinity_group: &authz::AntiAffinityGroup, + member: external::AntiAffinityGroupMember, + ) -> Result<(), Error> { + opctx + .authorize(authz::Action::Modify, authz_anti_affinity_group) + .await?; + + let instance_id = match member { + external::AntiAffinityGroupMember::Instance(id) => id, + }; + + let err = OptionalError::new(); + let conn = self.pool_connection_authorized(opctx).await?; + self.transaction_retry_wrapper("anti_affinity_group_member_delete") + .transaction(&conn, |conn| { + let err = err.clone(); + use db::schema::anti_affinity_group::dsl as group_dsl; + use db::schema::anti_affinity_group_instance_membership::dsl as membership_dsl; + use db::schema::instance::dsl as instance_dsl; + + async move { + // Check that the group exists + group_dsl::anti_affinity_group + .filter(group_dsl::time_deleted.is_null()) + .filter(group_dsl::id.eq(authz_anti_affinity_group.id())) + .select(group_dsl::id) + .first_async::(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource( + authz_anti_affinity_group, + ), + ) + }) + })?; + + // Check that the instance exists + instance_dsl::instance + .filter(instance_dsl::time_deleted.is_null()) + .filter(instance_dsl::id.eq(instance_id)) + .select(instance_dsl::id) + .first_async::(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByLookup( + ResourceType::Instance, + LookupType::ById(instance_id) + ), + ) + }) + })?; + + let rows = diesel::delete(membership_dsl::anti_affinity_group_instance_membership) + .filter(membership_dsl::group_id.eq(authz_anti_affinity_group.id())) + .filter(membership_dsl::instance_id.eq(instance_id)) + .execute_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel(e, ErrorHandler::Server) + }) + })?; + if rows == 0 { + return Err(err.bail(LookupType::ById(instance_id).into_not_found( + ResourceType::AntiAffinityGroupMember, + ))); + } + Ok(()) + } + }) + .await + .map_err(|e| { + if let Some(err) = err.take() { + return err; + } + public_error_from_diesel(e, ErrorHandler::Server) + })?; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use crate::db::lookup::LookupPath; + use crate::db::pub_test_utils::TestDatabase; + use nexus_db_model::Instance; + use nexus_types::external_api::params; + use omicron_common::api::external::{ + self, ByteCount, DataPageParams, IdentityMetadataCreateParams, + }; + use omicron_test_utils::dev; + use omicron_uuid_kinds::InstanceUuid; + use std::num::NonZeroU32; + + // Helper function for creating a project + async fn create_project( + opctx: &OpContext, + datastore: &DataStore, + name: &str, + ) -> (authz::Project, Project) { + let authz_silo = opctx.authn.silo_required().unwrap(); + let project = Project::new( + authz_silo.id(), + params::ProjectCreate { + identity: IdentityMetadataCreateParams { + name: name.parse().unwrap(), + description: "".to_string(), + }, + }, + ); + datastore.project_create(&opctx, project).await.unwrap() + } + + // Helper function for creating an affinity group with + // arbitrary configuration. + async fn create_affinity_group( + opctx: &OpContext, + datastore: &DataStore, + authz_project: &authz::Project, + name: &str, + ) -> CreateResult { + let group = AffinityGroup::new( + authz_project.id(), + params::AffinityGroupCreate { + identity: IdentityMetadataCreateParams { + name: name.parse().unwrap(), + description: "".to_string(), + }, + policy: external::AffinityPolicy::Fail, + failure_domain: external::FailureDomain::Sled, + }, + ); + datastore.affinity_group_create(&opctx, &authz_project, group).await + } + + // Helper function for creating an anti-affinity group with + // arbitrary configuration. + async fn create_anti_affinity_group( + opctx: &OpContext, + datastore: &DataStore, + authz_project: &authz::Project, + name: &str, + ) -> CreateResult { + let group = AntiAffinityGroup::new( + authz_project.id(), + params::AntiAffinityGroupCreate { + identity: IdentityMetadataCreateParams { + name: name.parse().unwrap(), + description: "".to_string(), + }, + policy: external::AffinityPolicy::Fail, + failure_domain: external::FailureDomain::Sled, + }, + ); + datastore + .anti_affinity_group_create(&opctx, &authz_project, group) + .await + } + + // Helper function for creating an instance without a VMM. + async fn create_instance_record( + opctx: &OpContext, + datastore: &DataStore, + authz_project: &authz::Project, + name: &str, + ) -> Instance { + let instance = Instance::new( + InstanceUuid::new_v4(), + authz_project.id(), + ¶ms::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: name.parse().unwrap(), + description: "".to_string(), + }, + ncpus: 2i64.try_into().unwrap(), + memory: ByteCount::from_gibibytes_u32(16), + hostname: "myhostname".try_into().unwrap(), + user_data: Vec::new(), + network_interfaces: + params::InstanceNetworkInterfaceAttachment::None, + external_ips: Vec::new(), + disks: Vec::new(), + boot_disk: None, + ssh_public_keys: None, + start: false, + auto_restart_policy: Default::default(), + }, + ); + + let instance = datastore + .project_create_instance(&opctx, &authz_project, instance) + .await + .unwrap(); + + set_instance_state_stopped(&datastore, instance.id()).await; + + instance + } + + // Helper for explicitly modifying instance state. + // + // The interaction we typically use to create and modify instance state + // is more complex in production, since it's the result of a back-and-forth + // between Nexus and Sled Agent, using carefully crafted rcgen values. + // + // Here, we just set the value of state explicitly. Be warned, there + // are no guardrails! + async fn set_instance_state_stopped( + datastore: &DataStore, + instance: uuid::Uuid, + ) { + use db::schema::instance::dsl; + diesel::update(dsl::instance) + .filter(dsl::id.eq(instance)) + .set(( + dsl::state.eq(db::model::InstanceState::NoVmm), + dsl::active_propolis_id.eq(None::), + )) + .execute_async( + &*datastore.pool_connection_for_tests().await.unwrap(), + ) + .await + .unwrap(); + } + + async fn set_instance_state_running( + datastore: &DataStore, + instance: uuid::Uuid, + ) { + use db::schema::instance::dsl; + diesel::update(dsl::instance) + .filter(dsl::id.eq(instance)) + .set(( + dsl::state.eq(db::model::InstanceState::Vmm), + dsl::active_propolis_id.eq(uuid::Uuid::new_v4()), + )) + .execute_async( + &*datastore.pool_connection_for_tests().await.unwrap(), + ) + .await + .unwrap(); + } + + #[tokio::test] + async fn affinity_groups_are_project_scoped() { + // Setup + let logctx = dev::test_setup_log("affinity_groups_are_project_scoped"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + let (authz_project, _) = + create_project(&opctx, &datastore, "my-project").await; + + let (authz_other_project, _) = + create_project(&opctx, &datastore, "my-other-project").await; + + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + + // To start: No groups exist + let groups = datastore + .affinity_group_list(&opctx, &authz_project, &pagbyid) + .await + .unwrap(); + assert!(groups.is_empty()); + + // Create a group + let group = create_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + // Now when we list groups, we'll see the one we created. + let groups = datastore + .affinity_group_list(&opctx, &authz_project, &pagbyid) + .await + .unwrap(); + assert_eq!(groups.len(), 1); + assert_eq!(groups[0], group); + + // This group won't appear in the other project + let groups = datastore + .affinity_group_list(&opctx, &authz_other_project, &pagbyid) + .await + .unwrap(); + assert!(groups.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn anti_affinity_groups_are_project_scoped() { + // Setup + let logctx = + dev::test_setup_log("anti_affinity_groups_are_project_scoped"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + let (authz_project, _) = + create_project(&opctx, &datastore, "my-project").await; + + let (authz_other_project, _) = + create_project(&opctx, &datastore, "my-other-project").await; + + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + + // To start: No groups exist + let groups = datastore + .anti_affinity_group_list(&opctx, &authz_project, &pagbyid) + .await + .unwrap(); + assert!(groups.is_empty()); + + // Create a group + let group = create_anti_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + // Now when we list groups, we'll see the one we created. + let groups = datastore + .anti_affinity_group_list(&opctx, &authz_project, &pagbyid) + .await + .unwrap(); + assert_eq!(groups.len(), 1); + assert_eq!(groups[0], group); + + // This group won't appear in the other project + let groups = datastore + .anti_affinity_group_list(&opctx, &authz_other_project, &pagbyid) + .await + .unwrap(); + assert!(groups.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn affinity_groups_prevent_project_deletion() { + // Setup + let logctx = + dev::test_setup_log("affinity_groups_prevent_project_deletion"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, mut project) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + // If we try to delete the project, we'll fail. + let err = datastore + .project_delete(&opctx, &authz_project, &project) + .await + .unwrap_err(); + assert!(matches!(err, Error::InvalidRequest { .. })); + assert!( + err.to_string() + .contains("project to be deleted contains an affinity group"), + "{err:?}" + ); + + // Delete the group, then try to delete the project again. + let (.., authz_group) = LookupPath::new(opctx, datastore) + .affinity_group_id(group.id()) + .lookup_for(authz::Action::Delete) + .await + .unwrap(); + datastore.affinity_group_delete(&opctx, &authz_group).await.unwrap(); + + // When the group was created, it bumped the rcgen in the project. If we + // have an old view of the project, we expect a "concurrent + // modification" error. + let err = datastore + .project_delete(&opctx, &authz_project, &project) + .await + .unwrap_err(); + assert!(err.to_string().contains("concurrent modification"), "{err:?}"); + + // If we update rcgen, however, and the group has been deleted, + // we can successfully delete the project. + project.rcgen = project.rcgen.next().into(); + datastore + .project_delete(&opctx, &authz_project, &project) + .await + .unwrap(); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn anti_affinity_groups_prevent_project_deletion() { + // Setup + let logctx = dev::test_setup_log( + "anti_affinity_groups_prevent_project_deletion", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, mut project) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_anti_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + // If we try to delete the project, we'll fail. + let err = datastore + .project_delete(&opctx, &authz_project, &project) + .await + .unwrap_err(); + assert!(matches!(err, Error::InvalidRequest { .. })); + assert!( + err.to_string().contains( + "project to be deleted contains an anti affinity group" + ), + "{err:?}" + ); + + // Delete the group, then try to delete the project again. + let (.., authz_group) = LookupPath::new(opctx, datastore) + .anti_affinity_group_id(group.id()) + .lookup_for(authz::Action::Delete) + .await + .unwrap(); + datastore + .anti_affinity_group_delete(&opctx, &authz_group) + .await + .unwrap(); + + // When the group was created, it bumped the rcgen in the project. If we + // have an old view of the project, we expect a "concurrent + // modification" error. + let err = datastore + .project_delete(&opctx, &authz_project, &project) + .await + .unwrap_err(); + assert!(err.to_string().contains("concurrent modification"), "{err:?}"); + + // If we update rcgen, however, and the group has been deleted, + // we can successfully delete the project. + project.rcgen = project.rcgen.next().into(); + datastore + .project_delete(&opctx, &authz_project, &project) + .await + .unwrap(); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn affinity_group_names_are_unique_in_project() { + // Setup + let logctx = + dev::test_setup_log("affinity_group_names_are_unique_in_project"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create two projects + let (authz_project1, _) = + create_project(&opctx, &datastore, "my-project-1").await; + let (authz_project2, _) = + create_project(&opctx, &datastore, "my-project-2").await; + + // We can create a group wiht the same name in different projects + let group = create_affinity_group( + &opctx, + &datastore, + &authz_project1, + "my-group", + ) + .await + .unwrap(); + create_affinity_group(&opctx, &datastore, &authz_project2, "my-group") + .await + .unwrap(); + + // If we try to create a new group with the same name in the same + // project, we'll see an error. + let err = create_affinity_group( + &opctx, + &datastore, + &authz_project1, + "my-group", + ) + .await + .unwrap_err(); + assert!( + matches!(&err, Error::ObjectAlreadyExists { + type_name, + object_name, + } if *type_name == ResourceType::AffinityGroup && + *object_name == "my-group"), + "Unexpected error: {err:?}" + ); + + // If we delete the group from the project, we can re-use the name. + let (.., authz_group) = LookupPath::new(opctx, datastore) + .affinity_group_id(group.id()) + .lookup_for(authz::Action::Delete) + .await + .unwrap(); + datastore.affinity_group_delete(&opctx, &authz_group).await.unwrap(); + + create_affinity_group(&opctx, &datastore, &authz_project1, "my-group") + .await + .expect("Should have been able to re-use name after deletion"); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn anti_affinity_group_names_are_unique_in_project() { + // Setup + let logctx = dev::test_setup_log( + "anti_affinity_group_names_are_unique_in_project", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create two projects + let (authz_project1, _) = + create_project(&opctx, &datastore, "my-project-1").await; + let (authz_project2, _) = + create_project(&opctx, &datastore, "my-project-2").await; + + // We can create a group wiht the same name in different projects + let group = create_anti_affinity_group( + &opctx, + &datastore, + &authz_project1, + "my-group", + ) + .await + .unwrap(); + create_anti_affinity_group( + &opctx, + &datastore, + &authz_project2, + "my-group", + ) + .await + .unwrap(); + + // If we try to create a new group with the same name in the same + // project, we'll see an error. + let err = create_anti_affinity_group( + &opctx, + &datastore, + &authz_project1, + "my-group", + ) + .await + .unwrap_err(); + assert!( + matches!(&err, Error::ObjectAlreadyExists { + type_name, + object_name, + } if *type_name == ResourceType::AntiAffinityGroup && + *object_name == "my-group"), + "Unexpected error: {err:?}" + ); + + // If we delete the group from the project, we can re-use the name. + let (.., authz_group) = LookupPath::new(opctx, datastore) + .anti_affinity_group_id(group.id()) + .lookup_for(authz::Action::Delete) + .await + .unwrap(); + datastore + .anti_affinity_group_delete(&opctx, &authz_group) + .await + .unwrap(); + + create_anti_affinity_group( + &opctx, + &datastore, + &authz_project1, + "my-group", + ) + .await + .expect("Should have been able to re-use name after deletion"); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn affinity_group_membership_add_list_remove() { + // Setup + let logctx = + dev::test_setup_log("affinity_group_membership_add_list_remove"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + let (.., authz_group) = LookupPath::new(opctx, datastore) + .affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + // A new group should have no members + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Create an instance without a VMM. + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + + // Add the instance as a member to the group + datastore + .affinity_group_member_add( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + + // We should now be able to list the new member + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert_eq!(members.len(), 1); + assert_eq!( + external::AffinityGroupMember::Instance(instance.id()), + members[0].clone().into() + ); + + // We can delete the member and observe an empty member list + datastore + .affinity_group_member_delete( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn anti_affinity_group_membership_add_list_remove() { + // Setup + let logctx = dev::test_setup_log( + "anti_affinity_group_membership_add_list_remove", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_anti_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + let (.., authz_group) = LookupPath::new(opctx, datastore) + .anti_affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + // A new group should have no members + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Create an instance without a VMM. + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + + // Add the instance as a member to the group + datastore + .anti_affinity_group_member_add( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + + // We should now be able to list the new member + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert_eq!(members.len(), 1); + assert_eq!( + external::AntiAffinityGroupMember::Instance(instance.id()), + members[0].clone().into() + ); + + // We can delete the member and observe an empty member list + datastore + .anti_affinity_group_member_delete( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn affinity_group_membership_add_remove_instance_with_vmm() { + // Setup + let logctx = dev::test_setup_log( + "affinity_group_membership_add_remove_instance_with_vmm", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + let (.., authz_group) = LookupPath::new(opctx, datastore) + .affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + // A new group should have no members + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Create an instance with a VMM. + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + set_instance_state_running(&datastore, instance.id()).await; + + // Cannot add the instance to the group while it's running. + let err = datastore + .affinity_group_member_add( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .expect_err( + "Shouldn't be able to add running instances to affinity groups", + ); + assert!(matches!(err, Error::InvalidRequest { .. })); + assert!( + err.to_string().contains( + "Instance cannot be added to affinity group in state" + ), + "{err:?}" + ); + + // If we stop the instance, we can add it to the group. + set_instance_state_stopped(&datastore, instance.id()).await; + datastore + .affinity_group_member_add( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + + // Now we can set the instance state to "running" once more. + set_instance_state_running(&datastore, instance.id()).await; + + // We should now be able to list the new member + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert_eq!(members.len(), 1); + assert_eq!( + external::AffinityGroupMember::Instance(instance.id()), + members[0].clone().into() + ); + + // We can delete the member and observe an empty member list -- even + // though it's running! + datastore + .affinity_group_member_delete( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn anti_affinity_group_membership_add_remove_instance_with_vmm() { + // Setup + let logctx = dev::test_setup_log( + "anti_affinity_group_membership_add_remove_instance_with_vmm", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_anti_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + let (.., authz_group) = LookupPath::new(opctx, datastore) + .anti_affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + // A new group should have no members + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Create an instance with a VMM. + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + set_instance_state_running(&datastore, instance.id()).await; + + // Cannot add the instance to the group while it's running. + let err = datastore + .anti_affinity_group_member_add( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .expect_err( + "Shouldn't be able to add running instances to anti-affinity groups", + ); + assert!(matches!(err, Error::InvalidRequest { .. })); + assert!( + err.to_string().contains( + "Instance cannot be added to anti-affinity group in state" + ), + "{err:?}" + ); + + // If we stop the instance, we can add it to the group. + set_instance_state_stopped(&datastore, instance.id()).await; + datastore + .anti_affinity_group_member_add( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + + // Now we can set the instance state to "running" once more. + set_instance_state_running(&datastore, instance.id()).await; + + // We should now be able to list the new member + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert_eq!(members.len(), 1); + assert_eq!( + external::AntiAffinityGroupMember::Instance(instance.id()), + members[0].clone().into() + ); + + // We can delete the member and observe an empty member list -- even + // though it's running! + datastore + .anti_affinity_group_member_delete( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn affinity_group_delete_group_deletes_members() { + // Setup + let logctx = + dev::test_setup_log("affinity_group_delete_group_deletes_members"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + let (.., authz_group) = LookupPath::new(opctx, datastore) + .affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + // A new group should have no members + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Create an instance without a VMM, add it to the group. + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + datastore + .affinity_group_member_add( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + + // Delete the group + datastore.affinity_group_delete(&opctx, &authz_group).await.unwrap(); + + // Confirm that no instance members exist + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn anti_affinity_group_delete_group_deletes_members() { + // Setup + let logctx = dev::test_setup_log( + "anti_affinity_group_delete_group_deletes_members", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_anti_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + let (.., authz_group) = LookupPath::new(opctx, datastore) + .anti_affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + // A new group should have no members + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Create an instance without a VMM, add it to the group. + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + datastore + .anti_affinity_group_member_add( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + + // Delete the group + datastore + .anti_affinity_group_delete(&opctx, &authz_group) + .await + .unwrap(); + + // Confirm that no instance members exist + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn affinity_group_delete_instance_deletes_membership() { + // Setup + let logctx = dev::test_setup_log( + "affinity_group_delete_instance_deletes_membership", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + let (.., authz_group) = LookupPath::new(opctx, datastore) + .affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + // A new group should have no members + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Create an instance without a VMM, add it to the group. + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + datastore + .affinity_group_member_add( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + + // Delete the instance + let (.., authz_instance) = LookupPath::new(opctx, datastore) + .instance_id(instance.id()) + .lookup_for(authz::Action::Delete) + .await + .unwrap(); + datastore + .project_delete_instance(&opctx, &authz_instance) + .await + .unwrap(); + + // Confirm that no instance members exist + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn anti_affinity_group_delete_instance_deletes_membership() { + // Setup + let logctx = dev::test_setup_log( + "anti_affinity_group_delete_instance_deletes_membership", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_anti_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + let (.., authz_group) = LookupPath::new(opctx, datastore) + .anti_affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + // A new group should have no members + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Create an instance without a VMM, add it to the group. + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + datastore + .anti_affinity_group_member_add( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + + // Delete the instance + let (.., authz_instance) = LookupPath::new(opctx, datastore) + .instance_id(instance.id()) + .lookup_for(authz::Action::Delete) + .await + .unwrap(); + datastore + .project_delete_instance(&opctx, &authz_instance) + .await + .unwrap(); + + // Confirm that no instance members exist + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn affinity_group_membership_for_deleted_objects() { + // Setup + let logctx = dev::test_setup_log( + "affinity_group_membership_for_deleted_objects", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + + struct TestArgs { + // Does the group exist? + group: bool, + // Does the instance exist? + instance: bool, + } + + let args = [ + TestArgs { group: false, instance: false }, + TestArgs { group: true, instance: false }, + TestArgs { group: false, instance: true }, + ]; + + for arg in args { + // Create an affinity group, and maybe delete it. + let group = create_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + let (.., authz_group) = LookupPath::new(opctx, datastore) + .affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + if !arg.group { + datastore + .affinity_group_delete(&opctx, &authz_group) + .await + .unwrap(); + } + + // Create an instance, and maybe delete it. + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + let (.., authz_instance) = LookupPath::new(opctx, datastore) + .instance_id(instance.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + if !arg.instance { + datastore + .project_delete_instance(&opctx, &authz_instance) + .await + .unwrap(); + } + + // Try to add the instance to the group. + // + // Expect to see specific errors, depending on whether or not the + // group/instance exist. + let err = datastore + .affinity_group_member_add( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .expect_err("Should have failed"); + + match (arg.group, arg.instance) { + (false, _) => { + assert!( + matches!(err, Error::ObjectNotFound { + type_name, .. + } if type_name == ResourceType::AffinityGroup), + "{err:?}" + ); + } + (true, false) => { + assert!( + matches!(err, Error::ObjectNotFound { + type_name, .. + } if type_name == ResourceType::Instance), + "{err:?}" + ); + } + (true, true) => { + panic!("If both exist, we won't throw an error") + } + } + + // Do the same thing, but for group membership removal. + let err = datastore + .affinity_group_member_delete( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .expect_err("Should have failed"); + match (arg.group, arg.instance) { + (false, _) => { + assert!( + matches!(err, Error::ObjectNotFound { + type_name, .. + } if type_name == ResourceType::AffinityGroup), + "{err:?}" + ); + } + (true, false) => { + assert!( + matches!(err, Error::ObjectNotFound { + type_name, .. + } if type_name == ResourceType::Instance), + "{err:?}" + ); + } + (true, true) => { + panic!("If both exist, we won't throw an error") + } + } + + // Cleanup, if we actually created anything. + if arg.instance { + datastore + .project_delete_instance(&opctx, &authz_instance) + .await + .unwrap(); + } + if arg.group { + datastore + .affinity_group_delete(&opctx, &authz_group) + .await + .unwrap(); + } + } + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn anti_affinity_group_membership_for_deleted_objects() { + // Setup + let logctx = dev::test_setup_log( + "anti_affinity_group_membership_for_deleted_objects", + ); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + + struct TestArgs { + // Does the group exist? + group: bool, + // Does the instance exist? + instance: bool, + } + + let args = [ + TestArgs { group: false, instance: false }, + TestArgs { group: true, instance: false }, + TestArgs { group: false, instance: true }, + ]; + + for arg in args { + // Create an anti-affinity group, and maybe delete it. + let group = create_anti_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + + let (.., authz_group) = LookupPath::new(opctx, datastore) + .anti_affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + if !arg.group { + datastore + .anti_affinity_group_delete(&opctx, &authz_group) + .await + .unwrap(); + } + + // Create an instance, and maybe delete it. + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + let (.., authz_instance) = LookupPath::new(opctx, datastore) + .instance_id(instance.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + if !arg.instance { + datastore + .project_delete_instance(&opctx, &authz_instance) + .await + .unwrap(); + } + + // Try to add the instance to the group. + // + // Expect to see specific errors, depending on whether or not the + // group/instance exist. + let err = datastore + .anti_affinity_group_member_add( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .expect_err("Should have failed"); + + match (arg.group, arg.instance) { + (false, _) => { + assert!( + matches!(err, Error::ObjectNotFound { + type_name, .. + } if type_name == ResourceType::AntiAffinityGroup), + "{err:?}" + ); + } + (true, false) => { + assert!( + matches!(err, Error::ObjectNotFound { + type_name, .. + } if type_name == ResourceType::Instance), + "{err:?}" + ); + } + (true, true) => { + panic!("If both exist, we won't throw an error") + } + } + + // Do the same thing, but for group membership removal. + let err = datastore + .anti_affinity_group_member_delete( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .expect_err("Should have failed"); + match (arg.group, arg.instance) { + (false, _) => { + assert!( + matches!(err, Error::ObjectNotFound { + type_name, .. + } if type_name == ResourceType::AntiAffinityGroup), + "{err:?}" + ); + } + (true, false) => { + assert!( + matches!(err, Error::ObjectNotFound { + type_name, .. + } if type_name == ResourceType::Instance), + "{err:?}" + ); + } + (true, true) => { + panic!("If both exist, we won't throw an error") + } + } + + // Cleanup, if we actually created anything. + if arg.instance { + datastore + .project_delete_instance(&opctx, &authz_instance) + .await + .unwrap(); + } + if arg.group { + datastore + .anti_affinity_group_delete(&opctx, &authz_group) + .await + .unwrap(); + } + } + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn affinity_group_membership_idempotency() { + // Setup + let logctx = + dev::test_setup_log("affinity_group_membership_idempotency"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + let (.., authz_group) = LookupPath::new(opctx, datastore) + .affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + // Create an instance + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + + // Add the instance to the group + datastore + .affinity_group_member_add( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + + // Add the instance to the group again + let err = datastore + .affinity_group_member_add( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap_err(); + assert!( + matches!( + err, + Error::ObjectAlreadyExists { + type_name: ResourceType::AffinityGroupMember, + .. + } + ), + "Error: {err:?}" + ); + + // We should still only observe a single member in the group. + // + // Two calls to "affinity_group_member_add" should be the same + // as a single call. + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert_eq!(members.len(), 1); + + // We should be able to delete the membership idempotently. + datastore + .affinity_group_member_delete( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + let err = datastore + .affinity_group_member_delete( + &opctx, + &authz_group, + external::AffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap_err(); + assert!( + matches!( + err, + Error::ObjectNotFound { + type_name: ResourceType::AffinityGroupMember, + .. + } + ), + "Error: {err:?}" + ); + + let members = datastore + .affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } + + #[tokio::test] + async fn anti_affinity_group_membership_idempotency() { + // Setup + let logctx = + dev::test_setup_log("anti_affinity_group_membership_idempotency"); + let db = TestDatabase::new_with_datastore(&logctx.log).await; + let (opctx, datastore) = (db.opctx(), db.datastore()); + + // Create a project and a group + let (authz_project, ..) = + create_project(&opctx, &datastore, "my-project").await; + let group = create_anti_affinity_group( + &opctx, + &datastore, + &authz_project, + "my-group", + ) + .await + .unwrap(); + let (.., authz_group) = LookupPath::new(opctx, datastore) + .anti_affinity_group_id(group.id()) + .lookup_for(authz::Action::Modify) + .await + .unwrap(); + + // Create an instance + let instance = create_instance_record( + &opctx, + &datastore, + &authz_project, + "my-instance", + ) + .await; + + // Add the instance to the group + datastore + .anti_affinity_group_member_add( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + + // Add the instance to the group again + let err = datastore + .anti_affinity_group_member_add( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap_err(); + assert!( + matches!( + err, + Error::ObjectAlreadyExists { + type_name: ResourceType::AntiAffinityGroupMember, + .. + } + ), + "Error: {err:?}" + ); + + // We should still only observe a single member in the group. + // + // Two calls to "anti_affinity_group_member_add" should be the same + // as a single call. + let pagparams_id = DataPageParams { + marker: None, + limit: NonZeroU32::new(100).unwrap(), + direction: dropshot::PaginationOrder::Ascending, + }; + let pagbyid = PaginatedBy::Id(pagparams_id); + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert_eq!(members.len(), 1); + + // We should be able to delete the membership idempotently. + datastore + .anti_affinity_group_member_delete( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap(); + let err = datastore + .anti_affinity_group_member_delete( + &opctx, + &authz_group, + external::AntiAffinityGroupMember::Instance(instance.id()), + ) + .await + .unwrap_err(); + assert!( + matches!( + err, + Error::ObjectNotFound { + type_name: ResourceType::AntiAffinityGroupMember, + .. + } + ), + "Error: {err:?}" + ); + + let members = datastore + .anti_affinity_group_member_list(&opctx, &authz_group, &pagbyid) + .await + .unwrap(); + assert!(members.is_empty()); + + // Clean up. + db.terminate().await; + logctx.cleanup_successful(); + } +} diff --git a/nexus/db-queries/src/db/datastore/instance.rs b/nexus/db-queries/src/db/datastore/instance.rs index 65d42fbe7c..9e4bec81d0 100644 --- a/nexus/db-queries/src/db/datastore/instance.rs +++ b/nexus/db-queries/src/db/datastore/instance.rs @@ -1387,6 +1387,13 @@ impl DataStore { })?; let instance_id = InstanceUuid::from_untyped_uuid(authz_instance.id()); + self.instance_affinity_group_memberships_delete(opctx, instance_id) + .await?; + self.instance_anti_affinity_group_memberships_delete( + opctx, + instance_id, + ) + .await?; self.instance_ssh_keys_delete(opctx, instance_id).await?; self.instance_mark_migrations_deleted(opctx, instance_id).await?; diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index b2d9f8f247..5c241578a5 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -49,6 +49,7 @@ use std::num::NonZeroU32; use std::sync::Arc; mod address_lot; +mod affinity; mod allow_list; mod auth; mod bfd; diff --git a/nexus/db-queries/src/db/datastore/project.rs b/nexus/db-queries/src/db/datastore/project.rs index 58b7b315c1..367e094b2e 100644 --- a/nexus/db-queries/src/db/datastore/project.rs +++ b/nexus/db-queries/src/db/datastore/project.rs @@ -225,6 +225,8 @@ impl DataStore { generate_fn_to_ensure_none_in_project!(project_image, name, String); generate_fn_to_ensure_none_in_project!(snapshot, name, String); generate_fn_to_ensure_none_in_project!(vpc, name, String); + generate_fn_to_ensure_none_in_project!(affinity_group, name, String); + generate_fn_to_ensure_none_in_project!(anti_affinity_group, name, String); /// Delete a project pub async fn project_delete( @@ -242,6 +244,9 @@ impl DataStore { self.ensure_no_project_images_in_project(opctx, authz_project).await?; self.ensure_no_snapshots_in_project(opctx, authz_project).await?; self.ensure_no_vpcs_in_project(opctx, authz_project).await?; + self.ensure_no_affinity_groups_in_project(opctx, authz_project).await?; + self.ensure_no_anti_affinity_groups_in_project(opctx, authz_project) + .await?; use db::schema::project::dsl; diff --git a/nexus/db-queries/src/db/lookup.rs b/nexus/db-queries/src/db/lookup.rs index c629dbfc42..d4591024e7 100644 --- a/nexus/db-queries/src/db/lookup.rs +++ b/nexus/db-queries/src/db/lookup.rs @@ -175,6 +175,16 @@ impl<'a> LookupPath<'a> { Instance::PrimaryKey(Root { lookup_root: self }, id) } + /// Select a resource of type AffinityGroup, identified by its id + pub fn affinity_group_id(self, id: Uuid) -> AffinityGroup<'a> { + AffinityGroup::PrimaryKey(Root { lookup_root: self }, id) + } + + /// Select a resource of type AntiAffinityGroup, identified by its id + pub fn anti_affinity_group_id(self, id: Uuid) -> AntiAffinityGroup<'a> { + AntiAffinityGroup::PrimaryKey(Root { lookup_root: self }, id) + } + /// Select a resource of type IpPool, identified by its name pub fn ip_pool_name<'b, 'c>(self, name: &'b Name) -> IpPool<'c> where @@ -645,7 +655,7 @@ lookup_resource! { lookup_resource! { name = "Project", ancestors = [ "Silo" ], - children = [ "Disk", "Instance", "Vpc", "Snapshot", "ProjectImage", "FloatingIp" ], + children = [ "AffinityGroup", "AntiAffinityGroup", "Disk", "Instance", "Vpc", "Snapshot", "ProjectImage", "FloatingIp" ], lookup_by_name = true, soft_deletes = true, primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] @@ -696,6 +706,24 @@ lookup_resource! { primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } +lookup_resource! { + name = "AffinityGroup", + ancestors = [ "Silo", "Project" ], + children = [], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] +} + +lookup_resource! { + name = "AntiAffinityGroup", + ancestors = [ "Silo", "Project" ], + children = [], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] +} + lookup_resource! { name = "InstanceNetworkInterface", ancestors = [ "Silo", "Project", "Instance" ], From 161f9d60759521080dcb84736e9077c631add27a Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Thu, 30 Jan 2025 13:04:02 -0800 Subject: [PATCH 2/6] fix policy tests --- .../src/policy_test/resource_builder.rs | 2 + nexus/db-queries/src/policy_test/resources.rs | 17 ++++ nexus/db-queries/tests/output/authz-roles.out | 84 +++++++++++++++++++ 3 files changed, 103 insertions(+) diff --git a/nexus/db-queries/src/policy_test/resource_builder.rs b/nexus/db-queries/src/policy_test/resource_builder.rs index b6d7d97553..310c11adf3 100644 --- a/nexus/db-queries/src/policy_test/resource_builder.rs +++ b/nexus/db-queries/src/policy_test/resource_builder.rs @@ -243,6 +243,8 @@ macro_rules! impl_dyn_authorized_resource_for_resource { } impl_dyn_authorized_resource_for_resource!(authz::AddressLot); +impl_dyn_authorized_resource_for_resource!(authz::AffinityGroup); +impl_dyn_authorized_resource_for_resource!(authz::AntiAffinityGroup); impl_dyn_authorized_resource_for_resource!(authz::Blueprint); impl_dyn_authorized_resource_for_resource!(authz::Certificate); impl_dyn_authorized_resource_for_resource!(authz::DeviceAccessToken); diff --git a/nexus/db-queries/src/policy_test/resources.rs b/nexus/db-queries/src/policy_test/resources.rs index 6ee92e167c..0465541053 100644 --- a/nexus/db-queries/src/policy_test/resources.rs +++ b/nexus/db-queries/src/policy_test/resources.rs @@ -300,6 +300,21 @@ async fn make_project( LookupType::ByName(vpc1_name.clone()), ); + let affinity_group_name = format!("{}-affinity-group1", project_name); + let affinity_group = authz::AffinityGroup::new( + project.clone(), + Uuid::new_v4(), + LookupType::ByName(affinity_group_name.clone()), + ); + + let anti_affinity_group_name = + format!("{}-anti-affinity-group1", project_name); + let anti_affinity_group = authz::AntiAffinityGroup::new( + project.clone(), + Uuid::new_v4(), + LookupType::ByName(anti_affinity_group_name.clone()), + ); + let instance_name = format!("{}-instance1", project_name); let instance = authz::Instance::new( project.clone(), @@ -313,6 +328,8 @@ async fn make_project( Uuid::new_v4(), LookupType::ByName(disk_name.clone()), )); + builder.new_resource(affinity_group.clone()); + builder.new_resource(anti_affinity_group.clone()); builder.new_resource(instance.clone()); builder.new_resource(authz::InstanceNetworkInterface::new( instance, diff --git a/nexus/db-queries/tests/output/authz-roles.out b/nexus/db-queries/tests/output/authz-roles.out index 4b24e649cc..e0d43250d1 100644 --- a/nexus/db-queries/tests/output/authz-roles.out +++ b/nexus/db-queries/tests/output/authz-roles.out @@ -306,6 +306,34 @@ resource: Disk "silo1-proj1-disk1" silo1-proj1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! +resource: AffinityGroup "silo1-proj1-affinity-group1" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-proj1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-proj1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + +resource: AntiAffinityGroup "silo1-proj1-anti-affinity-group1" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-proj1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-proj1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + resource: Instance "silo1-proj1-instance1" USER Q R LC RP M MP CC D @@ -474,6 +502,34 @@ resource: Disk "silo1-proj2-disk1" silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! +resource: AffinityGroup "silo1-proj2-affinity-group1" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + +resource: AntiAffinityGroup "silo1-proj2-anti-affinity-group1" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + resource: Instance "silo1-proj2-instance1" USER Q R LC RP M MP CC D @@ -810,6 +866,34 @@ resource: Disk "silo2-proj1-disk1" silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! +resource: AffinityGroup "silo2-proj1-affinity-group1" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + +resource: AntiAffinityGroup "silo2-proj1-anti-affinity-group1" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + resource: Instance "silo2-proj1-instance1" USER Q R LC RP M MP CC D From f2ebe31df00ef40a655d070288c467c2c23398a9 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 31 Jan 2025 14:45:01 -0800 Subject: [PATCH 3/6] Typed UUID --- nexus/db-queries/src/db/datastore/affinity.rs | 154 ++++++++++++------ 1 file changed, 106 insertions(+), 48 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/affinity.rs b/nexus/db-queries/src/db/datastore/affinity.rs index e09a0e4340..78690c746b 100644 --- a/nexus/db-queries/src/db/datastore/affinity.rs +++ b/nexus/db-queries/src/db/datastore/affinity.rs @@ -409,7 +409,7 @@ impl DataStore { use db::schema::affinity_group_instance_membership::dsl; dsl::affinity_group_instance_membership .filter(dsl::group_id.eq(authz_affinity_group.id())) - .filter(dsl::instance_id.eq(instance_id)) + .filter(dsl::instance_id.eq(instance_id.into_untyped_uuid())) .select(AffinityGroupInstanceMembership::as_select()) .get_result_async(&*conn) .await @@ -419,7 +419,7 @@ impl DataStore { e, ErrorHandler::NotFoundByLookup( ResourceType::AffinityGroupMember, - LookupType::by_id(instance_id), + LookupType::by_id(instance_id.into_untyped_uuid()), ), ) }) @@ -441,7 +441,7 @@ impl DataStore { use db::schema::anti_affinity_group_instance_membership::dsl; dsl::anti_affinity_group_instance_membership .filter(dsl::group_id.eq(authz_anti_affinity_group.id())) - .filter(dsl::instance_id.eq(instance_id)) + .filter(dsl::instance_id.eq(instance_id.into_untyped_uuid())) .select(AntiAffinityGroupInstanceMembership::as_select()) .get_result_async(&*conn) .await @@ -451,7 +451,7 @@ impl DataStore { e, ErrorHandler::NotFoundByLookup( ResourceType::AntiAffinityGroupMember, - LookupType::by_id(instance_id), + LookupType::by_id(instance_id.into_untyped_uuid()), ), ) }) @@ -508,7 +508,7 @@ impl DataStore { // issues, so we do the lookup manually. let instance_state = instance_dsl::instance .filter(instance_dsl::time_deleted.is_null()) - .filter(instance_dsl::id.eq(instance_id)) + .filter(instance_dsl::id.eq(instance_id.into_untyped_uuid())) .select(instance_dsl::state) .first_async(&conn) .await @@ -518,7 +518,7 @@ impl DataStore { e, ErrorHandler::NotFoundByLookup( ResourceType::Instance, - LookupType::ById(instance_id) + LookupType::ById(instance_id.into_untyped_uuid()) ), ) }) @@ -543,7 +543,7 @@ impl DataStore { diesel::insert_into(membership_dsl::affinity_group_instance_membership) .values(AffinityGroupInstanceMembership::new( AffinityGroupUuid::from_untyped_uuid(authz_affinity_group.id()), - InstanceUuid::from_untyped_uuid(instance_id), + instance_id, )) .execute_async(&conn) .await @@ -616,7 +616,7 @@ impl DataStore { // Check that the instance exists, and has no VMM let instance_state = instance_dsl::instance .filter(instance_dsl::time_deleted.is_null()) - .filter(instance_dsl::id.eq(instance_id)) + .filter(instance_dsl::id.eq(instance_id.into_untyped_uuid())) .select(instance_dsl::state) .first_async(&conn) .await @@ -626,7 +626,7 @@ impl DataStore { e, ErrorHandler::NotFoundByLookup( ResourceType::Instance, - LookupType::ById(instance_id) + LookupType::ById(instance_id.into_untyped_uuid()) ), ) }) @@ -651,7 +651,7 @@ impl DataStore { diesel::insert_into(membership_dsl::anti_affinity_group_instance_membership) .values(AntiAffinityGroupInstanceMembership::new( AntiAffinityGroupUuid::from_untyped_uuid(authz_anti_affinity_group.id()), - InstanceUuid::from_untyped_uuid(instance_id), + instance_id, )) .execute_async(&conn) .await @@ -752,7 +752,7 @@ impl DataStore { // Check that the instance exists instance_dsl::instance .filter(instance_dsl::time_deleted.is_null()) - .filter(instance_dsl::id.eq(instance_id)) + .filter(instance_dsl::id.eq(instance_id.into_untyped_uuid())) .select(instance_dsl::id) .first_async::(&conn) .await @@ -762,7 +762,7 @@ impl DataStore { e, ErrorHandler::NotFoundByLookup( ResourceType::Instance, - LookupType::ById(instance_id) + LookupType::ById(instance_id.into_untyped_uuid()) ), ) }) @@ -770,7 +770,7 @@ impl DataStore { let rows = diesel::delete(membership_dsl::affinity_group_instance_membership) .filter(membership_dsl::group_id.eq(authz_affinity_group.id())) - .filter(membership_dsl::instance_id.eq(instance_id)) + .filter(membership_dsl::instance_id.eq(instance_id.into_untyped_uuid())) .execute_async(&conn) .await .map_err(|e| { @@ -779,7 +779,7 @@ impl DataStore { }) })?; if rows == 0 { - return Err(err.bail(LookupType::ById(instance_id).into_not_found( + return Err(err.bail(LookupType::ById(instance_id.into_untyped_uuid()).into_not_found( ResourceType::AffinityGroupMember, ))); } @@ -841,7 +841,7 @@ impl DataStore { // Check that the instance exists instance_dsl::instance .filter(instance_dsl::time_deleted.is_null()) - .filter(instance_dsl::id.eq(instance_id)) + .filter(instance_dsl::id.eq(instance_id.into_untyped_uuid())) .select(instance_dsl::id) .first_async::(&conn) .await @@ -851,7 +851,7 @@ impl DataStore { e, ErrorHandler::NotFoundByLookup( ResourceType::Instance, - LookupType::ById(instance_id) + LookupType::ById(instance_id.into_untyped_uuid()) ), ) }) @@ -859,7 +859,7 @@ impl DataStore { let rows = diesel::delete(membership_dsl::anti_affinity_group_instance_membership) .filter(membership_dsl::group_id.eq(authz_anti_affinity_group.id())) - .filter(membership_dsl::instance_id.eq(instance_id)) + .filter(membership_dsl::instance_id.eq(instance_id.into_untyped_uuid())) .execute_async(&conn) .await .map_err(|e| { @@ -868,7 +868,7 @@ impl DataStore { }) })?; if rows == 0 { - return Err(err.bail(LookupType::ById(instance_id).into_not_found( + return Err(err.bail(LookupType::ById(instance_id.into_untyped_uuid()).into_not_found( ResourceType::AntiAffinityGroupMember, ))); } @@ -1487,7 +1487,9 @@ mod tests { .affinity_group_member_add( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -1499,7 +1501,9 @@ mod tests { .unwrap(); assert_eq!(members.len(), 1); assert_eq!( - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()) + ), members[0].clone().into() ); @@ -1508,7 +1512,9 @@ mod tests { .affinity_group_member_delete( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -1577,7 +1583,9 @@ mod tests { .anti_affinity_group_member_add( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -1589,7 +1597,9 @@ mod tests { .unwrap(); assert_eq!(members.len(), 1); assert_eq!( - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()) + ), members[0].clone().into() ); @@ -1598,7 +1608,9 @@ mod tests { .anti_affinity_group_member_delete( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -1668,7 +1680,9 @@ mod tests { .affinity_group_member_add( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .expect_err( @@ -1688,7 +1702,9 @@ mod tests { .affinity_group_member_add( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -1703,7 +1719,9 @@ mod tests { .unwrap(); assert_eq!(members.len(), 1); assert_eq!( - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()) + ), members[0].clone().into() ); @@ -1713,7 +1731,9 @@ mod tests { .affinity_group_member_delete( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -1783,7 +1803,7 @@ mod tests { .anti_affinity_group_member_add( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance(InstanceUuid::from_untyped_uuid(instance.id())), ) .await .expect_err( @@ -1803,7 +1823,9 @@ mod tests { .anti_affinity_group_member_add( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -1818,7 +1840,9 @@ mod tests { .unwrap(); assert_eq!(members.len(), 1); assert_eq!( - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()) + ), members[0].clone().into() ); @@ -1828,7 +1852,9 @@ mod tests { .anti_affinity_group_member_delete( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -1894,7 +1920,9 @@ mod tests { .affinity_group_member_add( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -1966,7 +1994,9 @@ mod tests { .anti_affinity_group_member_add( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -2041,7 +2071,9 @@ mod tests { .affinity_group_member_add( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -2121,7 +2153,9 @@ mod tests { .anti_affinity_group_member_add( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -2227,7 +2261,9 @@ mod tests { .affinity_group_member_add( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .expect_err("Should have failed"); @@ -2259,7 +2295,9 @@ mod tests { .affinity_group_member_delete( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .expect_err("Should have failed"); @@ -2383,7 +2421,9 @@ mod tests { .anti_affinity_group_member_add( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .expect_err("Should have failed"); @@ -2415,7 +2455,9 @@ mod tests { .anti_affinity_group_member_delete( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .expect_err("Should have failed"); @@ -2500,7 +2542,9 @@ mod tests { .affinity_group_member_add( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -2510,7 +2554,9 @@ mod tests { .affinity_group_member_add( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap_err(); @@ -2546,7 +2592,9 @@ mod tests { .affinity_group_member_delete( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -2554,7 +2602,9 @@ mod tests { .affinity_group_member_delete( &opctx, &authz_group, - external::AffinityGroupMember::Instance(instance.id()), + external::AffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap_err(); @@ -2619,7 +2669,9 @@ mod tests { .anti_affinity_group_member_add( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -2629,7 +2681,9 @@ mod tests { .anti_affinity_group_member_add( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap_err(); @@ -2665,7 +2719,9 @@ mod tests { .anti_affinity_group_member_delete( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap(); @@ -2673,7 +2729,9 @@ mod tests { .anti_affinity_group_member_delete( &opctx, &authz_group, - external::AntiAffinityGroupMember::Instance(instance.id()), + external::AntiAffinityGroupMember::Instance( + InstanceUuid::from_untyped_uuid(instance.id()), + ), ) .await .unwrap_err(); From 1ad01010bcd8e59581a267463b3efb671a748343 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 31 Jan 2025 16:00:03 -0800 Subject: [PATCH 4/6] review feedback --- common/src/api/external/mod.rs | 6 + nexus/db-queries/src/db/datastore/affinity.rs | 231 ++++++++++-------- 2 files changed, 139 insertions(+), 98 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index f3234d74b9..f4b657a1a5 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -294,6 +294,12 @@ impl<'a> From<&'a Name> for &'a str { } } +impl From for String { + fn from(name: Name) -> Self { + name.0 + } +} + /// `Name` instances are comparable like Strings, primarily so that they can /// be used as keys in trees. impl PartialEq for Name diff --git a/nexus/db-queries/src/db/datastore/affinity.rs b/nexus/db-queries/src/db/datastore/affinity.rs index 78690c746b..5cd7c57910 100644 --- a/nexus/db-queries/src/db/datastore/affinity.rs +++ b/nexus/db-queries/src/db/datastore/affinity.rs @@ -20,7 +20,6 @@ use crate::db::model::AffinityGroupUpdate; use crate::db::model::AntiAffinityGroup; use crate::db::model::AntiAffinityGroupInstanceMembership; use crate::db::model::AntiAffinityGroupUpdate; -use crate::db::model::InstanceState; use crate::db::model::Name; use crate::db::model::Project; use crate::db::pagination::paginated; @@ -477,6 +476,7 @@ impl DataStore { use db::schema::affinity_group::dsl as group_dsl; use db::schema::affinity_group_instance_membership::dsl as membership_dsl; use db::schema::instance::dsl as instance_dsl; + use db::schema::sled_resource::dsl as resource_dsl; async move { // Check that the group exists @@ -497,7 +497,8 @@ impl DataStore { }) })?; - // Check that the instance exists, and has no VMM + // Check that the instance exists, and has no sled + // reservation. // // NOTE: I'd prefer to use the "LookupPath" infrastructure // to look up the path, but that API does not give the @@ -506,11 +507,12 @@ impl DataStore { // Looking up the instance on a different database // connection than the transaction risks several concurrency // issues, so we do the lookup manually. - let instance_state = instance_dsl::instance + + let _check_instance_exists = instance_dsl::instance .filter(instance_dsl::time_deleted.is_null()) .filter(instance_dsl::id.eq(instance_id.into_untyped_uuid())) - .select(instance_dsl::state) - .first_async(&conn) + .select(instance_dsl::id) + .get_result_async::(&conn) .await .map_err(|e| { err.bail_retryable_or_else(e, |e| { @@ -523,21 +525,33 @@ impl DataStore { ) }) })?; + let has_reservation: bool = diesel::select( + diesel::dsl::exists( + resource_dsl::sled_resource + .filter(resource_dsl::instance_id.eq(instance_id.into_untyped_uuid())) + ) + ).get_result_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::Server, + ) + }) + })?; // NOTE: It may be possible to add non-stopped instances to // affinity groups, depending on where they have already // been placed. However, only operating on "stopped" // instances is much easier to work with, as it does not // require any understanding of the group policy. - match instance_state { - InstanceState::NoVmm => (), - other => { - return Err(err.bail(Error::invalid_request( - format!( - "Instance cannot be added to affinity group in state: {other}" - ) - ))); - }, + if has_reservation { + return Err(err.bail(Error::invalid_request( + format!( + "Instance cannot be added to affinity group with reservation" + ) + ))); } diesel::insert_into(membership_dsl::affinity_group_instance_membership) @@ -593,6 +607,7 @@ impl DataStore { use db::schema::anti_affinity_group::dsl as group_dsl; use db::schema::anti_affinity_group_instance_membership::dsl as membership_dsl; use db::schema::instance::dsl as instance_dsl; + use db::schema::sled_resource::dsl as resource_dsl; async move { // Check that the group exists @@ -613,12 +628,13 @@ impl DataStore { }) })?; - // Check that the instance exists, and has no VMM - let instance_state = instance_dsl::instance + // Check that the instance exists, and has no sled + // reservation. + let _check_instance_exists = instance_dsl::instance .filter(instance_dsl::time_deleted.is_null()) .filter(instance_dsl::id.eq(instance_id.into_untyped_uuid())) - .select(instance_dsl::state) - .first_async(&conn) + .select(instance_dsl::id) + .get_result_async::(&conn) .await .map_err(|e| { err.bail_retryable_or_else(e, |e| { @@ -631,21 +647,33 @@ impl DataStore { ) }) })?; + let has_reservation: bool = diesel::select( + diesel::dsl::exists( + resource_dsl::sled_resource + .filter(resource_dsl::instance_id.eq(instance_id.into_untyped_uuid())) + ) + ).get_result_async(&conn) + .await + .map_err(|e| { + err.bail_retryable_or_else(e, |e| { + public_error_from_diesel( + e, + ErrorHandler::Server, + ) + }) + })?; // NOTE: It may be possible to add non-stopped instances to - // anti affinity groups, depending on where they have already + // affinity groups, depending on where they have already // been placed. However, only operating on "stopped" // instances is much easier to work with, as it does not // require any understanding of the group policy. - match instance_state { - InstanceState::NoVmm => (), - other => { - return Err(err.bail(Error::invalid_request( - format!( - "Instance cannot be added to anti-affinity group in state: {other}" - ) - ))); - }, + if has_reservation { + return Err(err.bail(Error::invalid_request( + format!( + "Instance cannot be added to anti-affinity group with reservation" + ) + ))); } diesel::insert_into(membership_dsl::anti_affinity_group_instance_membership) @@ -728,7 +756,6 @@ impl DataStore { let err = err.clone(); use db::schema::affinity_group::dsl as group_dsl; use db::schema::affinity_group_instance_membership::dsl as membership_dsl; - use db::schema::instance::dsl as instance_dsl; async move { // Check that the group exists @@ -749,25 +776,6 @@ impl DataStore { }) })?; - // Check that the instance exists - instance_dsl::instance - .filter(instance_dsl::time_deleted.is_null()) - .filter(instance_dsl::id.eq(instance_id.into_untyped_uuid())) - .select(instance_dsl::id) - .first_async::(&conn) - .await - .map_err(|e| { - err.bail_retryable_or_else(e, |e| { - public_error_from_diesel( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::Instance, - LookupType::ById(instance_id.into_untyped_uuid()) - ), - ) - }) - })?; - let rows = diesel::delete(membership_dsl::affinity_group_instance_membership) .filter(membership_dsl::group_id.eq(authz_affinity_group.id())) .filter(membership_dsl::instance_id.eq(instance_id.into_untyped_uuid())) @@ -817,7 +825,6 @@ impl DataStore { let err = err.clone(); use db::schema::anti_affinity_group::dsl as group_dsl; use db::schema::anti_affinity_group_instance_membership::dsl as membership_dsl; - use db::schema::instance::dsl as instance_dsl; async move { // Check that the group exists @@ -838,25 +845,6 @@ impl DataStore { }) })?; - // Check that the instance exists - instance_dsl::instance - .filter(instance_dsl::time_deleted.is_null()) - .filter(instance_dsl::id.eq(instance_id.into_untyped_uuid())) - .select(instance_dsl::id) - .first_async::(&conn) - .await - .map_err(|e| { - err.bail_retryable_or_else(e, |e| { - public_error_from_diesel( - e, - ErrorHandler::NotFoundByLookup( - ResourceType::Instance, - LookupType::ById(instance_id.into_untyped_uuid()) - ), - ) - }) - })?; - let rows = diesel::delete(membership_dsl::anti_affinity_group_instance_membership) .filter(membership_dsl::group_id.eq(authz_anti_affinity_group.id())) .filter(membership_dsl::instance_id.eq(instance_id.into_untyped_uuid())) @@ -893,12 +881,16 @@ mod tests { use crate::db::lookup::LookupPath; use crate::db::pub_test_utils::TestDatabase; use nexus_db_model::Instance; + use nexus_db_model::Resources; + use nexus_db_model::SledResource; use nexus_types::external_api::params; use omicron_common::api::external::{ self, ByteCount, DataPageParams, IdentityMetadataCreateParams, }; use omicron_test_utils::dev; use omicron_uuid_kinds::InstanceUuid; + use omicron_uuid_kinds::PropolisUuid; + use omicron_uuid_kinds::SledUuid; use std::num::NonZeroU32; // Helper function for creating a project @@ -1006,14 +998,6 @@ mod tests { instance } - // Helper for explicitly modifying instance state. - // - // The interaction we typically use to create and modify instance state - // is more complex in production, since it's the result of a back-and-forth - // between Nexus and Sled Agent, using carefully crafted rcgen values. - // - // Here, we just set the value of state explicitly. Be warned, there - // are no guardrails! async fn set_instance_state_stopped( datastore: &DataStore, instance: uuid::Uuid, @@ -1032,16 +1016,28 @@ mod tests { .unwrap(); } - async fn set_instance_state_running( + // Helper for explicitly modifying sled resource usage + // + // The interaction we typically use to create and modify instance state + // is more complex in production, using a real allocation algorithm. + // + // Here, we just set the value of state explicitly. Be warned, there + // are no guardrails! + async fn allocate_instance_reservation( datastore: &DataStore, - instance: uuid::Uuid, + instance: InstanceUuid, ) { - use db::schema::instance::dsl; - diesel::update(dsl::instance) - .filter(dsl::id.eq(instance)) - .set(( - dsl::state.eq(db::model::InstanceState::Vmm), - dsl::active_propolis_id.eq(uuid::Uuid::new_v4()), + use db::schema::sled_resource::dsl; + diesel::insert_into(dsl::sled_resource) + .values(SledResource::new_for_vmm( + PropolisUuid::new_v4(), + instance, + SledUuid::new_v4(), + Resources::new( + 1, + ByteCount::from_kibibytes_u32(1).into(), + ByteCount::from_kibibytes_u32(1).into(), + ), )) .execute_async( &*datastore.pool_connection_for_tests().await.unwrap(), @@ -1050,6 +1046,20 @@ mod tests { .unwrap(); } + async fn delete_instance_reservation( + datastore: &DataStore, + instance: InstanceUuid, + ) { + use db::schema::sled_resource::dsl; + diesel::delete(dsl::sled_resource) + .filter(dsl::instance_id.eq(instance.into_untyped_uuid())) + .execute_async( + &*datastore.pool_connection_for_tests().await.unwrap(), + ) + .await + .unwrap(); + } + #[tokio::test] async fn affinity_groups_are_project_scoped() { // Setup @@ -1673,7 +1683,12 @@ mod tests { "my-instance", ) .await; - set_instance_state_running(&datastore, instance.id()).await; + + allocate_instance_reservation( + &datastore, + InstanceUuid::from_untyped_uuid(instance.id()), + ) + .await; // Cannot add the instance to the group while it's running. let err = datastore @@ -1691,13 +1706,17 @@ mod tests { assert!(matches!(err, Error::InvalidRequest { .. })); assert!( err.to_string().contains( - "Instance cannot be added to affinity group in state" + "Instance cannot be added to affinity group with reservation" ), "{err:?}" ); - // If we stop the instance, we can add it to the group. - set_instance_state_stopped(&datastore, instance.id()).await; + // If we have no reservation for the instance, we can add it to the group. + delete_instance_reservation( + &datastore, + InstanceUuid::from_untyped_uuid(instance.id()), + ) + .await; datastore .affinity_group_member_add( &opctx, @@ -1709,8 +1728,12 @@ mod tests { .await .unwrap(); - // Now we can set the instance state to "running" once more. - set_instance_state_running(&datastore, instance.id()).await; + // Now we can reserve a sled for the instance once more. + allocate_instance_reservation( + &datastore, + InstanceUuid::from_untyped_uuid(instance.id()), + ) + .await; // We should now be able to list the new member let members = datastore @@ -1796,7 +1819,11 @@ mod tests { "my-instance", ) .await; - set_instance_state_running(&datastore, instance.id()).await; + allocate_instance_reservation( + &datastore, + InstanceUuid::from_untyped_uuid(instance.id()), + ) + .await; // Cannot add the instance to the group while it's running. let err = datastore @@ -1812,13 +1839,17 @@ mod tests { assert!(matches!(err, Error::InvalidRequest { .. })); assert!( err.to_string().contains( - "Instance cannot be added to anti-affinity group in state" + "Instance cannot be added to anti-affinity group with reservation" ), "{err:?}" ); - // If we stop the instance, we can add it to the group. - set_instance_state_stopped(&datastore, instance.id()).await; + // If we have no reservation for the instance, we can add it to the group. + delete_instance_reservation( + &datastore, + InstanceUuid::from_untyped_uuid(instance.id()), + ) + .await; datastore .anti_affinity_group_member_add( &opctx, @@ -1830,8 +1861,12 @@ mod tests { .await .unwrap(); - // Now we can set the instance state to "running" once more. - set_instance_state_running(&datastore, instance.id()).await; + // Now we can reserve a sled for the instance once more. + allocate_instance_reservation( + &datastore, + InstanceUuid::from_untyped_uuid(instance.id()), + ) + .await; // We should now be able to list the new member let members = datastore @@ -2314,7 +2349,7 @@ mod tests { assert!( matches!(err, Error::ObjectNotFound { type_name, .. - } if type_name == ResourceType::Instance), + } if type_name == ResourceType::AffinityGroupMember), "{err:?}" ); } @@ -2474,7 +2509,7 @@ mod tests { assert!( matches!(err, Error::ObjectNotFound { type_name, .. - } if type_name == ResourceType::Instance), + } if type_name == ResourceType::AntiAffinityGroupMember), "{err:?}" ); } From 4d26262ab67d305d4c3052d1f95eef52f2cb7252 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 31 Jan 2025 16:03:08 -0800 Subject: [PATCH 5/6] comment --- nexus/db-queries/src/db/datastore/affinity.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/datastore/affinity.rs b/nexus/db-queries/src/db/datastore/affinity.rs index 5cd7c57910..101af1910d 100644 --- a/nexus/db-queries/src/db/datastore/affinity.rs +++ b/nexus/db-queries/src/db/datastore/affinity.rs @@ -664,7 +664,7 @@ impl DataStore { })?; // NOTE: It may be possible to add non-stopped instances to - // affinity groups, depending on where they have already + // anti-affinity groups, depending on where they have already // been placed. However, only operating on "stopped" // instances is much easier to work with, as it does not // require any understanding of the group policy. From 6ae191011d49b1adec0aba75b7f37b088d23b456 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Fri, 31 Jan 2025 16:03:59 -0800 Subject: [PATCH 6/6] clippy --- nexus/db-queries/src/db/datastore/affinity.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/affinity.rs b/nexus/db-queries/src/db/datastore/affinity.rs index 101af1910d..e55afe41ea 100644 --- a/nexus/db-queries/src/db/datastore/affinity.rs +++ b/nexus/db-queries/src/db/datastore/affinity.rs @@ -548,9 +548,7 @@ impl DataStore { // require any understanding of the group policy. if has_reservation { return Err(err.bail(Error::invalid_request( - format!( - "Instance cannot be added to affinity group with reservation" - ) + "Instance cannot be added to affinity group with reservation".to_string() ))); } @@ -670,9 +668,7 @@ impl DataStore { // require any understanding of the group policy. if has_reservation { return Err(err.bail(Error::invalid_request( - format!( - "Instance cannot be added to anti-affinity group with reservation" - ) + "Instance cannot be added to anti-affinity group with reservation".to_string() ))); }