Add StatefulSet as owner of Workload. #4799

mbobrovskyi · 2025-03-26T12:24:42Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

Add StatefulSet as owner of Workload.

Which issue(s) this PR fixes:

Fixes #4342
Fixes #4805
Fixes #6883
Fixes #7366

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Fixed the following bugs for the StatefulSet integration by ensuring the Workload object
has the ownerReference to the StatefulSet:
1. Kueue doesn't keep the StatefulSet as deactivated
2. Kueue marks the Workload as Finished if all StatefulSet's Pods are deleted
3. changing the "queue-name" label could occasionally result in the StatefulSet getting stuck

k8s-ci-robot · 2025-03-26T12:24:45Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

netlify · 2025-03-26T12:25:02Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`367ac58`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/6931169bda24f3000840cb1b

mbobrovskyi · 2025-03-26T13:46:21Z

/cc @mimowo

test/e2e/singlecluster/statefulset_test.go

mimowo · 2025-03-26T15:20:46Z

pkg/controller/jobs/statefulset/statefulset_reconciler.go

+		shouldUpdate = true
+		err = controllerutil.RemoveOwnerReference(sts, wl, r.client.Scheme())


Can you describe why we remove the owner reference from the workload on scaling down the StatefulSet?

IIUC this will result in deleting the workload, right? It seems safer to just update the count in the workload object, similarly as we would do for Jobs. For example, when we suspend a Job we don't delete the workload IIRC.

For example, when we suspend a Job we don't delete the workload IIRC.

It is a very interesting behavior in Job. I tried to suspend the job manually, but it automatically unsuspends itself. The only way to suspend the job is to deactivate the workload.

Can you describe why we remove the owner reference from the workload on scaling down the StatefulSet?

The problem is that after activation, the replicas can change (replicas=3 → replicas=0 → replicas=5). Our workload is admitted, and we can't change the PodSet count since this field is immutable. I think it's much easier to just delete it.

IIUC this will result in deleting the workload, right?

Yeah, you right. It should delete workload.

Yeah, for Job when the count is updated, then we update the count in workload.

So, my natural preference is to also update the count in the workload for STS, not sure it is much more complex. I'm not sure myself - considering the options.

The problem is that after activation, the replicas can change (replicas=3 → replicas=0 → replicas=5). Our workload is admitted, and we can't change the PodSet count since this field is immutable. I think it's much easier to just delete it.

I see, but to me it suggests we should also evict the workload for STS in that case, to be consistent with Job.
Can you try to prototype, maybe in a separate PR so that we can compare. I don't think we need to rush with decisions. I would prefer to do it well.

Thank you for the investigation and answers. The PR looks good assuming we take the approach of deleting the workload, but I would prefer to explore keeping and updating the workload, as we do for Jobs.

mimowo · 2025-03-27T06:12:51Z

/hold
to avoid premature merging, I need more time to decide on the approach.

mimowo · 2025-03-27T06:28:09Z

