Fix generateName for StatefulSets by including UID in workload name#9091
Fix generateName for StatefulSets by including UID in workload name#9091IrvingMg wants to merge 1 commit intokubernetes-sigs:mainfrom
Conversation
✅ Deploy Preview for kubernetes-sigs-kueue ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: IrvingMg The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
327ad6a to
d1344c5
Compare
d1344c5 to
6002a6c
Compare
|
@IrvingMg: The following test failed, say
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. DetailsInstructions 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. |
| remoteStatefulSet.Labels[kueue.MultiKueueOriginLabel] = origin | ||
|
|
||
| if remoteStatefulSet.Annotations == nil { | ||
| remoteStatefulSet.Annotations = map[string]string{} |
There was a problem hiding this comment.
| remoteStatefulSet.Annotations = map[string]string{} | |
| remoteStatefulSet.Annotations = make(map[string]string, 1) |
|
@IrvingMg do you understand the issue behind the e2e tests failing https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kueue/9091/pull-kueue-test-e2e-multikueue-main/2023423325627224064? Do we have a path forward for fixing it? |
Yes, I’ve been looking into it and am currently working on the fix. The issue seems to be related to splitting the reconciliation process after adding the StatefulSet pod reconciler, which is causing the workload to miss the StatefulSet owner reference. |
What type of PR is this?
/kind bug
What this PR does / why we need it:
GetWorkloadNamepreviously used only the StatefulSet name with an empty UID, causing workload name collisions when multiple StatefulSets shared agenerateNameprefix. This PR now includes the UID in the workload name hash.Since the UID is not available at webhook time, labels that depend on the workload name have been moved from the webhook to the reconciler.
For MultiKueue compatibility, this PR adds
GetOwnerUID(following the LWS pattern) so that worker-cluster copies use the management cluster’s UID viaMultiKueueOriginUIDAnnotation.Which issue(s) this PR fixes:
Fixes #8687
Special notes for your reviewer:
Does this PR introduce a user-facing change?