feat(KEP-3258): implement delayed admission check retries #7620

sohankunkerkar · 2025-11-12T17:13:09Z

Implements delayed retry mechanism for admission checks to prevent overwhelming external controllers and reduce control plane churn.

Problem:
Previously, when admission checks transitioned to Retry state, Kueue would immediately evict workloads and requeue them, causing excessive load on admission check controllers and unnecessary API server churn, particularly when retry conditions persisted predictably.

Solution:
This implementation adds two new fields to AdmissionCheckState:

requeueAfterSeconds: Specifies minimum wait time before retry
retryCount: Tracks retry attempts per admission check

Key Changes:

API Changes

Added requeueAfterSeconds and retryCount fields to AdmissionCheckState
Added +kubebuilder:validation:Minimum=0 to both fields (v1beta1 and v1beta2)

Controller Changes

Auto-increments retryCount on transition to Retry state
Calculates maximum retry time across all admission checks
Updates workload.status.requeueState.requeueAt with the maximum delay
Workload controller respects delayed retry times
RequeueAfterSeconds values persist across evictions
Refactored backoff calculation to use wait.NewBackoff() pattern

Behavior:
When multiple admission checks specify different delays, Kueue uses the maximum delay across all checks. Workloads are evicted immediately to release quota, but admission checks maintain their requeueAfterSeconds values, preventing race conditions where fast-responding checks could block slower ones from registering their delays.

Refs: KEP-3258
https://github.com/kubernetes-sigs/kueue/blob/main/keps/3258-delayed-admission-check-retries/README.md

What type of PR is this?

/kind feature
/kind api-change

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #3258

Special notes for your reviewer:

After discussing with @mimowo, taking over #7370 as the original author is on PTO.

Does this PR introduce a user-facing change?

AdmissionChecks: introduce new optional fields in the workload status for admission checks to control the delay by 
external and internal admission check controllers:
* requeueAfterSeconds: specifies minimum wait time before retry
* retryCount: Tracks retry attempts per admission check

netlify · 2025-11-12T17:13:40Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`5a7d9a4`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/6918c5c31e1de600082b5d4d

mimowo

LGTM overall, thank you 👍

pkg/controller/core/workload_controller.go

pkg/workload/workload.go

test/integration/singlecluster/controller/admissionchecks/provisioning/provisioning_test.go

pkg/controller/core/workload_controller_test.go

pkg/workload/workload.go

test/integration/singlecluster/scheduler/podsready/scheduler_test.go

mimowo

LGTM overall, left a couple comments, once this is done please follow up with refactoring of ProvisioningRequest to use the new API, this is already started by @dhenkel92 here: https://github.com/kubernetes-sigs/kueue/pull/7464/files#diff-d5b88e9a8af6b97ce61788f1307ec9ba1f4e3581a9c0634a151ce310c9ca3d91

pkg/controller/core/workload_controller.go

mimowo · 2025-11-14T07:52:46Z

pkg/controller/core/workload_controller.go

+					if wl.Status.RequeueState != nil {
+						wl.Status.RequeueState.RequeueAt = nil
+					}
+					return &wl, true, nil


Now we can also add workload.SetRequeuedCondition(&wl, kueue.WorkloadBackoffFinished, "The workload backoff was finished", true)

It is probably a follow up for later, but I would like to consolidate the two requests (here & pre-existing below) into one. They are really the same thing conceptually.

Still, this would mean a bigger diff so I'm ok as is for now.

Please also extend one of the test asserting to check that the Requeued=true condition is set.

test/integration/singlecluster/scheduler/delayedadmission/delayed_admission_test.go

test/integration/singlecluster/scheduler/podsready/scheduler_test.go

mimowo · 2025-11-14T17:25:23Z

pkg/controller/core/workload_controller.go

