enforce resource quota for all device backends - #2172
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Jay2006sawant The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughResource quota calculations are centralized in shared helpers. Scheduler admission and accelerator allocation now apply quota checks across NVIDIA and non-NVIDIA device backends. Tests cover request extraction, memory factors, allocation accounting, and quota rejection. ChangesResource quota enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant fitResourceQuota
participant FitPodQuota
participant QuotaManager
Scheduler->>fitResourceQuota: check pod quota
fitResourceQuota->>FitPodQuota: compute device-specific requests
FitPodQuota->>QuotaManager: validate memory and cores
QuotaManager-->>fitResourceQuota: return quota result
fitResourceQuota-->>Scheduler: allow or deny admission
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
bcd0850 to
e138494
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
pkg/device/quota_test.go (1)
231-296: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTests mutate the package-level
QuotaManagersingleton without restoring it. All five sites calldevice.NewQuotaManager()(or the in-packageNewQuotaManager()) and writeqm.Quotas["default"] = ...directly on the shared singleton, but onlydevice.DevicesMapis restored viat.Cleanup; the quota entry is never reset, leaving stale quota state for any later test in the same binary that reuses namespace"default".
pkg/device/quota_test.go#L231-L296: addt.Cleanupto delete/resetqm.Quotas["default"]afterTestFitAllocationQuota(and the adjacentTestFitPodQuotaMemoryFactor/TestFitPodQuotaNonNvidia).pkg/device/ascend/device_test.go#L2110-L2159: add the same cleanup forqm.Quotas["default"]inTestDevices_Fit_ResourceQuotaExceeded.pkg/device/cambricon/device_test.go#L1074-L1108: add the same cleanup inTestDevices_Fit_ResourceQuotaExceeded.pkg/device/hygon/device_test.go#L1167-L1201: add the same cleanup inTestDevices_Fit_ResourceQuotaExceeded.pkg/scheduler/webhook_test.go#L461-L499: add the same cleanup inTestFitResourceQuotaNonNvidiaExceeded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/device/quota_test.go` around lines 231 - 296, Add t.Cleanup to restore or delete qm.Quotas["default"] after each test mutates the shared quota singleton. Apply this in pkg/device/quota_test.go:231-296 for TestFitAllocationQuota and the adjacent quota tests, pkg/device/ascend/device_test.go:2110-2159, pkg/device/cambricon/device_test.go:1074-1108, pkg/device/hygon/device_test.go:1167-1201, and pkg/scheduler/webhook_test.go:461-499, preserving the existing DevicesMap cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/device/ascend/device.go`:
- Around line 520-524: The quota check in the device allocation flow should use
Ascend’s configured MemoryFactor instead of the hardcoded factor 1. Update the
FitAllocationQuota call to pass the same factor applied by
GenerateResourceRequests for both percentage-based memory requests (dev.Totalmem
* k.MemPercentagereq / 100) and normal Memreq, while preserving the existing
quota failure handling.
In `@pkg/device/hygon/device.go`:
- Around line 304-308: The quota checks do not consistently apply Hygon's memory
factor. In pkg/device/hygon/device.go lines 304-308, update the
FitAllocationQuota call in the Hygon allocation path to use the package
MemoryFactor instead of 1; in pkg/scheduler/webhook.go lines 132-137, extend
memoryFactor resolution beyond NVIDIA to include Hygon's MemoryFactor and
preserve the analogous behavior for future supported devices.
In `@pkg/device/quota.go`:
- Around line 132-138: Update FitPodQuota to immediately allow pods with no
request for the specified device, using the values returned by PodQuotaRequests
to detect zero resource usage before calling GetLocalCache().FitQuota; retain
the existing memoryFactor adjustment and quota evaluation for pods that request
the device.
---
Nitpick comments:
In `@pkg/device/quota_test.go`:
- Around line 231-296: Add t.Cleanup to restore or delete qm.Quotas["default"]
after each test mutates the shared quota singleton. Apply this in
pkg/device/quota_test.go:231-296 for TestFitAllocationQuota and the adjacent
quota tests, pkg/device/ascend/device_test.go:2110-2159,
pkg/device/cambricon/device_test.go:1074-1108,
pkg/device/hygon/device_test.go:1167-1201, and
pkg/scheduler/webhook_test.go:461-499, preserving the existing DevicesMap
cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d1472d5-4099-4327-a32c-66751bb4e290
📒 Files selected for processing (11)
pkg/device/ascend/device.gopkg/device/ascend/device_test.gopkg/device/cambricon/device.gopkg/device/cambricon/device_test.gopkg/device/hygon/device.gopkg/device/hygon/device_test.gopkg/device/nvidia/device.gopkg/device/quota.gopkg/device/quota_test.gopkg/scheduler/webhook.gopkg/scheduler/webhook_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/scheduler/webhook.go (1)
132-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDenial log doesn't identify which vendor/device caused the quota rejection.
Now that the loop checks every registered device type instead of only NVIDIA, the unchanged log line (
"... - Denying admission", nodeviceName) loses the ability to tell operators which backend's quota was exceeded. Also, the inlineif deviceName == nvidia.NvidiaGPUDevicespecial-case will need to be extended for every future vendor that needs a non-default factor; consider moving factor resolution behind theDevicesinterface/registry sofitResourceQuotastays vendor-agnostic.♻️ Include deviceName in the denial log
if !device.FitPodQuota(pod, deviceName, memoryFactor) { - klog.Infof(template+" - Denying admission", pod.Namespace, pod.Name, pod.UID) + klog.Infof(template+" - Denying admission, device %s exceeds quota", pod.Namespace, pod.Name, pod.UID, deviceName) return false }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/scheduler/webhook.go` around lines 132 - 141, Update the quota rejection log in the device-checking loop to include the failing deviceName, while preserving the existing admission-denial behavior. Keep factor selection vendor-agnostic by moving device-specific memory-factor resolution behind the Devices interface or registry rather than extending the inline NVIDIA special case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/scheduler/webhook.go`:
- Around line 133-136: Update the memoryFactor selection in fitResourceQuota to
use the configured factor for each device vendor, including Hygon and Ascend,
rather than defaulting to 1 for all non-NVIDIA devices. Reuse the existing
vendor configuration or shared interface used during resource request generation
so admission-time quota validation matches the scheduler’s authoritative check.
---
Nitpick comments:
In `@pkg/scheduler/webhook.go`:
- Around line 132-141: Update the quota rejection log in the device-checking
loop to include the failing deviceName, while preserving the existing
admission-denial behavior. Keep factor selection vendor-agnostic by moving
device-specific memory-factor resolution behind the Devices interface or
registry rather than extending the inline NVIDIA special case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f8af6780-7b5e-4081-9937-d7850095a2d2
📒 Files selected for processing (11)
pkg/device/ascend/device.gopkg/device/ascend/device_test.gopkg/device/cambricon/device.gopkg/device/cambricon/device_test.gopkg/device/hygon/device.gopkg/device/hygon/device_test.gopkg/device/nvidia/device.gopkg/device/quota.gopkg/device/quota_test.gopkg/scheduler/webhook.gopkg/scheduler/webhook_test.go
🚧 Files skipped from review as they are similar to previous changes (10)
- pkg/device/hygon/device_test.go
- pkg/device/hygon/device.go
- pkg/device/cambricon/device.go
- pkg/device/nvidia/device.go
- pkg/device/quota.go
- pkg/device/ascend/device_test.go
- pkg/scheduler/webhook_test.go
- pkg/device/cambricon/device_test.go
- pkg/device/quota_test.go
- pkg/device/ascend/device.go
There was a problem hiding this comment.
no ai disclosure, see https://github.com/Project-HAMi/HAMi/blob/master/CONTRIBUTING.md#ai-assistance-notice. fyi #2090 touches nvidia/device.go too.
| //This incurs an issue | ||
| memreq = dev.Totalmem * k.MemPercentagereq / 100 | ||
| } | ||
| if !device.FitAllocationQuota(pod.Namespace, k.Type, 1, int64(memreq), int64(k.Coresreq), tmpDevs, allocated) { |
There was a problem hiding this comment.
hygon already scales memnums by its own MemoryFactor in GenerateResourceRequests, so the hardcoded 1 here leaves the quota limit unscaled. what happens on a cluster w/ memoryFactor 10? ascend has the same issue.
| continue | ||
| for deviceName := range device.GetDevices() { | ||
| memoryFactor := int32(1) | ||
| if deviceName == nvidia.NvidiaGPUDevice { |
There was a problem hiding this comment.
this swaps one hardcoded nvidia branch for another, a MemoryFactor() on the Devices interface would cover every backend.
| }, | ||
| }, | ||
| } | ||
| qm := device.NewQuotaManager() |
There was a problem hiding this comment.
NewQuotaManager is a sync.Once singleton, so this writes to the global w/ no cleanup, unlike DevicesMap above.
DSFans2014
left a comment
DSFans2014
left a comment
There was a problem hiding this comment.
This PR adds shared quota helpers in pkg/device/quota.go and enforces namespace ResourceQuota for all registered device backends at admission time. It also adds pre-allocation quota checks in the scheduler Fit() path for Cambricon, Ascend, and Hygon, matching the existing NVIDIA behavior.
Why check all devices at admission but only Cambricon, Ascend, Hygon and NVIDIA in fit?
e30ab78 to
5c7e5f0
Compare
Signed-off-by: Jay2006sawant <jay242902@gmail.com>
5c7e5f0 to
81a5f49
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
pkg/device/mthreads/device.go (3)
97-99: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winComplete the request fallback before mutating resources.
When the GPU count comes from
ctr.Resources.Requestsandctr.Resources.Limitsis nil, the assignments below panic withassignment to entry in nil map. The same path reads memory only fromLimits, so a request-only memory value is ignored.Initialize
Limitsand fall back toRequestsfor the memory resource.Proposed fix
if ok { if count.Value() <= 0 { return false, fmt.Errorf("%s must be greater than 0", MthreadsResourceCount) } + if ctr.Resources.Limits == nil { + ctr.Resources.Limits = corev1.ResourceList{} + } if count.Value() > 1 { ... } mem, memok := ctr.Resources.Limits[corev1.ResourceName(MthreadsResourceMemory)] + if !memok { + mem, memok = ctr.Resources.Requests[corev1.ResourceName(MthreadsResourceMemory)] + }Also applies to: 110-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/device/mthreads/device.go` around lines 97 - 99, Update the resource fallback logic around the GPU count lookup to initialize ctr.Resources.Limits before any assignments when it is nil, and use ctr.Resources.Requests as the fallback source for the memory resource when no limit is present. Preserve the existing limit values when available and ensure both GPU and memory request-only paths can be written safely.
211-213: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBound memory and core quantities before narrowing them.
The current check only rejects out-of-range device counts. A large
memnumsmakesint(memnums) * 512overflow before it is returned asint32(memnum), and negativecorenumsturns into a large positiveCoresreq. Reject invalidmemnums * 512andcorenumsbefore conversion.Proposed fix
if ok { memnums, ok := mem.AsInt64() if ok { + if memnums < 0 || memnums > math.MaxInt32/512 { + return device.ContainerDeviceRequest{} + } memnum = int(memnums) * 512 } } ... if ok { corenums, ok := core.AsInt64() if ok { + if corenums < 0 || corenums > math.MaxInt32 { + return device.ContainerDeviceRequest{} + } corenum = int32(corenums) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/device/mthreads/device.go` around lines 211 - 213, Update the validation near the device quantity checks to reject negative or overflow-prone memnums before calculating memnums*512, and reject invalid corenums before narrowing it to the request’s core field. Preserve the existing invalid-device response and only perform the int32 conversions after all bounds checks pass.
100-103: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRequire an exact integer GPU count.
count.Value()rounds decimal quantities up, so0.5becomes one GPU and1.5becomes two GPUs. Usecount.AsInt64()and reject non-integral or oversized values before allocating cores, memory, or settingmthreads.com/request-gpu-num.Proposed fix
if ok { - if count.Value() <= 0 { + countValue, exact := count.AsInt64() + if !exact || countValue <= 0 || countValue > math.MaxInt32 { return false, fmt.Errorf("%s must be greater than 0", MthreadsResourceCount) } - if count.Value() > 1 { + if countValue > 1 {Use
countValuefor the later calculations and annotation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/device/mthreads/device.go` around lines 100 - 103, Update the GPU count validation in the device setup flow to use count.AsInt64(), rejecting non-integral or oversized quantities before allocating cores, memory, or setting mthreads.com/request-gpu-num. Store the validated integer as countValue and reuse it for subsequent calculations and annotation instead of count.Value().pkg/device/enflame/device.go (2)
406-408: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject negative
profile.CorePercentvalues.The new check validates only the upper bound. A negative value converts to a negative
profileCorePercent, passes the available-core check, and is stored as negativeUsedcores. Later usage accounting can decrease the recorded core usage.Proposed fix
-if profile.CorePercent > math.MaxInt32 { +if profile.CorePercent < 0 || profile.CorePercent > math.MaxInt32 {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/device/enflame/device.go` around lines 406 - 408, Update the validation around profile.CorePercent to reject values below zero as well as values above math.MaxInt32, returning the same common.ModeNotFit result for either invalid bound before converting or storing the value as profileCorePercent or Usedcores.
645-649: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject fractional values before integer conversion.
Both
float64branches accept values such as1.5and truncate them to1. This silently changes DRS capacity and custom-info values. Require an integral value before conversion inparseDRSCapacityandreadCustomInfoInt.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/device/enflame/device.go` around lines 645 - 649, Update the float64 handling in parseDRSCapacity and readCustomInfoInt to reject fractional values before converting to int32. Extend the existing NaN and int32-range validation with an integrality check, returning the current invalid-value error for non-integral inputs while preserving valid integral conversions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@pkg/device/enflame/device.go`:
- Around line 406-408: Update the validation around profile.CorePercent to
reject values below zero as well as values above math.MaxInt32, returning the
same common.ModeNotFit result for either invalid bound before converting or
storing the value as profileCorePercent or Usedcores.
- Around line 645-649: Update the float64 handling in parseDRSCapacity and
readCustomInfoInt to reject fractional values before converting to int32. Extend
the existing NaN and int32-range validation with an integrality check, returning
the current invalid-value error for non-integral inputs while preserving valid
integral conversions.
In `@pkg/device/mthreads/device.go`:
- Around line 97-99: Update the resource fallback logic around the GPU count
lookup to initialize ctr.Resources.Limits before any assignments when it is nil,
and use ctr.Resources.Requests as the fallback source for the memory resource
when no limit is present. Preserve the existing limit values when available and
ensure both GPU and memory request-only paths can be written safely.
- Around line 211-213: Update the validation near the device quantity checks to
reject negative or overflow-prone memnums before calculating memnums*512, and
reject invalid corenums before narrowing it to the request’s core field.
Preserve the existing invalid-device response and only perform the int32
conversions after all bounds checks pass.
- Around line 100-103: Update the GPU count validation in the device setup flow
to use count.AsInt64(), rejecting non-integral or oversized quantities before
allocating cores, memory, or setting mthreads.com/request-gpu-num. Store the
validated integer as countValue and reuse it for subsequent calculations and
annotation instead of count.Value().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6cfd702a-f6fd-4351-9409-cfd0fcf15cda
📒 Files selected for processing (27)
pkg/device/amd/device.gopkg/device/ascend/device.gopkg/device/ascend/device_test.gopkg/device/awsneuron/device.gopkg/device/biren/device.gopkg/device/cambricon/device.gopkg/device/cambricon/device_test.gopkg/device/devices.gopkg/device/devices_test.gopkg/device/enflame/device.gopkg/device/enflame/gcu.gopkg/device/hygon/device.gopkg/device/hygon/device_test.gopkg/device/iluvatar/device.gopkg/device/kunlun/device.gopkg/device/kunlun/vdevice.gopkg/device/metax/device.gopkg/device/metax/sdevice.gopkg/device/mthreads/device.gopkg/device/nvidia/device.gopkg/device/quota.gopkg/device/quota_test.gopkg/device/vastai/device.gopkg/scheduler/scheduler_test.gopkg/scheduler/score_test.gopkg/scheduler/webhook.gopkg/scheduler/webhook_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/device/hygon/device_test.go
- pkg/device/cambricon/device_test.go
- pkg/device/ascend/device_test.go
- pkg/device/quota.go
Signed-off-by: Jay2006sawant <jay242902@gmail.com>
|
@DSFans2014 Good catch. Admission now enforces quota for all registered devices (the #2157 bug). Fit() quota was NVIDIA-only before; I added it for Cambricon/Ascend/Hygon since they share the same vGPU allocation path. Other backends are covered at admission but not yet in Fit().happy to extend that in a follow-up if you'd like. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/device/nvidia/device_test.go`:
- Around line 1910-1915: Update TestMemoryFactor to capture the package-level
MemoryFactor before calling InitNvidiaDevice, then register t.Cleanup to restore
that value after the test. Keep the existing assertion unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b8e2481-335a-4ed4-9b71-4ddcc12bdf9e
📒 Files selected for processing (18)
pkg/device/amd/device.gopkg/device/ascend/device_test.gopkg/device/awsneuron/device.gopkg/device/biren/device.gopkg/device/cambricon/device.gopkg/device/devices.gopkg/device/enflame/device.gopkg/device/enflame/gcu.gopkg/device/hygon/device_test.gopkg/device/iluvatar/device.gopkg/device/kunlun/device.gopkg/device/kunlun/vdevice.gopkg/device/metax/device.gopkg/device/metax/sdevice.gopkg/device/mthreads/device.gopkg/device/nvidia/device_test.gopkg/device/quota_test.gopkg/device/vastai/device.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/device/hygon/device_test.go
| func TestMemoryFactor(t *testing.T) { | ||
| dev := InitNvidiaDevice(NvidiaConfig{ | ||
| ResourceCountName: "nvidia.com/gpu", | ||
| MemoryFactor: 3, | ||
| }) | ||
| assert.Equal(t, dev.MemoryFactor(), int32(3)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Restore the package-level MemoryFactor after this test.
InitNvidiaDevice changes the package-level MemoryFactor to 3. Restore the previous value with t.Cleanup; otherwise, later or parallel tests can observe state created by this test.
Restore the test state
func TestMemoryFactor(t *testing.T) {
+ oldMemoryFactor := MemoryFactor
+ t.Cleanup(func() {
+ MemoryFactor = oldMemoryFactor
+ })
dev := InitNvidiaDevice(NvidiaConfig{📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TestMemoryFactor(t *testing.T) { | |
| dev := InitNvidiaDevice(NvidiaConfig{ | |
| ResourceCountName: "nvidia.com/gpu", | |
| MemoryFactor: 3, | |
| }) | |
| assert.Equal(t, dev.MemoryFactor(), int32(3)) | |
| func TestMemoryFactor(t *testing.T) { | |
| oldMemoryFactor := MemoryFactor | |
| t.Cleanup(func() { | |
| MemoryFactor = oldMemoryFactor | |
| }) | |
| dev := InitNvidiaDevice(NvidiaConfig{ | |
| ResourceCountName: "nvidia.com/gpu", | |
| MemoryFactor: 3, | |
| }) | |
| assert.Equal(t, dev.MemoryFactor(), int32(3)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/device/nvidia/device_test.go` around lines 1910 - 1915, Update
TestMemoryFactor to capture the package-level MemoryFactor before calling
InitNvidiaDevice, then register t.Cleanup to restore that value after the test.
Keep the existing assertion unchanged.
Reminder: Answers must be written by human being. You can view the relevant rule here. |
3 participants
What type of PR is this?
/kind bug
What this PR does / why we need it:
HAMi's
QuotaManagertracks namespace memory and core usage in a device agnostic way, but quota enforcement was wired only for NVIDIA. The admission webhook skipped all non-NVIDIA devices, and only NVIDIA'sFit()path calledFitQuota()during scheduling.This PR adds shared quota helpers in
pkg/device/quota.goand enforces namespace ResourceQuota for all registered device backends at admission time. It also adds pre-allocation quota checks in the schedulerFit()path for Cambricon, Ascend, and Hygon, matching the existing NVIDIA behavior.Which issue(s) this PR fixes:
Fixes #2157
Special notes for your reviewer:
PodQuotaRequests,FitPodQuota,FitAllocationQuotafitResourceQuota()now iterates all devices inDevicesMapfitQuota()refactored to useFitAllocationQuotaFit()for Cambricon, Ascend, and Hygonmake testandmake verifypass locallyDoes this PR introduce a user-facing change?:
Yes. In multi-vendor clusters, pods requesting Cambricon, Ascend, Hygon, or other registered backends can now be denied at admission or scheduler Filter time when namespace ResourceQuota limits for vendor memory or core resources would be exceeded. Previously, only NVIDIA quota enforcement was active.
AI assistance disclosure:
I used AI assistance to understand the codebase and explore the quota enforcement flow. All code changes, tests, and verification were written and run by me. I reviewed and understand the full diff.
Summary by CodeRabbit