From 8d3922ec5c70ab0545a06f7c91ebfc8276012492 Mon Sep 17 00:00:00 2001 From: Saiyam Pathak Date: Thu, 2 Jul 2026 08:58:29 +0530 Subject: [PATCH 1/3] fix(nvidia): validate gpumem-percentage range to prevent silent unschedulability When a pod requests an out-of-range memory percentage (e.g. nvidia.com/gpumem-percentage > 100), the scheduler computed a memory request larger than any card's total memory, leaving the pod Pending forever with a misleading CardInsufficientMemory reason. A value of exactly 101 collided with the internal 'percentage not set' sentinel. - Reject percentages outside [0, 100] at admission time in MutateAdmission with a human-readable error message. Init containers are validated as well: they are scheduled via Resourcereqs but never passed to MutateAdmission by the webhook. - Clamp out-of-range percentages to 100 in GenerateResourceRequests as defense-in-depth for pods that bypass the admission webhook, mirroring the existing Coresreq > 100 clamp in Fit. Fixes #1781 Signed-off-by: Saiyam Pathak --- pkg/device/nvidia/device.go | 23 ++++++ pkg/device/nvidia/device_test.go | 126 +++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/pkg/device/nvidia/device.go b/pkg/device/nvidia/device.go index d626ee6a51..5edb742934 100644 --- a/pkg/device/nvidia/device.go +++ b/pkg/device/nvidia/device.go @@ -344,6 +344,16 @@ func (dev *NvidiaGPUDevices) GetNodeDevices(n corev1.Node) ([]*device.DeviceInfo func (dev *NvidiaGPUDevices) MutateAdmission(ctr *corev1.Container, p *corev1.Pod) (bool, error) { /*gpu related */ + if err := dev.validateMemoryPercentage(ctr); err != nil { + return false, err + } + // Init containers are scheduled by Resourcereqs but are not passed to + // MutateAdmission by the webhook, so validate them here as well. + for idx := range p.Spec.InitContainers { + if err := dev.validateMemoryPercentage(&p.Spec.InitContainers[idx]); err != nil { + return false, err + } + } priority, ok := ctr.Resources.Limits[corev1.ResourceName(dev.config.ResourcePriority)] if ok { ctr.Env = append(ctr.Env, corev1.EnvVar{ @@ -381,6 +391,15 @@ func (dev *NvidiaGPUDevices) MutateAdmission(ctr *corev1.Container, p *corev1.Po return hasResource, nil } +func (dev *NvidiaGPUDevices) validateMemoryPercentage(ctr *corev1.Container) error { + if pct, ok := resourceValue(ctr, corev1.ResourceName(dev.config.ResourceMemoryPercentageName)); ok { + if pct < 0 || pct > 100 { + return fmt.Errorf("invalid %s value %d in container %s: must be an integer between 0 and 100", dev.config.ResourceMemoryPercentageName, pct, ctr.Name) + } + } + return nil +} + func (dev *NvidiaGPUDevices) mutateContainerResource(ctr *corev1.Container) bool { _, resourceNameOK := ctr.Resources.Limits[corev1.ResourceName(dev.config.ResourceCountName)] if resourceNameOK { @@ -553,6 +572,10 @@ func (dev *NvidiaGPUDevices) GenerateResourceRequests(ctr *corev1.Container) dev mempnums, ok := mem.AsInt64() if ok { mempnum = int32(mempnums) + if mempnum < 0 || mempnum > 100 { + klog.ErrorS(nil, "memory percentage request out of range, clamping to 100", "container", ctr.Name, "requested", mempnum) + mempnum = 100 + } } } if mempnum == 101 && memnum == 0 { diff --git a/pkg/device/nvidia/device_test.go b/pkg/device/nvidia/device_test.go index bc9fdd6d70..1edb97e99c 100644 --- a/pkg/device/nvidia/device_test.go +++ b/pkg/device/nvidia/device_test.go @@ -128,6 +128,96 @@ func Test_MutateAdmission(t *testing.T) { } } +func Test_MutateAdmission_MemoryPercentageValidation(t *testing.T) { + gpuDevices := &NvidiaGPUDevices{ + config: NvidiaConfig{ + ResourceCountName: "nvidia.com/gpu", + ResourceMemoryName: "nvidia.com/gpumem", + ResourceMemoryPercentageName: "nvidia.com/gpumem-percentage", + ResourceCoreName: "nvidia.com/gpucores", + DefaultGPUNum: int32(1), + }, + } + tests := []struct { + name string + pct int64 + wantErr bool + }{ + { + name: "percentage of 0 is accepted", + pct: 0, + wantErr: false, + }, + { + name: "percentage of 100 is accepted", + pct: 100, + wantErr: false, + }, + { + name: "percentage of 101 is rejected", + pct: 101, + wantErr: true, + }, + { + name: "percentage above 100 is rejected", + pct: 150, + wantErr: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctr := &corev1.Container{ + Name: "test", + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "nvidia.com/gpu": *resource.NewQuantity(1, resource.BinarySI), + "nvidia.com/gpumem-percentage": *resource.NewQuantity(test.pct, resource.DecimalSI), + }, + }, + } + _, err := gpuDevices.MutateAdmission(ctr, &corev1.Pod{}) + if test.wantErr && err == nil { + t.Fatalf("expected MutateAdmission to reject percentage %d, but got no error", test.pct) + } + if !test.wantErr && err != nil { + t.Fatalf("expected MutateAdmission to accept percentage %d, but got error: %v", test.pct, err) + } + }) + t.Run(test.name+" (init container)", func(t *testing.T) { + ctr := &corev1.Container{ + Name: "test", + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "nvidia.com/gpu": *resource.NewQuantity(1, resource.BinarySI), + }, + }, + } + pod := &corev1.Pod{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: "init", + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "nvidia.com/gpu": *resource.NewQuantity(1, resource.BinarySI), + "nvidia.com/gpumem-percentage": *resource.NewQuantity(test.pct, resource.DecimalSI), + }, + }, + }, + }, + }, + } + _, err := gpuDevices.MutateAdmission(ctr, pod) + if test.wantErr && err == nil { + t.Fatalf("expected MutateAdmission to reject init container percentage %d, but got no error", test.pct) + } + if !test.wantErr && err != nil { + t.Fatalf("expected MutateAdmission to accept init container percentage %d, but got error: %v", test.pct, err) + } + }) + } +} + func TestMutateAdmissionDefaultsExclusiveCore(t *testing.T) { ptr := func(v int64) *int64 { return &v } clone := func(in corev1.ResourceList) corev1.ResourceList { @@ -1725,6 +1815,42 @@ func TestGenerateResourceRequests(t *testing.T) { Coresreq: 0, }, }, + { + name: "gpu count + memory percentage above 100 — clamped to 100", + ctr: &corev1.Container{ + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "nvidia.com/gpu": *resource.NewQuantity(1, resource.BinarySI), + "nvidia.com/gpumem-percentage": *resource.NewQuantity(150, resource.DecimalSI), + }, + }, + }, + want: device.ContainerDeviceRequest{ + Nums: 1, + Type: NvidiaGPUDevice, + Memreq: 0, + MemPercentagereq: 100, + Coresreq: 0, + }, + }, + { + name: "gpu count + memory percentage equal to sentinel 101 — clamped to 100", + ctr: &corev1.Container{ + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "nvidia.com/gpu": *resource.NewQuantity(1, resource.BinarySI), + "nvidia.com/gpumem-percentage": *resource.NewQuantity(101, resource.DecimalSI), + }, + }, + }, + want: device.ContainerDeviceRequest{ + Nums: 1, + Type: NvidiaGPUDevice, + Memreq: 0, + MemPercentagereq: 100, + Coresreq: 0, + }, + }, { name: "gpu count + explicit cores", ctr: &corev1.Container{ From 5f2ff40d798b0e27ea5dbd6f5264c78882acd3e0 Mon Sep 17 00:00:00 2001 From: Saiyam Pathak Date: Thu, 2 Jul 2026 12:51:33 +0530 Subject: [PATCH 2/3] fix(nvidia): clamp memory percentage before int32 cast to avoid overflow wraparound Values beyond int32 range (e.g. 2^32+50) wrapped to an in-range number after the cast and bypassed the clamp. Validate the raw int64 value first, then cast. Addresses review feedback on #1997. Signed-off-by: Saiyam Pathak --- pkg/device/nvidia/device.go | 8 ++++---- pkg/device/nvidia/device_test.go | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/pkg/device/nvidia/device.go b/pkg/device/nvidia/device.go index 5edb742934..dcc6afd030 100644 --- a/pkg/device/nvidia/device.go +++ b/pkg/device/nvidia/device.go @@ -571,11 +571,11 @@ func (dev *NvidiaGPUDevices) GenerateResourceRequests(ctr *corev1.Container) dev if ok { mempnums, ok := mem.AsInt64() if ok { - mempnum = int32(mempnums) - if mempnum < 0 || mempnum > 100 { - klog.ErrorS(nil, "memory percentage request out of range, clamping to 100", "container", ctr.Name, "requested", mempnum) - mempnum = 100 + if mempnums < 0 || mempnums > 100 { + klog.ErrorS(nil, "memory percentage request out of range, clamping to 100", "container", ctr.Name, "requested", mempnums) + mempnums = 100 } + mempnum = int32(mempnums) } } if mempnum == 101 && memnum == 0 { diff --git a/pkg/device/nvidia/device_test.go b/pkg/device/nvidia/device_test.go index 1edb97e99c..6ac58f7d38 100644 --- a/pkg/device/nvidia/device_test.go +++ b/pkg/device/nvidia/device_test.go @@ -1833,6 +1833,24 @@ func TestGenerateResourceRequests(t *testing.T) { Coresreq: 0, }, }, + { + name: "gpu count + memory percentage beyond int32 range — clamped to 100 without wrapping", + ctr: &corev1.Container{ + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + "nvidia.com/gpu": *resource.NewQuantity(1, resource.BinarySI), + "nvidia.com/gpumem-percentage": *resource.NewQuantity(int64(1)<<32+50, resource.DecimalSI), + }, + }, + }, + want: device.ContainerDeviceRequest{ + Nums: 1, + Type: NvidiaGPUDevice, + Memreq: 0, + MemPercentagereq: 100, + Coresreq: 0, + }, + }, { name: "gpu count + memory percentage equal to sentinel 101 — clamped to 100", ctr: &corev1.Container{ From d5a1fe38ed553c0f111f8c9f755b7f9c307fcaa5 Mon Sep 17 00:00:00 2001 From: Saiyam Pathak Date: Thu, 2 Jul 2026 14:28:01 +0530 Subject: [PATCH 3/3] fix(nvidia): drop init-container validation from MutateAdmission per review Init containers need more careful design at the admission layer; they remain protected by the out-of-range clamp in GenerateResourceRequests, which covers the scheduling path. Addresses review feedback from archlitchi on #1997. Signed-off-by: Saiyam Pathak --- pkg/device/nvidia/device.go | 7 ------- pkg/device/nvidia/device_test.go | 32 -------------------------------- 2 files changed, 39 deletions(-) diff --git a/pkg/device/nvidia/device.go b/pkg/device/nvidia/device.go index dcc6afd030..84285c2500 100644 --- a/pkg/device/nvidia/device.go +++ b/pkg/device/nvidia/device.go @@ -347,13 +347,6 @@ func (dev *NvidiaGPUDevices) MutateAdmission(ctr *corev1.Container, p *corev1.Po if err := dev.validateMemoryPercentage(ctr); err != nil { return false, err } - // Init containers are scheduled by Resourcereqs but are not passed to - // MutateAdmission by the webhook, so validate them here as well. - for idx := range p.Spec.InitContainers { - if err := dev.validateMemoryPercentage(&p.Spec.InitContainers[idx]); err != nil { - return false, err - } - } priority, ok := ctr.Resources.Limits[corev1.ResourceName(dev.config.ResourcePriority)] if ok { ctr.Env = append(ctr.Env, corev1.EnvVar{ diff --git a/pkg/device/nvidia/device_test.go b/pkg/device/nvidia/device_test.go index 6ac58f7d38..19d6723d62 100644 --- a/pkg/device/nvidia/device_test.go +++ b/pkg/device/nvidia/device_test.go @@ -183,38 +183,6 @@ func Test_MutateAdmission_MemoryPercentageValidation(t *testing.T) { t.Fatalf("expected MutateAdmission to accept percentage %d, but got error: %v", test.pct, err) } }) - t.Run(test.name+" (init container)", func(t *testing.T) { - ctr := &corev1.Container{ - Name: "test", - Resources: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "nvidia.com/gpu": *resource.NewQuantity(1, resource.BinarySI), - }, - }, - } - pod := &corev1.Pod{ - Spec: corev1.PodSpec{ - InitContainers: []corev1.Container{ - { - Name: "init", - Resources: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "nvidia.com/gpu": *resource.NewQuantity(1, resource.BinarySI), - "nvidia.com/gpumem-percentage": *resource.NewQuantity(test.pct, resource.DecimalSI), - }, - }, - }, - }, - }, - } - _, err := gpuDevices.MutateAdmission(ctr, pod) - if test.wantErr && err == nil { - t.Fatalf("expected MutateAdmission to reject init container percentage %d, but got no error", test.pct) - } - if !test.wantErr && err != nil { - t.Fatalf("expected MutateAdmission to accept init container percentage %d, but got error: %v", test.pct, err) - } - }) } }