[RFD 621 2/n] switch to Tufaceous v2 in Wicket and Installinator by iliana · Pull Request #10755 · oxidecomputer/omicron

iliana · 2026-07-06T22:21:53Z

This replaces Tufaceous v1 with Tufaceous v2 in Wicket, Installinator, and a couple of other crates. Closes #10733.

This PR is independent of (but somewhat useless without) #10676.

Most of the diff is like-for-like replacement but I did do a minor refactor to Installinator that greatly simplifies the code for downloading all of the artifacts listed in the Installinator document.

I manually tested this on berlin using the following procedure:

Merged in [RFD 621 1/n] releng: generate Tufaceous v2 repos #10676, ran cargo xtask releng, and copied both the generated repo.zip and repo-v2.zip to castle.
Started with whatever bits were previously on there (recent main + some commits that didn't touch these paths), and ran a clean slate.
Old Wicket, new Installinator: uploaded repo.zip to sled 16 and installed to sled 14.
New Wicket, new Installinator, v1 repo: uploaded repo.zip to sled 14 and installed to sled 16.
New Wicket, new Installinator, v2 repo: uploaded repo-v2.zip to 16 and installed to sleds 15 and 17.

This testing went well, and everything behaved as expected. However when I went to initialize the control plane I realized that the zone tarball names were wrong in the v2 repo. I fixed that and used Wicket to deploy the new repo-v2.zip to all of the sleds, and the control plane successfully came up.

iliana · 2026-07-06T22:39:37Z

On berlin, uploaded the (v1) repo to Nexus and ran oxide system update recovery-finish; Reconfigurator seems pleased:

{
  "components_by_release_version": {
    "21.0.0-0.local+git46fbd8adcfa": 87
  },
  "suspended": false,
  "target_release": {
    "time_requested": "2026-07-06T22:35:08.640751Z",
    "version": "21.0.0-0.local+git46fbd8adcfa"
  },
  "time_last_step_planned": "2026-07-06T22:38:15.024838Z"
}

update-common is used to validate that repositories generated by releng are usable. we want to keep this code around beyond the release, so it's better to remove update-common deps as we switch things over to tufaceous v2.

…0.wicket

jgallagher

Sorry for the long delay on review - most of my comments are just questions. I'd prefer to get a second look, probably from @sunshowers, time permitting.

jgallagher · 2026-07-27T14:29:14Z

 pub struct InstallinatorImageId {
    pub update_id: MupdateUuid,
+    #[schemars(schema_with = "ArtifactHash::v1_json_schema")]
    pub host_phase_2: ArtifactHash,


I think this needs an explanatory comment. My thought process on seeing a schema_with here is:

Why do we need to give schemars special instructions but not serde?

Does that mean tufaceous_artifact_v2::ArtifactHash isn't wire compatible with tufaceous_artifact::ArtifactHash?

but I feel like I'm missing something.

They are wire-compatible. I replaced serde_human_bytes with https://github.com/oxidecomputer/byte-wrapper in the Tufaceous v2 rewrite, and I also wanted to get rid of the poorly-written OpenAPI schema for ArtifactHash. So now by default, ArtifactHash uses the schema implementation from byte-wrapper which is much clearer.

To avoid unnecessary schema changes in existing uses I added this v1_json_schema method in oxidecomputer/tufaceous#47, which simply returns the old schema. The goal is to remove these where possible as follow-up PRs, doing the type versioning dance and/or teaching our OpenAPI drift tooling that this is an allowable exception to the compatibility rules since they are the same wire format.

I guess I could write a boilerplate comment and put it on all of them? (There's some in the Nexus PR as well.) Not sure how to succinctly write this.

teaching our OpenAPI drift tooling that this is an allowable exception to the compatibility rules since they are the same wire format

I think I'd vote for this - we've done this in the past where similar issues have come up, and it's nice in that it fixes this kind of change for everyone forever.

Do we have any examples of that, and does that change live in Omicron?

The one I'm thinking of was a change in drift (oxidecomputer/drift#20); I'd expect that to be the case most of the time. I'm assuming the differences in the OpenAPI spec are such that we can tell that they are actually equivalent, and it's just drift that doesn't realize it because of the particular shape?

It's somewhat more specific than I think drift should maybe care about, but I'm unsure what our development philosophy is there.

"host_phase_2": { - "format": "hex string (32 bytes)", - "type": "string" + "maxLength": 64, + "minLength": 64, + "pattern": "^[0-9a-fA-F]{64}$", + "type": "string", + "x-rust-type": { + "crate": "byte-wrapper", + "path": "byte_wrapper::HexArray::<32>", + "version": "0.1.0" + } },

Yeah, I do feel like this particular change is out of scope for a deterministic checker like drift (a smarter/ML based checker can figure this out but at the cost of nondeterminism). The new schema is certainly better than the old one, but I don't really know how drift can detect this.

One option here is that the OpenAPI manager gains a way to do a version override where it generates a new document with the same version number, with the knowledge that it is wire-compatible. But, to be candid, I'd be worried about exposing that to most people.

jgallagher · 2026-07-27T14:32:05Z

 #[serde(rename_all = "snake_case")]
 pub enum InstallinatorStepId {
    Download,
-    Format,


Was this unused before? Installinator doesn't touch U.2s, so maybe this was supposed to be for formatting M.2s if needed, but we haven't gotten around to that yet?

It was unused, yeah.

jgallagher · 2026-07-27T14:44:56Z

-    assert_eq!(expected_kinds, kinds, "all expected kinds present");
-    assert_eq!(
-        expected_installable_kinds, installable_kinds,
-        "all expected installable kinds present"


Did we lose a bit of test fidelity with these assertions going away? (Would we notice if the new expected_artifact_ids we get below was missing something?)

In the old code this was essentially testing that the manifest stored in the Omicron repo contained all of these artifacts. In v2, such a test would be ensuring that the list of fake artifacts in the Tufaceous library matches what we expect. The test belongs in Tufaceous, not Omicron, because if you've somehow merged a change in Tufaceous that has an insufficient set of artifacts and only notice once you try to update Omicron, something is wrong.

But if you go try to implement such a test in Tufaceous, you will realize that all you're doing is testing that the list of fake artifacts to add to the repo matches the list of generated fake artifacts. This is borderline tautological so the test doesn't exist there either (borderline because the method that adds a fake OS image generates three artifacts, not one, so maybe I should add some specific test coverage for that).

jgallagher · 2026-07-27T14:57:29Z

+        repo: &Arc<Repository>,
    ) -> Result<StepResult<Option<RotInterrogation>>, UpdateTerminalError> {
-        // We have a known set of bootloader FWID that don't have cabooses.
-        static KNOWN_MISSING_CABOOSE: [&str; 18] = [


I assume removing this is part of removing the TODO make this fatal at some point, and that all of these FWIDs correspond to truly ancient bootloaders we'll never see again?

Correct, this is part of that. I assume if we have any systems in the field with these very ancient bootloaders they will hit a reprogramming station first before being used in a development environment.

(Looking at git blame it seems that the ability to update the ROT bootloader landed in release 9, which was released July 2024.)

…0.wicket

sunshowers

did an initial review pass -- will probably want to do another round of review before accepting

sunshowers · 2026-07-27T21:25:04Z

+                    // We expect there is exactly one host phase 2 image.
+                    let host_phase_2_count = json
+                        .artifacts
+                        .iter()
+                        .filter(|artifact| {
+                            matches!(
+                                artifact.kind,
+                                InstallinatorArtifactKind::HostPhase2
+                            )
+                        })
+                        .count();
+                    ensure!(
+                        host_phase_2_count == 1,
+                        "expected a single host phase 2 \
+                        image but found {host_phase_2_count}"
+                    );


do we want to check here that there's >= 1 control plane image?

sunshowers · 2026-07-27T21:27:11Z

 hickory-resolver.workspace = true
 http.workspace = true
 http-body-util.workspace = true
 hubtools.workspace = true


is hubtools a dead dependency now?

Somehow no. Looks like there's still a call to archive.verify in here that loops through all available artifacts. Taking a look to see if that needs to be fixed...

sunshowers · 2026-07-27T21:27:56Z

-tufaceous-artifact.workspace = true
-tufaceous-lib.workspace = true
+tufaceous-artifact-v2.workspace = true
+tufaceous-v2.workspace = true


Is tufaceous-v2 used in installinator? or just tufaceous-artifact-v2

sunshowers · 2026-07-27T21:28:38Z

    /// Map of artifacts we ingested from the most-recently-uploaded TUF
    /// repository to a list of artifacts we're serving over the bootstrap
    /// network. In some cases the list of artifacts being served will have
    /// length 1 (when we're serving the artifact directly); in other cases the
    /// artifact in the TUF repo contains multiple nested artifacts inside it
    /// (e.g., RoT artifacts contain both A and B images), and we serve the list
    /// of extracted artifacts but not the original combination.
    ///
    /// Conceptually, this is a `BTreeMap<ArtifactId, Vec<ArtifactHashId>>`, but
    /// JSON requires string keys for maps, so we give back a vec of pairs
    /// instead.


I think this comment is now outdated.

sunshowers · 2026-07-27T21:29:17Z

+#[derive(
+    Debug,
+    Clone,
+    PartialEq,
+    Eq,
+    PartialOrd,
+    Ord,
+    Deserialize,
+    Serialize,
+    JsonSchema,
+)]
+pub struct ArtifactId {
+    pub tags: BTreeMap<String, String>,
+    pub version: ArtifactVersion,
+}


Worth adding doc comments?

sunshowers · 2026-07-27T22:08:27Z

+                    KnownArtifactTags::Rot(RotTags {
+                        rot_rkth: Some(RotKeyTableHash(rot_rkth)),
+                        ..
+                    }) if rot_rkth == &sign => versions.first(),
+                    _ => None,


same here, should this check for the rot_board and/or rot_slot to do proper dedup?

sunshowers · 2026-07-27T22:10:34Z

+            bail!(
+                "downloaded {} checksum failure: \
+                 expected {} but calculated {}",
+                id.kind,
+                hex::encode(&id.hash),
+                hex::encode(&computed_hash)
+            );


would be good to also log the file name here, since id.kind will just be "zone" for all control plane zones.

sunshowers · 2026-07-27T22:20:12Z

+            InstallinatorArtifactKind::MeasurementCorpus => {
+                "measurement_corpus".into()
+            }
+            InstallinatorArtifactKind::ControlPlane => unreachable!(),


is this actually unreachable? if so, why? worth adding a string here

sunshowers · 2026-07-27T22:24:08Z

-    ) -> Result<&'a ArtifactIdData, UpdateTerminalError> {
+    /// Via `client`, ask the target RoT for its CMPA page, then extract
+    /// and return the RKTH.
+    async fn get_rot_rkth(&self) -> Result<[u8; 32], UpdateTerminalError> {


gosh I really feel like we should use a newtype here rather than [u8; 32]

sunshowers · 2026-07-27T22:25:07Z

+                            }
+                            InstallinatorArtifactKind::HostPhase2 => {
+                                let id = MupdateOverrideHashId {
+                                    kind: "host_phase2".into(),


this used to be host_phase_2 -- worth matching that?

tufaceous v2 for wicket and installinator

26b79ec

iliana requested a review from jgallagher July 6, 2026 22:21

iliana changed the title ~~[RFD 621 2/n] switch to tufaceous v2 in wicket and installinator~~ [RFD 621 2/n] switch to Tufaceous v2 in Wicket and Installinator Jul 6, 2026

iliana added 2 commits July 7, 2026 08:52

avoid adding ErrorExt to update-common for now

bbf290e

update-common is used to validate that repositories generated by releng are usable. we want to keep this code around beyond the release, so it's better to remove update-common deps as we switch things over to tufaceous v2.

nit

1a3d052

iliana mentioned this pull request Jul 9, 2026

Add versioned API for rack commissioning (RFD 710) #10776

Closed

iliana added 2 commits July 9, 2026 09:25

Merge remote-tracking branch 'origin/main' into iliana/tufaceous-v2/2…

ec674b5

…0.wicket

update tufaceous

e9fd524

iliana requested a review from sunshowers July 9, 2026 20:38

iliana added 2 commits July 14, 2026 22:04

Merge remote-tracking branch 'origin/main' into iliana/tufaceous-v2/2…

91c8c6e

…0.wicket

Merge remote-tracking branch 'origin/main' into iliana/tufaceous-v2/2…

9346622

…0.wicket

iliana mentioned this pull request Jul 16, 2026

[RFD 621 3/n] switch to Tufaceous v2 in Nexus and Sled Agent #10774

Open

iliana added 4 commits July 20, 2026 10:38

Merge remote-tracking branch 'origin/main' into iliana/tufaceous-v2/2…

c07f141

…0.wicket

serve the v1 installinator document if present

7e63b23

tufaceous#51 is merged

4c96bef

Merge remote-tracking branch 'origin/main' into iliana/tufaceous-v2/2…

fbc3d08

…0.wicket

jgallagher reviewed Jul 27, 2026

View reviewed changes

Comment thread installinator/src/dispatch.rs

Comment thread wicket/src/ui/panes/update.rs Outdated

iliana added 6 commits July 27, 2026 11:32

Merge remote-tracking branch 'origin/main' into iliana/tufaceous-v2/2…

ac0c7d1

…0.wicket

flip

eef2c3d

remove mostly-unused field

b69d99b

we know the host OS version we're updating to

5424f49

comment on the kind values for older wicket

e2c7c78

ensure we extract some zones from the tarball

8a917b6

sunshowers reviewed Jul 27, 2026

View reviewed changes

Uh oh!

Conversation

iliana commented Jul 6, 2026

Uh oh!

iliana commented Jul 6, 2026

Uh oh!

jgallagher 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!

sunshowers Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

iliana Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Uh oh!

sunshowers 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sunshowers Jul 27, 2026 •

edited

Loading

iliana Jul 27, 2026 •

edited

Loading