Skip to content

Commit cf4ef41

Browse files
authored
Planner bg task: Save report even if we don't make a new target (#9088)
We talked about putting the background task's report in a watch channel, and we might still want to do that if we decide we want to expose it in the external API. But this change puts it into the _existing_ watch channel that omdb can poke for debugging, and is a really trivial change. With this it should be easy to compare the report of the current target blueprint with the report of running the planner again after it's realized.
1 parent 227d9df commit cf4ef41

File tree

3 files changed

+47
-28
lines changed

3 files changed

+47
-28
lines changed

dev-tools/omdb/src/bin/omdb/nexus.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,14 +1295,20 @@ fn print_task_blueprint_planner(details: &serde_json::Value) {
12951295
BlueprintPlannerStatus::Error(error) => {
12961296
println!(" task did not complete successfully: {error}");
12971297
}
1298-
BlueprintPlannerStatus::Unchanged { parent_blueprint_id } => {
1298+
BlueprintPlannerStatus::Unchanged { parent_blueprint_id, report } => {
12991299
println!(" plan unchanged from parent {parent_blueprint_id}");
1300+
println!("{report}");
13001301
}
1301-
BlueprintPlannerStatus::Planned { parent_blueprint_id, error } => {
1302+
BlueprintPlannerStatus::Planned {
1303+
parent_blueprint_id,
1304+
error,
1305+
report,
1306+
} => {
13021307
println!(
13031308
" planned new blueprint from parent {parent_blueprint_id}, \
13041309
but could not make it the target: {error}"
13051310
);
1311+
println!("{report}");
13061312
}
13071313
BlueprintPlannerStatus::Targeted { blueprint_id, report, .. } => {
13081314
println!(

nexus/src/app/background/tasks/blueprint_planner.rs

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,25 @@ impl BlueprintPlanner {
176176
}
177177
};
178178

179+
// We just ran the planner, so we should always get its report. This
180+
// output is for debugging only, though, so just make an empty one in
181+
// the unreachable arms.
182+
let report = match &blueprint.source {
183+
BlueprintSource::Planner(report) => Arc::clone(report),
184+
BlueprintSource::Rss
185+
| BlueprintSource::PlannerLoadedFromDatabase
186+
| BlueprintSource::ReconfiguratorCliEdit
187+
| BlueprintSource::Test => {
188+
warn!(
189+
&opctx.log,
190+
"ran planner, but got unexpected blueprint source; \
191+
generating an empty planning report";
192+
"source" => ?&blueprint.source,
193+
);
194+
Arc::new(PlanningReport::new())
195+
}
196+
};
197+
179198
// Compare the new blueprint to its parent.
180199
{
181200
let summary = blueprint.diff_since_blueprint(&parent);
@@ -188,6 +207,7 @@ impl BlueprintPlanner {
188207
);
189208
return BlueprintPlannerStatus::Unchanged {
190209
parent_blueprint_id,
210+
report,
191211
};
192212
}
193213
}
@@ -255,30 +275,13 @@ impl BlueprintPlanner {
255275
return BlueprintPlannerStatus::Planned {
256276
parent_blueprint_id,
257277
error: format!("{error}"),
278+
report,
258279
};
259280
}
260281
}
261282

262283
// We have a new target!
263284

264-
// We just ran the planner, so we should always get its report. This
265-
// output is for debugging only, though, so just make an empty one in
266-
// the unreachable arms.
267-
let report = match &blueprint.source {
268-
BlueprintSource::Planner(report) => Arc::clone(report),
269-
BlueprintSource::Rss
270-
| BlueprintSource::PlannerLoadedFromDatabase
271-
| BlueprintSource::ReconfiguratorCliEdit
272-
| BlueprintSource::Test => {
273-
warn!(
274-
&opctx.log,
275-
"ran planner, but got unexpected blueprint source; \
276-
generating an empty planning report";
277-
"source" => ?&blueprint.source,
278-
);
279-
Arc::new(PlanningReport::new())
280-
}
281-
};
282285
self.tx_blueprint.send_replace(Some(Arc::new((target, blueprint))));
283286
BlueprintPlannerStatus::Targeted {
284287
parent_blueprint_id,
@@ -313,6 +316,7 @@ mod test {
313316
use crate::app::background::tasks::blueprint_load::TargetBlueprintLoader;
314317
use crate::app::background::tasks::inventory_collection::InventoryCollector;
315318
use crate::app::{background::Activator, quiesce::NexusQuiesceHandle};
319+
use assert_matches::assert_matches;
316320
use nexus_inventory::now_db_precision;
317321
use nexus_test_utils_macros::nexus_test;
318322
use nexus_types::deployment::{
@@ -420,11 +424,12 @@ mod test {
420424
planner.activate(&opctx).await,
421425
)
422426
.expect("can't re-activate planner");
423-
assert_eq!(
427+
assert_matches!(
424428
status,
425429
BlueprintPlannerStatus::Unchanged {
426-
parent_blueprint_id: blueprint_id,
427-
}
430+
parent_blueprint_id,
431+
report: _,
432+
} if parent_blueprint_id == parent_blueprint_id
428433
);
429434

430435
// Enable execution.
@@ -488,11 +493,12 @@ mod test {
488493
planner.activate(&opctx).await,
489494
)
490495
.expect("can't re-activate planner");
491-
assert_eq!(
496+
assert_matches!(
492497
status,
493498
BlueprintPlannerStatus::Unchanged {
494-
parent_blueprint_id: blueprint_id,
495-
}
499+
parent_blueprint_id,
500+
report: _,
501+
} if parent_blueprint_id == blueprint_id
496502
);
497503
}
498504
}

nexus/types/src/internal_api/background.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -492,11 +492,18 @@ pub enum BlueprintPlannerStatus {
492492

493493
/// Planning produced a blueprint identital to the current target,
494494
/// so we threw it away and did nothing.
495-
Unchanged { parent_blueprint_id: BlueprintUuid },
495+
Unchanged {
496+
parent_blueprint_id: BlueprintUuid,
497+
report: Arc<PlanningReport>,
498+
},
496499

497500
/// Planning produced a new blueprint, but we failed to make it
498501
/// the current target and so deleted it.
499-
Planned { parent_blueprint_id: BlueprintUuid, error: String },
502+
Planned {
503+
parent_blueprint_id: BlueprintUuid,
504+
error: String,
505+
report: Arc<PlanningReport>,
506+
},
500507

501508
/// Planing succeeded, and we saved and made the new blueprint the
502509
/// current target.

0 commit comments

Comments
 (0)