-		if wl.Status.RequeueState != nil {
-			wl.Status.RequeueState = nil
+		if wl.Status.RequeueState != nil && wl.Status.RequeueState.RequeueAt != nil {
+			// Clear the requeue schedule while preserving the Count for historical tracking


I think we also want to clear the Count as before. This is because we re-activation is a manual process in spec, and if we don't clear the Count, then such a workload will be automatically put back as Deactivated, which is not good experience. So the user would need to clear the Count on their own.

This makes me think we should also clear admissionChecks.RetryCount fields on deactivation

Yeah, good catch! Just to sum up here-> we need to clear: RequeueState.Count, RequeueState.RequeueAt, all AdmissionChecks[].RetryCount fields, and all AdmissionChecks[].RequeueAfterSeconds fields.
Having said that, we can't set RequeueState = nil because that causes the validation error with SSA. We need to
clear the fields individually.

Having said that, we can't set RequeueState = nil because that causes the validation error with SSA. We need to
clear the fields individually.

Hm, ok, but this is surprising because the pre-existing code would do

if wl.Status.RequeueState != nil { wl.Status.RequeueState = nil }

didn't work on the current releases?

@mimowo Yeah, I made minimal changes to make this work. Let me know how you feel about this one?

This looks ok. Could you please also prepare a small PR with just this fix, and dedicated test.
I think if this didn't work in the past it is something where we should cherrypick the fix.

Ah, actually after you modified the ProvisioningRequest controller the fix is no longer needed. I think the fix was needed because the ProvRequest controller was changing the RequeueState using another admission manager.

pkg/controller/core/workload_controller.go

Implements delayed retry mechanism for admission checks to prevent overwhelming external controllers and reduce control plane churn. Problem: Previously, when admission checks transitioned to Retry state, Kueue would immediately evict workloads and requeue them, causing excessive load on admission check controllers and unnecessary API server churn, particularly when retry conditions persisted predictably. Solution: This implementation adds two new fields to AdmissionCheckState: - requeueAfterSeconds: Specifies minimum wait time before retry - retryCount: Tracks retry attempts per admission check Key Changes: API Changes - Added requeueAfterSeconds and retryCount fields to AdmissionCheckState - Added +kubebuilder:validation:Minimum=0 to both fields (v1beta1 and v1beta2) Controller Changes - Auto-increments retryCount on transition to Retry state - Calculates maximum retry time across all admission checks - Updates workload.status.requeueState.requeueAt with the maximum delay - Workload controller respects delayed retry times - RequeueAfterSeconds values persist across evictions - Refactored backoff calculation to use wait.NewBackoff() pattern Behavior: When multiple admission checks specify different delays, Kueue uses the maximum delay across all checks. Workloads are evicted immediately to release quota, but admission checks maintain their requeueAfterSeconds values, preventing race conditions where fast-responding checks could block slower ones from registering their delays. Refs: KEP-3258 https://github.com/DataDog/kueue/blob/main/keps/3258-delayed-admission-check-retries/README.md Signed-off-by: Sohan Kunkerkar <[email protected]> Co-authored-by: Daniel Henkel <[email protected]>

This change replaces the use of the shared RequeueState field with the new mechanism in the preprovision request admission check. Signed-off-by: Sohan Kunkerkar <[email protected]> Co-authored-by: Daniel Henkel <[email protected]>

mimowo · 2025-11-17T10:17:31Z

pkg/workload/admissionchecks.go

+	backoff := wait.NewBackoff(time.Duration(backoffBaseSeconds)*time.Second, time.Duration(backoffMaxSeconds)*time.Second, 2, 0.0001)
+	waitDuration := backoff.WaitTime(int(requeuingCount))
+
+	acState.RequeueAfterSeconds = ptr.To(int32(waitDuration.Truncate(time.Second).Seconds()))


We should also increment the RetryCount for ProvisioningRequest for informational purposes, even if we are not using that information directly.

EDIT: Ah I see it is already bumped here: https://github.com/kubernetes-sigs/kueue/pull/7620/files#diff-c703a3b5f20a31c414c5cafc7df771810f8e39ba1e8658d8b9f897bcb8af659cR95-R107

And this assert shows it gets set: https://github.com/kubernetes-sigs/kueue/pull/7620/files#diff-b524dd7e94e695c9007bf094b1ff9cd5a94952214c0db7de004c7ce5bea7ed1bR924-L924

And then incremented: https://github.com/kubernetes-sigs/kueue/pull/7620/files#diff-b524dd7e94e695c9007bf094b1ff9cd5a94952214c0db7de004c7ce5bea7ed1bL1351-R1368

mimowo

/lgtm
/approve
Please also address the small follow ups:

~~thread to use RetryCount for ProvReq controller for informational purposes~~
thread to simplify the code after the ProvReq controller is updated

k8s-ci-robot · 2025-11-17T10:19:03Z

LGTM label has been added.

Details

Git tree hash: 8081cc7c81df3b68b2123ffc41c4a3c9b508d39d

k8s-ci-robot · 2025-11-17T10:19:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mimowo, sohankunkerkar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [mimowo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…-sigs#7620) * feat(KEP-3258): implement delayed admission check retries Implements delayed retry mechanism for admission checks to prevent overwhelming external controllers and reduce control plane churn. Problem: Previously, when admission checks transitioned to Retry state, Kueue would immediately evict workloads and requeue them, causing excessive load on admission check controllers and unnecessary API server churn, particularly when retry conditions persisted predictably. Solution: This implementation adds two new fields to AdmissionCheckState: - requeueAfterSeconds: Specifies minimum wait time before retry - retryCount: Tracks retry attempts per admission check Key Changes: API Changes - Added requeueAfterSeconds and retryCount fields to AdmissionCheckState - Added +kubebuilder:validation:Minimum=0 to both fields (v1beta1 and v1beta2) Controller Changes - Auto-increments retryCount on transition to Retry state - Calculates maximum retry time across all admission checks - Updates workload.status.requeueState.requeueAt with the maximum delay - Workload controller respects delayed retry times - RequeueAfterSeconds values persist across evictions - Refactored backoff calculation to use wait.NewBackoff() pattern Behavior: When multiple admission checks specify different delays, Kueue uses the maximum delay across all checks. Workloads are evicted immediately to release quota, but admission checks maintain their requeueAfterSeconds values, preventing race conditions where fast-responding checks could block slower ones from registering their delays. Refs: KEP-3258 https://github.com/DataDog/kueue/blob/main/keps/3258-delayed-admission-check-retries/README.md Signed-off-by: Sohan Kunkerkar <[email protected]> Co-authored-by: Daniel Henkel <[email protected]> * pkg: use delayed ACs in preprovisioning check This change replaces the use of the shared RequeueState field with the new mechanism in the preprovision request admission check. Signed-off-by: Sohan Kunkerkar <[email protected]> Co-authored-by: Daniel Henkel <[email protected]> --------- Signed-off-by: Sohan Kunkerkar <[email protected]> Co-authored-by: Daniel Henkel <[email protected]>

mimowo · 2025-11-28T08:58:18Z

/remove-kind api-change
/kind feature

/release-note-edit

AdmissionChecks: introduce new optional fields in the workload status for admission checks to control the delay by 
external and internal admission check controllers:
* requeueAfterSeconds: specifies minimum wait time before retry
* retryCount: Tracks retry attempts per admission check

k8s-ci-robot requested review from kannon92 and tenzen-y November 12, 2025 17:13

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 12, 2025

sohankunkerkar mentioned this pull request Nov 12, 2025

feat(KEP-3258): implement delayed admission check retries #7370

Closed

sohankunkerkar changed the title ~~feat(KEP-3258): implement delayed admission check retries~~ [WIP] feat(KEP-3258): implement delayed admission check retries Nov 12, 2025

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 12, 2025

sohankunkerkar force-pushed the pr-7370 branch 10 times, most recently from 5cd940c to 5b6df07 Compare November 13, 2025 15:45

sohankunkerkar changed the title ~~[WIP] feat(KEP-3258): implement delayed admission check retries~~ feat(KEP-3258): implement delayed admission check retries Nov 13, 2025

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 13, 2025

mimowo reviewed Nov 13, 2025

View reviewed changes

sohankunkerkar force-pushed the pr-7370 branch 3 times, most recently from eb9934d to 1cd7565 Compare November 14, 2025 00:09

mimowo reviewed Nov 14, 2025

View reviewed changes

sohankunkerkar force-pushed the pr-7370 branch 2 times, most recently from 540f431 to 92e8f4b Compare November 14, 2025 15:10

sohankunkerkar force-pushed the pr-7370 branch from d59062d to a77160a Compare November 14, 2025 15:49

sohankunkerkar mentioned this pull request Nov 14, 2025

[WIP] Fix: Set Requeued condition for standalone workloads during admission check eviction #7673

Open

sohankunkerkar force-pushed the pr-7370 branch from a77160a to 943629b Compare November 14, 2025 17:18

mimowo reviewed Nov 14, 2025

View reviewed changes

pkg/controller/core/workload_controller.go Outdated Show resolved Hide resolved

sohankunkerkar force-pushed the pr-7370 branch 8 times, most recently from 51b4465 to 0f11ac7 Compare November 15, 2025 18:24

sohankunkerkar and others added 2 commits November 15, 2025 13:25

pkg: use delayed ACs in preprovisioning check

5a7d9a4

This change replaces the use of the shared RequeueState field with the new mechanism in the preprovision request admission check. Signed-off-by: Sohan Kunkerkar <[email protected]> Co-authored-by: Daniel Henkel <[email protected]>

sohankunkerkar force-pushed the pr-7370 branch from 0f11ac7 to 5a7d9a4 Compare November 15, 2025 18:26

mimowo reviewed Nov 17, 2025

View reviewed changes

k8s-ci-robot assigned mimowo Nov 17, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 17, 2025

k8s-ci-robot merged commit 57f960e into kubernetes-sigs:main Nov 17, 2025
22 checks passed

k8s-ci-robot added this to the v0.15 milestone Nov 17, 2025

This was referenced Nov 18, 2025

Fix RequeueState clearing when backoff finishes #7734

Merged

feat(KEP-3258): implement delayed admission check retries #7464

Closed

k8s-ci-robot removed the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Nov 28, 2025

feat(KEP-3258): implement delayed admission check retries #7620

feat(KEP-3258): implement delayed admission check retries #7620

Uh oh!

Conversation

sohankunkerkar commented Nov 12, 2025 • edited by k8s-ci-robot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

netlify bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Uh oh!

mimowo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mimowo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mimowo Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mimowo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Nov 17, 2025

Uh oh!

k8s-ci-robot commented Nov 17, 2025

Uh oh!

Uh oh!

mimowo commented Nov 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sohankunkerkar commented Nov 12, 2025 •

edited by k8s-ci-robot

Loading

netlify bot commented Nov 12, 2025 •

edited

Loading

mimowo Nov 17, 2025 •

edited

Loading

mimowo left a comment •

edited

Loading