Fix second-pass queue cleanup when workload no longer needs retry #8431

skools-here · 2025-12-30T09:37:31Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

Fixes cleanup of the second-pass scheduling queue by ensuring workloads are removed once they no longer require a retry, while preserving the initial backoff behavior.

Which issue(s) this PR fixes:

Fixes #8357

Special notes for your reviewer:

Cleanup is gated on retry iteration to avoid prematurely canceling the initial backoff window.

Does this PR introduce a user-facing change?

Fixed an issue where workloads could remain in the second-pass scheduling queue after they no longer required a retry.

netlify · 2025-12-30T09:37:37Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`4c1698c`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/69625e00dd751400086ea1b3

k8s-ci-robot · 2025-12-30T09:37:40Z

Welcome @skools-here!

It looks like this is your first PR to kubernetes-sigs/kueue 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kueue has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2025-12-30T09:37:41Z

Hi @skools-here. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

mbobrovskyi · 2025-12-30T09:44:37Z

/ok-to-test

skools-here · 2025-12-30T10:05:31Z

/retest-required

gabesaba

Thanks for the fix! I just have a small nitpick comment

/approve

gabesaba · 2026-01-08T00:16:42Z

pkg/cache/queue/manager.go

 		})
 		return true
 	}
+	if !workload.NeedsSecondPass(w) && iteration > 0 {


nit: else-if, and add a short comment to document this branch?

Suggested change

if !workload.NeedsSecondPass(w) && iteration > 0 {

else if iteration > 0 {

// some comment, including link to #8357

k8s-ci-robot · 2026-01-08T00:18:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gabesaba, skools-here

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~~ [gabesaba]

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

gabesaba · 2026-01-08T00:18:46Z

And please update the release-note

skools-here · 2026-01-08T13:29:43Z

Hey @gabesaba I fixed the changes and added release notes also kindly review!

mimowo · 2026-01-08T14:51:30Z

pkg/cache/queue/manager.go

 // QueueSecondPassIfNeeded queues for the second pass of scheduling with exponential
 // delay.
 func (m *Manager) QueueSecondPassIfNeeded(ctx context.Context, w *kueue.Workload, iteration int) bool {
+	log := ctrl.LoggerFrom(ctx)


Could you please add some unit test for that changes in, probably in TestQueueSecondPassIfNeeded?

Thanks! I’ll add unit coverage for this case and update the PR.

gabesaba · 2026-01-08T17:33:58Z

Hey @gabesaba I fixed the changes and added release notes also kindly review!

thank you! will wait for @mimowo's comment to be addressed.

skools-here · 2026-01-09T05:19:07Z

Hey @gabesaba I fixed the changes and added release notes also kindly review!

thank you! will wait for @mimowo's comment to be addressed.

I have completed what @mimowo said kindly take a look !

skools-here · 2026-01-09T19:16:52Z

/test pull-kueue-test-e2e-main-1-35

skools-here · 2026-01-09T19:31:19Z

The e2e job pull-kueue-test-e2e-main-1-35 failed twice without a test assertion failure. Unit tests and lint are passing locally. This seems like an infra flake.

gabesaba · 2026-01-10T06:04:39Z

This seems like an infra flake.

I think so too

Error response from daemon: Head "https://registry.k8s.io/v2/jobset/jobset/manifests/v0.10.1": dial tcp 34.96.108.209:443: i/o timeout

/retest

gabesaba · 2026-01-10T06:09:42Z

pkg/cache/queue/manager_test.go

+			if name == "workload no longer needs second pass after first iteration" {
+				manager.QueueSecondPassIfNeeded(
+					ctx,
+					baseWorkloadNotNeedingSecondPass.DeepCopy(),
+					1,
+				)
+			}


This is an anti-pattern - having a special case in the shared test logic for one of the test cases.

Additionally, are we testing in this case that the new logic works? I don't see anything which changes whether the 2nd pass is needed, nor checks if it is removed from the queue.

We might need another test fixture to test this. Let me know if you need help (though I'm OOO monday)

k8s-ci-robot · 2026-01-10T14:22:35Z

@skools-here: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kueue-test-unit-main	`4c1698c`	link	true	`/test pull-kueue-test-unit-main`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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. I understand the commands that are listed here.

Fix second-pass queue cleanup when workload no longer needs retry

a7f4086

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 30, 2025

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 30, 2025

k8s-ci-robot requested review from kannon92 and mimowo December 30, 2025 09:37

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 30, 2025

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 30, 2025

Fix import formatting

0791c91

gabesaba reviewed Jan 8, 2026

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 8, 2026

Clarify second-pass queue cleanup logic

4a14fc8

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jan 8, 2026

mimowo reviewed Jan 8, 2026

View reviewed changes

Add unit test for second-pass queue cleanup

2130ed9

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 8, 2026

Fix import order for manager tests

afdbbb2

gabesaba reviewed Jan 10, 2026

View reviewed changes

queue: fix second pass queue test handling deleted workloads

3ab949e

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 10, 2026

skools-here added 2 commits January 10, 2026 14:20

fixing manager_test.go

2ad6079

Fix second-pass queue cleanup when workload no longer needs retry

4c1698c

	if !workload.NeedsSecondPass(w) && iteration > 0 {
	else if iteration > 0 {
	// some comment, including link to #8357

Fix second-pass queue cleanup when workload no longer needs retry #8431

Are you sure you want to change the base?

Fix second-pass queue cleanup when workload no longer needs retry #8431

Conversation

skools-here commented Dec 30, 2025 • edited by mimowo 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 Dec 30, 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!

k8s-ci-robot commented Dec 30, 2025

Uh oh!

k8s-ci-robot commented Dec 30, 2025

Uh oh!

mbobrovskyi commented Dec 30, 2025

Uh oh!

skools-here commented Dec 30, 2025

Uh oh!

gabesaba left a comment

Choose a reason for hiding this comment

Uh oh!

gabesaba Jan 8, 2026 • 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 Jan 8, 2026

Uh oh!

gabesaba commented Jan 8, 2026

Uh oh!

skools-here commented Jan 8, 2026

Uh oh!

mimowo Jan 8, 2026 • 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!

skools-here Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

gabesaba commented Jan 8, 2026

Uh oh!

skools-here commented Jan 9, 2026

Uh oh!

skools-here commented Jan 9, 2026

Uh oh!

skools-here commented Jan 9, 2026

Uh oh!

gabesaba commented Jan 10, 2026

Uh oh!

gabesaba Jan 10, 2026

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jan 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

skools-here commented Dec 30, 2025 •

edited by mimowo

Loading

netlify bot commented Dec 30, 2025 •

edited

Loading

gabesaba Jan 8, 2026 •

edited

Loading

mimowo Jan 8, 2026 •

edited

Loading