Introduce WithLooseOnApply option for PatchAdmissionStatus #7246

mszadkow · 2025-10-13T12:51:33Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

As stated here

enabling the feature risks dropping some conditions set on the Workload by a controller, or user.

Thus to lower the risk to minimum we introduce an option that preserves strict setting for SSA.
That keeps the functionality unchanged when used with WorkloadRequestUseMergePatch disabled and for enabled let scheduler be more strict on patch and let it retry on the next scheduling cycle.

Which issue(s) this PR fixes:

Relates to #7035

Special notes for your reviewer:

Does this PR introduce a user-facing change?

WorkloadRequestUseMergePatch: use "strict" mode for admission patches during scheduling which
sends the ResourceVersion of the workload being admitted for comparing by kube-apiserver. 
This fixes the race-condition issue that Workload conditions added concurrently by other controllers
could be removed during scheduling.

netlify · 2025-10-13T12:51:41Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`d64dff1`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/68ee35ba2970260007cffd08

mimowo

LGTM overall, please also open another "test" PR which turns the feature gate as "true" so that we can make sure it works. We could then merge that PR in a follow up.

mimowo · 2025-10-13T12:56:33Z

pkg/scheduler/scheduler.go

 		err := workload.PatchAdmissionStatus(ctx, s.client, origWorkload, s.clock, func() (*kueue.Workload, bool, error) {
 			return newWorkload, true, nil
-		}, workload.WithLoose())
+		}, workload.WithLooseOnApply())


I think this should also be used here:

kueue/pkg/scheduler/scheduler.go

