Make wicketd + installinator cosmo aware #9038

labbott · 2025-09-17T16:35:29Z

This is the minimum set of changes to be able to update the next generation sled Cosmo. The most intrusive change is choosing a separate host phase 1 artifact.

This change does not attempt to differentiate between a Gimlet and a Cosmo at the inventory level which will need to happen separately. This change also does not support updating cosmos via reconfigurator driven update.

labbott · 2025-09-17T16:39:36Z

update-common/src/artifacts/update_plan.rs

+        // We expect to record a `KnownArtifactKind::Host`
+        // but we also need to ensure we end up with a unique
+        // entry of (name, version, ArtifactKind::HOST_PHASE_1)
+        // for use in the database
+        let gimlet_unique_artifact_id = ArtifactId {
+            name: format!("{}-gimlet", artifact_id.name),
+            version: artifact_id.version.clone(),
+            kind: KnownArtifactKind::Host.into(),
+        };
+
+        let cosmo_unique_artifact_id = ArtifactId {
+            name: format!("{}-cosmo", artifact_id.name),
+            version: artifact_id.version.clone(),
+            kind: KnownArtifactKind::Host.into(),
+        };
+


This is one of the weirder edge cases I needed to handle. I went back and forth about adding ArtifactKind::COSMO_HOST_PHASE_1 and ArtifactKind::GIMLET_HOST_PHASE_1 but decided against it and used the existing board entry to differentiate between the two artifacts like we do for SP images.

For compatibility with reconfigurator, I think it would be nicer to have separate ArtifactKind for each of the sled types. If we differentiate via board, the code in the reconfigurator planner would be a bit awkward.

Okay this is enough encouragement to get me to do it. It also means we shouldn't need the extra board information floating around.

This was easier than I expected and it seems much cleaner! Thanks for the nudge to do this.

Cargo.toml

karencfv

So nice to see the cosmo integration work reaching this part of the system!

nexus/reconfigurator/planning/src/mgs_updates/host_phase_1.rs

karencfv · 2025-09-17T22:31:12Z

update-common/src/artifacts/update_plan.rs

