[server] restore SMBIOS type 1 input on instance ensure - #1200
[server] restore SMBIOS type 1 input on instance ensure#1200zeeshanlakhani wants to merge 2 commits into
Conversation
The instance spec versioning rework in #1178 moved the API-to-internal conversion into per-version `SpecBuilder` paths, but dropped the assignment of the API spec's `smbios` field to the internal spec's `smbios_type1_input`. As a result, ensure requests with an explicit SMBIOS Type 1 input silently receive Propolis's default Type 1 values (`Oxide`/`OxVM`) instead. This breaks consumers of programmable SMBIOS (#977), like voxel and a4x2, where the SMBIOS manufacturer selects a4x2 hardware and the product and serial values identify the sled's configured `BaseboardId` during bootstrap. This change preserves `smbios` in `latest_to_spec_builder`, the common funnel for the versioned instance-spec conversion paths, and adds a regression test.
|
This looks great and I've verified it works right with voxel. Do we plan on updating the falcon pin from February? I've got some other propolis work I'll be making a PR for soon that would depend on this. |
we should, though not sure what the proper ordering of deps updates should be. |
|
@iximeow thoughts on this btw? |
|
@zeeshanlakhani would you be open to |
|
|
@iximeow @papertigers thoughts on this btw? We've kept the scope small vs what @sion42x was looking at in terms of including a larger portion of the spec. |
iximeow
left a comment
iximeow
left a comment
There was a problem hiding this comment.
sorry about the bug. this makes sense though I'd like to understand how I missed this in the refactor, particularly since there wasn't a set_smbios_type1_input before.
I'll convince myself of what happened there, probably push a commit that adjusts the test, and pull this in for the sake of getting this fixed for you; I realize this is an issue for voxel/a4x2/etc
| smbios: Some(SmbiosType1Input { | ||
| manufacturer: "a4x2".to_string(), | ||
| product_name: "913-0000019".to_string(), | ||
| serial_number: "2FAKE000".to_string(), | ||
| version: 2, | ||
| }), | ||
| }; | ||
|
|
||
| let spec = latest_to_spec_builder(api_spec).unwrap().finish(); | ||
| let smbios = | ||
| spec.smbios_type1_input.expect("SMBIOS type 1 input preserved"); | ||
| assert_eq!(smbios.manufacturer, "a4x2"); | ||
| assert_eq!(smbios.product_name, "913-0000019"); | ||
| assert_eq!(smbios.serial_number, "2FAKE000"); | ||
| assert_eq!(smbios.version, 2); |
There was a problem hiding this comment.
fwiw i'd throw a PartialEq on SmbiosType1Input and just make sure the original input is the same as the one we got back out. mostly so that if someone (hi Steve :) ) does add new type 1 fields later this correctly tests that those fields don't get set to some novel different default or something later on.
There was a problem hiding this comment.
Yep, I can definitely do that.
also get a little more exhaustive about the test conversions and which should succeed, which should fail, and how newer specs should be converted to older.
|
Other than clippy's issues, I like the additional testing here @iximeow. |
3 participants
An ensure request with an explicit SMBIOS Type 1 input silently got Propolis's defaults (
Oxide/OxVM) instead. This issue came up in voxel and a4x2 testing, where the manufacturer string is used for how those infra stacks select hardware, and the product-name and serial are used to carry the sled's configuredBaseboardIdduring the bootstrapping process.We ran into this after #1178 made it to the mainline, which moved API-to-internal conversion into per-version
SpecBuilderpaths and dropped thesmbios->smbios_type1_inputassignment along the way.This PR keeps
smbiosinlatest_to_spec_builder, where all the versioned paths go through.