From ae9501db807ade25c57a66f4ee05acf365df5105 Mon Sep 17 00:00:00 2001 From: Sara Elzayat Date: Thu, 4 Sep 2025 20:11:52 +0300 Subject: [PATCH 1/7] nexus-db-queries: track blueprint table coverage cumulatively --- .../db-queries/src/db/datastore/deployment.rs | 51 ++++++++++++------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index 4cae278a6bd..2e3a9dd1d93 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -3265,8 +3265,8 @@ mod tests { [blueprint1.id] ); - // Ensure every bp_* table received at least one row for this blueprint (issue #8455). - ensure_blueprint_fully_populated(&datastore, blueprint1.id).await; + // Start tracking cumulative blueprint table coverage. + let mut cumulative_counts = BlueprintTableCounts::new(&datastore, blueprint1.id).await; // Check the number of blueprint elements against our collection. assert_eq!( @@ -3632,6 +3632,9 @@ mod tests { assert_eq!(blueprint2, blueprint_read); assert_eq!(blueprint2.internal_dns_version, new_internal_dns_version); assert_eq!(blueprint2.external_dns_version, new_external_dns_version); + + let blueprint2_counts = BlueprintTableCounts::new(&datastore, blueprint2.id).await; + cumulative_counts.add(&blueprint2_counts); { let mut expected_ids = [blueprint1.id, blueprint2.id]; expected_ids.sort(); @@ -3723,6 +3726,9 @@ mod tests { .await .expect("failed to read collection back") ); + + let blueprint3_counts = BlueprintTableCounts::new(&datastore, blueprint3.id).await; + cumulative_counts.add(&blueprint3_counts); let bp3_target = BlueprintTarget { target_id: blueprint3.id, enabled: true, @@ -3777,6 +3783,9 @@ mod tests { .await .expect("failed to read collection back") ); + + let blueprint4_counts = BlueprintTableCounts::new(&datastore, blueprint4.id).await; + cumulative_counts.add(&blueprint4_counts); let bp4_target = BlueprintTarget { target_id: blueprint4.id, enabled: true, @@ -3835,6 +3844,9 @@ mod tests { .await .expect("failed to read collection back") ); + + let blueprint5_counts = BlueprintTableCounts::new(&datastore, blueprint5.id).await; + cumulative_counts.add(&blueprint5_counts); let bp5_target = BlueprintTarget { target_id: blueprint5.id, enabled: true, @@ -3863,6 +3875,9 @@ mod tests { .blueprint_insert(&opctx, &blueprint6) .await .expect("failed to insert blueprint"); + + let blueprint6_counts = BlueprintTableCounts::new(&datastore, blueprint6.id).await; + cumulative_counts.add(&blueprint6_counts); let bp6_target = BlueprintTarget { target_id: blueprint6.id, enabled: true, @@ -3875,6 +3890,8 @@ mod tests { datastore.blueprint_delete(&opctx, &authz_blueprint5).await.unwrap(); ensure_blueprint_fully_deleted(&datastore, blueprint5.id).await; + ensure_blueprint_fully_populated(&datastore, &cumulative_counts).await; + // Clean up. db.terminate().await; logctx.cleanup_successful(); @@ -4643,6 +4660,13 @@ mod tests { self.counts.values().all(|&count| count == 0) } + /// Add counts from another BlueprintTableCounts to this one. + fn add(&mut self, other: &BlueprintTableCounts) { + for (table, &count) in &other.counts { + *self.counts.entry(table.clone()).or_insert(0) += count; + } + } + /// Returns a list of table names that are empty. fn empty_tables(&self) -> Vec { self.counts @@ -4715,26 +4739,17 @@ mod tests { } } - // Verify that every blueprint-related table contains ≥1 row for `blueprint_id`. + // Verify that every blueprint-related table contains ≥1 row across test blueprints. // Complements `ensure_blueprint_fully_deleted`. async fn ensure_blueprint_fully_populated( - datastore: &DataStore, - blueprint_id: BlueprintUuid, + _datastore: &DataStore, + cumulative_counts: &BlueprintTableCounts, ) { - let counts = BlueprintTableCounts::new(datastore, blueprint_id).await; - - // Exception tables that may be empty in the representative blueprint: - // - MGS update tables: only populated when blueprint includes firmware - // updates - // - ClickHouse tables: only populated when blueprint includes - // ClickHouse configuration + // Exception tables that may be empty in the test blueprints: + // - ClickHouse tables: only populated when blueprint includes ClickHouse configuration // - debug log for planner reports: only populated when the blueprint // was produced by the planner (test blueprints generally aren't) let exception_tables = [ - "bp_pending_mgs_update_sp", - "bp_pending_mgs_update_rot", - "bp_pending_mgs_update_rot_bootloader", - "bp_pending_mgs_update_host_phase_1", "bp_clickhouse_cluster_config", "bp_clickhouse_keeper_zone_id_to_node_id", "bp_clickhouse_server_zone_id_to_node_id", @@ -4742,7 +4757,7 @@ mod tests { ]; // Check that all non-exception tables have at least one row - let empty_tables = counts.empty_tables(); + let empty_tables = cumulative_counts.empty_tables(); let problematic_tables: Vec<_> = empty_tables .into_iter() .filter(|table| !exception_tables.contains(&table.as_str())) @@ -4750,7 +4765,7 @@ mod tests { if !problematic_tables.is_empty() { panic!( - "Expected tables to be populated for blueprint {blueprint_id}: {:?}\n\n\ + "Expected tables to be populated across test blueprints: {:?}\n\n\ If every blueprint should be expected to have a value in this table, then this is a bug. \ Otherwise, you may need to add a table to the exception list in `ensure_blueprint_fully_populated()`. \ If you do this, please ensure that you add a test to `test_representative_blueprint()` that creates a \ From f5515cb4327e429bfa42bf283e2a0e589c612b8e Mon Sep 17 00:00:00 2001 From: Sara Elzayat Date: Thu, 4 Sep 2025 20:37:08 +0300 Subject: [PATCH 2/7] Fix formatting issues --- .../db-queries/src/db/datastore/deployment.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index 2e3a9dd1d93..279736ea5bd 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -3266,7 +3266,8 @@ mod tests { ); // Start tracking cumulative blueprint table coverage. - let mut cumulative_counts = BlueprintTableCounts::new(&datastore, blueprint1.id).await; + let mut cumulative_counts = + BlueprintTableCounts::new(&datastore, blueprint1.id).await; // Check the number of blueprint elements against our collection. assert_eq!( @@ -3633,7 +3634,8 @@ mod tests { assert_eq!(blueprint2.internal_dns_version, new_internal_dns_version); assert_eq!(blueprint2.external_dns_version, new_external_dns_version); - let blueprint2_counts = BlueprintTableCounts::new(&datastore, blueprint2.id).await; + let blueprint2_counts = + BlueprintTableCounts::new(&datastore, blueprint2.id).await; cumulative_counts.add(&blueprint2_counts); { let mut expected_ids = [blueprint1.id, blueprint2.id]; @@ -3727,7 +3729,8 @@ mod tests { .expect("failed to read collection back") ); - let blueprint3_counts = BlueprintTableCounts::new(&datastore, blueprint3.id).await; + let blueprint3_counts = + BlueprintTableCounts::new(&datastore, blueprint3.id).await; cumulative_counts.add(&blueprint3_counts); let bp3_target = BlueprintTarget { target_id: blueprint3.id, @@ -3784,7 +3787,8 @@ mod tests { .expect("failed to read collection back") ); - let blueprint4_counts = BlueprintTableCounts::new(&datastore, blueprint4.id).await; + let blueprint4_counts = + BlueprintTableCounts::new(&datastore, blueprint4.id).await; cumulative_counts.add(&blueprint4_counts); let bp4_target = BlueprintTarget { target_id: blueprint4.id, @@ -3845,7 +3849,8 @@ mod tests { .expect("failed to read collection back") ); - let blueprint5_counts = BlueprintTableCounts::new(&datastore, blueprint5.id).await; + let blueprint5_counts = + BlueprintTableCounts::new(&datastore, blueprint5.id).await; cumulative_counts.add(&blueprint5_counts); let bp5_target = BlueprintTarget { target_id: blueprint5.id, @@ -3876,7 +3881,8 @@ mod tests { .await .expect("failed to insert blueprint"); - let blueprint6_counts = BlueprintTableCounts::new(&datastore, blueprint6.id).await; + let blueprint6_counts = + BlueprintTableCounts::new(&datastore, blueprint6.id).await; cumulative_counts.add(&blueprint6_counts); let bp6_target = BlueprintTarget { target_id: blueprint6.id, From 75ec0e4f8982ac2893f93a1de1b0afafd62e8e68 Mon Sep 17 00:00:00 2001 From: Sara Elzayat Date: Wed, 1 Oct 2025 22:48:59 +0300 Subject: [PATCH 3/7] Update BlueprintTableCounts doc comment --- nexus/db-queries/src/db/datastore/deployment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index 279736ea5bd..367681a38cf 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -4589,7 +4589,7 @@ mod tests { ); } - /// Counts rows in blueprint-related tables for a specific blueprint ID. + /// Counts rows in blueprint-related tables. /// Used by both `ensure_blueprint_fully_populated` and `ensure_blueprint_fully_deleted`. struct BlueprintTableCounts { counts: BTreeMap, From e47d4d90cfc6fb46300ac2acc3db20af0d1846d0 Mon Sep 17 00:00:00 2001 From: Sara Elzayat Date: Wed, 1 Oct 2025 23:04:16 +0300 Subject: [PATCH 4/7] Add blueprint counter --- nexus/db-queries/src/db/datastore/deployment.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index 367681a38cf..3839f7966ce 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -4593,6 +4593,7 @@ mod tests { /// Used by both `ensure_blueprint_fully_populated` and `ensure_blueprint_fully_deleted`. struct BlueprintTableCounts { counts: BTreeMap, + num_blueprints: usize, } impl BlueprintTableCounts { @@ -4649,7 +4650,7 @@ mod tests { counts.insert(table_name.to_string(), count); } - let table_counts = BlueprintTableCounts { counts }; + let table_counts = BlueprintTableCounts { counts, num_blueprints: 1 }; // Verify no new blueprint tables were added without updating this function if let Err(msg) = @@ -4671,6 +4672,7 @@ mod tests { for (table, &count) in &other.counts { *self.counts.entry(table.clone()).or_insert(0) += count; } + self.num_blueprints += other.num_blueprints; } /// Returns a list of table names that are empty. From 54abd475077372fd87840bbde163acf0d1e7f6fc Mon Sep 17 00:00:00 2001 From: Sara Elzayat Date: Wed, 1 Oct 2025 23:47:10 +0300 Subject: [PATCH 5/7] Refactor ensure_blueprint_fully_populated as method --- .../db-queries/src/db/datastore/deployment.rs | 65 +++++++++---------- 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index 3839f7966ce..056d3db05e1 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -3896,7 +3896,7 @@ mod tests { datastore.blueprint_delete(&opctx, &authz_blueprint5).await.unwrap(); ensure_blueprint_fully_deleted(&datastore, blueprint5.id).await; - ensure_blueprint_fully_populated(&datastore, &cumulative_counts).await; + cumulative_counts.ensure_fully_populated(); // Clean up. db.terminate().await; @@ -4745,41 +4745,38 @@ mod tests { Ok(()) } } - } - // Verify that every blueprint-related table contains ≥1 row across test blueprints. - // Complements `ensure_blueprint_fully_deleted`. - async fn ensure_blueprint_fully_populated( - _datastore: &DataStore, - cumulative_counts: &BlueprintTableCounts, - ) { - // Exception tables that may be empty in the test blueprints: - // - ClickHouse tables: only populated when blueprint includes ClickHouse configuration - // - debug log for planner reports: only populated when the blueprint - // was produced by the planner (test blueprints generally aren't) - let exception_tables = [ - "bp_clickhouse_cluster_config", - "bp_clickhouse_keeper_zone_id_to_node_id", - "bp_clickhouse_server_zone_id_to_node_id", - "debug_log_blueprint_planning", - ]; - - // Check that all non-exception tables have at least one row - let empty_tables = cumulative_counts.empty_tables(); - let problematic_tables: Vec<_> = empty_tables - .into_iter() - .filter(|table| !exception_tables.contains(&table.as_str())) - .collect(); + /// Verify that every blueprint-related table contains ≥1 row across test blueprints. + /// Complements `ensure_blueprint_fully_deleted`. + fn ensure_fully_populated(&self) { + // Exception tables that may be empty in the test blueprints: + // - ClickHouse tables: only populated when blueprint includes ClickHouse configuration + // - debug log for planner reports: only populated when the blueprint + // was produced by the planner (test blueprints generally aren't) + let exception_tables = [ + "bp_clickhouse_cluster_config", + "bp_clickhouse_keeper_zone_id_to_node_id", + "bp_clickhouse_server_zone_id_to_node_id", + "debug_log_blueprint_planning", + ]; - if !problematic_tables.is_empty() { - panic!( - "Expected tables to be populated across test blueprints: {:?}\n\n\ - If every blueprint should be expected to have a value in this table, then this is a bug. \ - Otherwise, you may need to add a table to the exception list in `ensure_blueprint_fully_populated()`. \ - If you do this, please ensure that you add a test to `test_representative_blueprint()` that creates a \ - blueprint that _does_ populate this table and verifies it.", - problematic_tables - ); + // Check that all non-exception tables have at least one row + let empty_tables = self.empty_tables(); + let problematic_tables: Vec<_> = empty_tables + .into_iter() + .filter(|table| !exception_tables.contains(&table.as_str())) + .collect(); + + if !problematic_tables.is_empty() { + panic!( + "Expected tables to be populated across test blueprints: {:?}\n\n\ + If every blueprint should be expected to have a value in this table, then this is a bug. \ + Otherwise, you may need to add a table to the exception list in `ensure_fully_populated()`. \ + If you do this, please ensure that you add a test to `test_representative_blueprint()` that creates a \ + blueprint that _does_ populate this table and verifies it.", + problematic_tables + ); + } } } } From 8bb5e6f6fe2dbaffde7b5ec41e40f80f72898b1b Mon Sep 17 00:00:00 2001 From: Sara Elzayat Date: Wed, 1 Oct 2025 23:57:00 +0300 Subject: [PATCH 6/7] Add ClickHouse config to blueprint6 and remove exception tables --- .../db-queries/src/db/datastore/deployment.rs | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index 056d3db05e1..63065571fd0 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -3866,7 +3866,7 @@ mod tests { // Now make a new blueprint (with no meaningful changes) to ensure we // can delete the last test blueprint we generated above. - let blueprint6 = BlueprintBuilder::new_based_on( + let mut blueprint6 = BlueprintBuilder::new_based_on( &logctx.log, &blueprint5, &planning_input, @@ -3876,6 +3876,23 @@ mod tests { ) .expect("failed to create builder") .build(BlueprintSource::Test); + + // Add ClickHouse configuration to exercise those tables + if let Some((_, sled_config)) = blueprint6.sleds.iter().next() { + if let Some(zone_config) = sled_config.zones.iter().next() { + let zone_id = zone_config.id; + let mut cfg = ClickhouseClusterConfig::new( + format!("cluster-{TEST_NAME}"), + "test-secret".into(), + ); + cfg.max_used_keeper_id = KeeperId::from(1u64); + cfg.max_used_server_id = ServerId::from(1u64); + cfg.keepers.insert(zone_id, KeeperId::from(1u64)); + cfg.servers.insert(zone_id, ServerId::from(1u64)); + blueprint6.clickhouse_cluster_config = Some(cfg); + } + } + datastore .blueprint_insert(&opctx, &blueprint6) .await @@ -4750,13 +4767,9 @@ mod tests { /// Complements `ensure_blueprint_fully_deleted`. fn ensure_fully_populated(&self) { // Exception tables that may be empty in the test blueprints: - // - ClickHouse tables: only populated when blueprint includes ClickHouse configuration // - debug log for planner reports: only populated when the blueprint // was produced by the planner (test blueprints generally aren't) let exception_tables = [ - "bp_clickhouse_cluster_config", - "bp_clickhouse_keeper_zone_id_to_node_id", - "bp_clickhouse_server_zone_id_to_node_id", "debug_log_blueprint_planning", ]; From cf5a733d72a47c81198c54432583b55f41fa812f Mon Sep 17 00:00:00 2001 From: Sara Elzayat Date: Thu, 2 Oct 2025 00:39:27 +0300 Subject: [PATCH 7/7] Run cargo fmt --- nexus/db-queries/src/db/datastore/deployment.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/deployment.rs b/nexus/db-queries/src/db/datastore/deployment.rs index 63065571fd0..b7a1fe89504 100644 --- a/nexus/db-queries/src/db/datastore/deployment.rs +++ b/nexus/db-queries/src/db/datastore/deployment.rs @@ -4667,7 +4667,8 @@ mod tests { counts.insert(table_name.to_string(), count); } - let table_counts = BlueprintTableCounts { counts, num_blueprints: 1 }; + let table_counts = + BlueprintTableCounts { counts, num_blueprints: 1 }; // Verify no new blueprint tables were added without updating this function if let Err(msg) = @@ -4769,9 +4770,7 @@ mod tests { // Exception tables that may be empty in the test blueprints: // - debug log for planner reports: only populated when the blueprint // was produced by the planner (test blueprints generally aren't) - let exception_tables = [ - "debug_log_blueprint_planning", - ]; + let exception_tables = ["debug_log_blueprint_planning"]; // Check that all non-exception tables have at least one row let empty_tables = self.empty_tables();