feat(scheduler,admission): implement in-place pod resize accounting and queue admission - #2021
Merged
Merged
Conversation
Adds design document for effective-request accounting and best-effort webhook admission for in-place pod resizes. Fixes #1906. Signed-off-by: gshaibi <gshaibi@nvidia.com>
…nd queue admission Implements the design from PR #1997: 1. Effective-request accounting (KEP-1287): - Preserve ContainerStatus.Resources and AllocatedResources in the scheduler informer compact transform (pod_transform.go) so resize state is visible to the scheduler. - Compute effective pod requests as max(spec, enacted, allocated) per container; exclude spec when PodResizePending=Infeasible so infeasible resize targets do not inflate accounting. 2. Best-effort pods/resize admission webhook: - New PodResizeValidator validates pods/resize subresource updates. - Computes delta = max(proposed - old spec, 0) and rejects if any queue on the hierarchy would exceed its CPU/memory limit (all workloads) or CPU/memory quota (non-preemptible workloads). - failurePolicy=ignore so webhook unavailability never blocks a resize. Signed-off-by: gshaibi <gshaibi@nvidia.com>
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Signed-off-by: gshaibi <gshaibi@nvidia.com>
gshaibi
marked this pull request as draft
August 6, 2026 06:40
Signed-off-by: gshaibi <gshaibi@nvidia.com>
📊 Performance Benchmark ResultsComparing PR (
|
- operator: add pods/resize webhook entry (failurePolicy=Ignore) to validatingWCForKAIConfig so the handler is reachable in production - admission: register scheduling/v2 (Queue) and scheduling/v2alpha2 (PodGroup) in the manager scheme; add RBAC markers for PodGroups, Queues, and PriorityClasses so the client can fetch them at runtime - scheduler: guard isPodResizeInfeasible with Status==ConditionTrue to avoid acting on a stale or transitional condition - admission: replace specPodRequests delta with per-resource, per-container delta using max(enacted, allocated) as the effective-old baseline; skip resources unchanged in this resize so a stale infeasible spec target does not produce a spurious delta - admission: treat quota==0 as a finite boundary (use >= 0, matching the limit check) so non-preemptible upsizes are blocked when quota is zero - scheduler: apply the KEP-1287 effective-request model to restartable init containers (native sidecars) in initContainerEffects so a sidecar downsize in progress is not undercounted Signed-off-by: gshaibi <gshaibi@nvidia.com>
…pod resize validator - Add InPlacePodResize config struct to Admission API with ValidateQuota and BlockUpsizeOnBoundedQueues fields (defaults: true / false) - Add --validate-pod-resize-quota and --block-upsize-on-bounded-queues CLI flags - Wire flags from operator buildArgsList through to PodResizeValidator constructor - validateQuota=false: early-return allows all resizes without quota checks - blockUpsizeOnBoundedQueues=true: reject any upsize on queues with finite limit (all workloads) or finite quota (non-preemptible), regardless of allocation - Add tests covering both new modes Signed-off-by: gshaibi <gshaibi@nvidia.com>
…ake validate - Fix import ordering in app.go (goimports sort) - Fix struct field alignment in options.go (gofmt) - Remove trailing blank line in effective_requests.go - Regenerate CRD with inPlacePodResize fields - Regenerate admission RBAC with queues/podgroups/priorityclasses rules Signed-off-by: gshaibi <gshaibi@nvidia.com>
|
Total coverage: 54.6% -> 54.7% (delta 0.10%) Merging this branch changes the coverage (4 decrease, 4 increase)
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
…ine, and redistribution Three bugs in podResizeDelta addressed: - Sidecar upsizes bypassed admission: now iterates restartable init containers (RestartPolicy=Always) using InitContainerStatuses, matching the scheduler's initContainerEffects accounting. - Non-infeasible old spec was excluded from baseline: for normal/Deferred/InProgress pods the queue accounts for max(spec, enacted, allocated); the webhook now mirrors this instead of always using only max(enacted, allocated). - Per-container aggregation counted redistribution as growth: moving CPU from container A to container B produced a positive delta. Switched to pod-level aggregation (sum across all containers before diffing). The new structure: accumulate newSpecSum, oldSpecSum, effectiveOldSum at the pod level; skip a resource if newSpecSum[r] == oldSpecSum[r] (unchanged by this resize); delta = max(0, newSpecSum[r] - effectiveOldSum[r]). Add three tests: sidecar denied, non-infeasible baseline, redistribution allowed. Signed-off-by: gshaibi <gshaibi@nvidia.com>
…entation-2f767b # Conflicts: # pkg/operator/operands/admission/resources.go # pkg/scheduler/cache/pod_transform.go Signed-off-by: gshaibi <gshaibi@nvidia.com>
Signed-off-by: gshaibi <gshaibi@nvidia.com>
…helpers Replaces hand-rolled KEP-1287 aggregation with upstream resource.AggregateContainerRequests(UseStatusResources: true). - delete effective_requests.go entirely (3 funcs -> upstream) - collapse getPodResourceWithoutInitContainers + initContainerEffects into a single upstream call - webhook uses resource.IsPodResizeInfeasible instead of a local copy - retarget effective-request tests at the public aggregation path KNOWN FAILURE: upstream IsPodResizeInfeasible does not check condition.Status == True, so a stale Infeasible condition with Status=False is treated as infeasible. Signed-off-by: gshaibi <gshaibi@nvidia.com>
4 tasks
Signed-off-by: gshaibi <gshaibi@nvidia.com>
…ion test Upstream keys off Reason alone and ignores condition.Status, so the Status==True guard added in #2021 does not survive the move to k8s.io/component-helpers. Accepts upstream semantics and pins the difference so a future upstream tightening surfaces as a test failure rather than a silent change. Signed-off-by: gshaibi <gshaibi@nvidia.com>
…helpers-prototype refactor(scheduler): delegate effective-request accounting to k8s.io/component-helpers
…entation-2f767b Signed-off-by: gshaibi <gshaibi@nvidia.com>
…unting_test.go The effective_requests.go source it was named after was removed when aggregation moved to k8s.io/component-helpers. The tests now cover KAI's wiring of the upstream helper through getPodResourceRequest. Signed-off-by: gshaibi <gshaibi@nvidia.com>
…cePodVerticalScaling Signed-off-by: gshaibi <gshaibi@nvidia.com>
…g hierarchy - compactResizeConditions -> compactConditions, matching sibling naming (functions are named for the field they compact); drop stale 'generation-aware' wording - document that BlockUpsizeOnBoundedQueues has no effect when ValidateQuota is false, in API comments, CLI flag help, and CRD Signed-off-by: gshaibi <gshaibi@nvidia.com>
…cated resources Queue.Status.Allocated is derived from PodGroup.Status.ResourcesStatus.Allocated, which summed raw pod spec requests. An Infeasible resize target therefore inflated queue allocated (e.g. a rejected 64-CPU resize charged 64 CPU), diverging from scheduler-internal accounting and double-counting in the resize webhook's limit check. Found by live-cluster testing. Switch to the same upstream aggregation the scheduler uses (AggregateContainerRequests): KEP-1287 effective requests for allocated (UseStatusResources), spec-only for requested. This also closes a pre-existing undercount: init-phase peak and sidecars now count toward podgroup/queue allocated, matching what the scheduler has always reserved internally. Signed-off-by: gshaibi <gshaibi@nvidia.com>
…tion, dedup capacity checks Move IsPreemptible (with its PriorityClass fallback chain: specific -> global default -> DefaultPodGroupPriority) from podgroupcontroller utilities to pkg/common/podgroup, and use it in the resize webhook instead of a divergent local resolver. The webhook assumed priority 0 for unset/missing priority classes while the controller fell back to defaults - so a pod counted in AllocatedNonPreemptible could skip the webhook's quota check entirely. The checker and the accountant now share one implementation. Merge checkLimit and checkNonPreemptibleQuota into checkCapacityBound, parameterized by bound and allocated pool. The earlier quota>0 vs limit>=0 sentinel bug was drift between these two copies. Signed-off-by: gshaibi <gshaibi@nvidia.com>
E2E suite (test/e2e/suites/resize/) covers the resize admission webhook (deny past queue limit, allow within, downsizes) and effective-request accounting end to end (infeasible resize target must not inflate Queue.status.allocated). Skips on clusters older than 1.33. Resize calls retry on conflict: the kubelet writes pod status concurrently during a resize. User docs (docs/in-place-resize/) cover the effective-request charging model, webhook semantics including best-effort fail-open behavior, and the inPlacePodResize config fields. Signed-off-by: gshaibi <gshaibi@nvidia.com>
Signed-off-by: gshaibi <gshaibi@nvidia.com>
GetConnectivity stores the calling node's context; ginkgo cancels it when the node exits, so a TestContext created in BeforeEach fails every client call from the It body with 'context canceled'. Follow the quota-suite pattern: connectivity, skips, and cleanup live inside each spec. Signed-off-by: gshaibi <gshaibi@nvidia.com>
Replace the raw admission.Handler with the generic admission.Validator[*corev1.Pod] and register through the webhook builder with WithValidatorCustomPath, matching the sibling webhooks. The custom path is still required: the canonical /validate--v1-pod path belongs to the general pod validator. Removes the manual decoder, the scheme constructor parameter, and the request-encoding helpers in tests, which now call ValidateUpdate directly. Signed-off-by: gshaibi <gshaibi@nvidia.com>
Signed-off-by: gshaibi <gshaibi@nvidia.com>
…erred charge as a contract Generation-aware Infeasible was rejected during implementation (the observedGeneration field requires a non-GA gate; upstream reason-only semantics adopted and pinned by a characterization test) - update the design to match. Note that the Deferred max(spec, actual) charge is a dependency of deferred-resize eviction (#1872 / #2051): the reserved target prevents backfill thrash after victim eviction. Signed-off-by: gshaibi <gshaibi@nvidia.com>
itsomri
reviewed
Aug 16, 2026
itsomri
reviewed
Aug 16, 2026
itsomri
reviewed
Aug 16, 2026
…emantics Review feedback: the resize webhook's rules are disjoint from the pod validator's (pods/resize subresource only, no double validation) and the failure policies differ, which is why it is a separate endpoint. Define best-effort (fail-open) in the flag help. Signed-off-by: gshaibi <gshaibi@nvidia.com>
Signed-off-by: gshaibi <gshaibi@nvidia.com>
Signed-off-by: gshaibi <gshaibi@nvidia.com>
davidLif
reviewed
Aug 17, 2026
davidLif
left a comment
davidLif
left a comment
Collaborator
There was a problem hiding this comment.
ReclaimUnschedulableDistributedJob - degredation. This might be due to AggregateContainerRequests
Signed-off-by: gshaibi <gshaibi@nvidia.com>
Follow the DescribeAllocateElasticSpecs shape: Ordered container, queues and skips once in BeforeAll, TestContextCleanup in AfterEach, ClusterCleanup in AfterAll. Add feature_flags.SetInPlacePodResizeValidation to patch the kai config's inPlacePodResize settings (with admission rollout wait) and pin validateQuota=true for the suite, restoring defaults in AfterAll. Signed-off-by: gshaibi <gshaibi@nvidia.com>
…n comment Signed-off-by: gshaibi <gshaibi@nvidia.com>
Member
Author
I've rerun the test and it looks ok now, probably was just a hiccup. |
itsomri
reviewed
Aug 19, 2026
itsomri
reviewed
Aug 19, 2026
itsomri
reviewed
Aug 19, 2026
itsomri
reviewed
Aug 19, 2026
itsomri
reviewed
Aug 19, 2026
itsomri
reviewed
Aug 19, 2026
itsomri
previously approved these changes
Aug 19, 2026
gshaibi
force-pushed
the
claude/pr-1997-implementation-2f767b
branch
from
August 22, 2026 19:43
c9971a9 to
022046f
Compare
gshaibi
enabled auto-merge
August 23, 2026 12:59
itsomri
approved these changes
Aug 24, 2026
Collaborator
📝 Changelog fragment recordedThanks! This PR added the changelog fragment(s) below. Pending fragments are folded into |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Implements the design from #1997 (research issue #1906).
Effective-request accounting (KEP-1287 / in-place pod vertical scaling)
When Kubernetes performs an in-place pod resize, the pod may temporarily be running with different resources than its spec declares. This PR makes the scheduler charge for the effective resource consumption rather than the raw spec:
pkg/scheduler/cache/pod_transform.go— PreservesContainerStatus.AllocatedResourcesandContainerStatus.Resources(enacted resources) in the informer compact transform so resize state is visible to the scheduler without increasing informer memory footprint unnecessarily. Also retains thePodResizePendingcondition.pkg/scheduler/api/pod_info/effective_requests.go— Implements the effective-request model:max(spec, enacted, allocated)per container/resource for normal/Deferred/InProgress resizes;max(enacted, allocated)whenPodResizePending=Infeasible(so an infeasible target does not inflate accounting).pkg/scheduler/api/pod_info/pod_info.go—getPodResourceWithoutInitContainersnow uses the effective-request model.Best-effort
pods/resizeadmission webhookA new validating webhook rejects resize requests that would push a queue over its configured limits, without blocking legitimate resizes when the webhook is unavailable:
pkg/admission/webhook/v1alpha2/podhooks/pod_resize_validator.go— Computesdelta = max(proposed - old spec, 0)and walks the queue hierarchy. Rejects if any queue would exceed its CPU/memoryLimit(all workloads) orQuota(non-preemptible workloads only). Best-effort: lookup failures (PodGroup, Queue, PriorityClass) allow the resize.cmd/admission/app/app.go— Registers the webhook at/validate--v1-pod-resizewithfailurePolicy=ignoreso webhook unavailability never blocks a legitimate resize.Related Issues
Implements the design from #1997 (research issue #1906).
Related: #1894 (addresses the resize-status fragment), #1872 (deferred resize eviction — separate track, #2051).
Checklist
make changelog(or applied theskip-changeloglabel). Do not editCHANGELOG.mddirectly — pending fragments are folded into it at release time.Breaking Changes
None.
Additional Notes
The design document added by the first commit in this branch (
docs: propose in-place pod resize accounting and queue admission) describes the design rationale in full. The commits in this PR implement that design.Test coverage:
pkg/scheduler/api/pod_info/effective_requests_test.go— 7 tests covering: no-status fallback, normal upsize uses max, downsize-in-progress uses enacted, infeasible excludes spec, multiple containers,isPodResizeInfeasible,maxResourceListpkg/admission/webhook/v1alpha2/podhooks/pod_resize_validator_test.go— 9 tests covering: non-KAI pod allowed, no PodGroup allowed, downsize allowed, CPU limit exceeded denied, CPU limit not exceeded allowed, memory limit exceeded denied, unlimited limit allowed, non-preemptible quota exceeded denied, preemptible-only quota exceeded allowed