-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add resource limits #4605
Merged
Merged
Add resource limits #4605
Changes from 45 commits
Commits
Show all changes
53 commits
Select commit
Hold shift + click to select a range
b59092c
Outline basic CRUD actions for quotas
zephraph 159644c
Only allow viewing and updating quotas
zephraph 21ba99c
Start outlining quota database type
zephraph 0127dd1
Simplify quota model to be flatter
zephraph 5c50ce5
Rough API shape of resource quotas
zephraph 3346a33
Create quotas during silo creation
zephraph 501f5ac
Update comment for internal silo
zephraph 1894eb3
Merge branch 'main' into add-resource-limits
zephraph 9e51431
Drop inner silo quota view
zephraph caf371b
Convert storage/memory to bytecount instead of ints
zephraph 96877b2
WIP quota check CTE
zephraph 8025899
Add quota checks for cpu, memory, storage
zephraph a4002ff
Join do_update and quota_check into a single CTE step
zephraph 76c722f
Try (and fail) to resolve CTE type issues
zephraph f8e2afc
Resolve CTE compliation issues... ???
zephraph 3d4ebe2
Convert quota detlas to sql types; fix borrow issues
zephraph d317629
Merge branch 'main' into add-resource-limits
zephraph dcf974e
Remove a todo
zephraph 83c9872
Wire up quota limit error handling
zephraph 7e779a0
Fix missing quota specifier
zephraph 9859e12
SELECT got you down? Try JOIN today!
smklein 45eb0ef
Whoops, removing test cruft
smklein 55ff0b2
Only perform quota checks if allocating resources
zephraph 2ee6d36
Adjust some quotas in hopes of not failing all the tests
zephraph 4c58f9e
Add WIP integration test setup for quotas
zephraph 3b014c8
Show that quota checks are failing when they shouldn't
zephraph bd950bc
Ensure default silo gets constructed w/ a quota
zephraph 4b0d236
Flesh out basic quota tests
zephraph 0ad4340
Remove unnecessary transaction
zephraph 4f6d49f
Remove quotas tag, rename system silo list
zephraph 4111cc2
Drop custom quota configuration for standard half-rack config
zephraph 501563b
Fixup auth tests
zephraph f15a451
Better docs
zephraph 569b995
Delete quotas if the silo is deleted
zephraph ac8a2f3
Merge branch 'main' into add-resource-limits
zephraph 43beca8
Add a migration for silo_quotas and backfill data
zephraph 2c11210
Bump schema version
zephraph 025cd84
Merge branch 'main' into add-resource-limits
zephraph 8a7794b
Use insufficient capacity error
zephraph f8955a5
half_rack -> from_sled_count
zephraph 77f6d29
Merge branch 'main' into add-resource-limits
zephraph 760675d
Merge branch 'main' into add-resource-limits
zephraph 371bc98
Fix db deadlock caused by checking authz before silo is created
zephraph de951a7
Add quotas for silo cert tests
zephraph 410d3ae
Fix type failures
zephraph cd22388
Fix docs build
zephraph e86ec72
Update nexus/db-queries/src/db/queries/virtual_provisioning_collectio…
zephraph 5974325
Use an arbitrarily high value over something that looks real but isn't
zephraph c602e29
Enhance tests to check errors; fix overflow
zephraph fe87ca8
Drop expects for unwraps when we don't expect them to fail
zephraph 11ab326
Update comment
zephraph e369851
Fix clippy errors
zephraph 635d1ba
ci: add quota setup to deploy test
rcgoodfellow File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
use super::ByteCount; | ||
use crate::schema::silo_quotas; | ||
use chrono::{DateTime, Utc}; | ||
use nexus_types::external_api::{params, views}; | ||
use serde::{Deserialize, Serialize}; | ||
use uuid::Uuid; | ||
|
||
#[derive( | ||
Queryable, | ||
Insertable, | ||
Debug, | ||
Clone, | ||
Selectable, | ||
Serialize, | ||
Deserialize, | ||
AsChangeset, | ||
)] | ||
#[diesel(table_name = silo_quotas)] | ||
pub struct SiloQuotas { | ||
pub silo_id: Uuid, | ||
pub time_created: DateTime<Utc>, | ||
pub time_modified: DateTime<Utc>, | ||
|
||
/// The number of CPUs that this silo is allowed to use | ||
pub cpus: 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: ByteCount, | ||
storage: ByteCount, | ||
) -> Self { | ||
Self { | ||
silo_id, | ||
time_created: Utc::now(), | ||
time_modified: Utc::now(), | ||
cpus, | ||
memory, | ||
storage, | ||
} | ||
} | ||
|
||
pub fn from_sled_count(silo_id: Uuid, num_sleds: u32) -> Self { | ||
let count = params::SiloQuotasCreate::from_sled_count(num_sleds); | ||
Self::new( | ||
silo_id, | ||
count.cpus, | ||
count.memory.into(), | ||
count.storage.into(), | ||
) | ||
} | ||
} | ||
|
||
impl From<SiloQuotas> for views::SiloQuotas { | ||
fn from(silo_quotas: SiloQuotas) -> Self { | ||
Self { | ||
silo_id: silo_quotas.silo_id, | ||
cpus: silo_quotas.cpus, | ||
memory: silo_quotas.memory.into(), | ||
storage: silo_quotas.storage.into(), | ||
} | ||
} | ||
} | ||
|
||
impl From<views::SiloQuotas> for SiloQuotas { | ||
fn from(silo_quotas: views::SiloQuotas) -> Self { | ||
Self { | ||
silo_id: silo_quotas.silo_id, | ||
time_created: Utc::now(), | ||
time_modified: Utc::now(), | ||
cpus: silo_quotas.cpus, | ||
memory: silo_quotas.memory.into(), | ||
storage: silo_quotas.storage.into(), | ||
} | ||
} | ||
} | ||
|
||
// Describes a set of updates for the [`SiloQuotas`] model. | ||
#[derive(AsChangeset)] | ||
#[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>, | ||
} | ||
|
||
impl From<params::SiloQuotasUpdate> for SiloQuotasUpdate { | ||
fn from(params: params::SiloQuotasUpdate) -> Self { | ||
Self { | ||
cpus: params.cpus, | ||
memory: params.memory.map(|f| f.into()), | ||
storage: params.storage.map(|f| f.into()), | ||
time_modified: Utc::now(), | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,127 @@ | ||
use super::DataStore; | ||
use crate::authz; | ||
use crate::context::OpContext; | ||
use crate::db; | ||
use crate::db::error::public_error_from_diesel; | ||
use crate::db::error::ErrorHandler; | ||
use crate::db::pagination::paginated; | ||
use crate::db::pool::DbConnection; | ||
use async_bb8_diesel::AsyncRunQueryDsl; | ||
use diesel::prelude::*; | ||
use nexus_db_model::SiloQuotas; | ||
use nexus_db_model::SiloQuotasUpdate; | ||
use omicron_common::api::external::DataPageParams; | ||
use omicron_common::api::external::DeleteResult; | ||
use omicron_common::api::external::Error; | ||
use omicron_common::api::external::ListResultVec; | ||
use omicron_common::api::external::ResourceType; | ||
use omicron_common::api::external::UpdateResult; | ||
use uuid::Uuid; | ||
|
||
impl DataStore { | ||
/// Creates new quotas for a silo. This is grouped with silo creation | ||
/// and shouldn't be called outside of that flow. | ||
/// | ||
/// An authz check _cannot_ be performed here because the authz initialization | ||
/// isn't complete and will lead to a db deadlock. | ||
/// | ||
/// See https://github.com/oxidecomputer/omicron/blob/07eb7dafc20e35e44edf429fcbb759cbb33edd5f/nexus/db-queries/src/db/datastore/rack.rs#L407-L410 | ||
pub async fn silo_quotas_create( | ||
&self, | ||
conn: &async_bb8_diesel::Connection<DbConnection>, | ||
authz_silo: &authz::Silo, | ||
quotas: SiloQuotas, | ||
) -> Result<(), Error> { | ||
let silo_id = authz_silo.id(); | ||
use db::schema::silo_quotas::dsl; | ||
|
||
diesel::insert_into(dsl::silo_quotas) | ||
.values(quotas) | ||
.execute_async(conn) | ||
.await | ||
.map_err(|e| { | ||
public_error_from_diesel( | ||
e, | ||
ErrorHandler::Conflict( | ||
ResourceType::SiloQuotas, | ||
&silo_id.to_string(), | ||
), | ||
) | ||
}) | ||
.map(|_| ()) | ||
zephraph marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
pub async fn silo_quotas_delete( | ||
&self, | ||
opctx: &OpContext, | ||
conn: &async_bb8_diesel::Connection<DbConnection>, | ||
authz_silo: &authz::Silo, | ||
) -> DeleteResult { | ||
// Given that the quotas right now are somewhat of an extension of the | ||
// Silo we just check for delete permission on the silo itself. | ||
opctx.authorize(authz::Action::Delete, authz_silo).await?; | ||
|
||
use db::schema::silo_quotas::dsl; | ||
diesel::delete(dsl::silo_quotas) | ||
.filter(dsl::silo_id.eq(authz_silo.id())) | ||
.execute_async(conn) | ||
.await | ||
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; | ||
|
||
Ok(()) | ||
} | ||
|
||
pub async fn silo_update_quota( | ||
&self, | ||
opctx: &OpContext, | ||
authz_silo: &authz::Silo, | ||
updates: SiloQuotasUpdate, | ||
) -> UpdateResult<SiloQuotas> { | ||
opctx.authorize(authz::Action::Modify, authz_silo).await?; | ||
use db::schema::silo_quotas::dsl; | ||
let silo_id = authz_silo.id(); | ||
diesel::update(dsl::silo_quotas) | ||
.filter(dsl::silo_id.eq(silo_id)) | ||
.set(updates) | ||
.returning(SiloQuotas::as_returning()) | ||
.get_result_async(&*self.pool_connection_authorized(opctx).await?) | ||
.await | ||
.map_err(|e| { | ||
public_error_from_diesel( | ||
e, | ||
ErrorHandler::Conflict( | ||
ResourceType::SiloQuotas, | ||
&silo_id.to_string(), | ||
), | ||
) | ||
}) | ||
} | ||
|
||
pub async fn silo_quotas_view( | ||
&self, | ||
opctx: &OpContext, | ||
authz_silo: &authz::Silo, | ||
) -> Result<SiloQuotas, Error> { | ||
opctx.authorize(authz::Action::Read, authz_silo).await?; | ||
use db::schema::silo_quotas::dsl; | ||
dsl::silo_quotas | ||
.filter(dsl::silo_id.eq(authz_silo.id())) | ||
.first_async(&*self.pool_connection_authorized(opctx).await?) | ||
.await | ||
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) | ||
} | ||
|
||
pub async fn fleet_list_quotas( | ||
&self, | ||
opctx: &OpContext, | ||
pagparams: &DataPageParams<'_, Uuid>, | ||
) -> ListResultVec<SiloQuotas> { | ||
opctx.authorize(authz::Action::ListChildren, &authz::FLEET).await?; | ||
use db::schema::silo_quotas::dsl; | ||
paginated(dsl::silo_quotas, dsl::silo_id, pagparams) | ||
.select(SiloQuotas::as_select()) | ||
.load_async(&*self.pool_connection_authorized(opctx).await?) | ||
.await | ||
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would you need to go from view to model? Is this used?