Skip to content

fix: IPv4 CIDRPool gatewayIndex does not match leaf gateway address - #196

Open
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:bugfix/addressing-ipv4-cidrpool-gatewayindex-does-not
Open

fix: IPv4 CIDRPool gatewayIndex does not match leaf gateway address#196
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:bugfix/addressing-ipv4-cidrpool-gatewayindex-does-not

Conversation

@andrewwhitecdw

@andrewwhitecdw andrewwhitecdw commented Aug 17, 2026

Copy link
Copy Markdown

This PR addresses the following issue in pkg/networkoperatorplugin/spectrumx/addressing.go: IPv4 CIDRPool gatewayIndex does not match leaf gateway address.

Changes

  • pkg/networkoperatorplugin/spectrumx/addressing.go: IPv4 CIDRPool gatewayIndex does not match leaf gateway address.

Details

--- a/pkg/networkoperatorplugin/spectrumx/addressing.go
+++ b/pkg/networkoperatorplugin/spectrumx/addressing.go
@@ -1,5 +1,5 @@
-	return cidrPoolSettings{
-		gatewayIndex:         0,
-		perNodeNetworkPrefix: 31,
-		perNodeExclusions:    []PerNodeExclusion{{StartIndex: 1, EndIndex: 1}},
-	}
+	return cidrPoolSettings{
+		gatewayIndex:         1,
+		perNodeNetworkPrefix: 31,
+		perNodeExclusions:    []PerNodeExclusion{{StartIndex: 1, EndIndex: 1}},
+	}

Tests

  • pkg/networkoperatorplugin/spectrumx/addressing_test.go
--- /dev/null
+++ pkg/networkoperatorplugin/spectrumx/addressing_test.go
@@ -0,0 +1,32 @@
+package spectrumx
+
+import (
+	"testing"
+
+	"github.com/nvidia/k8s-launch-kit/pkg/config"
+)
+
+func TestIPv4PoolSettingsGatewayIndexMatchesLeafOffset(t *testing.T) {
+	spcx := &config.ProfileSpectrumX{}
+	settings := poolSettings(spcx)
+	if settings.perNodeNetworkPrefix != 31 {
+		t.Fatalf("IPv4 perNodeNetworkPrefix = %d, want 31", settings.perNodeNetworkPrefix)
+	}
+	if got, want := settings.gatewayIndex, 1; got != want {
+		t.Errorf("IPv4 gatewayIndex = %d, want %d (leaf/gateway is at offset 1 of each /31 block)", got, want)
+	}
+}
+
+func TestIPv6PoolSettingsGatewayIndexUnchanged(t *testing.T) {
+	spcx := &config.ProfileSpectrumX{IPVersion: config.SpectrumXIPVersionIPv6}
+	settings := poolSettings(spcx)
+	if got, want := settings.gatewayIndex, 2; got != want {
+		t.Errorf("IPv6 gatewayIndex = %d, want %d", got, want)
+	}
+}

Contributor guidelines

Per this repo's CONTRIBUTING.md:

  • All commits are signed off (Signed-off-by trailer, DCO).

Update (post-Greptile review): The existing IPv4 BuildCIDRPools test expectations have been updated to GatewayIndex: 1 to match the implementation change. The Spectrum-X addressing test suite now passes (go test ./pkg/networkoperatorplugin/spectrumx/...). Commit: 7b116ba14dbdeee1a457db743c2c4ce66e473b6b.

@greptile-apps

greptile-apps Bot commented Aug 17, 2026

Copy link
Copy Markdown

Greptile Summary

The PR corrects the Spectrum-X IPv4 CIDRPool gateway offset so it points to the leaf address and updates the affected expectations.

  • Changes the IPv4 gatewayIndex from 0 to 1, matching the leaf offset and per-node exclusion.
  • Updates all three existing IPv4 gateway-index assertions from 0 to 1.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
pkg/networkoperatorplugin/spectrumx/addressing.go Aligns the IPv4 CIDRPool gateway index with the leaf address and the existing exclusion at offset 1.
pkg/networkoperatorplugin/spectrumx/addressing_test.go Updates the three stale IPv4 gateway-index expectations, fully addressing the previous review finding.

Reviews (2): Last reviewed commit: "fix: IPv4 CIDRPool gatewayIndex does not..." | Re-trigger Greptile

}
return cidrPoolSettings{
gatewayIndex: 0,
gatewayIndex: 1,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 IPv4 test expectations remain stale

When the Spectrum-X package tests run, BuildCIDRPools propagates the new gateway index of 1 while three existing IPv4 tests still require 0, causing the test suite to fail. Update those expectations alongside this behavior change.

Knowledge Base Used: Spectrum-X addressing and topology

Signed-off-by: andrewwhitecdw <andrewwhitecdw@users.noreply.github.com>
@andrewwhitecdw
andrewwhitecdw force-pushed the bugfix/addressing-ipv4-cidrpool-gatewayindex-does-not branch from 0ca6401 to 7b116ba Compare August 17, 2026 22:02
@andrewwhitecdw

Copy link
Copy Markdown
Author

@greptile-apps Thanks for the review. I updated the stale IPv4 BuildCIDRPools test expectations to GatewayIndex: 1 so they agree with the implementation. go test ./pkg/networkoperatorplugin/spectrumx/... now passes.

@greptile-apps

greptile-apps Bot commented Aug 17, 2026

Copy link
Copy Markdown

The updates look correct. All three GatewayIndex: 0GatewayIndex: 1 occurrences in the existing BuildCIDRPools tests are now consistent with the fix, and the new unit tests in addressing_test.go directly assert both the IPv4 (gatewayIndex == 1) and IPv6 (gatewayIndex == 2) cases. The PR is in good shape — LGTM.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant