diff --git a/pkg/device/devices.go b/pkg/device/devices.go index 7817660aa5..0efbe0d4d8 100644 --- a/pkg/device/devices.go +++ b/pkg/device/devices.go @@ -679,13 +679,14 @@ func CheckUUID(annos map[string]string, id, useKey, noUseKey, deviceType string) return strings.TrimSpace(u) == id }) } - if userUUID, ok := annos[useKey]; ok { + // An empty value means "no constraint" rather than "match nothing". + if userUUID, ok := annos[useKey]; ok && strings.TrimSpace(userUUID) != "" { klog.V(5).Infof("check uuid for %s user uuid [%s], device id is %s", deviceType, userUUID, id) if !match(userUUID) { return false } } - if noUserUUID, ok := annos[noUseKey]; ok { + if noUserUUID, ok := annos[noUseKey]; ok && strings.TrimSpace(noUserUUID) != "" { klog.V(5).Infof("check uuid for %s not user uuid [%s], device id is %s", deviceType, noUserUUID, id) if match(noUserUUID) { return false @@ -693,3 +694,27 @@ func CheckUUID(annos map[string]string, id, useKey, noUseKey, deviceType string) } return true } + +// CheckType reports whether a device model is allowed by the use/noUse type +// constraints in annos. It mirrors CheckUUID but matches the card model as a +// case-insensitive substring instead of an exact device id. An empty value +// means "no constraint" rather than "match nothing". +func CheckType(annos map[string]string, cardType, useKey, noUseKey string) bool { + cardType = strings.ToUpper(cardType) + match := func(list string) bool { + return slices.ContainsFunc(strings.Split(list, ","), func(t string) bool { + return strings.Contains(cardType, strings.ToUpper(t)) + }) + } + if inuse, ok := annos[useKey]; ok && strings.TrimSpace(inuse) != "" { + if !match(inuse) { + return false + } + } + if noUse, ok := annos[noUseKey]; ok && strings.TrimSpace(noUse) != "" { + if match(noUse) { + return false + } + } + return true +} diff --git a/pkg/device/devices_test.go b/pkg/device/devices_test.go index a8c22cdd04..1d602442fc 100644 --- a/pkg/device/devices_test.go +++ b/pkg/device/devices_test.go @@ -1454,6 +1454,30 @@ func TestCheckUUID(t *testing.T) { id: "abc", want: true, }, + { + name: "empty GPUUseUUID annotation should not filter out any device", + annos: map[string]string{ + GPUUseUUID: "", + }, + id: "abc", + want: true, + }, + { + name: "whitespace-only GPUUseUUID annotation should not filter out any device", + annos: map[string]string{ + GPUUseUUID: " ", + }, + id: "abc", + want: true, + }, + { + name: "empty GPUNoUseUUID annotation should not exclude any device", + annos: map[string]string{ + GPUNoUseUUID: "", + }, + id: "abc", + want: true, + }, } for _, test := range tests { @@ -1464,6 +1488,71 @@ func TestCheckUUID(t *testing.T) { } } +func TestCheckType(t *testing.T) { + useKey := "example.com/use-gputype" + noUseKey := "example.com/nouse-gputype" + tests := []struct { + name string + annos map[string]string + cardType string + want bool + }{ + { + name: "no annotation is no constraint", + annos: map[string]string{}, + cardType: "NVIDIA-A100", + want: true, + }, + { + name: "use list matches by substring", + annos: map[string]string{useKey: "A100,V100"}, + cardType: "NVIDIA-A100", + want: true, + }, + { + name: "use list does not match", + annos: map[string]string{useKey: "V100"}, + cardType: "NVIDIA-A100", + want: false, + }, + { + name: "nouse list matches excludes the device", + annos: map[string]string{noUseKey: "A100"}, + cardType: "NVIDIA-A100", + want: false, + }, + { + name: "nouse list does not match keeps the device", + annos: map[string]string{noUseKey: "V100"}, + cardType: "NVIDIA-A100", + want: true, + }, + { + name: "empty use value is no constraint", + annos: map[string]string{useKey: ""}, + cardType: "NVIDIA-A100", + want: true, + }, + { + name: "whitespace-only nouse value excludes nothing", + annos: map[string]string{noUseKey: " "}, + cardType: "NVIDIA-A100", + want: true, + }, + { + name: "use satisfied and nouse matches still excludes", + annos: map[string]string{useKey: "A100", noUseKey: "A100"}, + cardType: "NVIDIA-A100", + want: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.want, CheckType(test.annos, test.cardType, useKey, noUseKey)) + }) + } +} + func TestDeviceUsageDeepCopy(t *testing.T) { tests := []struct { name string diff --git a/pkg/device/hygon/device.go b/pkg/device/hygon/device.go index af4c37fc65..a0b136137e 100644 --- a/pkg/device/hygon/device.go +++ b/pkg/device/hygon/device.go @@ -96,35 +96,7 @@ func (dev *DCUDevices) MutateAdmission(ctr *corev1.Container, p *corev1.Pod) (bo } func checkDCUtype(annos map[string]string, cardtype string) bool { - if inuse, ok := annos[DCUInUse]; ok { - if !strings.Contains(inuse, ",") { - if strings.Contains(strings.ToUpper(cardtype), strings.ToUpper(inuse)) { - return true - } - } else { - for val := range strings.SplitSeq(inuse, ",") { - if strings.Contains(strings.ToUpper(cardtype), strings.ToUpper(val)) { - return true - } - } - } - return false - } - if nouse, ok := annos[DCUNoUse]; ok { - if !strings.Contains(nouse, ",") { - if strings.Contains(strings.ToUpper(cardtype), strings.ToUpper(nouse)) { - return false - } - } else { - for val := range strings.SplitSeq(nouse, ",") { - if strings.Contains(strings.ToUpper(cardtype), strings.ToUpper(val)) { - return false - } - } - } - return true - } - return true + return device.CheckType(annos, cardtype, DCUInUse, DCUNoUse) } func (dev *DCUDevices) LockNode(n *corev1.Node, p *corev1.Pod) error { diff --git a/pkg/device/hygon/device_test.go b/pkg/device/hygon/device_test.go index ac251e07a3..d3197a60a3 100644 --- a/pkg/device/hygon/device_test.go +++ b/pkg/device/hygon/device_test.go @@ -191,6 +191,45 @@ func Test_checkDCUtype(t *testing.T) { }, want: true, }, + { + name: "empty use type annotation is no constraint", + args: struct { + annos map[string]string + cardtype string + }{ + annos: map[string]string{ + "hygon.com/use-dcutype": "", + }, + cardtype: "dcu", + }, + want: true, + }, + { + name: "empty nouse type annotation excludes nothing", + args: struct { + annos map[string]string + cardtype string + }{ + annos: map[string]string{ + "hygon.com/nouse-dcutype": "", + }, + cardtype: "dcu", + }, + want: true, + }, + { + name: "whitespace-only nouse type annotation excludes nothing", + args: struct { + annos map[string]string + cardtype string + }{ + annos: map[string]string{ + "hygon.com/nouse-dcutype": " ", + }, + cardtype: "dcu", + }, + want: true, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/pkg/device/nvidia/device.go b/pkg/device/nvidia/device.go index 3fda035905..dab398e67c 100644 --- a/pkg/device/nvidia/device.go +++ b/pkg/device/nvidia/device.go @@ -20,7 +20,6 @@ import ( "errors" "flag" "fmt" - "slices" "strconv" "strings" "sync" @@ -485,24 +484,7 @@ func resourcePresent(ctr *corev1.Container, name corev1.ResourceName) bool { } func checkGPUtype(annos map[string]string, cardtype string) bool { - cardtype = strings.ToUpper(cardtype) - if inuse, ok := annos[GPUInUse]; ok { - useTypes := strings.Split(inuse, ",") - if !slices.ContainsFunc(useTypes, func(useType string) bool { - return strings.Contains(cardtype, strings.ToUpper(useType)) - }) { - return false - } - } - if unuse, ok := annos[GPUNoUse]; ok { - unuseTypes := strings.Split(unuse, ",") - if slices.ContainsFunc(unuseTypes, func(unuseType string) bool { - return strings.Contains(cardtype, strings.ToUpper(unuseType)) - }) { - return false - } - } - return true + return device.CheckType(annos, cardtype, GPUInUse, GPUNoUse) } func assertNuma(annos map[string]string) bool { diff --git a/pkg/device/nvidia/device_test.go b/pkg/device/nvidia/device_test.go index a98f79f157..25f936e1d8 100644 --- a/pkg/device/nvidia/device_test.go +++ b/pkg/device/nvidia/device_test.go @@ -2420,6 +2420,16 @@ func TestCheckGPUtype_NoUse(t *testing.T) { assert.Equal(t, checkGPUtype(annos, "NVIDIA-V100"), true) } +func TestCheckGPUtype_EmptyAnnotation(t *testing.T) { + // An empty use/nouse type annotation means "no constraint": strings.Contains + // treats "" as a substring of every card type, so without a guard an empty + // nouse-gputype would wrongly exclude every device. + assert.Equal(t, checkGPUtype(map[string]string{GPUInUse: ""}, "NVIDIA-A100"), true) + assert.Equal(t, checkGPUtype(map[string]string{GPUInUse: " "}, "NVIDIA-A100"), true) + assert.Equal(t, checkGPUtype(map[string]string{GPUNoUse: ""}, "NVIDIA-A100"), true) + assert.Equal(t, checkGPUtype(map[string]string{GPUNoUse: " "}, "NVIDIA-A100"), true) +} + func TestCheckType_AllocateMode(t *testing.T) { dev := &NvidiaGPUDevices{} req := device.ContainerDeviceRequest{Type: NvidiaGPUDevice}