Skip to content

Commit 4f53200

Browse files
committed
Fixed minor CR comments
Signed-off-by: itsomri <omric@nvidia.com>
1 parent d82510b commit 4f53200

4 files changed

Lines changed: 18 additions & 11 deletions

File tree

pkg/scheduler/api/node_info/numa_topology.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package node_info
55

66
import (
7+
"slices"
78
"sort"
89
"strconv"
910
"strings"
@@ -220,7 +221,9 @@ func allocatablePrefixSums(zones []*NumaZone, indices []int) map[int][]float64 {
220221
// vendor variants) collapse to one entry; the reported name preserved is the last seen.
221222
func awareIndices(resources sets.Set[v1.ResourceName], vectorMap *resource_info.ResourceVectorMap) ([]int, map[int]v1.ResourceName) {
222223
names := map[int]v1.ResourceName{}
223-
for name := range resources {
224+
resourceNames := sets.List(resources)
225+
slices.Sort(resourceNames)
226+
for _, name := range resourceNames {
224227
if idx := vectorMap.GetIndex(name); idx >= 0 {
225228
names[idx] = name
226229
}

pkg/scheduler/api/node_info/numa_topology_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@ import (
1616

1717
// zoneAmount reads a zone vector's amount for a resource in its natural unit (cpu in cores, others
1818
// by count), translating from the vector's milli-cpu storage.
19-
func zoneAmount(vec resource_info.ResourceVector, vm *resource_info.ResourceVectorMap, name v1.ResourceName) int64 {
19+
func zoneAmount(vec resource_info.ResourceVector, vm *resource_info.ResourceVectorMap, name v1.ResourceName) float64 {
2020
idx := vm.GetIndex(name)
2121
val := vec.Get(idx)
2222
if idx == resource_info.CPUIndex {
23-
return int64(val) / 1000
23+
return val / 1000
2424
}
25-
return int64(val)
25+
return val
2626
}
2727

2828
func numaNodeZone(name string, available map[string]string) nrtv1alpha2.Zone {
@@ -159,8 +159,8 @@ func TestBuildNumaTopology(t *testing.T) {
159159
assert.Equal(t, TopologyPolicySingleNUMANode, nt.Policy)
160160
assert.Len(t, nt.Zones, 2, "only NUMA-node zones are kept")
161161

162-
assert.Equal(t, int64(2), zoneAmount(nt.Zones[0].Available, nt.VectorMap, "nvidia.com/gpu"))
163-
assert.Equal(t, int64(2), zoneAmount(nt.Zones[0].Allocatable, nt.VectorMap, "nvidia.com/gpu"), "allocatable is populated")
162+
assert.Equal(t, float64(2), zoneAmount(nt.Zones[0].Available, nt.VectorMap, "nvidia.com/gpu"))
163+
assert.Equal(t, float64(2), zoneAmount(nt.Zones[0].Allocatable, nt.VectorMap, "nvidia.com/gpu"), "allocatable is populated")
164164

165165
assert.True(t, nt.Resources.HasAll("cpu", "memory", "nvidia.com/gpu"))
166166
assert.Equal(t, 3, nt.Resources.Len())
@@ -178,8 +178,8 @@ func TestBuildNumaTopology(t *testing.T) {
178178

179179
nt := BuildNumaTopology(nrt, resource_info.NewResourceVectorMap())
180180

181-
assert.Equal(t, int64(4), zoneAmount(nt.Zones[0].Available, nt.VectorMap, "cpu"), "available reflects free capacity")
182-
assert.Equal(t, int64(8), zoneAmount(nt.Zones[0].Allocatable, nt.VectorMap, "cpu"), "allocatable reflects total capacity")
181+
assert.Equal(t, float64(4), zoneAmount(nt.Zones[0].Available, nt.VectorMap, "cpu"), "available reflects free capacity")
182+
assert.Equal(t, float64(8), zoneAmount(nt.Zones[0].Allocatable, nt.VectorMap, "cpu"), "allocatable reflects total capacity")
183183
})
184184
}
185185

@@ -210,8 +210,8 @@ func TestNumaTopologyClone(t *testing.T) {
210210
clone.Zones[0].Available[cpuIdx] -= 1000
211211
clone.Zones[0].Allocatable[cpuIdx] -= 2000
212212

213-
assert.Equal(t, int64(4), zoneAmount(orig.Zones[0].Available, orig.VectorMap, "cpu"), "original available ledger unchanged")
214-
assert.Equal(t, int64(4), zoneAmount(orig.Zones[0].Allocatable, orig.VectorMap, "cpu"), "original allocatable ledger unchanged")
213+
assert.Equal(t, float64(4), zoneAmount(orig.Zones[0].Available, orig.VectorMap, "cpu"), "original available ledger unchanged")
214+
assert.Equal(t, float64(4), zoneAmount(orig.Zones[0].Allocatable, orig.VectorMap, "cpu"), "original allocatable ledger unchanged")
215215
assert.Nil(t, (*NumaTopology)(nil).Clone())
216216
}
217217

pkg/scheduler/plugins/numa/restricted_evaluator_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func assertPlacement(t *testing.T, placement pod_info.NUMAPlacement, want map[in
6161
assert.Lenf(t, got, len(amounts), "resource count on zone %d", z)
6262
for name, wantQty := range amounts {
6363
gotQty := got[name]
64-
assert.Equalf(t, wantQty.Value(), gotQty.Value(), "zone %d resource %s", z, name)
64+
assert.Equalf(t, 0, gotQty.Cmp(wantQty), "zone %d resource %s", z, name)
6565
}
6666
}
6767
}

pkg/scheduler/test_utils/test_utils.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,10 @@ func matchNUMAZonesAvailable(
365365
for name, want := range resources {
366366
expected := resource_info.NewResourceVectorFromResourceList(v1.ResourceList{name: resource.MustParse(want)}, vectorMap)
367367
idx := vectorMap.GetIndex(name)
368+
if idx < 0 {
369+
t.Errorf("Test number: %d, name: %v, has failed. Node %v zone %d resource %v missing from vector map", testNumber, testName, nodeName, zoneIndex, name)
370+
continue
371+
}
368372
if zone.Available.Get(idx) != expected.Get(idx) {
369373
t.Errorf("Test number: %d, name: %v, has failed. Node %v zone %d resource %v: actual Available %v, was expecting %v", testNumber, testName, nodeName, zoneIndex, name, zone.Available.Get(idx), want)
370374
}

0 commit comments

Comments
 (0)