fix: process init containers in admission mutation and quota checks - #2168
fix: process init containers in admission mutation and quota checks#2168Gaurav-205 wants to merge 1 commit into
Conversation
Signed-off-by: Gaurav-205 <gauravkhandelwal205@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Gaurav-205 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 |
📝 WalkthroughWalkthroughThe scheduler webhook now processes init-container device resources during admission and quota evaluation. A regression test verifies admission and scheduler-name patching for a pod whose GPU request exists only in an init container. ChangesInit-container GPU accounting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AdmissionRequest
participant webhook.Handle
participant device.GetDevices
participant fitResourceQuota
AdmissionRequest->>webhook.Handle: submit Pod admission request
webhook.Handle->>device.GetDevices: mutate init-container resources
webhook.Handle->>fitResourceQuota: evaluate init-container quota requests
webhook.Handle-->>AdmissionRequest: return admission result and schedulerName patch
Possibly related issues
Possibly related PRs
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/scheduler/webhook_test.go (1)
853-856: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for init-container quota enforcement.
This fixture specifies only
hami.io/gpu; therefore the new memory/core accumulation atpkg/scheduler/webhook.goLines 170-175 remains zero and the test still passes if that quota loop is removed. Add an init-container case with GPU memory/core values and a constrained quota that verifies the expected admission result.Also applies to: 898-913
🤖 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_test.go` around lines 853 - 856, Add a test case in the existing webhook quota tests around the resource fixture and the related assertions to cover init-container GPU memory/core accumulation. Configure init-container resources with nonzero GPU memory and core values, apply a quota constrained below their accumulated usage, and assert the expected admission result so removing the init-container quota loop causes the test to fail.
🤖 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.
Nitpick comments:
In `@pkg/scheduler/webhook_test.go`:
- Around line 853-856: Add a test case in the existing webhook quota tests
around the resource fixture and the related assertions to cover init-container
GPU memory/core accumulation. Configure init-container resources with nonzero
GPU memory and core values, apply a quota constrained below their accumulated
usage, and assert the expected admission result so removing the init-container
quota loop causes the test to fail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c9410f69-10af-4a39-a447-11bb1035df91
📒 Files selected for processing (2)
pkg/scheduler/webhook.gopkg/scheduler/webhook_test.go
|
Thank you for your contribution @Gaurav-205 please check |
Summary
While
pkg/device/devices.goaccounts for bothInitContainersandContainersduring scheduling and resource allocation fitting, the mutating admission webhook (pkg/scheduler/webhook.go) and resource quota checks (fitResourceQuota) previously only iteratedpod.Spec.Containers.When a Pod requested accelerator resources (e.g.,
nvidia.com/gpuorhami.io/gpu) exclusively inside an init container:config.SchedulerName) was not assigned to the Pod.fitResourceQuota()skipped init container resource limits/requests.This PR updates
pkg/scheduler/webhook.goto iterate overpod.Spec.InitContainersin bothMutateAdmissionandfitResourceQuota(), ensuring consistent resource mutation, scheduler injection, and quota enforcement.Changes
Handle()inpkg/scheduler/webhook.goto processpod.Spec.InitContainers.fitResourceQuota()inpkg/scheduler/webhook.goto calculate init container resource requests.TestInitContainerOnlyGPUAdmissioninpkg/scheduler/webhook_test.go.Fixes #2167
Summary by CodeRabbit
New Features
Bug Fixes