[1/n] [reconfigurator-cli] in cmds-mupdate-update-flow, apply sled agent configs

[sunshowers]

In cmds-mupdate-update-flow.txt, add sled-set omicron-config latest commands before setting the mupdate override field for these sleds. Without these, the simulated sled-agent's reported reconciled generation lags behind the parent blueprint's per-sled generation, which ends up breaking forthcoming work to detect stale inventory collections while initially setting the "will remove mupdate override" field within blueprints.

This test-only PR is separated out into its own commit to try and minimize the diff for the forthcoming functional change.

As a result of this change, within the test, sled 0 now applies a previously-pending zone expungement (internal_dns 427ec88f). As a result, the planner confirms the expungement and adds a replacement zone (cfc72ddd, artifact 2.0.0). But this is actually a bug -- on a sled with a mupdate override, replacement zones should come from the install dataset. See #10420.

[jgallagher]

With apologies - I think these comments all point to possible other problems not related to the generation fix coming in #10409. They can certainly be fixed separately, but wanted to point them out to see if I was misunderstanding them.

[jgallagher]

I'm not sure this is wrong for the purposes of the test, but this combo of Nexus having an artifact image source and other zones being sourced from install-dataset shouldn't be possible in practice, right? Either a mupdate has happened (everything is on install-dataset), or we've recovered (everything is an artifact).

IIUC, the test is wanting to show that any zones that were sourced from an artifact are converted to install-dataset after a mupdate. We only have Nexus here as a shortcut to have a shorter test, right? That's probably worth either a comment noting the weird combo, or we could set all the other zones to artifact too for a more typical-looking inventory?

[sunshowers]

Yeah this is a bit contrived (especially since this is a manual blueprint edit). I think it would be easiest to just add a comment.

[jgallagher]

This isn't a change in this PR, but this description seems to have atrophied. Differences between the description and the actual diff:

• sled 1's host phase 2 contents change from artifact to current contents (this is surprising since it has the mupdate override error?)
• sled 1 has a pending MGS update cleared (also surprising to me, both in that we have 2 pending updates and that we'd clear this, although maybe that's normal if there's a mupdate override error?)
• sled 0 and 2 both also have their measurements changed from unknown to install dataset (seems fine / expected)

[sunshowers]

The code doing this is

omicron/nexus/reconfigurator/planning/src/blueprint_editor/sled_editor.rs

Lines 774 to 813 in 7c6ad41

	(Err(message), bp_override) => {
	// If there's an error obtaining mupdate override information:
	//
	// * Don't set zone image sources to InstallDataset, because
	// Sled Agent *will* run zones out of the install dataset in
	// case of an error, but will not run zones with an artifact
	// source.
	// * Do clear host phase 2 and pending MGS updates to reflect
	// the reality that Sled Agent will not run zones in this
	// case.
	// * Reset measurements to whatever we had from before
	let prev_mgs_update = match pending_mgs_update {
	Entry::Vacant(_) => None,
	Entry::Occupied(entry) => Some(Box::new(entry.remove())),
	};
	let prev_host_phase_2 = self.set_host_phase_2(
	BlueprintHostPhase2DesiredSlots::current_contents(),
	);
	let prev_measurements =
	self.reset_to_parent_blueprint_measurements();
	// Also update the cached value inside `noop_sled_info`.
	if let NoopConvertSledInfoMut::Ok(mut info) = noop_sled_info {
	info.status = NoopConvertSledStatus::Ineligible(
	NoopConvertSledIneligibleReason::MupdateOverrideError {
	message: message.clone(),
	},
	);
	}
	Ok(EnsureMupdateOverrideAction::GetOverrideError {
	message: message.clone(),
	bp_override,
	prev_mgs_update,
	prev_host_phase_2,
	prev_measurements,
	})

. In general, an error in the mupdate override is a pretty exceptional situation and (from my recollection working on this) the bias is towards progressing as little as possible.

[jgallagher]

Is the image source of this addition correct? This sled has a mupdate override in place, which means we don't know what version it's running; if we're adding a zone, should we be sourcing it from the install dataset?

[sunshowers]

Interesting, yeah -- both

omicron/sled-agent/config-reconciler/src/ledger.rs

Lines 455 to 481 in 7c6ad41

	// * If the config has a remove_mupdate_override set, then all zones
	// should have their image source set to InstallDataset.
	if let Some(mupdate_override_id) = new_config.remove_mupdate_override {
	for zone in &new_config.zones {
	match zone.image_source {
	OmicronZoneImageSource::InstallDataset => {}
	OmicronZoneImageSource::Artifact { hash } => {
	validation_errors.push(format!(
	"remove mupdate override \
	set to {mupdate_override_id}, but zone {} \
	has image source Artifact with hash {hash}",
	zone.id,
	));
	}
	}
	}
	if new_config.host_phase_2
	!= HostPhase2DesiredSlots::current_contents()
	{
	validation_errors.push(format!(
	"remove mupdate override set to {mupdate_override_id}, but \
	host phase 2 contents are not set to current: {:#?}",
	new_config.host_phase_2,
	));
	}
	}

and blippy will reject this config. I'll file a followup to fix this.

[jgallagher]

Hmm! I think the second note here is #10420; the first note is the thing we were talking about last week, right? Blippy is overly aggressive about Nexus generation mismatches in cases like this where we've mupdated but haven't yet seen all the sleds in inventory?

[sunshowers]

Yep! Will update the comment to clarify this