Planner flip-flops on which zone to update

[plotnick]

In our first test of the blueprint-planner background task of #8287, with blueprint execution disabled, we found on setting the target release that the planner produced two blueprints: the first attempted to expunge an internal-dns zone, and the second to update in-place a crucible zone. But it should instead have waited for the first step to complete (which it wouldn't have without enabling execution).

[jgallagher]

I think we should consider whether a fix for this issue can also cover #8284 and maybe #8285. I'm wondering if we should have ... something (a pattern? something enforced in the type system?) ... that lets steps or phases of the planner block subsequent phases (maybe just "block any subsequent update phases"? this is all not that thought out). Totally spitballing to get the ball rolling, #8269 changes do_plan to look like this:

fn do_plan(&mut self) -> Result<(), Error> {
    self.do_plan_expunge()?;
    self.do_plan_add()?;
    self.do_plan_decommission()?;
    if let UpdateStepResult::ContinueToNextStep =
        self.do_plan_mgs_updates()?
    {
        self.do_plan_zone_updates()?;
    }
    self.do_plan_cockroachdb_settings();
    Ok(())
}

but maybe we want something richer than UpdateStepResult? E.g.,

fn do_plan(&mut self) -> Result<PlanSummary, Error> {
    let expunge_summary = self.do_plan_expunge()?;
    let add_summary = self.do_plan_add()?;
    let decommission_summary = self.do_plan_decommission()?;
    // we might skip MGS updates (e.g., if `add_summary` says we're trying to restore services?)
    let mgs_update_summary = self.do_plan_mgs_updates(
        &expunge_summary,
        &add_summary,
        &decommission_summary,
    )?;
    // we might skip zone updates (e.g., if we're trying to restore services or `mgs_summary` says it's doing an update)
    let zone_update_summary = self.do_plan_zone_updates(
        &expunge_summary,
        &add_summary,
        &decommission_summary,
        &mgs_update_summary,
    )?;
    self.do_plan_cockroachdb_settings();

    // return all phase summaries so we can emit notes in the blueprint
    Ok(PlanSummary {
        expunge_summary,
        add_summary,
        decommission_summary,
        mgs_update_summary,
        zone_update_summary,
    })
}

[davepacheco]

That direction makes sense to me. I wrote up a straw man for this in RFD 565.
https://rfd.shared.oxide.computer/rfd/565#_planner_implementation
that accounts for the sequence and when to bail out early. It definitely might be wrong. I only thought about it, not implemented or modeled it!

It's not (yet) obvious to me that this would benefit particularly from type system support.

[jgallagher]

It's not (yet) obvious to me that this would benefit particularly from type system support.

Yeah, fair. I didn't necessarily mean anything fancy here; in that hypothetical snippet I dropped above, we have some amount of type system support in that calling do_plan_zone_updates at all requires an instance of the *_summary types from the prior phases. That's "some amount of type system support" in that it would be nontrivial to reorder or skip phases.