From caf371b902a5257cff3bedb327b5eaf53b3bf51d Mon Sep 17 00:00:00 2001 From: Justin Bennett Date: Tue, 5 Dec 2023 14:44:05 -0500 Subject: [PATCH] Convert storage/memory to bytecount instead of ints --- nexus/db-model/src/quota.rs | 33 ++++++++++++++++------ nexus/db-model/src/schema.rs | 4 +-- nexus/db-queries/src/db/datastore/silo.rs | 4 +-- nexus/db-queries/src/db/fixed_data/silo.rs | 12 ++------ nexus/src/app/external_endpoints.rs | 1 + nexus/src/app/rack.rs | 2 +- nexus/test-utils/src/resource_helpers.rs | 4 +-- nexus/tests/integration_tests/endpoints.rs | 5 ++++ nexus/tests/integration_tests/silos.rs | 7 +++++ nexus/types/src/external_api/params.rs | 18 +++++++++--- nexus/types/src/external_api/views.rs | 4 +-- schema/crdb/dbinit.sql | 4 +-- 12 files changed, 64 insertions(+), 34 deletions(-) diff --git a/nexus/db-model/src/quota.rs b/nexus/db-model/src/quota.rs index 277ebb7888..b42456b330 100644 --- a/nexus/db-model/src/quota.rs +++ b/nexus/db-model/src/quota.rs @@ -1,3 +1,4 @@ +use super::ByteCount; use crate::schema::silo_quotas; use chrono::{DateTime, Utc}; use nexus_types::external_api::{params, views}; @@ -20,13 +21,25 @@ pub struct SiloQuotas { pub time_created: DateTime, pub time_modified: DateTime, + /// The number of CPUs that this silo is allowed to use pub cpus: i64, - pub memory: i64, - pub storage: i64, + + /// The amount of memory (in bytes) that this silo is allowed to use + #[diesel(column_name = memory_bytes)] + pub memory: ByteCount, + + /// The amount of storage (in bytes) that this silo is allowed to use + #[diesel(column_name = storage_bytes)] + pub storage: ByteCount, } impl SiloQuotas { - pub fn new(silo_id: Uuid, cpus: i64, memory: i64, storage: i64) -> Self { + pub fn new( + silo_id: Uuid, + cpus: i64, + memory: ByteCount, + storage: ByteCount, + ) -> Self { Self { silo_id, time_created: Utc::now(), @@ -43,8 +56,8 @@ impl From for views::SiloQuotas { Self { silo_id: silo_quotas.silo_id, cpus: silo_quotas.cpus, - memory: silo_quotas.memory, - storage: silo_quotas.storage, + memory: silo_quotas.memory.into(), + storage: silo_quotas.storage.into(), } } } @@ -56,8 +69,8 @@ impl From for SiloQuotas { time_created: Utc::now(), time_modified: Utc::now(), cpus: silo_quotas.cpus, - memory: silo_quotas.memory, - storage: silo_quotas.storage, + memory: silo_quotas.memory.into(), + storage: silo_quotas.storage.into(), } } } @@ -67,7 +80,9 @@ impl From for SiloQuotas { #[diesel(table_name = silo_quotas)] pub struct SiloQuotasUpdate { pub cpus: Option, + #[diesel(column_name = memory_bytes)] pub memory: Option, + #[diesel(column_name = storage_bytes)] pub storage: Option, pub time_modified: DateTime, } @@ -76,8 +91,8 @@ impl From for SiloQuotasUpdate { fn from(params: params::SiloQuotasUpdate) -> Self { Self { cpus: params.cpus, - memory: params.memory, - storage: params.storage, + memory: params.memory.map(|f| f.into()), + storage: params.storage.map(|f| f.into()), time_modified: Utc::now(), } } diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index d2367db33b..a266a54b6e 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -415,8 +415,8 @@ table! { time_created -> Timestamptz, time_modified -> Timestamptz, cpus -> Int8, - memory -> Int8, - storage -> Int8, + memory_bytes -> Int8, + storage_bytes -> Int8, } } diff --git a/nexus/db-queries/src/db/datastore/silo.rs b/nexus/db-queries/src/db/datastore/silo.rs index 9d53b18496..6e36f163d9 100644 --- a/nexus/db-queries/src/db/datastore/silo.rs +++ b/nexus/db-queries/src/db/datastore/silo.rs @@ -263,8 +263,8 @@ impl DataStore { SiloQuotas::new( authz_silo.id(), new_silo_params.quotas.cpus, - new_silo_params.quotas.memory, - new_silo_params.quotas.storage, + new_silo_params.quotas.memory.into(), + new_silo_params.quotas.storage.into(), ), ) .await?; diff --git a/nexus/db-queries/src/db/fixed_data/silo.rs b/nexus/db-queries/src/db/fixed_data/silo.rs index ea9a9778da..d02db783a3 100644 --- a/nexus/db-queries/src/db/fixed_data/silo.rs +++ b/nexus/db-queries/src/db/fixed_data/silo.rs @@ -25,11 +25,7 @@ lazy_static! { description: "default silo".to_string(), }, // TODO: Should the default silo have a quota? If so, what should the defaults be? - quotas: params::SiloQuotasCreate { - cpus: 0, - memory: 0, - storage: 0, - }, + quotas: params::SiloQuotasCreate::empty(), discoverable: false, identity_mode: shared::SiloIdentityMode::LocalOnly, admin_group_name: None, @@ -56,11 +52,7 @@ lazy_static! { description: "Built-in internal Silo.".to_string(), }, // The internal silo contains no virtual resources, so it has no allotted capacity. - quotas: params::SiloQuotasCreate { - cpus: 0, - memory: 0, - storage: 0, - }, + quotas: params::SiloQuotasCreate::empty(), discoverable: false, identity_mode: shared::SiloIdentityMode::LocalOnly, admin_group_name: None, diff --git a/nexus/src/app/external_endpoints.rs b/nexus/src/app/external_endpoints.rs index f95c64d3eb..62238a651b 100644 --- a/nexus/src/app/external_endpoints.rs +++ b/nexus/src/app/external_endpoints.rs @@ -827,6 +827,7 @@ mod test { name: name.parse().unwrap(), description: String::new(), }, + quotas: params::SiloQuotasCreate::empty(), discoverable: false, identity_mode, admin_group_name: None, diff --git a/nexus/src/app/rack.rs b/nexus/src/app/rack.rs index be0934afdf..9f63e312ae 100644 --- a/nexus/src/app/rack.rs +++ b/nexus/src/app/rack.rs @@ -204,7 +204,7 @@ impl super::Nexus { description: "built-in recovery Silo".to_string(), }, // TODO: Should the recovery silo have a quota? If so, what should the defaults be? - quotas: params::SiloQuotasCreate { cpus: 0, memory: 0, storage: 0 }, + quotas: params::SiloQuotasCreate::empty(), discoverable: false, identity_mode: SiloIdentityMode::LocalOnly, admin_group_name: None, diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 5687e19c69..2ec41830bc 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -264,8 +264,8 @@ pub async fn create_silo( }, quotas: params::SiloQuotasCreate { cpus: 36, - memory: 1000, - storage: 100000, + memory: ByteCount::from_gibibytes_u32(1000), + storage: ByteCount::from_gibibytes_u32(100000), }, discoverable, identity_mode, diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 536b96f7ae..f5450fc633 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -91,6 +91,11 @@ lazy_static! { name: DEMO_SILO_NAME.clone(), description: String::from(""), }, + quotas: params::SiloQuotasCreate { + cpus: 100, + memory: ByteCount::from_gibibytes_u32(100), + storage: ByteCount::from_gibibytes_u32(100), + }, discoverable: true, identity_mode: shared::SiloIdentityMode::SamlJit, admin_group_name: None, diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs index e5f41d294d..84199c48cc 100644 --- a/nexus/tests/integration_tests/silos.rs +++ b/nexus/tests/integration_tests/silos.rs @@ -65,6 +65,7 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) { name: cptestctx.silo_name.clone(), description: "a silo".to_string(), }, + quotas: params::SiloQuotasCreate::empty(), discoverable: false, identity_mode: shared::SiloIdentityMode::LocalOnly, admin_group_name: None, @@ -281,6 +282,7 @@ async fn test_silo_admin_group(cptestctx: &ControlPlaneTestContext) { name: "silo-name".parse().unwrap(), description: "a silo".to_string(), }, + quotas: params::SiloQuotasCreate::empty(), discoverable: false, identity_mode: shared::SiloIdentityMode::SamlJit, admin_group_name: Some("administrator".into()), @@ -2253,6 +2255,7 @@ async fn test_silo_authn_policy(cptestctx: &ControlPlaneTestContext) { name: silo_name, description: String::new(), }, + quotas: params::SiloQuotasCreate::empty(), discoverable: false, identity_mode: shared::SiloIdentityMode::LocalOnly, admin_group_name: None, @@ -2329,6 +2332,7 @@ async fn check_fleet_privileges( name: SILO_NAME.parse().unwrap(), description: String::new(), }, + quotas: params::SiloQuotasCreate::empty(), discoverable: false, identity_mode: shared::SiloIdentityMode::LocalOnly, admin_group_name: None, @@ -2357,6 +2361,7 @@ async fn check_fleet_privileges( name: SILO_NAME.parse().unwrap(), description: String::new(), }, + quotas: params::SiloQuotasCreate::empty(), discoverable: false, identity_mode: shared::SiloIdentityMode::LocalOnly, admin_group_name: None, @@ -2384,6 +2389,7 @@ async fn check_fleet_privileges( name: SILO_NAME.parse().unwrap(), description: String::new(), }, + quotas: params::SiloQuotasCreate::empty(), discoverable: false, identity_mode: shared::SiloIdentityMode::LocalOnly, admin_group_name: None, @@ -2416,6 +2422,7 @@ async fn check_fleet_privileges( name: SILO_NAME.parse().unwrap(), description: String::new(), }, + quotas: params::SiloQuotasCreate::empty(), discoverable: false, identity_mode: shared::SiloIdentityMode::LocalOnly, admin_group_name: None, diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 293a139506..0604fa72d6 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -294,15 +294,25 @@ pub struct SiloCreate { #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct SiloQuotasCreate { pub cpus: i64, - pub memory: i64, - pub storage: i64, + pub memory: ByteCount, + pub storage: ByteCount, +} + +impl SiloQuotasCreate { + pub fn empty() -> Self { + Self { + cpus: 0, + memory: ByteCount::from(0), + storage: ByteCount::from(0), + } + } } #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct SiloQuotasUpdate { pub cpus: Option, - pub memory: Option, - pub storage: Option, + pub memory: Option, + pub storage: Option, } /// Create-time parameters for a `User` diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index d00e095150..8cb1595112 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -53,8 +53,8 @@ pub struct Silo { pub struct SiloQuotas { pub silo_id: Uuid, pub cpus: i64, - pub memory: i64, - pub storage: i64, + pub memory: ByteCount, + pub storage: ByteCount, } // IDENTITY PROVIDER diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 214a783434..c1316d4e61 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -832,8 +832,8 @@ CREATE TABLE IF NOT EXISTS omicron.public.silo_quotas ( time_created TIMESTAMPTZ NOT NULL, time_modified TIMESTAMPTZ NOT NULL, cpus INT8 NOT NULL, - memory INT8 NOT NULL, - storage INT8 NOT NULL + memory_bytes INT8 NOT NULL, + storage_bytes INT8 NOT NULL ); /*