From 95f12ef0d05ff84479d61f902e53bfa35299043c Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 9 May 2023 15:53:38 -0400 Subject: [PATCH 01/11] WIP: Flesh out sled_instance resource --- common/src/sql/dbinit.sql | 44 +++++++++++++++++--- nexus/db-model/src/instance.rs | 2 +- nexus/db-model/src/lib.rs | 1 + nexus/db-model/src/schema.rs | 18 +++++++- nexus/db-model/src/sled_instance.rs | 24 +++++++++++ nexus/db-queries/src/authz/api_resources.rs | 8 ++++ nexus/db-queries/src/db/datastore/vpc.rs | 2 +- nexus/db-queries/src/db/lookup.rs | 9 ++++ nexus/src/app/sled.rs | 46 ++++++++++++++------- nexus/src/external_api/http_entrypoints.rs | 7 ++-- 10 files changed, 134 insertions(+), 27 deletions(-) create mode 100644 nexus/db-model/src/sled_instance.rs diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 957f2d3da7..ce79d64896 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -783,13 +783,14 @@ CREATE TABLE omicron.public.instance ( time_state_updated TIMESTAMPTZ NOT NULL, state_generation INT NOT NULL, /* - * Server where the VM is currently running, if any. Note that when we - * support live migration, there may be multiple servers associated with + * Sled where the VM is currently running, if any. Note that when we + * support live migration, there may be multiple sleds associated with * this Instance, but only one will be truly active. Still, consumers of * this information should consider whether they also want to know the other - * servers involved in the migration. + * sleds involved in the migration. */ - active_server_id UUID, + active_sled_id UUID, + /* Identifies the underlying propolis-server backing the instance. */ active_propolis_id UUID NOT NULL, active_propolis_ip INET, @@ -827,10 +828,43 @@ CREATE UNIQUE INDEX ON omicron.public.instance ( -- Allow looking up instances by server. This is particularly -- useful for resource accounting within a sled. CREATE INDEX ON omicron.public.instance ( - active_server_id + active_sled_id ) WHERE time_deleted IS NULL; +/* + * A special view of an instance provided to operators for insights into what's running + * on a sled. + */ + +CREATE VIEW omicron.public.sled_instance +AS SELECT + instance.id, + instance.name, + silo.name as silo_name, + project.name as project_name, + instance.active_sled_id, + instance.time_created, + instance.time_modified, + instance.migration_id, + instance.ncpus, + instance.memory, + instance.state +FROM + omicron.public.instance as instance +JOIN + omicron.public.project as project +ON + instance.project_id = project.id +JOIN + omicron.public.silo as silo +ON + project.silo_id = silo.id +WHERE + instance.active_sled_id IS NOT NULL +AND instance.time_deleted IS NULL; + + /* * Guest-Visible, Virtual Disks */ diff --git a/nexus/db-model/src/instance.rs b/nexus/db-model/src/instance.rs index 5d722912da..4e51b02780 100644 --- a/nexus/db-model/src/instance.rs +++ b/nexus/db-model/src/instance.rs @@ -124,7 +124,7 @@ pub struct InstanceRuntimeState { // // TODO(#2315): This should be optional so that it can be cleared when the // instance is not active. - #[diesel(column_name = active_server_id)] + #[diesel(column_name = active_sled_id)] pub sled_id: Uuid, /// The ID of the Propolis server hosting the current incarnation of this /// instance. diff --git a/nexus/db-model/src/lib.rs b/nexus/db-model/src/lib.rs index 5f36cc8eef..47698a8e0e 100644 --- a/nexus/db-model/src/lib.rs +++ b/nexus/db-model/src/lib.rs @@ -63,6 +63,7 @@ mod silo_group; mod silo_user; mod silo_user_password_hash; mod sled; +mod sled_instance; mod sled_resource; mod sled_resource_kind; mod snapshot; diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index d9de507c62..28e6bfb152 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -141,7 +141,7 @@ table! { state -> crate::InstanceStateEnum, time_state_updated -> Timestamptz, state_generation -> Int8, - active_server_id -> Uuid, + active_sled_id -> Uuid, active_propolis_id -> Uuid, active_propolis_ip -> Nullable, target_propolis_id -> Nullable, @@ -153,6 +153,22 @@ table! { } } +table! { + sled_instance (id) { + id -> Uuid, + name -> Text, + silo_name -> Text, + project_name -> Text, + time_created -> Timestamptz, + time_modified -> Timestamptz, + state -> crate::InstanceStateEnum, + active_sled_id -> Uuid, + migration_id -> Nullable, + ncpus -> Int8, + memory -> Int8, + } +} + table! { metric_producer (id) { id -> Uuid, diff --git a/nexus/db-model/src/sled_instance.rs b/nexus/db-model/src/sled_instance.rs new file mode 100644 index 0000000000..30cd9cf7d5 --- /dev/null +++ b/nexus/db-model/src/sled_instance.rs @@ -0,0 +1,24 @@ +use db_macros::Resource; +use nexus_types::identity::Resource; +use serde::{Deserialize, Serialize}; +use uuid::Uuid; + +use crate::{schema::sled_instance, InstanceState, Name}; + +/// An operator view of an instance as exposed by the sled API. +#[derive(Queryable, Debug, Selectable, Resource, Serialize, Deserialize)] +#[diesel(table_name = sled_instance)] +pub struct SledInstance { + #[diesel(embed)] + identity: SledInstanceIdentity, + + pub silo_name: Name, + pub project_name: Name, + + pub state: InstanceState, + + pub migration_id: Option, + + pub ncpus: i64, + pub memory: i64, +} diff --git a/nexus/db-queries/src/authz/api_resources.rs b/nexus/db-queries/src/authz/api_resources.rs index d5402629d6..f46c4f3ca9 100644 --- a/nexus/db-queries/src/authz/api_resources.rs +++ b/nexus/db-queries/src/authz/api_resources.rs @@ -979,6 +979,14 @@ authz_resource! { polar_snippet = FleetChild, } +authz_resource! { + name = "SledInstance", + parent = "Sled", + primary_key = Uuid, + roles_allowed = false, + polar_snippet = FleetChild, +} + authz_resource! { name = "PhysicalDisk", parent = "Fleet", diff --git a/nexus/db-queries/src/db/datastore/vpc.rs b/nexus/db-queries/src/db/datastore/vpc.rs index 025f85b942..0a60400618 100644 --- a/nexus/db-queries/src/db/datastore/vpc.rs +++ b/nexus/db-queries/src/db/datastore/vpc.rs @@ -363,7 +363,7 @@ impl DataStore { .on(instance::id .eq(instance_network_interface::instance_id)), ) - .inner_join(sled::table.on(sled::id.eq(instance::active_server_id))) + .inner_join(sled::table.on(sled::id.eq(instance::active_sled_id))) .filter(instance_network_interface::vpc_id.eq(vpc_id)) .filter(instance_network_interface::time_deleted.is_null()) .filter(instance::time_deleted.is_null()) diff --git a/nexus/db-queries/src/db/lookup.rs b/nexus/db-queries/src/db/lookup.rs index 778af923bd..0ff17df778 100644 --- a/nexus/db-queries/src/db/lookup.rs +++ b/nexus/db-queries/src/db/lookup.rs @@ -733,6 +733,15 @@ lookup_resource! { primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } +lookup_resource! { + name = "SledInstance", + ancestors = [], + children = [], + lookup_by_name = true, + soft_deletes = false, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ], +} + lookup_resource! { name = "PhysicalDisk", ancestors = [], diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs index 8188a118c7..c839600261 100644 --- a/nexus/src/app/sled.rs +++ b/nexus/src/app/sled.rs @@ -15,6 +15,7 @@ use crate::internal_api::params::{ SledRole, ZpoolPutRequest, }; use nexus_db_queries::context::OpContext; +use nexus_db_queries::db::lookup; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; @@ -27,6 +28,14 @@ use uuid::Uuid; impl super::Nexus { // Sleds + pub fn sled_lookup<'a>( + &'a self, + opctx: &'a OpContext, + sled_id: &Uuid, + ) -> LookupResult> { + let sled = LookupPath::new(opctx, &self.db_datastore).sled_id(*sled_id); + Ok(sled) + } // TODO-robustness we should have a limit on how many sled agents there can // be (for graceful degradation at large scale). @@ -61,7 +70,7 @@ impl super::Nexus { Ok(()) } - pub async fn sleds_list( + pub async fn sled_list( &self, opctx: &OpContext, pagparams: &DataPageParams<'_, Uuid>, @@ -69,16 +78,12 @@ impl super::Nexus { self.db_datastore.sled_list(&opctx, pagparams).await } - pub async fn sled_lookup( + pub async fn sled_list_instances( &self, opctx: &OpContext, - sled_id: &Uuid, - ) -> LookupResult { - let (.., db_sled) = LookupPath::new(opctx, &self.db_datastore) - .sled_id(*sled_id) - .fetch() - .await?; - Ok(db_sled) + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + self.db_datastore.sled_list_instances(&opctx, pagparams).await } pub async fn sled_client( @@ -92,7 +97,8 @@ impl super::Nexus { // Franky, returning an "Arc" here without a connection pool is a little // silly; it's not actually used if each client connection exists as a // one-shot. - let sled = self.sled_lookup(&self.opctx_alloc, id).await?; + let (.., sled) = + self.sled_lookup(&self.opctx_alloc, id)?.fetch().await?; let log = self.log.new(o!("SledAgent" => id.clone().to_string())); let dur = std::time::Duration::from_secs(60); @@ -343,8 +349,12 @@ impl super::Nexus { // Lookup the physical host IP of the sled hosting this instance let instance_sled_id = db_instance.runtime().sled_id; - let physical_host_ip = - *self.sled_lookup(&self.opctx_alloc, &instance_sled_id).await?.ip; + let physical_host_ip = *self + .sled_lookup(&self.opctx_alloc, &instance_sled_id)? + .fetch() + .await? + .1 + .ip; let mut last_sled_id: Option = None; loop { @@ -355,7 +365,7 @@ impl super::Nexus { }; let sleds_page = - self.sleds_list(&self.opctx_alloc, &pagparams).await?; + self.sled_list(&self.opctx_alloc, &pagparams).await?; let mut join_handles = Vec::with_capacity(sleds_page.len() * instance_nics.len()); @@ -438,8 +448,12 @@ impl super::Nexus { // Lookup the physical host IP of the sled hosting this instance let instance_sled_id = db_instance.runtime().sled_id; - let physical_host_ip = - *self.sled_lookup(&self.opctx_alloc, &instance_sled_id).await?.ip; + let physical_host_ip = *self + .sled_lookup(&self.opctx_alloc, &instance_sled_id)? + .fetch() + .await? + .1 + .ip; let mut last_sled_id: Option = None; @@ -451,7 +465,7 @@ impl super::Nexus { }; let sleds_page = - self.sleds_list(&self.opctx_alloc, &pagparams).await?; + self.sled_list(&self.opctx_alloc, &pagparams).await?; let mut join_handles = Vec::with_capacity(sleds_page.len() * instance_nics.len()); diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 56c6ebf26b..9fdc24187c 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -3729,7 +3729,7 @@ async fn sled_list( let query = query_params.into_inner(); let opctx = crate::context::op_context_for_external_api(&rqctx).await?; let sleds = nexus - .sleds_list(&opctx, &data_page_params_for(&rqctx, &query)?) + .sled_list(&opctx, &data_page_params_for(&rqctx, &query)?) .await? .into_iter() .map(|s| s.into()) @@ -3765,8 +3765,9 @@ async fn sled_view( let nexus = &apictx.nexus; let path = path_params.into_inner(); let opctx = crate::context::op_context_for_external_api(&rqctx).await?; - let sled_info = nexus.sled_lookup(&opctx, &path.sled_id).await?; - Ok(HttpResponseOk(sled_info.into())) + let (.., sled) = + nexus.sled_lookup(&opctx, &path.sled_id)?.fetch().await?; + Ok(HttpResponseOk(sled.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } From 79ee535288ed32e1cc06932db20efaad1bb2bac8 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Mon, 15 May 2023 17:42:13 -0400 Subject: [PATCH 02/11] Fixup several failures --- common/src/api/external/mod.rs | 1 + nexus/db-model/src/lib.rs | 1 + nexus/db-model/src/sled_instance.rs | 32 +++++++++++--- nexus/db-queries/src/authz/api_resources.rs | 2 +- nexus/db-queries/src/db/datastore/mod.rs | 1 + .../src/db/datastore/sled_instance.rs | 43 +++++++++++++++++++ 6 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 nexus/db-queries/src/db/datastore/sled_instance.rs diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 3fe60ba28a..261a443cb5 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -705,6 +705,7 @@ pub enum ResourceType { Rack, Service, Sled, + SledInstance, Switch, SagaDbg, Snapshot, diff --git a/nexus/db-model/src/lib.rs b/nexus/db-model/src/lib.rs index 7daa28096c..1cca2b804e 100644 --- a/nexus/db-model/src/lib.rs +++ b/nexus/db-model/src/lib.rs @@ -137,6 +137,7 @@ pub use silo_group::*; pub use silo_user::*; pub use silo_user_password_hash::*; pub use sled::*; +pub use sled_instance::*; pub use sled_resource::*; pub use sled_resource_kind::*; pub use snapshot::*; diff --git a/nexus/db-model/src/sled_instance.rs b/nexus/db-model/src/sled_instance.rs index 30cd9cf7d5..a02a117dee 100644 --- a/nexus/db-model/src/sled_instance.rs +++ b/nexus/db-model/src/sled_instance.rs @@ -1,17 +1,22 @@ -use db_macros::Resource; -use nexus_types::identity::Resource; -use serde::{Deserialize, Serialize}; +use crate::schema::sled_instance; +use crate::InstanceState; +use crate::Name; +use db_macros::Asset; +use nexus_types::external_api::views; +use nexus_types::identity::Asset; +use serde::Deserialize; +use serde::Serialize; use uuid::Uuid; -use crate::{schema::sled_instance, InstanceState, Name}; - /// An operator view of an instance as exposed by the sled API. -#[derive(Queryable, Debug, Selectable, Resource, Serialize, Deserialize)] +#[derive(Queryable, Debug, Selectable, Asset, Serialize, Deserialize)] #[diesel(table_name = sled_instance)] pub struct SledInstance { #[diesel(embed)] identity: SledInstanceIdentity, + pub name: Name, + pub silo_name: Name, pub project_name: Name, @@ -22,3 +27,18 @@ pub struct SledInstance { pub ncpus: i64, pub memory: i64, } + +impl From for views::SledInstance { + from (sled_instance: SledInstance) -> Self { + Self { + identity: sled_instance.identity(), + name: sled_instance.name, + silo_name: sled_instance.silo_name, + project_name: sled_instance.project_name, + state: sled_instance.state, + migration_id: sled_instance.migration_id, + ncpus: sled_instance.ncpus, + memory: sled_instance.memory, + } + } +} \ No newline at end of file diff --git a/nexus/db-queries/src/authz/api_resources.rs b/nexus/db-queries/src/authz/api_resources.rs index 96588f0572..c4e6c89c41 100644 --- a/nexus/db-queries/src/authz/api_resources.rs +++ b/nexus/db-queries/src/authz/api_resources.rs @@ -976,7 +976,7 @@ authz_resource! { authz_resource! { name = "SledInstance", - parent = "Sled", + parent = "Fleet", primary_key = Uuid, roles_allowed = false, polar_snippet = FleetChild, diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index d1e8d0dc64..11089fe2fb 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -66,6 +66,7 @@ mod silo; mod silo_group; mod silo_user; mod sled; +mod sled_instance; mod snapshot; mod ssh_key; mod switch; diff --git a/nexus/db-queries/src/db/datastore/sled_instance.rs b/nexus/db-queries/src/db/datastore/sled_instance.rs new file mode 100644 index 0000000000..55b1125b3b --- /dev/null +++ b/nexus/db-queries/src/db/datastore/sled_instance.rs @@ -0,0 +1,43 @@ +use super::DataStore; + +use crate::authz; +use crate::context::OpContext; +use crate::db; +use crate::db::error::public_error_from_diesel_pool; +use crate::db::error::ErrorHandler; +use crate::db::lookup::SledInstance; +use crate::db::pagination::paginated; +use async_bb8_diesel::AsyncRunQueryDsl; +use diesel::prelude::*; +use nexus_db_model::Name; +use omicron_common::api::external::http_pagination::PaginatedBy; +use omicron_common::api::external::ListResultVec; +use ref_cast::RefCast; +use uuid::Uuid; + +impl DataStore { + pub async fn sled_instance_list( + &self, + opctx: &OpContext, + sled_id: Uuid, + pagparams: &PaginatedBy<'_>, + ) -> ListResultVec { + opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?; + use db::schema::sled_instance::dsl; + match pagparams { + PaginatedBy::Id(pagparams) => { + paginated(dsl::sled_instance, dsl::id, &pagparams) + } + PaginatedBy::Name(pagparams) => paginated( + dsl::sled_instance, + dsl::name, + &pagparams.map_name(|n| Name::ref_cast(n)), + ), + } + .filter(dsl::active_sled_id.eq(sled_id)) + .select(SledInstance::as_select()) + .load_async::(self.pool_authorized(opctx).await?) + .await + .map_error(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + } +} From 9f2fc76a6d4133eb92e94edbb3b46d028a07cd1e Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 16 May 2023 11:50:43 -0400 Subject: [PATCH 03/11] Add view of sled instance --- nexus/db-model/src/sled_instance.rs | 19 +++++++++---------- .../src/db/datastore/sled_instance.rs | 1 - nexus/types/src/external_api/views.rs | 17 ++++++++++++++++- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/nexus/db-model/src/sled_instance.rs b/nexus/db-model/src/sled_instance.rs index a02a117dee..e3a901264d 100644 --- a/nexus/db-model/src/sled_instance.rs +++ b/nexus/db-model/src/sled_instance.rs @@ -14,31 +14,30 @@ use uuid::Uuid; pub struct SledInstance { #[diesel(embed)] identity: SledInstanceIdentity, + active_sled_id: Uuid, + pub migration_id: Option, pub name: Name, - pub silo_name: Name, pub project_name: Name, pub state: InstanceState, - - pub migration_id: Option, - pub ncpus: i64, pub memory: i64, } impl From for views::SledInstance { - from (sled_instance: SledInstance) -> Self { + fn from(sled_instance: SledInstance) -> Self { Self { identity: sled_instance.identity(), - name: sled_instance.name, - silo_name: sled_instance.silo_name, - project_name: sled_instance.project_name, - state: sled_instance.state, + name: sled_instance.name.into(), + active_sled_id: sled_instance.active_sled_id, + silo_name: sled_instance.silo_name.into(), + project_name: sled_instance.project_name.into(), + state: *sled_instance.state.state(), migration_id: sled_instance.migration_id, ncpus: sled_instance.ncpus, memory: sled_instance.memory, } } -} \ No newline at end of file +} diff --git a/nexus/db-queries/src/db/datastore/sled_instance.rs b/nexus/db-queries/src/db/datastore/sled_instance.rs index 55b1125b3b..c0b0d4289e 100644 --- a/nexus/db-queries/src/db/datastore/sled_instance.rs +++ b/nexus/db-queries/src/db/datastore/sled_instance.rs @@ -7,7 +7,6 @@ use crate::db::error::public_error_from_diesel_pool; use crate::db::error::ErrorHandler; use crate::db::lookup::SledInstance; use crate::db::pagination::paginated; -use async_bb8_diesel::AsyncRunQueryDsl; use diesel::prelude::*; use nexus_db_model::Name; use omicron_common::api::external::http_pagination::PaginatedBy; diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index 7108acc545..892ded7cc9 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -12,7 +12,7 @@ use api_identity::ObjectIdentity; use chrono::DateTime; use chrono::Utc; use omicron_common::api::external::{ - ByteCount, Digest, IdentityMetadata, Ipv4Net, Ipv6Net, Name, + ByteCount, Digest, IdentityMetadata, InstanceState, Ipv4Net, Ipv6Net, Name, ObjectIdentity, RoleName, SemverVersion, }; use schemars::JsonSchema; @@ -278,6 +278,21 @@ pub struct Sled { pub usable_physical_ram: ByteCount, } +/// An operator's view of an instance running on a given sled +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct SledInstance { + #[serde(flatten)] + pub identity: AssetIdentityMetadata, + pub active_sled_id: Uuid, + pub migration_id: Option, + pub name: Name, + pub silo_name: Name, + pub project_name: Name, + pub state: InstanceState, + pub ncpus: i64, + pub memory: i64, +} + // SWITCHES /// An operator's view of a Switch. From a0817cf10ff5b5bbdc3492de6a301931f5572aec Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 16 May 2023 21:51:54 -0400 Subject: [PATCH 04/11] Wire up the last of the API --- .../src/db/datastore/sled_instance.rs | 30 +++++--------- nexus/src/app/mod.rs | 1 + nexus/src/app/sled.rs | 3 +- nexus/src/app/sled_instance.rs | 18 +++++++++ nexus/src/external_api/http_entrypoints.rs | 40 ++++++++++++++++++- 5 files changed, 70 insertions(+), 22 deletions(-) create mode 100644 nexus/src/app/sled_instance.rs diff --git a/nexus/db-queries/src/db/datastore/sled_instance.rs b/nexus/db-queries/src/db/datastore/sled_instance.rs index c0b0d4289e..b1724556d2 100644 --- a/nexus/db-queries/src/db/datastore/sled_instance.rs +++ b/nexus/db-queries/src/db/datastore/sled_instance.rs @@ -5,13 +5,12 @@ use crate::context::OpContext; use crate::db; use crate::db::error::public_error_from_diesel_pool; use crate::db::error::ErrorHandler; -use crate::db::lookup::SledInstance; use crate::db::pagination::paginated; +use async_bb8_diesel::AsyncRunQueryDsl; use diesel::prelude::*; -use nexus_db_model::Name; -use omicron_common::api::external::http_pagination::PaginatedBy; +use nexus_db_model::SledInstance; +use omicron_common::api::external::DataPageParams; use omicron_common::api::external::ListResultVec; -use ref_cast::RefCast; use uuid::Uuid; impl DataStore { @@ -19,24 +18,15 @@ impl DataStore { &self, opctx: &OpContext, sled_id: Uuid, - pagparams: &PaginatedBy<'_>, + pagparams: &DataPageParams<'_, Uuid>, ) -> ListResultVec { opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?; use db::schema::sled_instance::dsl; - match pagparams { - PaginatedBy::Id(pagparams) => { - paginated(dsl::sled_instance, dsl::id, &pagparams) - } - PaginatedBy::Name(pagparams) => paginated( - dsl::sled_instance, - dsl::name, - &pagparams.map_name(|n| Name::ref_cast(n)), - ), - } - .filter(dsl::active_sled_id.eq(sled_id)) - .select(SledInstance::as_select()) - .load_async::(self.pool_authorized(opctx).await?) - .await - .map_error(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + paginated(dsl::sled_instance, dsl::id, &pagparams) + .filter(dsl::active_sled_id.eq(sled_id)) + .select(SledInstance::as_select()) + .load_async::(self.pool_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) } } diff --git a/nexus/src/app/mod.rs b/nexus/src/app/mod.rs index 13f2e03222..59fc057f4e 100644 --- a/nexus/src/app/mod.rs +++ b/nexus/src/app/mod.rs @@ -44,6 +44,7 @@ pub mod saga; mod session; mod silo; mod sled; +mod sled_instance; mod snapshot; mod switch; pub mod test_interfaces; diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs index c3272bd1ad..3d8c96d97e 100644 --- a/nexus/src/app/sled.rs +++ b/nexus/src/app/sled.rs @@ -81,9 +81,10 @@ impl super::Nexus { pub async fn sled_list_instances( &self, opctx: &OpContext, + sled_id: Uuid, pagparams: &DataPageParams<'_, Uuid>, ) -> ListResultVec { - self.db_datastore.sled_list_instances(&opctx, pagparams).await + self.db_datastore.sled_instance_list(&opctx, sled_id, pagparams).await } pub async fn sled_client( diff --git a/nexus/src/app/sled_instance.rs b/nexus/src/app/sled_instance.rs new file mode 100644 index 0000000000..eca896242a --- /dev/null +++ b/nexus/src/app/sled_instance.rs @@ -0,0 +1,18 @@ +use crate::authz; +use crate::db; +use nexus_db_queries::context::OpContext; +use omicron_common::api::external::DataPageParams; +use omicron_common::api::external::ListResultVec; +use uuid::Uuid; + +impl super::Nexus { + pub async fn sled_instance_list( + &self, + opctx: &OpContext, + sled_id: Uuid, + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?; + self.db_datastore.sled_instance_list(opctx, sled_id, pagparams).await + } +} diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 54a98071b7..60005bcb42 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -42,7 +42,8 @@ use ipnetwork::IpNetwork; use nexus_db_queries::db::lookup::ImageLookup; use nexus_db_queries::db::lookup::ImageParentLookup; use nexus_types::{ - external_api::views::Switch, identity::AssetIdentityMetadata, + external_api::views::{SledInstance, Switch}, + identity::AssetIdentityMetadata, }; use omicron_common::api::external::http_pagination::data_page_params_for; use omicron_common::api::external::http_pagination::marker_for_name; @@ -190,6 +191,7 @@ pub fn external_api() -> NexusApiDescription { api.register(rack_view)?; api.register(sled_list)?; api.register(sled_view)?; + api.register(sled_instance_list)?; api.register(sled_physical_disk_list)?; api.register(physical_disk_list)?; api.register(switch_list)?; @@ -3657,6 +3659,42 @@ async fn sled_view( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +/// List instances running on a given sled +#[endpoint { + method = GET, + path = "/v1/system/hardware/sleds/{sled_id}/instances", + tags = ["system"], +}] +async fn sled_instance_list( + rqctx: RequestContext>, + path_params: Path, + query_params: Query, +) -> Result>, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let query = query_params.into_inner(); + let opctx = crate::context::op_context_for_external_api(&rqctx).await?; + let sled_instances = nexus + .sled_instance_list( + &opctx, + path.sled_id, + &data_page_params_for(&rqctx, &query)?, + ) + .await? + .into_iter() + .map(|s| s.into()) + .collect(); + Ok(HttpResponseOk(ScanById::results_page( + &query, + sled_instances, + &|_, sled_instance: &SledInstance| sled_instance.identity.id, + )?)) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + // Physical disks /// List physical disks From 5ad7c28bb7fe35b9fafa586a0a5a282a8e924a09 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 16 May 2023 22:20:01 -0400 Subject: [PATCH 05/11] Bump generated artifacts --- nexus/tests/output/nexus_tags.txt | 1 + openapi/nexus.json | 152 ++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+) diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index 57606a51e4..794345f63d 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -135,6 +135,7 @@ silo_policy_view GET /v1/system/silos/{silo}/policy silo_user_list GET /v1/system/users silo_user_view GET /v1/system/users/{user_id} silo_view GET /v1/system/silos/{silo} +sled_instance_list GET /v1/system/hardware/sleds/{sled_id}/instances sled_list GET /v1/system/hardware/sleds sled_physical_disk_list GET /v1/system/hardware/sleds/{sled_id}/disks sled_view GET /v1/system/hardware/sleds/{sled_id} diff --git a/openapi/nexus.json b/openapi/nexus.json index 1a6d37c49b..d226e6b85c 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -3588,6 +3588,75 @@ } } }, + "/v1/system/hardware/sleds/{sled_id}/instances": { + "get": { + "tags": [ + "system" + ], + "summary": "List instances running on a given sled", + "operationId": "sled_instance_list", + "parameters": [ + { + "in": "path", + "name": "sled_id", + "description": "ID of the sled", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + }, + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + } + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/IdSortMode" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SledInstanceResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": { + "required": [] + } + } + }, "/v1/system/hardware/switches": { "get": { "tags": [ @@ -10741,6 +10810,89 @@ "usable_physical_ram" ] }, + "SledInstance": { + "description": "An operator's view of an instance running on a given sled", + "type": "object", + "properties": { + "active_sled_id": { + "type": "string", + "format": "uuid" + }, + "id": { + "description": "unique, immutable, system-controlled identifier for each resource", + "type": "string", + "format": "uuid" + }, + "memory": { + "type": "integer", + "format": "int64" + }, + "migration_id": { + "nullable": true, + "type": "string", + "format": "uuid" + }, + "name": { + "$ref": "#/components/schemas/Name" + }, + "ncpus": { + "type": "integer", + "format": "int64" + }, + "project_name": { + "$ref": "#/components/schemas/Name" + }, + "silo_name": { + "$ref": "#/components/schemas/Name" + }, + "state": { + "$ref": "#/components/schemas/InstanceState" + }, + "time_created": { + "description": "timestamp when this resource was created", + "type": "string", + "format": "date-time" + }, + "time_modified": { + "description": "timestamp when this resource was last modified", + "type": "string", + "format": "date-time" + } + }, + "required": [ + "active_sled_id", + "id", + "memory", + "name", + "ncpus", + "project_name", + "silo_name", + "state", + "time_created", + "time_modified" + ] + }, + "SledInstanceResultsPage": { + "description": "A single page of results", + "type": "object", + "properties": { + "items": { + "description": "list of items on this page of results", + "type": "array", + "items": { + "$ref": "#/components/schemas/SledInstance" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, "SledResultsPage": { "description": "A single page of results", "type": "object", From 1c3bee7be8f1447bfb4819351d317784cedae82b Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 16 May 2023 23:18:43 -0400 Subject: [PATCH 06/11] Add authz endpoint --- nexus/tests/integration_tests/endpoints.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 72ccc6c516..140fab4805 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -50,6 +50,9 @@ lazy_static! { pub static ref HARDWARE_SLED_DISK_URL: String = format!("/v1/system/hardware/sleds/{}/disks", SLED_AGENT_UUID); + pub static ref SLED_INSTANCES_URL: String = + format!("/v1/system/hardware/sleds/{}/instances", SLED_AGENT_UUID); + // Global policy pub static ref SYSTEM_POLICY_URL: &'static str = "/v1/system/policy"; @@ -1428,6 +1431,13 @@ lazy_static! { allowed_methods: vec![AllowedMethod::Get], }, + VerifyEndpoint { + url: &SLED_INSTANCES_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![AllowedMethod::Get], + }, + VerifyEndpoint { url: &HARDWARE_SLED_URL, visibility: Visibility::Protected, From 18806370d9cf2b1da84065708d12947035cff7e4 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 16 May 2023 23:22:49 -0400 Subject: [PATCH 07/11] Drop duplicate sled definition --- nexus/src/app/sled.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs index 3d8c96d97e..f37235b430 100644 --- a/nexus/src/app/sled.rs +++ b/nexus/src/app/sled.rs @@ -78,15 +78,6 @@ impl super::Nexus { self.db_datastore.sled_list(&opctx, pagparams).await } - pub async fn sled_list_instances( - &self, - opctx: &OpContext, - sled_id: Uuid, - pagparams: &DataPageParams<'_, Uuid>, - ) -> ListResultVec { - self.db_datastore.sled_instance_list(&opctx, sled_id, pagparams).await - } - pub async fn sled_client( &self, id: &Uuid, From ae608a5e8fe357ecd2bcfd875b2e78782b504af1 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 17 May 2023 13:45:13 -0400 Subject: [PATCH 08/11] Fix authz failure --- nexus/db-queries/src/db/datastore/sled_instance.rs | 4 ++-- nexus/src/app/sled_instance.rs | 9 +++++++-- nexus/src/external_api/http_entrypoints.rs | 3 ++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/sled_instance.rs b/nexus/db-queries/src/db/datastore/sled_instance.rs index b1724556d2..9ba6861cec 100644 --- a/nexus/db-queries/src/db/datastore/sled_instance.rs +++ b/nexus/db-queries/src/db/datastore/sled_instance.rs @@ -17,13 +17,13 @@ impl DataStore { pub async fn sled_instance_list( &self, opctx: &OpContext, - sled_id: Uuid, + authz_sled: &authz::Sled, pagparams: &DataPageParams<'_, Uuid>, ) -> ListResultVec { opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?; use db::schema::sled_instance::dsl; paginated(dsl::sled_instance, dsl::id, &pagparams) - .filter(dsl::active_sled_id.eq(sled_id)) + .filter(dsl::active_sled_id.eq(authz_sled.id())) .select(SledInstance::as_select()) .load_async::(self.pool_authorized(opctx).await?) .await diff --git a/nexus/src/app/sled_instance.rs b/nexus/src/app/sled_instance.rs index eca896242a..c885e12782 100644 --- a/nexus/src/app/sled_instance.rs +++ b/nexus/src/app/sled_instance.rs @@ -1,6 +1,7 @@ use crate::authz; use crate::db; use nexus_db_queries::context::OpContext; +use nexus_db_queries::db::lookup; use omicron_common::api::external::DataPageParams; use omicron_common::api::external::ListResultVec; use uuid::Uuid; @@ -9,10 +10,14 @@ impl super::Nexus { pub async fn sled_instance_list( &self, opctx: &OpContext, - sled_id: Uuid, + sled_lookup: &lookup::Sled<'_>, pagparams: &DataPageParams<'_, Uuid>, ) -> ListResultVec { + let (.., authz_sled) = + sled_lookup.lookup_for(authz::Action::Read).await?; opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?; - self.db_datastore.sled_instance_list(opctx, sled_id, pagparams).await + self.db_datastore + .sled_instance_list(opctx, &authz_sled, pagparams) + .await } } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 60005bcb42..0a45d1fdc0 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -3676,10 +3676,11 @@ async fn sled_instance_list( let path = path_params.into_inner(); let query = query_params.into_inner(); let opctx = crate::context::op_context_for_external_api(&rqctx).await?; + let sled_lookup = nexus.sled_lookup(&opctx, &path.sled_id)?; let sled_instances = nexus .sled_instance_list( &opctx, - path.sled_id, + &sled_lookup, &data_page_params_for(&rqctx, &query)?, ) .await? From 35a52e0864d7d2723e15731e823285e62ddd1ed5 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 17 May 2023 14:39:09 -0400 Subject: [PATCH 09/11] Add test --- nexus/tests/integration_tests/sleds.rs | 43 ++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/nexus/tests/integration_tests/sleds.rs b/nexus/tests/integration_tests/sleds.rs index eb9048b3fd..de585b7b2d 100644 --- a/nexus/tests/integration_tests/sleds.rs +++ b/nexus/tests/integration_tests/sleds.rs @@ -7,12 +7,16 @@ use camino::Utf8Path; use dropshot::test_util::ClientTestContext; use nexus_test_interface::NexusServer; +use nexus_test_utils::resource_helpers::create_instance; use nexus_test_utils::resource_helpers::create_physical_disk; +use nexus_test_utils::resource_helpers::create_project; use nexus_test_utils::resource_helpers::delete_physical_disk; use nexus_test_utils::resource_helpers::objects_list_page_authz; +use nexus_test_utils::resource_helpers::populate_ip_pool; use nexus_test_utils::start_sled_agent; use nexus_test_utils::SLED_AGENT_UUID; use nexus_test_utils_macros::nexus_test; +use omicron_nexus::external_api::views::SledInstance; use omicron_nexus::external_api::views::{ PhysicalDisk, PhysicalDiskType, Sled, }; @@ -35,6 +39,13 @@ async fn physical_disks_list( objects_list_page_authz::(client, url).await.items } +async fn sled_instance_list( + client: &ClientTestContext, + url: &str, +) -> Vec { + objects_list_page_authz::(client, url).await.items +} + #[nexus_test] async fn test_sleds_list(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; @@ -119,3 +130,35 @@ async fn test_physical_disk_create_list_delete( delete_physical_disk(&internal_client, "v", "s", "m", sled_id).await; assert!(physical_disks_list(&external_client, &disks_url).await.is_empty()); } + +#[nexus_test] +async fn test_sled_instance_list(cptestctx: &ControlPlaneTestContext) { + let external_client = &cptestctx.external_client; + + // Verify that there is one sled to begin with. + let sleds_url = "/v1/system/hardware/sleds"; + assert_eq!(sleds_list(&external_client, &sleds_url).await.len(), 1); + + // Verify that there are no instances. + let instances_url = + format!("/v1/system/hardware/sleds/{SLED_AGENT_UUID}/instances"); + assert!(sled_instance_list(&external_client, &instances_url) + .await + .is_empty()); + + // Create an IP pool and project that we'll use for testing. + populate_ip_pool(&external_client, "default", None).await; + let project = create_project(&external_client, "test-project").await; + let instance = + create_instance(&external_client, "test-project", "test-instance") + .await; + + let sled_instances = + sled_instance_list(&external_client, &instances_url).await; + + // Ensure 1 instance was created on the sled + assert_eq!(sled_instances.len(), 1); + + assert_eq!(project.identity.name, sled_instances[0].project_name); + assert_eq!(instance.identity.name, sled_instances[0].name); +} From 3e5913ee8afe2f7610212b461cdc868568bf2b0f Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 17 May 2023 16:02:31 -0400 Subject: [PATCH 10/11] Remove extra where clause, format output --- common/src/sql/dbinit.sql | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index dc3ff2a51a..3bab2ece12 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -886,18 +886,13 @@ AS SELECT instance.memory, instance.state FROM - omicron.public.instance as instance -JOIN - omicron.public.project as project -ON - instance.project_id = project.id -JOIN - omicron.public.silo as silo -ON - project.silo_id = silo.id + omicron.public.instance AS instance + JOIN omicron.public.project AS project ON + instance.project_id = project.id + JOIN omicron.public.silo AS silo ON + project.silo_id = silo.id WHERE - instance.active_sled_id IS NOT NULL -AND instance.time_deleted IS NULL; + instance.time_deleted IS NULL; /* From dca43e892454fcf48ca0fb8630bba70600a54605 Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Wed, 17 May 2023 16:03:19 -0400 Subject: [PATCH 11/11] Remove by name lookup for sled instance --- nexus/db-queries/src/db/lookup.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/lookup.rs b/nexus/db-queries/src/db/lookup.rs index 663ece0e40..80597821b2 100644 --- a/nexus/db-queries/src/db/lookup.rs +++ b/nexus/db-queries/src/db/lookup.rs @@ -764,7 +764,7 @@ lookup_resource! { name = "SledInstance", ancestors = [], children = [], - lookup_by_name = true, + lookup_by_name = false, soft_deletes = false, primary_key_columns = [ { column_name = "id", rust_type = Uuid } ], }