pkg/controller/jobs/pod/pod_controller.go

 // Finished means whether the job is completed/failed or not,
 // condition represents the workload finished condition.
 func (p *Pod) Finished() (message string, success, finished bool) {
+	if p.isServing() {


I think this is a bugfix for an issue on its own. Let me open so that we can fix issues one-by-one rather than multiple in one PR, because then some overlapping issues make it hard to reason about the solution. Solving one-by-one will also allow us to build a good test coverage. Opened: #4805

I can move these changes to a separate PR, but I'm not sure how to test them yet. We still need the the OwnerReference of the StatefulSet to prevent it from being deleted.

mbobrovskyi · 2025-08-16T11:10:12Z

/remove-lifecycle rotten

mimowo · 2025-12-05T16:04:45Z

/release-note-edit

Fixed the following bugs for the StatefulSet integration by ensuring the Workload object
has the ownerReference to the StatefulSet:
1. Kueue doesn't keep the StatefulSet as deactivated
2. Kueue marks the Workload as Finished if all StatefulSet's Pods are deleted
3. changing the "queue-name" label could occasionally result in the StatefulSet getting stuck

mimowo

Thank you, I have revisted this PR. I take it.

cc @sohankunkerkar I know you prepared the alternative, but I think we need to go with one. Since both PRs implement pretty much the same approach it is a great signal the approach makes sense, because both of you converged on the same approach. Thank you folks 👍

/lgtm
/approve
/cherrypick release-0.14
/cherrypick release-0.13

k8s-ci-robot · 2025-12-05T16:15:21Z

LGTM label has been added.

Git tree hash: 0dfe16eb2f5ecd05a0be0160bf60fd404f68d48d

k8s-ci-robot · 2025-12-05T16:15:22Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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

mimowo · 2025-12-05T17:04:20Z

/unhold

mimowo · 2025-12-05T17:15:39Z

https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kueue/4799/pull-kueue-populator-test-e2e-main/1996989187164737536
does not seem related
/test pull-kueue-populator-test-e2e-main

mimowo · 2025-12-05T17:29:29Z

/cherrypick release-0.14
/cherrypick release-0.15

k8s-infra-cherrypick-robot · 2025-12-05T17:30:07Z

@mimowo: new pull request created: #8102

In response to this:

/cherrypick release-0.14
/cherrypick release-0.15

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-12-05T17:30:43Z

@mimowo: new pull request created: #8103

In response to this:

/cherrypick release-0.14
/cherrypick release-0.15

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.

…#8104) * Add StatefulSet as owner of Workload. * Fix after cherry-pick.

mimowo · 2025-12-05T18:47:34Z

/cherrypick release-0.15

k8s-infra-cherrypick-robot · 2025-12-05T18:48:13Z

@mimowo: new pull request created: #8105

In response to this:

/cherrypick release-0.15

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 added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/bug Categorizes issue or PR as related to a bug. labels Mar 26, 2025

k8s-ci-robot requested review from PBundyra and tenzen-y March 26, 2025 12:24

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

mbobrovskyi force-pushed the fix/sts-workloads-get-deleted-on-preemption branch from 8cffe93 to d8935ee Compare March 26, 2025 13:22

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 Mar 26, 2025

mbobrovskyi force-pushed the fix/sts-workloads-get-deleted-on-preemption branch from d8935ee to 8634d41 Compare March 26, 2025 13:30

mbobrovskyi marked this pull request as ready for review March 26, 2025 13:32

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 Mar 26, 2025

k8s-ci-robot requested a review from kannon92 March 26, 2025 13:32

mbobrovskyi force-pushed the fix/sts-workloads-get-deleted-on-preemption branch 2 times, most recently from 422353b to 28a1799 Compare March 26, 2025 13:43

k8s-ci-robot requested a review from mimowo March 26, 2025 13:46

mbobrovskyi force-pushed the fix/sts-workloads-get-deleted-on-preemption branch 2 times, most recently from 7245480 to 4422733 Compare March 26, 2025 14:17

mimowo reviewed Mar 26, 2025

View reviewed changes

mimowo mentioned this pull request Mar 26, 2025

Add LeaderWorkerSet as owner of Workload. #4790

Merged

mbobrovskyi force-pushed the fix/sts-workloads-get-deleted-on-preemption branch 2 times, most recently from 356b381 to 5b20f33 Compare March 26, 2025 15:57

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 27, 2025

mimowo reviewed Mar 27, 2025

View reviewed changes

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

k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 16, 2025

mbobrovskyi mentioned this pull request Nov 5, 2025

[WIP] test/e2e: fix flaky Statefulset e2e tests #7512

Closed

sohankunkerkar mentioned this pull request Nov 5, 2025

Flaky E2E test: ManageJobsWithoutQueueName when manageJobsWithoutQueueName=true should suspend the pods created by a StatefulSet in the test namespace without queue-name label #7366

Closed

mbobrovskyi mentioned this pull request Dec 4, 2025

pkg/controller: Fix workload deletion during StatefulSet queue name change #8040

Closed

mbobrovskyi force-pushed the fix/sts-workloads-get-deleted-on-preemption branch from cb269ed to 9568409 Compare December 4, 2025 03:35

Add StatefulSet as owner of Workload.

367ac58

mbobrovskyi force-pushed the fix/sts-workloads-get-deleted-on-preemption branch from 9568409 to 367ac58 Compare December 4, 2025 05:05

mimowo reviewed Dec 5, 2025

View reviewed changes

k8s-ci-robot assigned mimowo Dec 5, 2025

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

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

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 5, 2025

k8s-ci-robot merged commit 98c941f into kubernetes-sigs:main Dec 5, 2025
28 checks passed

k8s-ci-robot added this to the v0.16 milestone Dec 5, 2025

k8s-infra-cherrypick-robot mentioned this pull request Dec 5, 2025

[release-0.14] Add StatefulSet as owner of Workload. #8102

Closed

k8s-infra-cherrypick-robot mentioned this pull request Dec 5, 2025

[release-0.15] Add StatefulSet as owner of Workload. #8103

Closed

mbobrovskyi mentioned this pull request Dec 5, 2025

Automated cherry pick of #4799: Add StatefulSet as owner of Workload. #8104

Merged

k8s-ci-robot pushed a commit that referenced this pull request Dec 5, 2025

Automated cherry pick of #4799: Add StatefulSet as owner of Workload. (…

9bdeca8

…#8104) * Add StatefulSet as owner of Workload. * Fix after cherry-pick.

k8s-infra-cherrypick-robot mentioned this pull request Dec 5, 2025

[release-0.15] Add StatefulSet as owner of Workload. #8105

Merged

		shouldUpdate = true
		err = controllerutil.RemoveOwnerReference(sts, wl, r.client.Scheme())

Add StatefulSet as owner of Workload. #4799

Add StatefulSet as owner of Workload. #4799

Conversation

mbobrovskyi commented Mar 26, 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!

k8s-ci-robot commented Mar 26, 2025

Uh oh!

netlify bot commented Mar 26, 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!

mbobrovskyi commented Mar 26, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mimowo Mar 26, 2025

Choose a reason for hiding this comment

Uh oh!

mbobrovskyi Mar 26, 2025

Choose a reason for hiding this comment

Uh oh!

mbobrovskyi Mar 26, 2025

Choose a reason for hiding this comment

Uh oh!

mbobrovskyi Mar 26, 2025

Choose a reason for hiding this comment

Uh oh!

mimowo Mar 26, 2025

Choose a reason for hiding this comment

Uh oh!

mimowo Mar 26, 2025

Choose a reason for hiding this comment

Uh oh!

mimowo commented Mar 27, 2025

Uh oh!

mimowo Mar 27, 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!

mbobrovskyi Apr 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!

mbobrovskyi commented Aug 16, 2025

Uh oh!

mimowo commented Dec 5, 2025

Uh oh!

mimowo left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Dec 5, 2025

Uh oh!

k8s-ci-robot commented Dec 5, 2025

Uh oh!

mimowo commented Dec 5, 2025

Uh oh!

mimowo commented Dec 5, 2025

Uh oh!

Uh oh!

mimowo commented Dec 5, 2025

Uh oh!

k8s-infra-cherrypick-robot commented Dec 5, 2025

Uh oh!

k8s-infra-cherrypick-robot commented Dec 5, 2025

Uh oh!

mimowo commented Dec 5, 2025

Uh oh!

mbobrovskyi commented Mar 26, 2025 •

edited by k8s-ci-robot

Loading

netlify bot commented Mar 26, 2025 •

edited

Loading

mimowo Mar 27, 2025 •

edited

Loading

mbobrovskyi Apr 14, 2025 •

edited

Loading