Skip to content

Commit 94f0731

Browse files
nexus-db-queries: verify blueprint test covers all tables (#8455) (#8667)
Add ensure_blueprint_fully_populated() to check all bp_* tables get populated by representative blueprint. Complements existing ensure_blueprint_fully_deleted(). Add ClickHouse config to representative blueprint. Fixes #8455 --------- Co-authored-by: David Pacheco <[email protected]>
1 parent 32f0348 commit 94f0731

File tree

1 file changed

+199
-82
lines changed

1 file changed

+199
-82
lines changed

nexus/db-queries/src/db/datastore/deployment.rs

Lines changed: 199 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -3039,94 +3039,20 @@ mod tests {
30393039
pub should_write_data: Option<Arc<AtomicBool>>,
30403040
}
30413041

3042-
// This is a not-super-future-maintainer-friendly helper to check that all
3043-
// the subtables related to blueprints have been pruned of a specific
3044-
// blueprint ID. If additional blueprint tables are added in the future,
3045-
// this function will silently ignore them unless they're manually added.
3042+
// Check that all the subtables related to blueprints have been pruned of a specific
3043+
// blueprint ID. Uses the shared BlueprintTableCounts struct.
30463044
async fn ensure_blueprint_fully_deleted(
30473045
datastore: &DataStore,
30483046
blueprint_id: BlueprintUuid,
30493047
) {
3050-
let conn = datastore.pool_connection_for_tests().await.unwrap();
3051-
3052-
macro_rules! query_count {
3053-
($table:ident, $blueprint_id_col:ident) => {{
3054-
use nexus_db_schema::schema::$table::dsl;
3055-
let result = dsl::$table
3056-
.filter(
3057-
dsl::$blueprint_id_col
3058-
.eq(to_db_typed_uuid(blueprint_id)),
3059-
)
3060-
.count()
3061-
.get_result_async(&*conn)
3062-
.await;
3063-
(stringify!($table), result)
3064-
}};
3065-
}
3048+
let counts = BlueprintTableCounts::new(datastore, blueprint_id).await;
30663049

3067-
// These tables start with `bp_` but do not represent the contents of a
3068-
// specific blueprint. It should be uncommon to add things to this
3069-
// list.
3070-
let tables_ignored: BTreeSet<_> = ["bp_target"].into_iter().collect();
3071-
3072-
let mut tables_checked = BTreeSet::new();
3073-
for (table_name, result) in [
3074-
query_count!(blueprint, id),
3075-
query_count!(bp_sled_metadata, blueprint_id),
3076-
query_count!(bp_omicron_dataset, blueprint_id),
3077-
query_count!(bp_omicron_physical_disk, blueprint_id),
3078-
query_count!(bp_omicron_zone, blueprint_id),
3079-
query_count!(bp_omicron_zone_nic, blueprint_id),
3080-
query_count!(bp_clickhouse_cluster_config, blueprint_id),
3081-
query_count!(bp_clickhouse_keeper_zone_id_to_node_id, blueprint_id),
3082-
query_count!(bp_clickhouse_server_zone_id_to_node_id, blueprint_id),
3083-
query_count!(bp_oximeter_read_policy, blueprint_id),
3084-
query_count!(bp_pending_mgs_update_sp, blueprint_id),
3085-
query_count!(bp_pending_mgs_update_rot, blueprint_id),
3086-
query_count!(bp_pending_mgs_update_rot_bootloader, blueprint_id),
3087-
query_count!(bp_pending_mgs_update_host_phase_1, blueprint_id),
3088-
] {
3089-
let count: i64 = result.unwrap();
3090-
assert_eq!(
3091-
count, 0,
3092-
"nonzero row count for blueprint \
3093-
{blueprint_id} in table {table_name}"
3094-
);
3095-
tables_checked.insert(table_name);
3096-
}
3097-
3098-
// Look for likely blueprint-related tables that we didn't check.
3099-
let mut query = QueryBuilder::new();
3100-
query.sql(
3101-
"SELECT table_name \
3102-
FROM information_schema.tables \
3103-
WHERE table_name LIKE 'bp\\_%'",
3050+
// All tables should be empty (no exceptions for deleted blueprints)
3051+
assert!(
3052+
counts.all_empty(),
3053+
"Blueprint {blueprint_id} not fully deleted. Non-empty tables: {:?}",
3054+
counts.non_empty_tables()
31043055
);
3105-
let tables_unchecked: Vec<String> = query
3106-
.query::<diesel::sql_types::Text>()
3107-
.load_async(&*conn)
3108-
.await
3109-
.expect("Failed to query information_schema for tables")
3110-
.into_iter()
3111-
.filter(|f: &String| {
3112-
let t = f.as_str();
3113-
!tables_ignored.contains(t) && !tables_checked.contains(t)
3114-
})
3115-
.collect();
3116-
if !tables_unchecked.is_empty() {
3117-
// If you see this message, you probably added a blueprint table
3118-
// whose name started with `bp_*`. Add it to the block above so
3119-
// that this function checks whether deleting a blueprint deletes
3120-
// rows from that table. (You may also find you need to update
3121-
// blueprint_delete() to actually delete said rows.)
3122-
panic!(
3123-
"found table(s) that look related to blueprints, but \
3124-
aren't covered by ensure_blueprint_fully_deleted(). \
3125-
Please add them to that function!\n
3126-
Found: {}",
3127-
tables_unchecked.join(", ")
3128-
);
3129-
}
31303056
}
31313057

31323058
// Create a fake set of `SledDetails`, either with a subnet matching
@@ -3287,6 +3213,9 @@ mod tests {
32873213
[blueprint1.id]
32883214
);
32893215

3216+
// Ensure every bp_* table received at least one row for this blueprint (issue #8455).
3217+
ensure_blueprint_fully_populated(&datastore, blueprint1.id).await;
3218+
32903219
// Check the number of blueprint elements against our collection.
32913220
assert_eq!(
32923221
blueprint1.sleds.len(),
@@ -4583,4 +4512,192 @@ mod tests {
45834512
found these zones not in service: {not_in_service:?}"
45844513
);
45854514
}
4515+
4516+
/// Counts rows in blueprint-related tables for a specific blueprint ID.
4517+
/// Used by both `ensure_blueprint_fully_populated` and `ensure_blueprint_fully_deleted`.
4518+
struct BlueprintTableCounts {
4519+
counts: BTreeMap<String, i64>,
4520+
}
4521+
4522+
impl BlueprintTableCounts {
4523+
/// Create a new BlueprintTableCounts by querying all blueprint tables.
4524+
async fn new(
4525+
datastore: &DataStore,
4526+
blueprint_id: BlueprintUuid,
4527+
) -> BlueprintTableCounts {
4528+
let conn = datastore.pool_connection_for_tests().await.unwrap();
4529+
4530+
macro_rules! query_count {
4531+
($table:ident, $blueprint_id_col:ident) => {{
4532+
use nexus_db_schema::schema::$table::dsl;
4533+
let result = dsl::$table
4534+
.filter(
4535+
dsl::$blueprint_id_col
4536+
.eq(to_db_typed_uuid(blueprint_id)),
4537+
)
4538+
.count()
4539+
.get_result_async(&*conn)
4540+
.await;
4541+
(stringify!($table), result)
4542+
}};
4543+
}
4544+
4545+
let mut counts = BTreeMap::new();
4546+
for (table_name, result) in [
4547+
query_count!(blueprint, id),
4548+
query_count!(bp_sled_metadata, blueprint_id),
4549+
query_count!(bp_omicron_dataset, blueprint_id),
4550+
query_count!(bp_omicron_physical_disk, blueprint_id),
4551+
query_count!(bp_omicron_zone, blueprint_id),
4552+
query_count!(bp_omicron_zone_nic, blueprint_id),
4553+
query_count!(bp_clickhouse_cluster_config, blueprint_id),
4554+
query_count!(
4555+
bp_clickhouse_keeper_zone_id_to_node_id,
4556+
blueprint_id
4557+
),
4558+
query_count!(
4559+
bp_clickhouse_server_zone_id_to_node_id,
4560+
blueprint_id
4561+
),
4562+
query_count!(bp_oximeter_read_policy, blueprint_id),
4563+
query_count!(bp_pending_mgs_update_sp, blueprint_id),
4564+
query_count!(bp_pending_mgs_update_rot, blueprint_id),
4565+
query_count!(
4566+
bp_pending_mgs_update_rot_bootloader,
4567+
blueprint_id
4568+
),
4569+
query_count!(bp_pending_mgs_update_host_phase_1, blueprint_id),
4570+
] {
4571+
let count: i64 = result.unwrap();
4572+
counts.insert(table_name.to_string(), count);
4573+
}
4574+
4575+
let table_counts = BlueprintTableCounts { counts };
4576+
4577+
// Verify no new blueprint tables were added without updating this function
4578+
if let Err(msg) =
4579+
table_counts.verify_all_tables_covered(datastore).await
4580+
{
4581+
panic!("{}", msg);
4582+
}
4583+
4584+
table_counts
4585+
}
4586+
4587+
/// Returns true if all tables are empty (0 rows).
4588+
fn all_empty(&self) -> bool {
4589+
self.counts.values().all(|&count| count == 0)
4590+
}
4591+
4592+
/// Returns a list of table names that are empty.
4593+
fn empty_tables(&self) -> Vec<String> {
4594+
self.counts
4595+
.iter()
4596+
.filter_map(
4597+
|(table, &count)| {
4598+
if count == 0 { Some(table.clone()) } else { None }
4599+
},
4600+
)
4601+
.collect()
4602+
}
4603+
4604+
/// Returns a list of table names that are non-empty.
4605+
fn non_empty_tables(&self) -> Vec<String> {
4606+
self.counts
4607+
.iter()
4608+
.filter_map(
4609+
|(table, &count)| {
4610+
if count > 0 { Some(table.clone()) } else { None }
4611+
},
4612+
)
4613+
.collect()
4614+
}
4615+
4616+
/// Get all table names that were checked.
4617+
fn tables_checked(&self) -> BTreeSet<&str> {
4618+
self.counts.keys().map(|s| s.as_str()).collect()
4619+
}
4620+
4621+
/// Verify no new blueprint tables were added without updating this function.
4622+
async fn verify_all_tables_covered(
4623+
&self,
4624+
datastore: &DataStore,
4625+
) -> Result<(), String> {
4626+
let conn = datastore.pool_connection_for_tests().await.unwrap();
4627+
4628+
// Tables prefixed with `bp_` that are *not* specific to a single blueprint
4629+
// and therefore intentionally ignored. There is only one of these right now.
4630+
let tables_ignored: BTreeSet<_> =
4631+
["bp_target"].into_iter().collect();
4632+
let tables_checked = self.tables_checked();
4633+
4634+
let mut query = QueryBuilder::new();
4635+
query.sql(
4636+
"SELECT table_name FROM information_schema.tables WHERE table_name LIKE 'bp\\_%'",
4637+
);
4638+
let tables_unchecked: Vec<String> = query
4639+
.query::<diesel::sql_types::Text>()
4640+
.load_async(&*conn)
4641+
.await
4642+
.expect("Failed to query information_schema for tables")
4643+
.into_iter()
4644+
.filter(|f: &String| {
4645+
let t = f.as_str();
4646+
!tables_ignored.contains(t) && !tables_checked.contains(t)
4647+
})
4648+
.collect();
4649+
4650+
if !tables_unchecked.is_empty() {
4651+
Err(format!(
4652+
"found blueprint-related table(s) not covered by BlueprintTableCounts: {}\n\n\
4653+
If you see this message, you probably added a blueprint table whose name started with `bp_*`. \
4654+
Add it to the query list in BlueprintTableCounts::new() so that this function checks the table. \
4655+
You may also need to update blueprint deletion/insertion code to handle rows in that table.",
4656+
tables_unchecked.join(", ")
4657+
))
4658+
} else {
4659+
Ok(())
4660+
}
4661+
}
4662+
}
4663+
4664+
// Verify that every blueprint-related table contains ≥1 row for `blueprint_id`.
4665+
// Complements `ensure_blueprint_fully_deleted`.
4666+
async fn ensure_blueprint_fully_populated(
4667+
datastore: &DataStore,
4668+
blueprint_id: BlueprintUuid,
4669+
) {
4670+
let counts = BlueprintTableCounts::new(datastore, blueprint_id).await;
4671+
4672+
// Exception tables that may be empty in the representative blueprint:
4673+
// - MGS update tables: only populated when blueprint includes firmware updates
4674+
// - ClickHouse tables: only populated when blueprint includes ClickHouse configuration
4675+
let exception_tables = [
4676+
"bp_pending_mgs_update_sp",
4677+
"bp_pending_mgs_update_rot",
4678+
"bp_pending_mgs_update_rot_bootloader",
4679+
"bp_pending_mgs_update_host_phase_1",
4680+
"bp_clickhouse_cluster_config",
4681+
"bp_clickhouse_keeper_zone_id_to_node_id",
4682+
"bp_clickhouse_server_zone_id_to_node_id",
4683+
];
4684+
4685+
// Check that all non-exception tables have at least one row
4686+
let empty_tables = counts.empty_tables();
4687+
let problematic_tables: Vec<_> = empty_tables
4688+
.into_iter()
4689+
.filter(|table| !exception_tables.contains(&table.as_str()))
4690+
.collect();
4691+
4692+
if !problematic_tables.is_empty() {
4693+
panic!(
4694+
"Expected tables to be populated for blueprint {blueprint_id}: {:?}\n\n\
4695+
If every blueprint should be expected to have a value in this table, then this is a bug. \
4696+
Otherwise, you may need to add a table to the exception list in `ensure_blueprint_fully_populated()`. \
4697+
If you do this, please ensure that you add a test to `test_representative_blueprint()` that creates a \
4698+
blueprint that _does_ populate this table and verifies it.",
4699+
problematic_tables
4700+
);
4701+
}
4702+
}
45864703
}

0 commit comments

Comments
 (0)