fix: populate device.Mainboard from Chassis resource data - #452
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a long-standing gap in the Redfish inventory collector where device.Mainboard was never populated, by deriving motherboard identity from compatible Chassis resources (including Supermicro’s OEM extension for board serials) while preventing later chassis entries from overwriting the first valid mainboard.
Changes:
- Add
collectMainboardand wire it intochassisAttributesalongside existing chassis-based collectors. - Extract Supermicro motherboard serial via
Oem.Supermicro.BoardSerialNumber(best-effort; empty if absent/unparseable). - Add unit tests covering population behavior, “do not overwrite” behavior, and OEM serial parsing.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/redfishwrapper/inventory.go | Calls collectMainboard during chassis processing so device.Mainboard is populated during inventory collection. |
| internal/redfishwrapper/inventory_collect.go | Implements collectMainboard plus Supermicro OEM serial extraction helper. |
| internal/redfishwrapper/inventory_collect_test.go | Adds focused tests for mainboard collection, overwrite prevention, and OEM serial parsing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6 tasks
joelrebel
reviewed
Jul 14, 2026
mcanevet
added a commit
to mcanevet/bmclib
that referenced
this pull request
Jul 28, 2026
Redfish inventory never populated device.Mainboard. Some vendors (e.g. Supermicro) don't expose a dedicated motherboard resource: the Chassis resource's Model is the board's own model, while PartNumber/SerialNumber describe the case rather than the board itself. redfishwrapper's collectMainboard stays vendor neutral, per PR bmc-toolbox#452 review feedback. Supermicro's own board serial (from its Chassis OEM extension) is filled in by each provider after the generic inventory collection runs: providers/supermicro and providers/redfish both need it, since the generic redfish provider is tried before the Supermicro-specific one and usually succeeds against Supermicro BMCs too, short-circuiting before the vendor-specific provider ever runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mcanevet
force-pushed
the
fix/mainboard-info
branch
from
July 28, 2026 08:58
bd64668 to
9d3fb56
Compare
Contributor
Author
|
@joelrebel done |
joelrebel
requested changes
Jul 31, 2026
mcanevet
added a commit
to mcanevet/bmclib
that referenced
this pull request
Jul 31, 2026
Redfish inventory never populated device.Mainboard. Some vendors (e.g. Supermicro) don't expose a dedicated motherboard resource: the Chassis resource's Model is the board's own model, while PartNumber/SerialNumber describe the case rather than the board itself. redfishwrapper's collectMainboard stays vendor neutral, per PR bmc-toolbox#452 review feedback. Supermicro's own board serial (from its Chassis OEM extension) is filled in by each provider after the generic inventory collection runs: providers/supermicro and providers/redfish both need it, since the generic redfish provider is tried before the Supermicro-specific one and usually succeeds against Supermicro BMCs too, short-circuiting before the vendor-specific provider ever runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mcanevet
force-pushed
the
fix/mainboard-info
branch
from
July 31, 2026 06:33
9d3fb56 to
712fcab
Compare
Redfish inventory never populated device.Mainboard. Some vendors (e.g. Supermicro) don't expose a dedicated motherboard resource: the Chassis resource's Model is the board's own model, while PartNumber/SerialNumber describe the case rather than the board itself. redfishwrapper's collectMainboard stays vendor neutral, per PR bmc-toolbox#452 review feedback. Supermicro's own board serial (from its Chassis OEM extension) is filled in by each provider after the generic inventory collection runs: providers/supermicro and providers/redfish both need it, since the generic redfish provider is tried before the Supermicro-specific one and usually succeeds against Supermicro BMCs too, short-circuiting before the vendor-specific provider ever runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
populateMainboardSerial in providers/redfish/redfish.go duplicated the identical function already added to providers/supermicro/supermicro.go. The generic redfish provider should stay vendor neutral; Supermicro-specific enrichment belongs only in the Supermicro provider.
joelrebel
force-pushed
the
fix/mainboard-info
branch
from
August 5, 2026 05:41
712fcab to
39bbb95
Compare
joelrebel
approved these changes
Aug 5, 2026
Contributor
Merge Queue Status
This pull request spent 13 seconds in the queue, including 1 second running CI. Required conditions to merge
|
mergify Bot
added a commit
to tinkerbell/tinkerbell
that referenced
this pull request
Aug 10, 2026
## Summary - Adds `Hardware.status.bmcInventory`, populated out-of-band via the BMC (Redfish/IPMI through bmclib), covering BIOS, BMC (incl. its own management NIC), mainboard, CPUs, memory, host NICs, drives, storage controllers, PSUs, TPMs, and GPUs. - Rufio's Machine controller collects this by reusing the BMC connection it already opens for power polling — no second connection. Collection only actually runs once per 24h per Machine (or immediately if a `tinkerbell.org/refresh-inventory: "true"` annotation is set on the Machine), independent of the 3-minute power-poll cadence. - Every list field is sorted by a stable per-component key before being compared/written, so BMCs returning components in non-deterministic order don't cause a spurious status diff (and reconcile loop) every poll. - Status writes go through Server-Side Apply under a dedicated `machine-controller` field manager, so this can be extended independently of other status sub-fields later. - BMC unreachable/unsupported (e.g. IPMI-only hardware, which bmclib can't collect inventory from at all) is treated as a permanent, non-fatal condition: logged, an `InventoryUnreachable` Warning event is emitted, and the Machine's own reconciliation and last-known-good inventory are left untouched. - Every field on the new API types is optional — field coverage varies significantly by BMC vendor/protocol, so an absent field reflects what the BMC reports, not an error. While validating this against real hardware, I found and fixed several upstream bmclib Redfish-collection bugs/gaps that were producing missing or wrong inventory data. Those are submitted separately, upstream, and are independent of this PR (Tinkerbell degrades gracefully to missing data without them, and gets more accurate/complete data once they land): - bmc-toolbox/bmclib#449 — NIC port MAC misattribution when NetworkPort IDs collide across adapters - bmc-toolbox/bmclib#450 — DIMM capacity used the wrong Redfish field (VolatileSizeMiB instead of CapacityMiB) - bmc-toolbox/bmclib#451 — drive collection incorrectly gated on Storage.DrivesCount - bmc-toolbox/bmclib#452 — Mainboard left unpopulated instead of sourced from the Chassis resource - bmc-toolbox/bmclib#453 — BIOS.Vendor left unpopulated instead of sourced from the system manufacturer - bmc-toolbox/bmclib#454 — collect the BMC's own out-of-band management NIC (distinct from host NICs) This is a UI-less, additive change. A follow-up PR surfaces this data in the Hardware detail page. ## Test plan - [x] `make generate manifests` — CRD and deepcopy regenerated, no unrelated diff - [x] Unit tests for `bmcInventoryFromDevice` (full fixture + nil-safety) - [x] `sortDevice` determinism test (two orderings of the same logical inventory produce identical mapped output) - [x] `dueForInventoryRefresh` table test (never collected, stale, fresh, manual-refresh annotation override) - [x] Reconciler-level tests against a fake BMC provider: successful collection, an IPMI-only-style hard error, refresh cadence (3 calls → 1 actual `Inventory()` call), no-linked-Hardware no-op, and an end-to-end smoke test through `Reconcile()` - [x] Verified live against real hardware in my own cluster
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR implement/change/remove?
device.Mainboardwas never populated for any vendor — no code path set it,so it always stayed at
common.NewDevice()'s empty default.Some vendors (e.g. Supermicro) don't expose a dedicated motherboard
resource: on a real Supermicro AS-1114S-WN10RT-EU,
Chassis.Modelis theboard's own model (
H12SSW-NTR) whileChassis.PartNumber/SerialNumberdescribe the case, and the board's own serial number is under the
Supermicro OEM extension (
Oem.Supermicro.BoardSerialNumber), distinctfrom the case serial.
Adds
collectMainboard, wired alongside the existingcollectEnclosure, topopulate
device.Mainboardfrom these fields. Best-effort: falls back to noserial number on chassis without the Supermicro OEM extension, and does
nothing on chassis without a board model at all.
chassisAttributescalls this once per compatibleChassismember, andsome vendors (e.g. Dell) have more than one compatible Chassis ID in
KnownChassisOdataIDs. Sincedevice.Mainboardis a single field, not aslice,
collectMainboardonly populates it from the first chassis thatprovides a model — this prevents an unrelated chassis processed later (e.g.
a drive enclosure/backplane with its own
Modelset) from silentlyoverwriting a correct motherboard entry.
Checklist
The HW vendor this change applies to (if applicable)
Supermicro (data source), but the "don't overwrite" guard is written
generically for any vendor with multiple compatible Chassis resources
(e.g. Dell).
The HW model number, product name this change applies to (if applicable)
Observed on a Supermicro AS-1114S-WN10RT-EU (motherboard: H12SSW-NTR).
The BMC firmware and/or BIOS versions that this change applies to (if applicable)
Observed on BMC firmware 01.04.04, BIOS 2.9.
What version of tooling - vendor specific or opensource does this change depend on (if applicable)
N/A
Description for changelog/release notes