fix(types): dedup registered instance types by published name

src/compute-plane-services/nvca/pkg/types/resource_types.go

@@ -401,6 +401,33 @@ func roundUpCPUToInteger(cpu resource.Quantity) uint64 {
 	return uint64(cpu.Value())
 }
 
+// instanceTypePrecedes reports whether a must be considered before b when building
+// a registration, ordering by descending capacity: GPU count first, then the
+// non-GPU resources, with FullName as a final tie-break.
+//
+// This must be a strict ordering that totally orders any two instance types. Ties
+// are resolved by whichever entry is considered first, and that entry supplies the
+// resource profile published for a name it collides on. The input arrives in node
+// listing order, which is not stable, so a comparator that leaves equal-capacity
+// entries unordered would let the published profile change from one reconcile to
+// the next.
+func instanceTypePrecedes(a, b InstanceType) bool {
+	if a.GPUCount != b.GPUCount {
+		return a.GPUCount > b.GPUCount
+	}
+	for _, c := range []int{
+		a.CPU.Cmp(b.CPU),
+		a.SystemMemory.Cmp(b.SystemMemory),
+		a.Storage.Cmp(b.Storage),
+		a.GPUMemoryPerGPU.Cmp(b.GPUMemoryPerGPU),
+	} {
+		if c != 0 {
+			return c > 0
+		}
+	}
+	return a.FullName < b.FullName
+}
+
 func (g BackendGPU) toDynamicRegistration(allowMultiNodeWorkloads bool, infraOverhead corev1.ResourceList) (rg RegistrationGPU) {
 	rg.Name = string(g.Name)
 	// use the Capacity AS IS from BackendGPU for
@@ -421,8 +448,15 @@ func (g BackendGPU) toDynamicRegistration(allowMultiNodeWorkloads bool, infraOve
 	// instance type. This doesn't fix the cross-cluster instance type compatibility issue
 	// (what if "GPU.NCP.A100-6_1x" looks different between two target-able clusters), but does fix the problem
 	// on individual clusters with intentionally different machine sizes for the same GPU.
+	//
+	// The same applies to nodes carrying different products that normalize to one GPUName
+	// (ex. A100 SXM4 and A100 PCIe): they collapse to a single registered instance type,
+	// derived from the largest node because of the ordering below. Until instance names can
+	// distinguish them, the smaller node is not separately addressable. That is preferable to
+	// registering both under one name, which makes a single-instance request create one
+	// instance per duplicate.
 	sort.Slice(g.InstanceTypes, func(i, j int) bool {
-		return g.InstanceTypes[i].GPUCount >= g.InstanceTypes[j].GPUCount
+		return instanceTypePrecedes(g.InstanceTypes[i], g.InstanceTypes[j])
 	})
 
 	instSetByIDStr := map[string]RegistrationInstanceType{}
@@ -434,9 +468,18 @@ func (g BackendGPU) toDynamicRegistration(allowMultiNodeWorkloads bool, infraOve
 
 		// Calculate the per-GPU memory count for instance multiples.
 		for i := uint64(1); i < it.GPUCount; i *= 2 {
-			// Use the full GPU name to dedup entries, since some features
-			// are captured in the full name only.
-			instIDStr := fmt.Sprintf("%s-%dx", it.FullName, i)
+			// Dedup on the published instance type name, which is the only handle
+			// downstream services have on an instance type. Two registered entries
+			// sharing a name are indistinguishable to them: a request naming it
+			// resolves to every match and is dispatched once per match, so a request
+			// for one instance creates one instance per duplicate.
+			//
+			// FullName is not a safe key here because several distinct products
+			// normalize to the same GPUName, and therefore to the same instance name:
+			// board SKUs of one model (NVIDIA-A100-SXM4-80GB vs NVIDIA-A100-80GB-PCIe),
+			// capacity variants (NVIDIA-A100-SXM4-40GB vs -80GB), and time-sliced
+			// nodes (the "-SHARED" suffix is dropped by ParseGPUName).
+			instIDStr := it.Name.WithMultiplier(i)
 
 			if _, ok := instSetByIDStr[instIDStr]; ok {
 				continue
@@ -451,7 +494,7 @@ func (g BackendGPU) toDynamicRegistration(allowMultiNodeWorkloads bool, infraOve
 			rg.InstanceTypes = append(rg.InstanceTypes, instType)
 		}
 
-		lastInstIDStr := fmt.Sprintf("%s-%dx", it.FullName, it.GPUCount)
+		lastInstIDStr := it.Name.WithMultiplier(it.GPUCount)
 		if _, ok := instSetByIDStr[lastInstIDStr]; ok {
 			continue
 		}

src/compute-plane-services/nvca/pkg/types/resource_types_test.go

@@ -21,6 +21,8 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"maps"
+	"slices"
 	"strings"
 	"sync/atomic"
 	"testing"
@@ -400,22 +402,9 @@ func TestToRegistration(t *testing.T) {
 					Storage:       "85Gi",
 					NodeType:      RegistrationInstanceTypeNodeTypeSingle,
 				},
-				{
-					Name:          "ON-PREM.GPU.A100_1x",
-					Value:         "ON-PREM.GPU.A100",
-					Description:   "Desc",
-					Default:       false,
-					CPUCores:      2,
-					CPU:           "2",
-					SystemMemory:  "2Gi",
-					GPUCount:      1,
-					GPUMemory:     "80Gi",
-					CPUArch:       "amd64",
-					OS:            "Linux",
-					DriverVersion: "525.2.02",
-					Storage:       "512Gi",
-					NodeType:      RegistrationInstanceTypeNodeTypeSingle,
-				},
+				// The 80GB node also yields an "ON-PREM.GPU.A100_1x", since ParseGPUName
+				// normalizes both capacities to "A100". Only one entry may be registered
+				// under a given name, so it is dropped in favor of the larger node's.
 				{
 					Name:          "ON-PREM.GPU.A100_2x",
 					Value:         "ON-PREM.GPU.A100",
@@ -593,6 +582,111 @@ func TestToRegistration(t *testing.T) {
 	assert.Equal(t, expRegistrationGPUs, inBackendGPUs.ToRegistration(true, corev1.ResourceList{}))
 }
 
+// Two board SKUs of one GPU model normalize to the same GPUName, and so to the same
+// instance type name. Registering both makes a single-instance request resolve to two
+// destinations upstream and create two instances, so only one may be published.
+func TestToRegistrationDedupsInstanceNamesAcrossBoardSKUs(t *testing.T) {
+	newIT := func(fullName string, gpuCount uint64, cpu, mem string) InstanceType {
+		return InstanceType{
+			Name:            "NCP.GPU.A100",
+			FullName:        fullName,
+			Description:     fullName,
+			CPU:             resource.MustParse(cpu),
+			SystemMemory:    resource.MustParse(mem),
+			GPUCount:        gpuCount,
+			GPUMemoryPerGPU: resource.MustParse("81920Mi"),
+			CPUArch:         "amd64",
+			OS:              "linux",
+			DriverVersion:   "570.211.01",
+			Storage:         resource.MustParse("512Gi"),
+		}
+	}
+
+	// An 8-GPU DGX (SXM4) and a 2-GPU VMware host (PCIe), both reporting A100.
+	in := BackendGPUs{
+		{
+			Name: "A100",
+			InstanceTypes: []InstanceType{
+				newIT("NVIDIA-A100-SXM4-80GB", 8, "256", "2015Gi"),
+				newIT("NVIDIA-A100-80GB-PCIe", 2, "48", "62Gi"),
+			},
+		},
+	}
+
+	got := in.ToRegistration(false, corev1.ResourceList{})
+	require.Len(t, got, 1)
+
+	seen := map[string]int{}
+	for _, it := range got[0].InstanceTypes {
+		seen[it.Name]++
+	}
+	for name, count := range seen {
+		assert.Equalf(t, 1, count, "instance type %q registered %d times", name, count)
+	}
+
+	// The PCIe node contributes no new names: 1x/2x collide with the DGX subdivisions.
+	assert.Equal(t,
+		[]string{"NCP.GPU.A100_1x", "NCP.GPU.A100_2x", "NCP.GPU.A100_4x", "NCP.GPU.A100_8x"},
+		slices.Sorted(maps.Keys(seen)),
+	)
+
+	// The surviving entries are the DGX-derived ones, since the larger node sorts first.
+	for _, it := range got[0].InstanceTypes {
+		assert.Equal(t, "NVIDIA-A100-SXM4-80GB", it.Description)
+	}
+}
+
+// Nodes arrive in listing order, which is not stable. When two SKUs of equal GPU count
+// collide on a published name, the winner supplies that name's resource profile, so the
+// choice must not depend on the order they happen to be listed in.
+func TestToRegistrationCollisionWinnerIsIndependentOfInputOrder(t *testing.T) {
+	newIT := func(fullName, cpu, mem, storage string) InstanceType {
+		return InstanceType{
+			Name:            "NCP.GPU.A100",
+			FullName:        fullName,
+			Description:     fullName,
+			CPU:             resource.MustParse(cpu),
+			SystemMemory:    resource.MustParse(mem),
+			GPUCount:        2,
+			GPUMemoryPerGPU: resource.MustParse("81920Mi"),
+			CPUArch:         "amd64",
+			OS:              "linux",
+			DriverVersion:   "570.211.01",
+			Storage:         resource.MustParse(storage),
+		}
+	}
+
+	// Same GPU count, different machines. Only the resource profile distinguishes them.
+	sxm := newIT("NVIDIA-A100-SXM4-80GB", "64", "503Gi", "424Gi")
+	pcie := newIT("NVIDIA-A100-80GB-PCIe", "48", "62Gi", "973Gi")
+
+	profileOf := func(its []InstanceType) []RegistrationInstanceType {
+		got := BackendGPUs{{Name: "A100", InstanceTypes: its}}.ToRegistration(false, corev1.ResourceList{})
+		require.Len(t, got, 1)
+
+		// Both SKUs subdivide to the same two names, so the collision keeps one entry
+		// each. Asserted here so the comparisons below cannot hold vacuously on an
+		// empty registration.
+		names := make([]string, 0, len(got[0].InstanceTypes))
+		for _, it := range got[0].InstanceTypes {
+			names = append(names, it.Name)
+		}
+		require.Equal(t, []string{"NCP.GPU.A100_1x", "NCP.GPU.A100_2x"}, names)
+
+		return got[0].InstanceTypes
+	}
+
+	forward := profileOf([]InstanceType{sxm, pcie})
+	reversed := profileOf([]InstanceType{pcie, sxm})
+
+	assert.Equal(t, forward, reversed, "published registration must not depend on node listing order")
+
+	// The larger machine wins the collision, consistent with the descending ordering.
+	for _, it := range forward {
+		assert.Equal(t, "NVIDIA-A100-SXM4-80GB", it.Description)
+	}
+}
+
 func Test_calcFractionCPU(t *testing.T) {
 	type spec struct {
 		q         resource.Quantity