Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
29 changes: 27 additions & 2 deletions pkg/device/devices.go
Original file line number Diff line number Diff line change
Expand Up @@ -679,17 +679,42 @@ 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
}
}
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
}
89 changes: 89 additions & 0 deletions pkg/device/devices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
30 changes: 1 addition & 29 deletions pkg/device/hygon/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
39 changes: 39 additions & 0 deletions pkg/device/hygon/device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
20 changes: 1 addition & 19 deletions pkg/device/nvidia/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"errors"
"flag"
"fmt"
"slices"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions pkg/device/nvidia/device_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Loading