feat: collect the BMC's own management NIC from Manager EthernetInterfaces by mcanevet · Pull Request #454 · bmc-toolbox/bmclib

mcanevet · 2026-07-09T16:18:12Z

What does this PR implement/change/remove?

device.BMC.NIC was always left nil: bmcAttributes only populated the
BMC's identity/firmware fields (vendor, model, serial, firmware) from the
Manager resource. The BMC's own network interface(s) — the one you actually
connect to for out-of-band management — live under a separate Redfish
subtree, /redfish/v1/Managers/{id}/EthernetInterfaces, distinct from the
host's NICs under ComputerSystem, which collectNICs already handles.

Adds collectBMCNIC, wired into bmcAttributes, to walk the Manager's
EthernetInterfaces and populate device.BMC.NIC. Filters the same
00:00:00:00:00:00 placeholder MAC convention used elsewhere in this
package, falling back to PermanentMACAddress when MACAddress is empty
or the placeholder. Best-effort: does nothing if the Manager has no
EthernetInterfaces link or every interface lacks a usable MAC.

Checklist

Tests added
Similar commits squashed

The HW vendor this change applies to (if applicable)

Generic — Manager.EthernetInterfaces is a standard Redfish resource, not
vendor-specific. Validated end-to-end on Supermicro.

The HW model number, product name this change applies to (if applicable)

Validated on a Supermicro AS-1114S-WN10RT-EU, which exposed two Manager
EthernetInterfaces (a dedicated "ToHost" port and a shared NIC port).

The BMC firmware and/or BIOS versions that this change applies to (if applicable)

Observed on BMC firmware 01.04.04.

What version of tooling - vendor specific or opensource does this change depend on (if applicable)

N/A

Description for changelog/release notes

feat: populate device.BMC.NIC (the BMC's own out-of-band management
network interface) from the Manager resource's EthernetInterfaces,
previously never collected. Distinct from device.NICs, which lists the
host's own network interfaces.

Copilot

Pull request overview

This PR updates the Redfish inventory collection to populate device.BMC.NIC by walking the BMC Manager’s EthernetInterfaces subtree, so the out-of-band management NIC is captured separately from host NICs collected under ComputerSystem.

Changes:

Wire BMC NIC collection into bmcAttributes so device.BMC.NIC is populated from Manager/EthernetInterfaces.
Add collectBMCNIC to translate Manager EthernetInterface data into common.NICPort entries.
Add a unit test and fixtures covering Manager EthernetInterfaces inventory.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/redfishwrapper/inventory.go	Calls into new BMC NIC collection during BMC attribute gathering.
internal/redfishwrapper/inventory_collect.go	Adds `collectBMCNIC` to build `device.BMC.NIC` from Manager EthernetInterfaces.
internal/redfishwrapper/inventory_test.go	Adds a new test asserting `device.BMC.NIC` is populated and host NICs remain unaffected.
internal/redfishwrapper/fixtures/bmc_nic/ethernet_interfaces.json	Adds a fixture for the Manager EthernetInterface collection.
internal/redfishwrapper/fixtures/bmc_nic/ethernet_1.json	Adds a fixture for a Manager EthernetInterface member resource.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

joelrebel

Thanks for this contribution

joelrebel · 2026-07-16T18:50:16Z

@mcanevet this requires the conflicts to be resolved

mcanevet · 2026-07-28T11:38:24Z

@joelrebel rebased

joelrebel · 2026-08-05T05:47:56Z

@mcanevet another conflict here, if you can resolve it please

…faces device.BMC.NIC was always left nil: bmcAttributes only populated the BMC's identity/firmware fields from the Manager resource. The BMC's own network interface(s) live under /redfish/v1/Managers/{id}/EthernetInterfaces, a separate Redfish subtree from the host's NICs (ComputerSystem), which collectNICs already handles. Add collectBMCNIC to walk the Manager's EthernetInterfaces and populate device.BMC.NIC, filtering the same 00:00:00:00:00:00 placeholder MAC convention used elsewhere and falling back to PermanentMACAddress.

mcanevet · 2026-08-05T07:57:59Z

@joelrebel done

joelrebel

Thanks

mergify · 2026-08-06T06:40:51Z

## 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

Copilot AI review requested due to automatic review settings July 9, 2026 16:18

Copilot started reviewing on behalf of mcanevet July 9, 2026 16:18 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread internal/redfishwrapper/inventory.go Outdated

Comment thread internal/redfishwrapper/inventory_collect.go Outdated

Comment thread internal/redfishwrapper/fixtures/bmc_nic/ethernet_1.json Outdated

mcanevet force-pushed the feat/bmc-management-nic branch from f98f719 to c799658 Compare July 9, 2026 16:40

mcanevet mentioned this pull request Jul 10, 2026

Add out-of-band BMC hardware inventory to Hardware status tinkerbell/tinkerbell#844

Merged

6 tasks

joelrebel reviewed Jul 14, 2026

View reviewed changes

Comment thread internal/redfishwrapper/inventory_collect.go

joelrebel previously approved these changes Jul 14, 2026

View reviewed changes

mcanevet dismissed joelrebel’s stale review via c2dba8a July 28, 2026 11:38

mcanevet force-pushed the feat/bmc-management-nic branch from c799658 to c2dba8a Compare July 28, 2026 11:38

mcanevet force-pushed the feat/bmc-management-nic branch from c2dba8a to 88b6b25 Compare July 31, 2026 06:27

mcanevet force-pushed the feat/bmc-management-nic branch from 88b6b25 to b536841 Compare August 5, 2026 07:57

joelrebel approved these changes Aug 6, 2026

View reviewed changes

joelrebel added the ready-to-merge label Aug 6, 2026

mergify Bot added the queued label Aug 6, 2026

mergify Bot merged commit 83f0b7a into bmc-toolbox:main Aug 6, 2026
4 of 5 checks passed

mergify Bot removed the queued label Aug 6, 2026

Navigation Menu

Uh oh!

feat: collect the BMC's own management NIC from Manager EthernetInterfaces - #454

feat: collect the BMC's own management NIC from Manager EthernetInterfaces#454
mergify[bot] merged 1 commit into
bmc-toolbox:mainfrom
mcanevet:feat/bmc-management-nic

mcanevet commented Jul 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joelrebel left a comment

Uh oh!

joelrebel commented Jul 16, 2026

Uh oh!

mcanevet commented Jul 28, 2026

Uh oh!

joelrebel commented Aug 5, 2026

Uh oh!

mcanevet commented Aug 5, 2026

Uh oh!

joelrebel left a comment

Uh oh!

mergify Bot commented Aug 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mcanevet commented Jul 9, 2026

What does this PR implement/change/remove?

Checklist

The HW vendor this change applies to (if applicable)

The HW model number, product name this change applies to (if applicable)

The BMC firmware and/or BIOS versions that this change applies to (if applicable)

What version of tooling - vendor specific or opensource does this change depend on (if applicable)

Description for changelog/release notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joelrebel left a comment

Choose a reason for hiding this comment

Uh oh!

joelrebel commented Jul 16, 2026

Uh oh!

mcanevet commented Jul 28, 2026

Uh oh!

joelrebel commented Aug 5, 2026

Uh oh!

mcanevet commented Aug 5, 2026

Uh oh!

joelrebel left a comment

Choose a reason for hiding this comment

Uh oh!

mergify Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Queue Status

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mergify Bot commented Aug 6, 2026 •

edited

Loading