Skip to content
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.

Sign up for GitHub

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

[reconfigurator][support-bundles] Choose datasets for support bundles from current blueprint #7330

Closed
wants to merge 3 commits into from
Closed

[reconfigurator][support-bundles] Choose datasets for support bundles from current blueprint #7330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d8786b
add is_current_target_blueprint() extension for specific blueprint_id…
jgallagher Jan 10, 2025
885e64f
add blueprint_dataset_filter() diesel extension trait
jgallagher Jan 10, 2025
40ead1d
support bundles: choose dataset from blueprint
jgallagher Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 131 additions & 2 deletions nexus/db-model/src/deployment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,7 @@ impl BpClickhouseServerZoneIdToNodeId {
}
}

mod diesel_util {
mod diesel_blueprint_zone_filter {
use crate::{
schema::bp_omicron_zone::disposition, to_db_bp_zone_disposition,
DbBpZoneDisposition,
Expand Down Expand Up @@ -1146,4 +1146,133 @@ mod diesel_util {
EqAny<disposition, BoxedIterator<DbBpZoneDisposition>>;
}

pub use diesel_util::ApplyBlueprintZoneFilterExt;
pub use diesel_blueprint_zone_filter::ApplyBlueprintZoneFilterExt;

mod diesel_blueprint_dataset_filter {
use crate::{
schema::bp_omicron_dataset::disposition, to_db_bp_dataset_disposition,
DbBpDatasetDisposition,
};
use diesel::{
helper_types::EqAny, prelude::*, query_dsl::methods::FilterDsl,
};
use nexus_types::deployment::{
BlueprintDatasetDisposition, BlueprintDatasetFilter,
};

/// An extension trait to apply a [`BlueprintDatasetFilter`] to a Diesel
/// expression.
///
/// This is applicable to any Diesel expression which includes the
/// `bp_omicron_dataset` table.
///
/// This needs to live here, rather than in `nexus-db-queries`, because it
/// names the `DbBpDatasetDisposition` type which is private to this crate.
pub trait ApplyBlueprintDatasetFilterExt {
type Output;

/// Applies a [`BlueprintDatasetFilter`] to a Diesel expression.
fn blueprint_dataset_filter(
self,
filter: BlueprintDatasetFilter,
) -> Self::Output;
}

impl<E> ApplyBlueprintDatasetFilterExt for E
where
E: FilterDsl<BlueprintDatasetFilterQuery>,
{
type Output = E::Output;

fn blueprint_dataset_filter(
self,
filter: BlueprintDatasetFilter,
) -> Self::Output {
// This is only boxed for ease of reference above.
let all_matching_dispositions: BoxedIterator<
DbBpDatasetDisposition,
> = Box::new(
BlueprintDatasetDisposition::all_matching(filter)
.map(to_db_bp_dataset_disposition),
);

FilterDsl::filter(
self,
disposition.eq_any(all_matching_dispositions),
)
}
}

type BoxedIterator<T> = Box<dyn Iterator<Item = T>>;
type BlueprintDatasetFilterQuery =
EqAny<disposition, BoxedIterator<DbBpDatasetDisposition>>;
}

pub use diesel_blueprint_dataset_filter::ApplyBlueprintDatasetFilterExt;

mod diesel_is_current_target_blueprint {
use crate::schema;
use crate::schema::bp_target;
use diesel::dsl::Desc;
use diesel::dsl::EqAny;
use diesel::dsl::Limit;
use diesel::dsl::OrderBy;
use diesel::dsl::Select;
use diesel::prelude::*;
use diesel::sql_types;

// Writing the return type of diesel queries is not awesome. Build up the
// actual return type via a sequence of type aliases that correspond to each
// of the steps of the query implementation below.
type SelectBlueprintId = Select<bp_target::table, bp_target::blueprint_id>;
type OrderByVersion = OrderBy<SelectBlueprintId, Desc<bp_target::version>>;
type IsCurrentTargetBlueprintExpr<T> = EqAny<T, Limit<OrderByVersion>>;

/// An extension trait to check that a `Uuid` expression is equal
/// to the current target blueprint.
///
/// The typical use of this will be to search one of the various blueprint
/// tables, restricted to only rows from the current target blueprint.
///
/// `diesel` does not know about our strongly-typed UUID wrappers, so
/// technically this trait could apply to any expression that evaluates to a
/// `sql_types::Uuid`. To restrict this to only blueprint IDs, we also
/// restrict this via a `Sealed` trait to only specific columns known to
/// hold blueprint IDs. This list may need to be expanded if we write new
/// blueprint ID expression sources or need to query against an expression
/// more complex than a column.
pub trait ExprIsCurrentTargetBlueprintExt:
Expression<SqlType = sql_types::Uuid> + Sized + Sealed
{
fn is_current_target_blueprint(
self,
) -> IsCurrentTargetBlueprintExpr<Self>;
}

impl<E> ExprIsCurrentTargetBlueprintExt for E
where
E: Expression<SqlType = sql_types::Uuid> + Sized + Sealed,
{
fn is_current_target_blueprint(
self,
) -> IsCurrentTargetBlueprintExpr<Self> {
use bp_target::dsl;

self.eq_any(
dsl::bp_target
.select(dsl::blueprint_id)
.order_by(dsl::version.desc())
.limit(1),
)
}
}

// The specific list of columns for which `ExprIsCurrentTargetBlueprintExt`
// exists.
pub trait Sealed {}
impl Sealed for schema::bp_omicron_dataset::blueprint_id {}
impl Sealed for schema::bp_omicron_physical_disk::blueprint_id {}
impl Sealed for schema::bp_omicron_zone::blueprint_id {}
}

pub use diesel_is_current_target_blueprint::ExprIsCurrentTargetBlueprintExt;
1 change: 1 addition & 0 deletions nexus/db-model/src/schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2021,6 +2021,7 @@ allow_tables_to_appear_in_same_query!(
allow_tables_to_appear_in_same_query!(hw_baseboard_id, inv_sled_agent,);

allow_tables_to_appear_in_same_query!(
bp_omicron_dataset,
bp_omicron_zone,
bp_target,
dataset,
Expand Down
136 changes: 76 additions & 60 deletions nexus/db-queries/src/db/datastore/support_bundle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use crate::context::OpContext;
use crate::db;
use crate::db::error::public_error_from_diesel;
use crate::db::error::ErrorHandler;
use crate::db::model::Dataset;
use crate::db::model::DatasetKind;
use crate::db::model::SupportBundle;
use crate::db::model::SupportBundleState;
Expand All @@ -20,7 +19,10 @@ use crate::transaction_retry::OptionalError;
use async_bb8_diesel::AsyncRunQueryDsl;
use diesel::prelude::*;
use futures::FutureExt;
use nexus_types::identity::Asset;
use nexus_db_model::ApplyBlueprintDatasetFilterExt;
use nexus_db_model::BpOmicronDataset;
use nexus_db_model::ExprIsCurrentTargetBlueprintExt;
use nexus_types::deployment::BlueprintDatasetFilter;
use omicron_common::api::external;
use omicron_common::api::external::CreateResult;
use omicron_common::api::external::DataPageParams;
Expand All @@ -31,7 +33,6 @@ use omicron_uuid_kinds::BlueprintUuid;
use omicron_uuid_kinds::GenericUuid;
use omicron_uuid_kinds::OmicronZoneUuid;
use omicron_uuid_kinds::SupportBundleUuid;
use omicron_uuid_kinds::ZpoolUuid;
use uuid::Uuid;

const CANNOT_ALLOCATE_ERR_MSG: &'static str =
Expand Down Expand Up @@ -93,21 +94,28 @@ impl DataStore {
let err = err.clone();

async move {
use db::schema::dataset::dsl as dataset_dsl;
use db::schema::bp_omicron_dataset::dsl as dataset_dsl;
use db::schema::support_bundle::dsl as support_bundle_dsl;

// Observe all "non-deleted, debug datasets".
// Observe all in-service `Debug` dataset in the current
// target blueprint.
//
// Return the first one we find that doesn't already
// have a support bundle allocated to it.
let free_dataset = dataset_dsl::dataset
.filter(dataset_dsl::time_deleted.is_null())
let free_dataset = dataset_dsl::bp_omicron_dataset
.filter(
db::schema::bp_omicron_dataset::blueprint_id
.is_current_target_blueprint(),
)
.blueprint_dataset_filter(
BlueprintDatasetFilter::InService,
)
.filter(dataset_dsl::kind.eq(DatasetKind::Debug))
.left_join(support_bundle_dsl::support_bundle.on(
dataset_dsl::id.eq(support_bundle_dsl::dataset_id),
))
.filter(support_bundle_dsl::dataset_id.is_null())
.select(Dataset::as_select())
.select(BpOmicronDataset::as_select())
.first_async(&conn)
.await
.optional()?;
Expand All @@ -129,8 +137,8 @@ impl DataStore {

let bundle = SupportBundle::new(
reason_for_creation,
ZpoolUuid::from_untyped_uuid(dataset.pool_id),
dataset.id(),
dataset.pool_id.into(),
dataset.id.into(),
this_nexus_id,
);

Expand Down Expand Up @@ -259,9 +267,7 @@ impl DataStore {

// For this blueprint: The set of expunged debug datasets
let invalid_datasets = blueprint
.all_omicron_datasets(
nexus_types::deployment::BlueprintDatasetFilter::Expunged,
)
.all_omicron_datasets(BlueprintDatasetFilter::Expunged)
.filter_map(|(_sled_id, dataset_config)| {
if matches!(
dataset_config.kind,
Expand Down Expand Up @@ -486,6 +492,7 @@ mod test {
use omicron_uuid_kinds::DatasetUuid;
use omicron_uuid_kinds::PhysicalDiskUuid;
use omicron_uuid_kinds::SledUuid;
use omicron_uuid_kinds::ZpoolUuid;
use rand::Rng;

// Pool/Dataset pairs, for debug datasets only.
Expand All @@ -501,18 +508,6 @@ mod test {
}

impl TestSled {
fn new_with_pool_count(pool_count: usize) -> Self {
Self {
sled: SledUuid::new_v4(),
pools: (0..pool_count)
.map(|_| TestPool {
pool: ZpoolUuid::new_v4(),
dataset: DatasetUuid::new_v4(),
})
.collect(),
}
}

fn new_from_blueprint(blueprint: &Blueprint) -> Vec<Self> {
let mut sleds = vec![];
for (sled, datasets) in &blueprint.blueprint_datasets {
Expand Down Expand Up @@ -579,17 +574,6 @@ mod test {
.zpool_insert(opctx, zpool)
.await
.expect("failed to upsert zpool");

let dataset = Dataset::new(
pool.dataset,
pool.pool.into_untyped_uuid(),
None,
DebugDatasetKind,
);
datastore
.dataset_upsert(dataset)
.await
.expect("failed to upsert dataset");
}
}
}
Expand All @@ -601,8 +585,42 @@ mod test {
opctx: &OpContext,
pool_count: usize,
) -> TestSled {
let sled = TestSled::new_with_pool_count(pool_count);
sled.create_database_records(&datastore, &opctx).await;
let mut rng = SimRngState::from_seed(&format!(
"create_sled_and_zpools-{pool_count}"
));
let (example, bp1) = ExampleSystemBuilder::new_with_rng(
&opctx.log,
rng.next_system_rng(),
)
.nsleds(1)
.create_zones(false)
.ndisks_per_sled(
pool_count.try_into().expect("reasonable number of pools"),
)
.build();

bp_insert_and_make_target(opctx, datastore, &example.initial_blueprint)
.await;
bp_insert_and_make_target(opctx, datastore, &bp1).await;

// Flatten the blueprint down to just a `TestSled` with its Debug
// datasets.
assert_eq!(bp1.blueprint_datasets.len(), 1);
let (&sled, datasets_config) = bp1
.blueprint_datasets
.first_key_value()
.expect("blueprint has 1 sled");

let pools = datasets_config
.datasets
.values()
.filter(|d| d.kind == DebugDatasetKind)
.map(|d| TestPool { pool: d.pool.id(), dataset: d.id })
.collect::<Vec<_>>();
assert_eq!(pools.len(), pool_count);

let sled = TestSled { sled, pools };
sled.create_database_records(datastore, opctx).await;
sled
}

Expand Down Expand Up @@ -973,22 +991,21 @@ mod test {
let (opctx, datastore) = (db.opctx(), db.datastore());

let mut rng = SimRngState::from_seed(TEST_NAME);
let (_example, mut bp1) = ExampleSystemBuilder::new_with_rng(
let (example, bp1) = ExampleSystemBuilder::new_with_rng(
&logctx.log,
rng.next_system_rng(),
)
.build();

// Weirdly, the "ExampleSystemBuilder" blueprint has a parent blueprint,
// but which isn't exposed through the API. Since we're only able to see
// the blueprint it emits, that means we can't actually make it the
// target because "the parent blueprint is not the current target".
//
// Instead of dealing with that, we lie: claim this is the primordial
// blueprint, with no parent.
//
// Regardless, make this starter blueprint our target.
bp1.parent_blueprint_id = None;
// Insert both the primordial blueprint created by
// `ExampleSystemBuilder` and the useful blueprint (its child), making
// the latter the target for the rest of the test.
bp_insert_and_make_target(
&opctx,
&datastore,
&example.initial_blueprint,
)
.await;
bp_insert_and_make_target(&opctx, &datastore, &bp1).await;

// Manually perform the equivalent of blueprint execution to populate
Expand Down Expand Up @@ -1080,22 +1097,21 @@ mod test {
let (opctx, datastore) = (db.opctx(), db.datastore());

let mut rng = SimRngState::from_seed(TEST_NAME);
let (_example, mut bp1) = ExampleSystemBuilder::new_with_rng(
let (example, bp1) = ExampleSystemBuilder::new_with_rng(
&logctx.log,
rng.next_system_rng(),
)
.build();

// Weirdly, the "ExampleSystemBuilder" blueprint has a parent blueprint,
// but which isn't exposed through the API. Since we're only able to see
// the blueprint it emits, that means we can't actually make it the
// target because "the parent blueprint is not the current target".
//
// Instead of dealing with that, we lie: claim this is the primordial
// blueprint, with no parent.
//
// Regardless, make this starter blueprint our target.
bp1.parent_blueprint_id = None;
// Insert both the primordial blueprint created by
// `ExampleSystemBuilder` and the useful blueprint (its child), making
// the latter the target for the rest of the test.
bp_insert_and_make_target(
&opctx,
&datastore,
&example.initial_blueprint,
)
.await;
bp_insert_and_make_target(&opctx, &datastore, &bp1).await;

// Manually perform the equivalent of blueprint execution to populate
Expand Down
Loading
Loading