Convert storage/memory to bytecount instead of ints

oxidecomputer · Dec 5, 2023 · caf371b · caf371b
1 parent 9e51431
commit caf371b
Show file tree

Hide file tree

Showing 12 changed files with 64 additions and 34 deletions.
diff --git 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<Utc>,
     pub time_modified: DateTime<Utc>,
 
+    /// 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<SiloQuotas> 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<views::SiloQuotas> 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<views::SiloQuotas> for SiloQuotas {
 #[diesel(table_name = silo_quotas)]
 pub struct SiloQuotasUpdate {
     pub cpus: Option<i64>,
+    #[diesel(column_name = memory_bytes)]
     pub memory: Option<i64>,
+    #[diesel(column_name = storage_bytes)]
     pub storage: Option<i64>,
     pub time_modified: DateTime<Utc>,
 }
@@ -76,8 +91,8 @@ impl From<params::SiloQuotasUpdate> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<i64>,
-    pub memory: Option<i64>,
-    pub storage: Option<i64>,
+    pub memory: Option<ByteCount>,
+    pub storage: Option<ByteCount>,
 }
 
 /// Create-time parameters for a `User`

diff --git 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
@@ -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
 );
 
 /*