[tenzen-y]

What type of PR is this?

/kind bug
/area integrations

What this PR does / why we need it:

I fixed potential atomicity problem described in #14137

Which issue(s) this PR fixes:

Fixes #14137

Special notes for your reviewer:

Does this PR introduce a user-facing change?

LeaderWorkerSet: Fixed a race when an existing Workload’s queue name and the LWS’s
kueue.x-k8s.io/admission-gated-by annotation changed during the same reconciliation. Kueue now persists both
changes atomically, preventing the Workload from entering the queue without its admission gate if the second
update is delayed or fails.

Summary by CodeRabbit

• Bug Fixes
  • Workload queue metadata and admission-gating updates are now saved together, preventing partial updates.
  • Admission-gating events are recorded only after a successful workload update.
  • Failed updates now return an error without persisting incomplete changes.

[netlify]

✅ Deploy Preview for kubernetes-sigs-kueue ready!

Name	Link
🔨 Latest commit	9bf78da
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/6a7a30b5f9854700089398cb
😎 Deploy Preview	https://deploy-preview-14143--kubernetes-sigs-kueue.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[tenzen-y]

cc @sohankunkerkar

[coderabbitai]

📝 Walkthrough

Walkthrough

The reconciler now persists workload queue and AdmissionGatedBy metadata together. Tests cover successful updates, event emission, update failures, and unchanged state after failure.

Changes

Workload metadata reconciliation

Layer / File(s)	Summary
Batch workload metadata updates pkg/controller/jobs/leaderworkerset/leaderworkerset_reconciler.go	updateWorkload combines queue-name and AdmissionGatedBy changes in one client update. It records the event after a successful update.
Validate atomic update behavior pkg/controller/jobs/leaderworkerset/leaderworkerset_reconciler_test.go	Tests verify atomic persistence, UpdatedWorkload event emission, propagated update errors, and unchanged workload state after failure. An update interceptor injects workload update failures.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• kubernetes-sigs/kueue issue 14137 — It requests the same atomic updateWorkload behavior and corresponding success and failure tests.

Possibly related PRs

• kubernetes-sigs/kueue#13931 — Applies a similar atomic AdmissionGatedBy update and post-success event pattern in another reconciler.
• kubernetes-sigs/kueue#14133 — Uses the same metadata update and event-recording pattern in another workload controller.

Suggested reviewers: mbobrovskyi, ivnovakov

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: making LeaderWorkerSet workload updates atomic.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@pkg/controller/jobs/leaderworkerset/leaderworkerset_reconciler_test.go`:
- Around line 2213-2220: Update the interceptor in the atomic Workload update
test to count every Workload Update invocation instead of failing the first one
via workloadUpdateErr. For the scenario where both queue name and
AdmissionGatedBy change, assert exactly one Workload update and retain
assertions for the resulting state; add or update the corresponding unit and
integration coverage for this controller behavior.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 85141326-f013-40db-aeb7-6d45dbb84a46

📥 Commits

Reviewing files that changed from the base of the PR and between c3ca632 and 9bf78da.

📒 Files selected for processing (2)

• pkg/controller/jobs/leaderworkerset/leaderworkerset_reconciler.go
• pkg/controller/jobs/leaderworkerset/leaderworkerset_reconciler_test.go

[mimowo]

/lgtm
/approve
/cherrypick release-0.19
/cherrypick release-0.18
Thank you for hardening LWS 👍

[kubernetes-prow]

LGTM label has been added.

Details

Git tree hash: d329b766dae060ef771f65d628d5797b8b9d598b

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mimowo, tenzen-y

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,tenzen-y]

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

[tenzen-y]

/cherrypick release-0.19
/cherrypick release-0.18

[k8s-infra-cherrypick-robot]

@tenzen-y: once the present PR merges, I will cherry-pick it on top of release-0.18, release-0.19 in new PRs and assign them to you.

Details

In response to this:

/cherrypick release-0.19
/cherrypick release-0.18

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]

@tenzen-y: new pull request created: #14144

Details

In response to this:

/cherrypick release-0.19
/cherrypick release-0.18

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]

@tenzen-y: new pull request created: #14145

Details

In response to this:

/cherrypick release-0.19
/cherrypick release-0.18

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.

[tenzen-y]

/release-note-edit

LeaderWorkerSet: Fix a bug where LWS is potentially pushed into the queue even though it has the admissionGatedBy annotation.

[mimowo]

/release-note-edit

LeaderWorkerSet: Fixed a race when an existing Workload’s queue name and the LWS’s
kueue.x-k8s.io/admission-gated-by annotation changed during the same reconciliation. Kueue now persists both
changes atomically, preventing the Workload from entering the queue without its admission gate if the second
update is delayed or fails.