Line 317 in 8749fd8

}, workload.WithLoose()); err != nil {

Do we have an integration or e2e test which covers that?

I am working on unit tests, but sure I can work on integration test as well.

Maybe we already have an integration test, you can check by enabling the feature gate, and say injecting a panic in that place.

I think I found one that fails from time to time because of the change:
Scheduler Fair Sharing Suite: [It] Scheduler when ClusterQueue head has inadmissible workload sticky workload deleted, next workload can admit

This one flakes not only on this PR: #7250

I was asking if we have an integration test already which exercises the other use of PatchAdmissionStatus in schedule

kueue/pkg/scheduler/scheduler.go

Line 317 in 8749fd8

}, workload.WithLoose()); err != nil {

And in my first comment I also referenced the other place to update for consistency, as both are from scheduler.

Nice, does it flake if you run the tests in a loop with the changes in this PR, plus enabling the feature gate?

Testing now

Please summarize the results of the testing.

A successful with flag enabled run of:
ginkgo --json-report ./ginkgo.report -focus "SchedulerWithWaitForPodsReady" -r --race --procs=4 --repeat=20
No failures.

mimowo · 2025-10-13T12:57:43Z

Please also add a release note

pkg/scheduler/scheduler_test.go

pkg/workload/workload.go

mimowo · 2025-10-15T13:05:28Z

Thanks 👍
/lgtm
/approve
/cherrypick release-0.14

k8s-infra-cherrypick-robot · 2025-10-15T13:05:31Z

@mimowo: once the present PR merges, I will cherry-pick it on top of release-0.14 in a new PR and assign it to you.

In response to this:

Thanks 👍
/lgtm
/approve
/cherrypick release-0.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot · 2025-10-15T13:05:39Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

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

k8s-ci-robot · 2025-10-15T13:05:39Z

LGTM label has been added.

Git tree hash: a15d5e4b1bb22cc8643db3e0d1042f3cca620f39

mimowo · 2025-10-15T13:10:38Z

We need to adjust the release note to be user-oriented
/release-note-edit

WorkloadRequestUseMergePatch: use "strict" for admission patches during scheduling by comparing 
ResourceVersion of the workload. This prevents occasional dropping of conditions added by other
controllers.

mimowo · 2025-10-15T13:11:50Z

/release-note-edit

WorkloadRequestUseMergePatch: use "strict" mode for admission patches during scheduling which
sends the ResourceVersion of the workload being admitted for comparing by kube-apiserver. 
This prevents occasional dropping of conditions added by other controllers.

mimowo · 2025-10-15T13:12:38Z

/remove-kind feature
/kind bug

mimowo · 2025-10-15T13:14:47Z

/release-note-edit

WorkloadRequestUseMergePatch: use "strict" mode for admission patches during scheduling which
sends the ResourceVersion of the workload being admitted for comparing by kube-apiserver. 
This fixes the race-condition issue that Workload conditions added concurrently by other controllers
could be removed during scheduling.

mimowo · 2025-10-15T13:34:15Z

/cherrypick release-0.14

k8s-infra-cherrypick-robot · 2025-10-15T13:34:18Z

@mimowo: new pull request created: #7279

In response to this:

Thanks 👍
/lgtm
/approve
/cherrypick release-0.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-infra-cherrypick-robot · 2025-10-15T13:34:53Z

@mimowo: new pull request could not be created: failed to create pull request against kubernetes-sigs/kueue#release-0.14 from head k8s-infra-cherrypick-robot:cherry-pick-7246-to-release-0.14: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for k8s-infra-cherrypick-robot:cherry-pick-7246-to-release-0.14."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request","status":"422"}

In response to this:

/cherrypick release-0.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

mimowo · 2025-10-15T13:35:13Z

@mszadkow please prepare the cherrypick manually

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 13, 2025

k8s-ci-robot requested review from mimowo and tenzen-y October 13, 2025 12:51

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 13, 2025

mimowo reviewed Oct 13, 2025

View reviewed changes

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 13, 2025

mimowo reviewed Oct 13, 2025

View reviewed changes

pkg/scheduler/scheduler_test.go Show resolved Hide resolved

mszadkow mentioned this pull request Oct 14, 2025

[Cleanup] Add scheduler test with WorkloadRequestUseMergePatch enabled #7256

Merged

mszadkow force-pushed the feat/7035-WorkloadRequestUseMergePatch-to-beta branch from 952183b to 3013cf0 Compare October 14, 2025 09:43

mimowo reviewed Oct 14, 2025

View reviewed changes

pkg/workload/workload.go Outdated Show resolved Hide resolved

mimowo reviewed Oct 14, 2025

View reviewed changes

pkg/workload/workload.go Show resolved Hide resolved

mszadkow force-pushed the feat/7035-WorkloadRequestUseMergePatch-to-beta branch from 3013cf0 to 6a7a46c Compare October 14, 2025 11:28

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2025

mszadkow force-pushed the feat/7035-WorkloadRequestUseMergePatch-to-beta branch from 6a7a46c to 0543121 Compare October 14, 2025 11:31

mimowo reviewed Oct 14, 2025

View reviewed changes

pkg/workload/workload.go Outdated Show resolved Hide resolved

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2025

mszadkow added 2 commits October 14, 2025 13:36

Introduce WithLooseOnApply option for PatchAdmissionStatus

1409bf4

Adjust scheduler unit tests

d64dff1

mszadkow force-pushed the feat/7035-WorkloadRequestUseMergePatch-to-beta branch from 0543121 to d64dff1 Compare October 14, 2025 11:36

k8s-ci-robot assigned mimowo Oct 15, 2025

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

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

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. and removed kind/feature Categorizes issue or PR as related to a new feature. labels Oct 15, 2025

k8s-ci-robot merged commit a4a39cf into kubernetes-sigs:main Oct 15, 2025
23 checks passed

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

k8s-infra-cherrypick-robot mentioned this pull request Oct 15, 2025

[release-0.14] Introduce WithLooseOnApply option for PatchAdmissionStatus #7279

Merged

mszadkow deleted the feat/7035-WorkloadRequestUseMergePatch-to-beta branch October 15, 2025 13:48

Introduce WithLooseOnApply option for PatchAdmissionStatus #7246

Introduce WithLooseOnApply option for PatchAdmissionStatus #7246

Uh oh!

Conversation

mszadkow commented Oct 13, 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 Oct 13, 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!

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!

mimowo Oct 14, 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!

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!

mszadkow Oct 15, 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!

Choose a reason for hiding this comment

Uh oh!

mimowo commented Oct 13, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mimowo commented Oct 15, 2025

Uh oh!

k8s-infra-cherrypick-robot commented Oct 15, 2025

Uh oh!

k8s-ci-robot commented Oct 15, 2025

Uh oh!

k8s-ci-robot commented Oct 15, 2025

Uh oh!

mimowo commented Oct 15, 2025

Uh oh!

mimowo commented Oct 15, 2025

Uh oh!

mimowo commented Oct 15, 2025

Uh oh!

mimowo commented Oct 15, 2025

Uh oh!

Uh oh!

mimowo commented Oct 15, 2025

Uh oh!

k8s-infra-cherrypick-robot commented Oct 15, 2025

Uh oh!

k8s-infra-cherrypick-robot commented Oct 15, 2025

Uh oh!

mimowo commented Oct 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

mszadkow commented Oct 13, 2025 •

edited by k8s-ci-robot

Loading

netlify bot commented Oct 13, 2025 •

edited

Loading

mimowo Oct 14, 2025 •

edited

Loading

mszadkow Oct 15, 2025 •

edited

Loading