-        if self.host_phase_1.is_some() || self.host_phase_2_hash.is_some() {
+        if self.cosmo_host_phase_1.is_some()
+            || self.gimlet_host_phase_1.is_some()
+            || self.host_phase_2_hash.is_some()


Just for my own understanding. Host phase 2 artifacts will always be the same regardless of the type of sled, yes? Do you have a link to an RFD, or somewhere where I can read more on how this works?

@karencfv that's correct: the Phase 2 image is common across sled types. Phase 1 differs only because we have different firmware blobs and things (from AMD) that we vary across microarchitectures (Milan vs Turin) that need to be loaded by the PSP; the x86 code (phbl and the OS bits) are identical, however.

yeah I don't think I know an RFD off the top of my head

The split is discussed in RFD 241. RFD 281 discusses the split, but doesn't go into much detail. RFD 284 discusses the phase 1 loading process in detail.

To my knowledge, no RFD has been updated to refer to the board-specific ROM images that are now created during image generation.

Thanks for the explanation!

karencfv · 2025-09-17T22:39:25Z

update-common/src/artifacts/update_plan.rs

+        // We expect to record a `KnownArtifactKind::Host`
+        // but we also need to ensure we end up with a unique
+        // entry of (name, version, ArtifactKind::HOST_PHASE_1)
+        // for use in the database
+        let gimlet_unique_artifact_id = ArtifactId {
+            name: format!("{}-gimlet", artifact_id.name),
+            version: artifact_id.version.clone(),
+            kind: KnownArtifactKind::Host.into(),
+        };
+
+        let cosmo_unique_artifact_id = ArtifactId {
+            name: format!("{}-cosmo", artifact_id.name),
+            version: artifact_id.version.clone(),
+            kind: KnownArtifactKind::Host.into(),
+        };
+


For compatibility with reconfigurator, I think it would be nicer to have separate ArtifactKind for each of the sled types. If we differentiate via board, the code in the reconfigurator planner would be a bit awkward.

wicket-common/src/update_events.rs

sled-hardware/src/illumos/mod.rs

wicketd/src/update_tracker.rs

jgallagher · 2025-09-19T15:44:50Z

wicketd/src/update_tracker.rs

            update_cx,
            &mut host_registrar,
            plan,
+            host_type.clone(),


I think if we reuse OxideSled that Dan added to sled-hardware, we can drop these .clone() too (since it's Copy).

It turns out we can't drop all the clone because the are wrapped in a StepResult

jgallagher · 2025-09-19T15:50:34Z

update-common/src/artifacts/update_plan.rs

+        (
+            ExtractedArtifactDataHandle,
+            ExtractedArtifactDataHandle,
+            ExtractedArtifactDataHandle,


Could we replace this tuple with a struct to give these names, and the same for a struct containing three &mut HashingNamedUtf8TempFiles for the args to F? (Maybe this also bleeds into the tufaceous API?)

Tuples where all the values are the same type make me nervous in terms of ordering - all the callers just have to get it right, and there isn't any compiler help since swapping any of them still typechecks.

I tweaked the tufaceous API to also handle this and I do think it's cleaner

jgallagher · 2025-09-19T15:57:06Z

update-common/src/artifacts/update_plan.rs

+        let gimlet_phase_1_id = ArtifactId {
+            name: format!("{}-gimlet", artifact_id.name),
+            version: artifact_id.version.clone(),
+            kind: ArtifactKind::TRAMPOLINE_PHASE_1,


Should we have separate GIMLET and COSMO flavors of TRAMPOLINE_PHASE_1? I don't think we want the name to be the only way to distinguish them. (Maybe this doesn't matter for wicket, since it has access to the named fields here, and doesn't yet matter for Nexus, since it doesn't kick off mupdates. But that last bit will change eventually.)

Good catch, I can add that as well. I think the lack of Nexus work is why I didn't catch this sooner.

labbott · 2025-09-19T18:51:58Z

update-common/src/artifacts/update_plan.rs

+        (
+            ExtractedArtifactDataHandle,
+            ExtractedArtifactDataHandle,
+            ExtractedArtifactDataHandle,


I tweaked the tufaceous API to also handle this and I do think it's cleaner

labbott · 2025-09-19T18:52:58Z

wicketd/src/update_tracker.rs

            update_cx,
            &mut host_registrar,
            plan,
+            host_type.clone(),


It turns out we can't drop all the clone because the are wrapped in a StepResult

labbott · 2025-09-19T18:55:37Z

update-common/src/artifacts/update_plan.rs

+struct HostArtifactSource<'a> {
+    gimlet_host_phase_1: &'a mut HashingNamedUtf8TempFile,
+    cosmo_host_phase_1: &'a mut HashingNamedUtf8TempFile,
+    phase_2: &'a mut HashingNamedUtf8TempFile,
+}


It turned out to be easier to have a separate struct vs just using the HostPhaseImageSource from tufaceous directly

jgallagher

🚀

dancrossnyc · 2025-09-20T11:39:21Z

nexus/reconfigurator/planning/src/mgs_updates/host_phase_1.rs

-        if artifact.id.kind == ArtifactKind::HOST_PHASE_1 {
+        // TODO-correctness we only support gimlet at the moment, need
+        // to tell if this target is a gimlet or a comso
+        if artifact.id.kind == ArtifactKind::GIMLET_HOST_PHASE_1 {


I wonder if we could also test for ArtifactKind::COSMO_HOST_PHASE_1 and push into phase_1_artifacts here? Perhaps they're not being generated yet, and perhaps there's a further reason I'm not aware of for not doing so?

I also vaguely wonder whether a match on artifact.id.kind might be more expressive than an if ... else if ... chain.

That would be the next step of work. I purposely didn't want to plumb through the cosmo portions because I wasn't 100% sure how we wanted to differentiate between cosmo/gimlet at the reconfigurator level.

This is the minimum set of changes to be able to update the next generation sled Cosmo. The most intrusive change is choosing a separate host phase 1 artifact. This change does not attempt to differentiate between a Gimlet and a Cosmo at the inventory level or ignition level which will need to happen later via API changes.

labbott commented Sep 17, 2025

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

karencfv reviewed Sep 17, 2025

View reviewed changes

dancrossnyc reviewed Sep 18, 2025

View reviewed changes

sled-hardware/src/illumos/mod.rs Outdated Show resolved Hide resolved

labbott force-pushed the labbott/cosmo_awareness branch from c8a2cd1 to bbcb178 Compare September 19, 2025 15:16

dancrossnyc reviewed Sep 19, 2025

View reviewed changes

wicketd/src/update_tracker.rs Outdated Show resolved Hide resolved

jgallagher reviewed Sep 19, 2025

View reviewed changes

labbott commented Sep 19, 2025

View reviewed changes

jgallagher approved these changes Sep 19, 2025

View reviewed changes

labbott force-pushed the labbott/cosmo_awareness branch 2 times, most recently from 86d2762 to ce95d1d Compare September 20, 2025 00:04

dancrossnyc reviewed Sep 20, 2025

View reviewed changes

labbott force-pushed the labbott/cosmo_awareness branch 2 times, most recently from c0209ca to 066fb48 Compare September 20, 2025 23:23

labbott merged commit 9a946c0 into main Sep 21, 2025
18 checks passed

labbott deleted the labbott/cosmo_awareness branch September 21, 2025 14:31

labbott mentioned this pull request Oct 6, 2025

MUPdate support for cosmo #8623

Closed

Make wicketd + installinator cosmo aware #9038

Make wicketd + installinator cosmo aware #9038

Uh oh!

Conversation

labbott commented Sep 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

karencfv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgallagher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants