Skip to content

fix: skip hardware fabric default when Spectrum-X forces ethernet - #207

Open
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:bugfix/defaults-skip-hardware-fabric-default-when
Open

fix: skip hardware fabric default when Spectrum-X forces ethernet#207
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:bugfix/defaults-skip-hardware-fabric-default-when

Conversation

@andrewwhitecdw

Copy link
Copy Markdown

This PR addresses the following issue in pkg/resolve/defaults.go: skip hardware fabric default when Spectrum-X forces ethernet.

Changes

  • pkg/resolve/defaults.go: skip hardware fabric default when Spectrum-X forces ethernet.

Details

--- a/pkg/resolve/defaults.go
+++ b/pkg/resolve/defaults.go
@@ -1,3 +1,5 @@
-	// --fabric ----------------------------------------------------------
-	if cfg.Profile.Fabric == "" && opts.Fabric == "" {
-		fabric, ok, reason := dominantLinkType(cfg.ClusterConfig)
+	// --fabric ----------------------------------------------------------
+	// Spectrum-X forces ethernet fabric; skip hardware linkType default
+	// when active so the Spectrum-X branch can default it to ethernet.
+	if cfg.Profile.Fabric == "" && opts.Fabric == "" && !opts.SpectrumX && !cfgHasSpectrumX {
+		fabric, ok, reason := dominantLinkType(cfg.ClusterConfig)

Tests

  • pkg/resolve/defaults_test.go
--- a/pkg/resolve/defaults_test.go
+++ b/pkg/resolve/defaults_test.go
@@ -0,0 +1,66 @@
+func TestApplyHardwareDefaults_SpectrumXSkipsFabricHardwareDefault(t *testing.T) {
+	// An InfiniBand cluster should not leave fabric=infiniband when the
+	// user enabled Spectrum-X on the CLI; the Spectrum-X branch must force
+	// ethernet.
+	cfg := &config.LaunchKitConfig{
+		ClusterConfig: []config.ClusterConfig{
+			{Identifier: "group-1", LinkType: "InfiniBand"},
+		},
+		Profile: &config.Profile{},
+	}
+	opts := options.Options{SpectrumX: true}
+
+	decisions := ApplyHardwareDefaults(cfg, opts)
+
+	if cfg.Profile.Fabric != "ethernet" {
+		t.Errorf("Spectrum-X should force fabric=ethernet, got %q", cfg.Profile.Fabric)
+	}
+
+	found := false
+	for _, d := range decisions {
+		if d.Flag == "--fabric" && d.Value == "ethernet" && d.Reason == "implied by --spectrum-x" {
+			found = true
+		}
+	}
+	if !found {
+		t.Errorf("expected Spectrum-X fabric default decision, got %v", decisions)
+	}
+}
+
+func TestApplyHardwareDefaults_ConfigSpectrumXSkipsFabricHardwareDefault(t *testing.T) {
+	// Spectrum-X can also come from the config file; hardware default must
+	// still be skipped so fabric is forced to ethernet.
+	cfg := &config.LaunchKitConfig{
+		ClusterConfig: []config.ClusterConfig{
+			{Identifier: "group-1", LinkType: "InfiniBand"},
+		},
+		Profile: &config.Profile{
+			SpectrumX: &config.ProfileSpectrumX{Enable: true},
+		},
+	}
+	opts := options.Options{}
+
+	decisions := ApplyHardwareDefaults(cfg, opts)
+
+	if cfg.Profile.Fabric != "ethernet" {
+		t.Errorf("Spectrum-X should force fabric=ethernet, got %q", cfg.Profile.Fabric)
+	}
+
+	found := false
+	for _, d := range decisions {
+		if d.Flag == "--fabric" && d.Value == "ethernet" && d.Reason == "implied by --spectrum-x" {
+			found = true
+		}
+	}
+	if !found {
+		t.Errorf("expected Spectrum-X fabric default decision, got %v", decisions)
+	}
+}

Contributor guidelines

Per this repo's CONTRIBUTING.md:

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

Signed-off-by: andrewwhitecdw <andrewwhitecdw@users.noreply.github.com>
@greptile-apps

greptile-apps Bot commented Aug 17, 2026

Copy link
Copy Markdown

Greptile Summary

This PR prevents hardware discovery from assigning an InfiniBand fabric before the Spectrum-X-specific defaulting branch can force Ethernet.

  • Skips dominantLinkType fabric defaulting when Spectrum-X is enabled through either CLI options or configuration.
  • Preserves the existing non-Spectrum-X hardware-defaulting path.

Confidence Score: 5/5

The PR appears safe to merge because every newly skipped hardware-default path reaches the Spectrum-X branch that assigns Ethernet.

The new condition uses the same Spectrum-X activation signals as the subsequent defaulting branch, while explicit CLI fabric values retain precedence and contradictory values remain subject to validation.

Important Files Changed

Filename Overview
pkg/resolve/defaults.go The updated guard aligns with the downstream Spectrum-X activation condition, which reliably assigns Ethernet while leaving ordinary hardware defaulting unchanged.

Reviews (1): Last reviewed commit: "fix: skip hardware fabric default when S..." | Re-trigger Greptile

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