Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions pkg/device/ascend/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"errors"
"flag"
"fmt"
"math"
"slices"
"sort"
"strconv"
Expand Down Expand Up @@ -312,6 +313,10 @@ func (dev *Devices) GenerateResourceRequests(ctr *corev1.Container) device.Conta
klog.V(3).Infof("Counting %s devices", dev.config.CommonWord)
if n, ok := v.AsInt64(); ok {
klog.Info("Found AscendDevices devices")
if n <= 0 || n > math.MaxInt32 {
klog.ErrorS(nil, "ascend device count request is out of range", "container", ctr.Name, "request", n)
return device.ContainerDeviceRequest{}
}
memnum := 0
mem, ok := ctr.Resources.Limits[ascendResourceMem]
if !ok {
Expand All @@ -320,9 +325,24 @@ func (dev *Devices) GenerateResourceRequests(ctr *corev1.Container) device.Conta
if ok {
memnums, ok := mem.AsInt64()
if ok {
// Ascend memory is expressed in MB, so a byte-suffixed quantity such as
// 16Gi is several orders of magnitude past the int32 request field and is
// almost always a wrong-unit mistake rather than a real request.
if memnums < 0 || memnums > math.MaxInt32 {
klog.ErrorS(nil, "ascend device memory request is out of range; memory unit is treated as MB not Byte, so a quantity such as 16Gi is invalid, request 16384 for 16GB instead",
"container", ctr.Name, "request", mem.String(), "device", dev.config.CommonWord)
return device.ContainerDeviceRequest{}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}
if dev.config.MemoryFactor > 1 {
rawMemnums := memnums
// memnums is already bounded by math.MaxInt32 and MemoryFactor is an
// int32, so this product cannot overflow int64 before it is checked.
memnums = memnums * int64(dev.config.MemoryFactor)
if memnums > math.MaxInt32 {
klog.ErrorS(nil, "ascend device memory request overflows int32 after applying memory factor; memory unit is treated as MB not Byte",
"container", ctr.Name, "raw", rawMemnums, "scaled", memnums, "factor", dev.config.MemoryFactor)
return device.ContainerDeviceRequest{}
}
klog.V(4).Infof("Update Ascend memory request. before %d, after %d, factor %d", rawMemnums, memnums, dev.config.MemoryFactor)
}
// If "core" is requested, it explicitly indicates the use of soft-partitioning.
Expand All @@ -347,10 +367,17 @@ func (dev *Devices) GenerateResourceRequests(ctr *corev1.Container) device.Conta
// Process Core Resources
corenum := int32(0)
if ascendResourceCore != "" {
if cv, ok := ctr.Resources.Limits[ascendResourceCore]; ok {
corenum = int32(cv.Value())
} else if cv, ok := ctr.Resources.Requests[ascendResourceCore]; ok {
corenum = int32(cv.Value())
cv, ok := ctr.Resources.Limits[ascendResourceCore]
if !ok {
cv, ok = ctr.Resources.Requests[ascendResourceCore]
}
if ok {
corenums := cv.Value()
if corenums < 0 || corenums > math.MaxInt32 {
klog.ErrorS(nil, "ascend device core request is out of range", "container", ctr.Name, "request", cv.String())
return device.ContainerDeviceRequest{}
}
corenum = int32(corenums)
}
}

Expand Down
164 changes: 164 additions & 0 deletions pkg/device/ascend/device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1439,6 +1439,170 @@ func Test_GenerateResourceRequestsFactor(t *testing.T) {
}
}

// Test_GenerateResourceRequests_OutOfRangeValues covers requests whose values do
// not fit the int32 fields of ContainerDeviceRequest. Before the range guards
// these narrowed silently: a byte-suffixed memory quantity such as 16Gi wrapped
// to 0 on the soft-partitioning path, which made the scheduler's memory fit
// check pass against any device.
func Test_GenerateResourceRequests_OutOfRangeValues(t *testing.T) {
// Soft-partitioning layout: requesting a core count makes
// GenerateResourceRequests use the raw memory value instead of trimming it
// to a template, which is where the int32 wrap used to happen.
coreModeConfig := VNPUConfig{
CommonWord: "Ascend910B3",
ResourceName: "huawei.com/Ascend910B3",
ResourceCoreName: "huawei.com/Ascend910B3-core",
ResourceMemoryName: "huawei.com/Ascend910B3-memory",
MemoryAllocatable: int64(65536),
MemoryCapacity: int64(65536),
}

tests := []struct {
name string
dev Devices
args corev1.Container
want device.ContainerDeviceRequest
}{
{
// 16Gi is 17179869184, which wrapped to 0 when narrowed to int32.
// Ascend memory is denominated in MB, so this is a wrong-unit request.
name: "memory requested in bytes exceeds int32 range on soft-partitioning path",
dev: Devices{config: coreModeConfig},
args: corev1.Container{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"huawei.com/Ascend910B3": resource.MustParse("1"),
"huawei.com/Ascend910B3-core": resource.MustParse("10"),
"huawei.com/Ascend910B3-memory": resource.MustParse("16Gi"),
},
},
},
want: device.ContainerDeviceRequest{},
},
{
name: "memory requested in bytes exceeds int32 range on trim path",
dev: Devices{config: coreModeConfig},
args: corev1.Container{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"huawei.com/Ascend910B3": resource.MustParse("1"),
"huawei.com/Ascend910B3-memory": resource.MustParse("16Gi"),
},
},
},
want: device.ContainerDeviceRequest{},
},
{
name: "oversized device count exceeds int32 range",
dev: Devices{config: coreModeConfig},
args: corev1.Container{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"huawei.com/Ascend910B3": resource.MustParse("2200000000"),
},
},
},
want: device.ContainerDeviceRequest{},
},
{
name: "negative core request",
dev: Devices{config: coreModeConfig},
args: corev1.Container{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"huawei.com/Ascend910B3": resource.MustParse("1"),
"huawei.com/Ascend910B3-core": resource.MustParse("-1"),
},
},
},
want: device.ContainerDeviceRequest{},
},
{
name: "oversized core request exceeds int32 range",
dev: Devices{config: coreModeConfig},
args: corev1.Container{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"huawei.com/Ascend910B3": resource.MustParse("1"),
"huawei.com/Ascend910B3-core": resource.MustParse("2200000000"),
},
},
},
want: device.ContainerDeviceRequest{},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
result := test.dev.GenerateResourceRequests(&test.args)
assert.Equal(t, result, test.want)
})
}
}

