You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
metalman: declarative bond and VLAN host networking for VirtualMedia installs - #1109
Adds an optional network block to Machine netboot specs so Metalman can provision hosts whose only uplink is an LACP bond, with tagged VLANs, static routes, and DNS. When it is set, VirtualMedia boots a local installer from the ISO instead of fetching GRUB config over a single NIC. The installer brings up the bond and VLANs, then fetches its install instructions from a new /bootmedia//install endpoint, which uses the same operation and source-IP checks as the existing media callbacks. The same config is written to the installed host as netplan or systemd-networkd files. Machines without network keep the existing single-NIC flow.
The bond-member and VLAN-link rules on HostNetworkSpec exceeded the API
server CEL cost budget in the MachineOperation CRD, where PXESpec is
embedded in the unbounded status.targets list, so the CRD was rejected.
Declare them on MachineSpec (spec.pxe) and HostSpec (spec.host.netboot)
instead, and bound interface names with maxLength 15.
Evaluate the Machine and MachineOperation CRDs with the apiextensions
schema validator, CEL validator, and structural defaulting, covering the
reference bond/VLAN topology and each rejection rule.
…etplan
When the frozen netboot spec declares a host network, build an ISO that
carries the installer and media.cpio and boots it locally, serve KEY=VALUE
install instructions at /bootmedia/<id>/install with the grub.cfg
authorization, and render the declared netplan into NoCloud network-config.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Validation permits VLAN MTUs above the effective parent MTU and address ordering that selects a non-lease provisioning source, causing installation to fail before instruction retrieval or receive a 404 from /install.
Adds declarative host networking to Metalman’s VirtualMedia provisioning flow, enabling installation over bond and VLAN uplinks without requiring firmware-level networking. Machines without a network block retain the existing single-NIC flow.
Changes:
Adds network API types, validation, generated schemas, and configuration examples.
Packages local installer media and provides an operation-authorized /install endpoint.
Renders runtime and persistent network configuration, with tests for validation, rendering, archives, and installer behavior.
File
Description
internal/metalman/netboot/oci_cache.go
Adds local-installer capability metadata.
internal/metalman/netboot/netboot.go
Exposes declared networking to templates.
internal/metalman/netboot/media.go
Builds local installer ISOs with network configuration.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
A transient MachineOperation list failure still returns a permanent 404 instead of a retryable 503, aborting valid local boots, and the failing-List path lacks test coverage.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Return 503 for transient callback owner lookup failures
internal/metalman/netboot/media_http.go:388
A transient MachineOperation list failure returns 404 from ServeHTTP's initial callbackOwner lookup (media_http.go:367-374), before this install handler can run. The installer treats 404 as permanent, so a temporary API outage aborts an otherwise valid local boot instead of retrying. Return 503 for errMediaLookup in that initial branch too, and add a failing-List case alongside the existing failing-Get test.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
A critical VLAN name-matching bug lets valid configurations pass topology validation but causes the installer to reject distinct names as duplicates and stop before fetching install instructions.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Use fixed-string matching for declared network names
internal/metalman/hostnetwork/runtime.go:51
Declaring VLAN e0-64 (ID 64) before e0.64 (ID 65) on the same ethernet passes topology validation, but the installer rejects the second name as a duplicate and stops before fetching install instructions. net_declared in images/netboot/assets/init:230 uses grep -qx, which treats the dot as a wildcard matching the hyphen. Change that lookup to grep -Fqx and add a regression case for these distinct names.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed to evaluate reserved interface names causing installer failures and large media payloads potentially exhausting controller memory.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The change spans signed-GRUB boot and persistent networking on amd64/arm64 UEFI, while mocked tests leave bootability and bond/VLAN connectivity unverified without representative VM or hardware validation.
When UEFI firmware boots the bootstrap ISO as a CD-ROM, Ubuntu GRUB roots
itself at (cd0)/grub on the ISO9660 file system rather than the FAT EFI
image, so it found no config and stopped at the grub> prompt. Copy the same
config to the GRUB prefix paths in the ISO9660 tree as well. Found by
booting the local installer ISO in QEMU with OVMF.
The declared bond/VLAN network is authoritative and never runs DHCP, so
dhcpLeases[0].holdLease would be silently ignored. Reject it in CEL and in
hostnetwork.Validate, and document the rule.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The cross-layer boot and networking changes need representative amd64/arm64 runtime validation, since mocked tests do not establish local-media boot, real LACP/VLAN activation, or post-install connectivity.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The firmware/GRUB boot and bond/VLAN networking changes require representative end-to-end boot and first-reboot validation beyond the existing rendering, packaging, and mocked-command tests.
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
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.
Adds an optional
networkblock to Machine netboot specs so Metalman can provision hosts whose only uplink is an LACP bond, with tagged VLANs, static routes, and DNS. When it is set, VirtualMedia boots a local installer from the ISO instead of fetching GRUB config over a single NIC. The installer brings up the bond and VLANs, then fetches its install instructions from a new /bootmedia//install endpoint, which uses the same operation and source-IP checks as the existing media callbacks. The same config is written to the installed host as netplan or systemd-networkd files. Machines withoutnetworkkeep the existing single-NIC flow.