Skip to content

✨ Add networkRanges to DHCP for multi-subnet support#553

Open
holmboe wants to merge 1 commit intometal3-io:mainfrom
holmboe:dhcp-multi-range
Open

✨ Add networkRanges to DHCP for multi-subnet support#553
holmboe wants to merge 1 commit intometal3-io:mainfrom
holmboe:dhcp-multi-range

Conversation

@holmboe
Copy link
Contributor

@holmboe holmboe commented Feb 17, 2026

Summary

  • Add DHCPRange struct and NetworkRanges []DHCPRange field to the DHCP CRD type
  • Operator concatenates primary range + networkRanges into a single semicolon-separated DHCP_RANGE env var
  • Relax validation so primary flat fields (networkCIDR/rangeBegin/rangeEnd) are optional when networkRanges is provided
  • Each network range is independently validated (CIDR parse, IPs within CIDR)
  • Fully backward compatible: existing CRs with only flat fields continue to work

Companion PR

Test plan

  • make test passes — includes new validation and buildDHCPRange test cases
  • Verify CRD schema includes networkRanges field
  • Apply Ironic CR with networkRanges only (no primary range) — validates new code path
  • Apply Ironic CR with both primary range and networkRanges — verify semicolon-separated DHCP_RANGE
  • Apply Ironic CR with primary range only — verify backward compatibility

Add DHCPRange struct and NetworkRanges field to the DHCP CRD type,
allowing multiple DHCP ranges for subnets served via relay agents.
The operator concatenates all ranges into a semicolon-separated
DHCP_RANGE env var. The primary flat fields remain supported for
backward compatibility.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Henrik Holmboe <henrik@dynamist.se>
@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign elfosardo for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 17, 2026
@metal3-io-bot
Copy link
Contributor

Hi @holmboe. Thanks for your PR.

I'm waiting for a metal3-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@metal3-io-bot metal3-io-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 17, 2026
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ok-to-test

@metal3-io-bot metal3-io-bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 23, 2026
NetworkCIDR string `json:"networkCIDR,omitempty"`

// RangeBegin is the first IP that can be given to hosts. Must be inside NetworkCIDR.
RangeBegin string `json:"rangeBegin,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should we handle these fields? Does it make sense to call them deprecated and recommend using NetworkRanges?

prefix, err := netip.ParsePrefix(dhcp.NetworkCIDR)
if err != nil {
return "" // don't disable your webhooks people
var parts []string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the linter issue

return err
// Validate primary range if present
if dhcp.RangeBegin != "" || dhcp.RangeEnd != "" {
if dhcp.NetworkCIDR == "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you find a way to reuse validateDHCPRange for the old fields? Maybe create a temporary NetworkRange variable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants