From 48b262b17c3ac622858437aa113cf045972fd4a5 Mon Sep 17 00:00:00 2001 From: Alexander Maslennikov Date: Fri, 31 Jul 2026 15:00:25 +0200 Subject: [PATCH] Improve Spectrum-X CIDRPool diagnostics Signed-off-by: Alexander Maslennikov --- README.md | 6 + docs/user/spectrum-x.md | 22 ++ .../spectrumx/addressing.go | 188 +++++++++++++++++- .../spectrumx/addressing_test.go | 132 ++++++++++++ pkg/networkoperatorplugin/templates.go | 26 ++- pkg/networkoperatorplugin/templates_test.go | 26 +++ skills/k8s-launch-kit-generate/SKILL.md | 6 + 7 files changed, 394 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 1f73add..a0dd729 100644 --- a/README.md +++ b/README.md @@ -736,6 +736,12 @@ for 2-tier IPv4 allocation and `10` for 3-tier IPv4 allocation. `ipVersion: ipv6` is accepted in the config and CLI, but Spectrum-X CIDRPool rendering currently supports IPv4 static allocations only. +CIDRPool allocation requires exact, case-sensitive equality between selected +`clusterConfig.workerNodes` values and topology host endpoint `node` values. +Generation errors summarize both name sets and identify missing rail/plane +coverage, including likely case or short-name/FQDN mismatches. See the +[Spectrum-X troubleshooting guidance](docs/user/spectrum-x.md#troubleshooting-cidrpool-allocation-errors). + For non-Spectrum-X profiles, leaving both the flag and `selectedRelease` empty continues to render the newest gates (treated as "latest"). diff --git a/docs/user/spectrum-x.md b/docs/user/spectrum-x.md index c5857c3..6ebaccc 100644 --- a/docs/user/spectrum-x.md +++ b/docs/user/spectrum-x.md @@ -172,6 +172,28 @@ l8k generate \ IPv4 CIDRPool rendering is supported. IPv6 is accepted in config for forward compatibility but is not rendered into CIDRPools yet. +### Troubleshooting CIDRPool allocation errors + +CIDRPool generation matches every selected `clusterConfig.workerNodes` value +against topology host endpoint `node` values using an exact, case-sensitive +comparison. l8k does not automatically equate short names with FQDNs because +that could allocate an address to the wrong node. + +When no workers match, the error reports the topology file path and a sorted +summary of selected workers, topology hosts, exact matches, missing workers, +and topology-only hosts. It also calls out likely case or short-name/FQDN +mismatches. If no names are similar, verify that `--topology-file` points to +the topology export for the cluster represented by `cluster-config.yaml`. + +When a worker matches by name but is absent from a generated pool, the error +reports that worker's available rail/plane coverage. Check every host +endpoint's `attributes.rail`; for `swplb`, also check the connected leaf +endpoint's `attributes.plane`. Each selected worker must have a link for every +rail, and for every rail/plane combination in `swplb` mode. + +Large node lists are sorted and limited to the first eight entries, followed +by a `(+N more)` count, so generation failures remain readable. + ## DRA Workload Allocation For RA2.2 and RA2.3, set `profile.spectrumX.useDRA: true` to render ResourceClaimTemplate-based workload allocation. diff --git a/pkg/networkoperatorplugin/spectrumx/addressing.go b/pkg/networkoperatorplugin/spectrumx/addressing.go index 34e6edc..2d4bfea 100644 --- a/pkg/networkoperatorplugin/spectrumx/addressing.go +++ b/pkg/networkoperatorplugin/spectrumx/addressing.go @@ -146,6 +146,16 @@ type poolKey struct { plane int } +const diagnosticListLimit = 8 + +type topologyMatchSummary struct { + selected []string + topology []string + matched []string + missing []string + topologyOnly []string +} + // BuildCIDRPools builds nv-ipam CIDRPool data for the Spectrum-X profiles from // a topology.json that follows the reference generator or NVIDIA AIR schema. func BuildCIDRPools(cfg *config.LaunchKitConfig, group config.ClusterConfig) ([]CIDRPool, error) { @@ -177,19 +187,19 @@ func BuildCIDRPools(cfg *config.LaunchKitConfig, group config.ClusterConfig) ([] allocations := allocationsByPool(links, allowedNodes, spcx) poolKeys := sortedPoolKeys(allocations) if len(poolKeys) == 0 { - return nil, fmt.Errorf("no Spectrum-X topology allocations matched clusterConfig group %q", group.Identifier) + summary := summarizeTopologyMatches(links, allowedNodes) + return nil, fmt.Errorf("no Spectrum-X topology allocations matched %s in topology file %q: %s; %s", + clusterConfigGroupLabel(group), topologyPath, formatTopologyMatchSummary(summary), topologyMismatchHint(summary)) } pools := make([]CIDRPool, 0, len(poolKeys)) + coverage := topologyCoverage(links, spcx) for _, key := range poolKeys { staticAllocations := allocations[key] - if len(staticAllocations) == 0 { - return nil, fmt.Errorf("CIDR pool %s has no host static allocations in topology file %s", - poolName(key, group.MergedIdentifier, spcx), topologyPath) - } if len(allowedNodes) > 0 { if missing := missingNodes(allowedNodes, staticAllocations); len(missing) > 0 { - return nil, fmt.Errorf("CIDR pool %s is missing topology allocations for worker nodes: %s", - poolName(key, group.MergedIdentifier, spcx), strings.Join(missing, ", ")) + return nil, fmt.Errorf("CIDRPool %s for %s is missing topology allocations for workers %s in topology file %q: %s; %s", + poolName(key, group.MergedIdentifier, spcx), clusterConfigGroupLabel(group), formatLimitedList(missing), + topologyPath, formatMissingCoverage(missing, coverage, spcx), topologyAttributeHint(spcx)) } } cidr := poolCIDR(staticAllocations[0].Prefix, spcx) @@ -401,10 +411,7 @@ func allocationsByPool(links []hostLink, allowedNodes map[string]struct{}, spcx continue } } - key := poolKey{rail: link.rail} - if spcx.MultiplaneMode == "swplb" { - key.plane = link.plane - } + key := poolKeyForLink(link, spcx) if seen[key] == nil { seen[key] = map[string]struct{}{} } @@ -421,6 +428,14 @@ func allocationsByPool(links []hostLink, allowedNodes map[string]struct{}, spcx return result } +func poolKeyForLink(link hostLink, spcx *config.ProfileSpectrumX) poolKey { + key := poolKey{rail: link.rail} + if spcx.MultiplaneMode == "swplb" { + key.plane = link.plane + } + return key +} + func sortedPoolKeys(allocations map[poolKey][]StaticAllocation) []poolKey { keys := make([]poolKey, 0, len(allocations)) for key := range allocations { @@ -515,3 +530,154 @@ func missingNodes(nodes map[string]struct{}, allocations []StaticAllocation) []s sort.Strings(missing) return missing } + +func clusterConfigGroupLabel(group config.ClusterConfig) string { + if group.Identifier != "" { + return fmt.Sprintf("clusterConfig group %q", group.Identifier) + } + return "unnamed clusterConfig group" +} + +func summarizeTopologyMatches(links []hostLink, selected map[string]struct{}) topologyMatchSummary { + topology := map[string]struct{}{} + for _, link := range links { + topology[link.node] = struct{}{} + } + + summary := topologyMatchSummary{ + selected: sortedSet(selected), + topology: sortedSet(topology), + } + for _, node := range summary.selected { + if _, ok := topology[node]; ok { + summary.matched = append(summary.matched, node) + } else { + summary.missing = append(summary.missing, node) + } + } + for _, node := range summary.topology { + if _, ok := selected[node]; !ok { + summary.topologyOnly = append(summary.topologyOnly, node) + } + } + return summary +} + +func formatTopologyMatchSummary(summary topologyMatchSummary) string { + return fmt.Sprintf("topology match summary: selected workers=%s, topology host nodes=%s, exact matches=%s, missing workers=%s, topology-only hosts=%s", + formatLimitedList(summary.selected), formatLimitedList(summary.topology), formatLimitedList(summary.matched), + formatLimitedList(summary.missing), formatLimitedList(summary.topologyOnly)) +} + +func topologyMismatchHint(summary topologyMatchSummary) string { + if selected, topology, ok := similarNodePair(summary.selected, summary.topology, strings.EqualFold); ok { + return fmt.Sprintf("possible case mismatch between selected worker %q and topology host %q; host endpoint node values must exactly match clusterConfig.workerNodes", + selected, topology) + } + shortNameEqual := func(left, right string) bool { + return strings.EqualFold(shortHostname(left), shortHostname(right)) + } + if selected, topology, ok := similarNodePair(summary.selected, summary.topology, shortNameEqual); ok { + return fmt.Sprintf("possible short-name/FQDN mismatch between selected worker %q and topology host %q; host endpoint node values must exactly match clusterConfig.workerNodes", + selected, topology) + } + return "the topology may describe a different cluster; host endpoint node values must exactly match clusterConfig.workerNodes" +} + +func similarNodePair(selected, topology []string, matches func(string, string) bool) (string, string, bool) { + for _, selectedNode := range selected { + for _, topologyNode := range topology { + if selectedNode != topologyNode && matches(selectedNode, topologyNode) { + return selectedNode, topologyNode, true + } + } + } + return "", "", false +} + +func shortHostname(node string) string { + short, _, _ := strings.Cut(node, ".") + return short +} + +func topologyCoverage(links []hostLink, spcx *config.ProfileSpectrumX) map[string][]poolKey { + coverageSets := map[string]map[poolKey]struct{}{} + for _, link := range links { + if coverageSets[link.node] == nil { + coverageSets[link.node] = map[poolKey]struct{}{} + } + coverageSets[link.node][poolKeyForLink(link, spcx)] = struct{}{} + } + + coverage := make(map[string][]poolKey, len(coverageSets)) + for node, keys := range coverageSets { + allocations := make(map[poolKey][]StaticAllocation, len(keys)) + for key := range keys { + allocations[key] = nil + } + coverage[node] = sortedPoolKeys(allocations) + } + return coverage +} + +func formatMissingCoverage(missing []string, coverage map[string][]poolKey, spcx *config.ProfileSpectrumX) string { + present := make([]string, 0, len(missing)) + absent := make([]string, 0, len(missing)) + for _, node := range missing { + keys := coverage[node] + if len(keys) == 0 { + absent = append(absent, node) + continue + } + present = append(present, fmt.Sprintf("%s=%s", node, formatPoolKeys(keys, spcx))) + } + + details := make([]string, 0, 2) + if len(present) > 0 { + details = append(details, "available topology coverage for missing workers "+formatLimitedList(present)) + } + if len(absent) > 0 { + details = append(details, "workers absent from topology "+formatLimitedList(absent)) + } + return strings.Join(details, "; ") +} + +func formatPoolKeys(keys []poolKey, spcx *config.ProfileSpectrumX) string { + names := make([]string, 0, len(keys)) + for _, key := range keys { + names = append(names, poolName(key, "", spcx)) + } + return formatLimitedList(names) +} + +func topologyAttributeHint(spcx *config.ProfileSpectrumX) string { + if spcx.MultiplaneMode == "swplb" { + return "check host attributes.rail and leaf attributes.plane for the missing rail/plane links" + } + return "check host attributes.rail for the missing rail links" +} + +func sortedSet(values map[string]struct{}) []string { + result := make([]string, 0, len(values)) + for value := range values { + result = append(result, value) + } + sort.Strings(result) + return result +} + +func formatLimitedList(values []string) string { + values = append([]string(nil), values...) + sort.Strings(values) + shown := values + remaining := 0 + if len(values) > diagnosticListLimit { + shown = values[:diagnosticListLimit] + remaining = len(values) - diagnosticListLimit + } + formatted := "[" + strings.Join(shown, ", ") + "]" + if remaining > 0 { + formatted += fmt.Sprintf(" (+%d more)", remaining) + } + return formatted +} diff --git a/pkg/networkoperatorplugin/spectrumx/addressing_test.go b/pkg/networkoperatorplugin/spectrumx/addressing_test.go index 4923dab..f70c1ff 100644 --- a/pkg/networkoperatorplugin/spectrumx/addressing_test.go +++ b/pkg/networkoperatorplugin/spectrumx/addressing_test.go @@ -17,6 +17,7 @@ package spectrumx import ( + "fmt" "os" "path/filepath" "testing" @@ -262,6 +263,137 @@ func TestBuildCIDRPoolsRejectsIPv6UntilRenderable(t *testing.T) { require.ErrorContains(t, err, "currently supports ipVersion=ipv4 only") } +func TestBuildCIDRPoolsReportsWrongTopologyFile(t *testing.T) { + topologyPath := writeSingleLinkTopology(t, "stale-worker") + cfg := spectrumXTestConfig(topologyPath, "hwplb") + + _, err := BuildCIDRPools(cfg, config.ClusterConfig{ + WorkerNodes: []string{"compute-b", "compute-a"}, + }) + + require.ErrorContains(t, err, "no Spectrum-X topology allocations matched unnamed clusterConfig group") + require.ErrorContains(t, err, "selected workers=[compute-a, compute-b]") + require.ErrorContains(t, err, "topology host nodes=[stale-worker]") + require.ErrorContains(t, err, "exact matches=[]") + require.ErrorContains(t, err, "missing workers=[compute-a, compute-b]") + require.ErrorContains(t, err, "topology-only hosts=[stale-worker]") + require.ErrorContains(t, err, "topology may describe a different cluster") + require.ErrorContains(t, err, "must exactly match clusterConfig.workerNodes") +} + +func TestBuildCIDRPoolsReportsLikelyNodeNameMismatch(t *testing.T) { + tests := []struct { + name string + topologyNode string + workerNode string + wantHint string + }{ + { + name: "case mismatch", + topologyNode: "compute-a", + workerNode: "Compute-A", + wantHint: `possible case mismatch between selected worker "Compute-A" and topology host "compute-a"`, + }, + { + name: "short name and FQDN mismatch", + topologyNode: "compute-a.example.test", + workerNode: "compute-a", + wantHint: `possible short-name/FQDN mismatch between selected worker "compute-a" and topology host "compute-a.example.test"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + topologyPath := writeSingleLinkTopology(t, tt.topologyNode) + cfg := spectrumXTestConfig(topologyPath, "hwplb") + + _, err := BuildCIDRPools(cfg, config.ClusterConfig{WorkerNodes: []string{tt.workerNode}}) + + require.ErrorContains(t, err, tt.wantHint) + require.ErrorContains(t, err, "must exactly match clusterConfig.workerNodes") + }) + } +} + +func TestBuildCIDRPoolsReportsMissingRailPlaneCoverage(t *testing.T) { + topologyPath := writeTopology(t, `{ + "nodes": [ + {"name": "compute-a", "role": "host", "type": "default"}, + {"name": "compute-b", "role": "host", "type": "default"}, + {"name": "leaf-a", "role": "leaf", "type": "cumulus"} + ], + "links": [ + [ + {"node": "leaf-a", "interface": "swp1s0", "attributes": {"role": "leaf", "plane": 0, "pod": 0, "su": 0}}, + {"node": "compute-a", "interface": "eth_p0_r0", "attributes": {"role": "host", "rail": 0, "pod": 0, "su": 0}} + ], + [ + {"node": "leaf-a", "interface": "swp2s0", "attributes": {"role": "leaf", "plane": 0, "pod": 0, "su": 0}}, + {"node": "compute-b", "interface": "eth_p0_r0", "attributes": {"role": "host", "rail": 0, "pod": 0, "su": 0}} + ], + [ + {"node": "leaf-a", "interface": "swp1s1", "attributes": {"role": "leaf", "plane": 1, "pod": 0, "su": 0}}, + {"node": "compute-a", "interface": "eth_p1_r0", "attributes": {"role": "host", "rail": 0, "pod": 0, "su": 0}} + ] + ] +}`) + cfg := spectrumXTestConfig(topologyPath, "swplb") + + _, err := BuildCIDRPools(cfg, config.ClusterConfig{ + Identifier: "gpu-workers", + WorkerNodes: []string{"compute-a", "compute-b"}, + }) + + require.ErrorContains(t, err, `CIDRPool rail-0-plane-1 for clusterConfig group "gpu-workers"`) + require.ErrorContains(t, err, "missing topology allocations for workers [compute-b]") + require.ErrorContains(t, err, "available topology coverage for missing workers [compute-b=[rail-0-plane-0]]") + require.ErrorContains(t, err, "check host attributes.rail and leaf attributes.plane") +} + +func TestBuildCIDRPoolsAIRDiagnosticsUseNormalizedHostNames(t *testing.T) { + cfg := spectrumXTestConfig(filepath.Join("testdata", "air-simple-quadplane.json"), "swplb") + + _, err := BuildCIDRPools(cfg, config.ClusterConfig{WorkerNodes: []string{"unrelated-worker"}}) + + require.ErrorContains(t, err, "topology host nodes=[worker-su01-rack01-h01, worker-su01-rack01-h02]") + require.ErrorContains(t, err, "topology-only hosts=[worker-su01-rack01-h01, worker-su01-rack01-h02]") +} + +func TestFormatLimitedList(t *testing.T) { + values := []string{"node-09", "node-03", "node-01", "node-07", "node-05", "node-10", "node-08", "node-02", "node-06", "node-04"} + + require.Equal(t, + "[node-01, node-02, node-03, node-04, node-05, node-06, node-07, node-08] (+2 more)", + formatLimitedList(values)) +} + +func spectrumXTestConfig(topologyPath, multiplaneMode string) *config.LaunchKitConfig { + return &config.LaunchKitConfig{ + Profile: &config.Profile{SpectrumX: &config.ProfileSpectrumX{ + Enable: true, + TopologyType: config.SpectrumXTopology2Tier, + IPVersion: config.SpectrumXIPVersionIPv4, + TopologyFile: topologyPath, + MultiplaneMode: multiplaneMode, + NumberOfPlanes: 2, + }}, + } +} + +func writeSingleLinkTopology(t *testing.T, hostNode string) string { + t.Helper() + return writeTopology(t, fmt.Sprintf(`{ + "nodes": [ + {"name": %q, "role": "host", "type": "default"}, + {"name": "leaf-a", "role": "leaf", "type": "cumulus"} + ], + "links": [[ + {"node": "leaf-a", "interface": "swp1s0", "attributes": {"role": "leaf", "plane": 0, "pod": 0, "su": 0}}, + {"node": %q, "interface": "eth_p0_r0", "attributes": {"role": "host", "rail": 0, "pod": 0, "su": 0}} + ]] +}`, hostNode, hostNode)) +} + func writeTopology(t *testing.T, content string) string { t.Helper() path := filepath.Join(t.TempDir(), "topology.json") diff --git a/pkg/networkoperatorplugin/templates.go b/pkg/networkoperatorplugin/templates.go index a5d7bf8..2334906 100644 --- a/pkg/networkoperatorplugin/templates.go +++ b/pkg/networkoperatorplugin/templates.go @@ -646,6 +646,8 @@ type templateContext struct { ClusterConfig *config.ClusterConfig } +const templateGroupDiagnosticLimit = 8 + // ProcessTemplate processes a Go template file with the given config. // Returns a map of filename → rendered content. Templates that reference // .ClusterConfig are rendered once per group (producing separate files), @@ -793,7 +795,7 @@ func ProcessTemplate(templatePath string, cfg *config.LaunchKitConfig, groupFilt } var buf bytes.Buffer if err := tmpl.Execute(&buf, ctx); err != nil { - return nil, fmt.Errorf("failed to execute template %s for group %s: %w", templatePath, groups[i].Identifier, err) + return nil, fmt.Errorf("failed to execute template %s for %s: %w", templatePath, templateGroupLabel(groups[i]), err) } id := renderGroup.Identifier fileName := baseName @@ -809,6 +811,28 @@ func ProcessTemplate(templatePath string, cfg *config.LaunchKitConfig, groupFilt return results, nil } +func templateGroupLabel(group config.ClusterConfig) string { + if group.Identifier != "" { + return fmt.Sprintf("clusterConfig group %q", group.Identifier) + } + if len(group.WorkerNodes) > 0 { + workers := append([]string(nil), group.WorkerNodes...) + sort.Strings(workers) + shown := workers + remaining := 0 + if len(workers) > templateGroupDiagnosticLimit { + shown = workers[:templateGroupDiagnosticLimit] + remaining = len(workers) - templateGroupDiagnosticLimit + } + label := fmt.Sprintf("clusterConfig workers [%s]", strings.Join(shown, ", ")) + if remaining > 0 { + label += fmt.Sprintf(" (+%d more)", remaining) + } + return label + } + return "unnamed clusterConfig group" +} + // groupFabric returns the discovered fabric ("Ethernet" or "InfiniBand") // for a group, plus a bool indicating whether the field is set. Reads // directly from `group.LinkType`, which `discoverGroupFabric` populates diff --git a/pkg/networkoperatorplugin/templates_test.go b/pkg/networkoperatorplugin/templates_test.go index 6b074f6..aacd3ac 100644 --- a/pkg/networkoperatorplugin/templates_test.go +++ b/pkg/networkoperatorplugin/templates_test.go @@ -1451,6 +1451,32 @@ spec: }) } +func TestProcessTemplateErrorIdentifiesUnnamedGroupByWorkers(t *testing.T) { + templatePath := filepath.Join(t.TempDir(), "broken.yaml") + require.NoError(t, os.WriteFile(templatePath, []byte(`{{index .ClusterConfig.WorkerNodes 1}}`), 0o600)) + cfg := &config.LaunchKitConfig{ + ClusterConfig: []config.ClusterConfig{{ + WorkerNodes: []string{"compute-a"}, + }}, + } + + _, err := ProcessTemplate(templatePath, cfg, "") + + require.ErrorContains(t, err, "for clusterConfig workers [compute-a]") + require.NotContains(t, err.Error(), "for group :") +} + +func TestTemplateGroupLabelBoundsAndSortsWorkers(t *testing.T) { + group := config.ClusterConfig{WorkerNodes: []string{ + "node-09", "node-03", "node-01", "node-07", "node-05", + "node-10", "node-08", "node-02", "node-06", "node-04", + }} + + require.Equal(t, + "clusterConfig workers [node-01, node-02, node-03, node-04, node-05, node-06, node-07, node-08] (+2 more)", + templateGroupLabel(group)) +} + func TestVersionGE(t *testing.T) { cases := []struct { have, target string diff --git a/skills/k8s-launch-kit-generate/SKILL.md b/skills/k8s-launch-kit-generate/SKILL.md index aeba42f..9fd0cee 100644 --- a/skills/k8s-launch-kit-generate/SKILL.md +++ b/skills/k8s-launch-kit-generate/SKILL.md @@ -158,6 +158,12 @@ win when a one-off override is needed. - NVIDIA AIR topology support requires the documented one-based node/interface naming contract (`su`, `h`, `leaf-p

`, `r`, `railp

`, and `pod` for 3-tier). See `docs/user/spectrum-x.md` in the l8k repository. +- Spectrum-X CIDRPool allocation matches `clusterConfig.workerNodes` to + topology host endpoint `node` values exactly and case-sensitively. A zero-match + error usually means the wrong topology file, a case difference, or a + short-name/FQDN difference. Partial-pool errors report the worker's available + rail/plane coverage; check host `attributes.rail` and, for `swplb`, leaf + `attributes.plane`. - RA2.2 and RA2.3 v1alpha2 `SpectrumXRailPoolConfig` output intentionally omits the removed `spec.withBCM` field; current CRDs reject it during strict decoding.