// Test_GenerateResourceRequests_MemoryFactorOverflow covers a raw memory request
// that fits in int32 on its own but overflows once MemoryFactor is applied.
// 300000000 fits in int32, but 300000000 * 10 exceeds math.MaxInt32.
func Test_GenerateResourceRequests_MemoryFactorOverflow(t *testing.T) {
tests := []struct {
name string
dev Devices
args corev1.Container
}{
{
// Soft-partitioning keeps the scaled value, so this narrowed to a
// negative Memreq before the guard.
name: "scaled memory overflows int32 on soft-partitioning path",
dev: Devices{
config: VNPUConfig{
CommonWord: "Ascend910B3",
ResourceName: "huawei.com/Ascend910B3",
ResourceCoreName: "huawei.com/Ascend910B3-core",
ResourceMemoryName: "huawei.com/Ascend910B3-memory",
MemoryAllocatable: int64(65536),
MemoryCapacity: int64(65536),
MemoryFactor: int32(10),
},
},
args: corev1.Container{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"huawei.com/Ascend910B3": resource.MustParse("1"),
"huawei.com/Ascend910B3-core": resource.MustParse("10"),
"huawei.com/Ascend910B3-memory": resource.MustParse("300000000"),
},
},
},
},
{
name: "scaled memory overflows int32 on trim path",
dev: Devices{
config: VNPUConfig{
CommonWord: "Ascend910A",
ResourceName: "huawei.com/Ascend910A",
ResourceMemoryName: "huawei.com/Ascend910A-memory",
MemoryAllocatable: int64(32768),
MemoryCapacity: int64(32768),
MemoryFactor: int32(10),
},
},
args: corev1.Container{
Resources: corev1.ResourceRequirements{
Limits: corev1.ResourceList{
"huawei.com/Ascend910A": resource.MustParse("1"),
"huawei.com/Ascend910A-memory": resource.MustParse("300000000"),
},
},
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
result := test.dev.GenerateResourceRequests(&test.args)
assert.Equal(t, result, device.ContainerDeviceRequest{})
})
}
}

func TestDevices_LockNode(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading