From d1e503b195d53784b9ff6f98770c561a4658e01b Mon Sep 17 00:00:00 2001 From: Aditya Raut Date: Fri, 31 Jul 2026 23:34:55 +0530 Subject: [PATCH 1/4] fix(device,plugin): stop MIG usage corruption and fd leak Two independent bugs found while auditing MIG resource accounting: 1. pkg/device/nvidia/device.go: CustomFilterRule was checking MIG template/slot sizes against the raw, unresolved request.Memreq. For percentage-based memory requests (MemPercentagereq set, Memreq == 0) this comparison is always trivially true, so the filter would admit a device even when no MIG slot is actually big enough. AddResourceUsage's "fresh template" branch then had no guard for that case either: if no template fit, it fell through silently and still incremented Usedmem/Usedcores, corrupting the node's accounting. Fixed by resolving the actual memory request before calling CustomFilterRule, and by adding the same found-guard the sibling ("reuse existing slot") branch already had. 2. pkg/device-plugin/nvidiadevice/nvinternal/plugin/lock.go: the *os.File returned by os.Create in createMigApplyLock was never closed, leaking one fd per MIG apply on every ApplyMigTemplate call. Added regression tests for both accounting-corruption paths and the percentage-based MIG filter check. Signed-off-by: Aditya Raut --- .../nvidiadevice/nvinternal/plugin/lock.go | 4 +- pkg/device/nvidia/device.go | 14 ++++- pkg/device/nvidia/device_test.go | 51 +++++++++++++++++++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/pkg/device-plugin/nvidiadevice/nvinternal/plugin/lock.go b/pkg/device-plugin/nvidiadevice/nvinternal/plugin/lock.go index 25e1d1a0d5..6219898bd8 100644 --- a/pkg/device-plugin/nvidiadevice/nvinternal/plugin/lock.go +++ b/pkg/device-plugin/nvidiadevice/nvinternal/plugin/lock.go @@ -61,12 +61,12 @@ func createMigApplyLock(file string) error { klog.Infof("MIG apply lock file already exists: %s", MigApplyLockFile) return nil } - _, err := os.Create(file) + f, err := os.Create(file) if err != nil { klog.Errorf("Failed to create MIG apply lock file: %v", err) return err } - return nil + return f.Close() } // RemoveMigApplyLock removes the lock file for MIG apply operation diff --git a/pkg/device/nvidia/device.go b/pkg/device/nvidia/device.go index 57553c5210..0fe49b3486 100644 --- a/pkg/device/nvidia/device.go +++ b/pkg/device/nvidia/device.go @@ -681,6 +681,7 @@ func (dev *NvidiaGPUDevices) AddResourceUsage(pod *corev1.Pod, n *device.DeviceU n.Used++ if n.Mode == MigMode { if dev.migNeedsReset(n) { + found := false OuterLoop: for tidx, templates := range n.MigTemplate { for idx, template := range templates { @@ -700,10 +701,14 @@ func (dev *NvidiaGPUDevices) AddResourceUsage(pod *corev1.Pod, n *device.DeviceU } n.MigUsage.Index = int32(tidx) n.MigUsage.UsageList[usageListIdx].InUse = true + found = true break OuterLoop } } } + if !found { + return errors.New("mig template allocate resource fail") + } } else { found := false for idx, val := range n.MigUsage.UsageList { @@ -839,7 +844,14 @@ func (nv *NvidiaGPUDevices) Fit(devices []*device.DeviceUsage, request device.Co klog.V(5).InfoS(common.CardComputeUnitsExhausted, "pod", klog.KObj(pod), "device", dev.ID, "device index", i) continue } - if !nv.CustomFilterRule(allocated, request, tmpDevs[k.Type], dev) { + // CustomFilterRule must see the resolved memory request: for + // percentage-based requests (MemPercentagereq set, Memreq == 0), + // the raw request.Memreq is still 0, which would make its MIG + // template/slot size checks trivially pass regardless of whether + // any slot is actually big enough. + resolvedReq := request + resolvedReq.Memreq = memreq + if !nv.CustomFilterRule(allocated, resolvedReq, tmpDevs[k.Type], dev) { reason[common.CardNotFoundCustomFilterRule]++ klog.V(5).InfoS(common.CardNotFoundCustomFilterRule, "pod", klog.KObj(pod), "device", dev.ID, "device index", i) continue diff --git a/pkg/device/nvidia/device_test.go b/pkg/device/nvidia/device_test.go index b980b12155..c5094f2078 100644 --- a/pkg/device/nvidia/device_test.go +++ b/pkg/device/nvidia/device_test.go @@ -2693,6 +2693,25 @@ func TestAddResourceUsage_MigNonResetNoSlot(t *testing.T) { assert.Assert(t, strings.Contains(err.Error(), "mig template allocate resource fail")) } +func TestAddResourceUsage_MigResetNoFit(t *testing.T) { + dev := InitNvidiaDevice(NvidiaConfig{}) + usage := &device.DeviceUsage{ + Mode: MigMode, + MigTemplate: []device.Geometry{ + { + {Name: "1g.5gb", Memory: 1024, Core: 14, Count: 1}, + }, + }, + } + ctr := &device.ContainerDevice{UUID: "GPU-0", Usedmem: 4096} + err := dev.AddResourceUsage(&corev1.Pod{}, usage, ctr) + assert.Assert(t, err != nil) + assert.Assert(t, strings.Contains(err.Error(), "mig template allocate resource fail")) + // No template fit: usage counters must not reflect a phantom allocation. + assert.Equal(t, usage.Usedmem, int32(0)) + assert.Assert(t, !strings.Contains(ctr.UUID, "[")) +} + func TestCustomFilterRule_MigEmptyUsageWithTemplate(t *testing.T) { dev := InitNvidiaDevice(NvidiaConfig{}) devusage := &device.DeviceUsage{ @@ -2857,6 +2876,38 @@ func TestFit_MutexPolicy(t *testing.T) { assert.Equal(t, fit, false) } +func TestFit_MigPercentageRequestRejectsUndersizedTemplate(t *testing.T) { + config := NvidiaConfig{ + ResourceCountName: "nvidia.com/gpu", + ResourceMemoryName: "nvidia.com/gpumem", + ResourceCoreName: "nvidia.com/gpucores", + ResourceMemoryPercentageName: "nvidia.com/gpumem-percentage", + } + nv := InitNvidiaDevice(config) + + // The only MIG template offers 1024MiB slots, but the pod requests 50% + // of an 8192MiB card (= 4096MiB) via MemPercentagereq rather than an + // explicit Memreq. CustomFilterRule must be evaluated against the + // resolved 4096MiB figure, not the raw (still zero) Memreq field, or + // it would wrongly report a fit against any unused slot regardless of + // size. + devices := []*device.DeviceUsage{ + { + ID: "dev-0", Index: 0, Used: 0, Count: 1, + Totalmem: 8192, Totalcore: 100, Type: NvidiaGPUDevice, Health: true, + Mode: MigMode, + MigTemplate: []device.Geometry{ + { + {Name: "1g.5gb", Memory: 1024, Core: 14, Count: 1}, + }, + }, + }, + } + req := device.ContainerDeviceRequest{Nums: 1, MemPercentagereq: 50, Coresreq: 10, Type: NvidiaGPUDevice} + fit, _, _ := nv.Fit(devices, req, &corev1.Pod{}, &device.NodeInfo{}, &device.PodDevices{}) + assert.Equal(t, fit, false) +} + func TestFit_TopologyExactMatch(t *testing.T) { config := NvidiaConfig{ ResourceCountName: "nvidia.com/gpu", From 9cc940aa34543b5475e8d5c84382c73c5e67be7f Mon Sep 17 00:00:00 2001 From: Aditya Raut Date: Tue, 4 Aug 2026 14:39:03 +0530 Subject: [PATCH 2/4] fix(device): don't bump n.Used on failed MIG allocation AddResourceUsage incremented n.Used before checking whether a MIG template/slot actually fit the request, in both the fresh-template and reuse-slot branches. On a no-fit error, n.Used stayed bumped even though nothing was allocated, corrupting device usage accounting. Signed-off-by: Aditya Raut --- pkg/device/nvidia/device.go | 2 +- pkg/device/nvidia/device_test.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/device/nvidia/device.go b/pkg/device/nvidia/device.go index 0fe49b3486..c326c7968a 100644 --- a/pkg/device/nvidia/device.go +++ b/pkg/device/nvidia/device.go @@ -678,7 +678,6 @@ func (dev *NvidiaGPUDevices) migNeedsReset(n *device.DeviceUsage) bool { } func (dev *NvidiaGPUDevices) AddResourceUsage(pod *corev1.Pod, n *device.DeviceUsage, ctr *device.ContainerDevice) error { - n.Used++ if n.Mode == MigMode { if dev.migNeedsReset(n) { found := false @@ -728,6 +727,7 @@ func (dev *NvidiaGPUDevices) AddResourceUsage(pod *corev1.Pod, n *device.DeviceU } } } + n.Used++ n.Usedcores += ctr.Usedcores n.Usedmem += ctr.Usedmem return nil diff --git a/pkg/device/nvidia/device_test.go b/pkg/device/nvidia/device_test.go index c5094f2078..199fa7effc 100644 --- a/pkg/device/nvidia/device_test.go +++ b/pkg/device/nvidia/device_test.go @@ -2691,6 +2691,7 @@ func TestAddResourceUsage_MigNonResetNoSlot(t *testing.T) { err := dev.AddResourceUsage(&corev1.Pod{}, usage, ctr) assert.Assert(t, err != nil) assert.Assert(t, strings.Contains(err.Error(), "mig template allocate resource fail")) + assert.Equal(t, usage.Used, int32(0)) } func TestAddResourceUsage_MigResetNoFit(t *testing.T) { @@ -2709,6 +2710,7 @@ func TestAddResourceUsage_MigResetNoFit(t *testing.T) { assert.Assert(t, strings.Contains(err.Error(), "mig template allocate resource fail")) // No template fit: usage counters must not reflect a phantom allocation. assert.Equal(t, usage.Usedmem, int32(0)) + assert.Equal(t, usage.Used, int32(0)) assert.Assert(t, !strings.Contains(ctr.UUID, "[")) } From 6ad97295648baa9e0a711de097cb8942792ccc38 Mon Sep 17 00:00:00 2001 From: Aditya Raut Date: Fri, 7 Aug 2026 08:09:10 +0530 Subject: [PATCH 3/4] fix(scheduler): set LimitSet in ResourceQuota test fixtures #2313 added a LimitSet flag that FitQuota now gates on instead of Limit != 0, and updated TestFitResourceQuota's fixture accordingly, but missed three sibling tests in the same file that also build device.Quota directly: TestFitResourceQuotaNonNvidia, TestFitResourceQuotaCountsEveryDevice, and TestFitResourceQuotaAscendMemoryFactor. Their fixtures defaulted to LimitSet: false, so FitQuota treated the configured limits as unset and admitted every pod, failing the denial assertions in all three tests on current master. Set LimitSet: true on the five affected fixture entries to match what AddQuota produces, mirroring the fix already applied to TestFitResourceQuota. Signed-off-by: Aditya Raut --- pkg/scheduler/webhook_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/scheduler/webhook_test.go b/pkg/scheduler/webhook_test.go index 1ab6745089..10762221b4 100644 --- a/pkg/scheduler/webhook_test.go +++ b/pkg/scheduler/webhook_test.go @@ -486,13 +486,13 @@ func TestFitResourceQuotaNonNvidia(t *testing.T) { // One MLU vmemory unit is 256 MiB, so a limit of 100 units leaves room for // 25600 MiB. Comparing the request against the raw 100 would deny every pod. qm.Quotas["mlu-mem"] = &device.DeviceQuota{ - "cambricon.com/mlu.smlu.vmemory": &device.Quota{Used: 0, Limit: 100}, + "cambricon.com/mlu.smlu.vmemory": &device.Quota{Used: 0, Limit: 100, LimitSet: true}, } qm.Quotas["mlu-core"] = &device.DeviceQuota{ - "cambricon.com/mlu.smlu.vcore": &device.Quota{Used: 20, Limit: 50}, + "cambricon.com/mlu.smlu.vcore": &device.Quota{Used: 20, Limit: 50, LimitSet: true}, } qm.Quotas["dcu-mem"] = &device.DeviceQuota{ - "hygon.com/dcumem": &device.Quota{Used: 0, Limit: 1000}, + "hygon.com/dcumem": &device.Quota{Used: 0, Limit: 1000, LimitSet: true}, } t.Cleanup(func() { for _, ns := range []string{"mlu-mem", "mlu-core", "dcu-mem"} { @@ -590,7 +590,7 @@ func TestFitResourceQuotaCountsEveryDevice(t *testing.T) { qm := device.NewQuotaManager() // 60 units is 15360 MiB of headroom. qm.Quotas["mlu-multi"] = &device.DeviceQuota{ - "cambricon.com/mlu.smlu.vmemory": &device.Quota{Used: 0, Limit: 60}, + "cambricon.com/mlu.smlu.vmemory": &device.Quota{Used: 0, Limit: 60, LimitSet: true}, } t.Cleanup(func() { delete(qm.Quotas, "mlu-multi") }) @@ -649,7 +649,7 @@ func TestFitResourceQuotaAscendMemoryFactor(t *testing.T) { qm := device.NewQuotaManager() qm.Quotas["ascend"] = &device.DeviceQuota{ - "huawei.com/Ascend910B-memory": &device.Quota{Used: 0, Limit: 8192}, + "huawei.com/Ascend910B-memory": &device.Quota{Used: 0, Limit: 8192, LimitSet: true}, } t.Cleanup(func() { delete(qm.Quotas, "ascend") }) From 16d1b3cc775304e8638d6026c45e35f25741146b Mon Sep 17 00:00:00 2001 From: Aditya Raut Date: Fri, 7 Aug 2026 09:48:24 +0530 Subject: [PATCH 4/4] refactor(device): trim comments to one sentence per review feedback archlitchi asked for the comments around the CustomFilterRule fix and its test in device.go/device_test.go to be trimmed to one sentence. Signed-off-by: Aditya Raut --- pkg/device/nvidia/device.go | 6 +----- pkg/device/nvidia/device_test.go | 7 +------ 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/pkg/device/nvidia/device.go b/pkg/device/nvidia/device.go index c326c7968a..c7eaec0039 100644 --- a/pkg/device/nvidia/device.go +++ b/pkg/device/nvidia/device.go @@ -844,11 +844,7 @@ func (nv *NvidiaGPUDevices) Fit(devices []*device.DeviceUsage, request device.Co klog.V(5).InfoS(common.CardComputeUnitsExhausted, "pod", klog.KObj(pod), "device", dev.ID, "device index", i) continue } - // CustomFilterRule must see the resolved memory request: for - // percentage-based requests (MemPercentagereq set, Memreq == 0), - // the raw request.Memreq is still 0, which would make its MIG - // template/slot size checks trivially pass regardless of whether - // any slot is actually big enough. + // CustomFilterRule must see the resolved memory request, not the raw (possibly zero) Memreq field. resolvedReq := request resolvedReq.Memreq = memreq if !nv.CustomFilterRule(allocated, resolvedReq, tmpDevs[k.Type], dev) { diff --git a/pkg/device/nvidia/device_test.go b/pkg/device/nvidia/device_test.go index 199fa7effc..77465e444e 100644 --- a/pkg/device/nvidia/device_test.go +++ b/pkg/device/nvidia/device_test.go @@ -2887,12 +2887,7 @@ func TestFit_MigPercentageRequestRejectsUndersizedTemplate(t *testing.T) { } nv := InitNvidiaDevice(config) - // The only MIG template offers 1024MiB slots, but the pod requests 50% - // of an 8192MiB card (= 4096MiB) via MemPercentagereq rather than an - // explicit Memreq. CustomFilterRule must be evaluated against the - // resolved 4096MiB figure, not the raw (still zero) Memreq field, or - // it would wrongly report a fit against any unused slot regardless of - // size. + // The only MIG template offers 1024MiB slots, but the pod requests 4096MiB (50% of 8192MiB) via MemPercentagereq. devices := []*device.DeviceUsage{ { ID: "dev-0", Index: 0, Used: 0, Count: 1,