v1beta2: Delete .enable field from FairSharing API in config #7583

mbobrovskyi · 2025-11-07T15:45:47Z

What type of PR is this?

/kind cleanup
/kind api-change

What this PR does / why we need it:

Delete .enable field from FairSharing API in config.

Which issue(s) this PR fixes:

Fixes #5032

Special notes for your reviewer:

Does this PR introduce a user-facing change?

v1beta2: Delete .enable field from FairSharing API in config

netlify · 2025-11-07T15:45:54Z

✅ Deploy Preview for kubernetes-sigs-kueue ready!

Name	Link
🔨 Latest commit	`34a35cd`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/69158e9f76f277000866c7ec
😎 Deploy Preview	https://deploy-preview-7583--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.

apis/config/v1beta2/configuration_types.go

mbobrovskyi · 2025-11-07T15:57:45Z

/cc @mimowo

mimowo · 2025-11-07T16:18:30Z

apis/config/v1beta1/configuration_conversion.go


 func Convert_v1beta1_Configuration_To_v1beta2_Configuration(in *Configuration, out *v1beta2.Configuration, s conversionapi.Scope) error {
+	if in.FairSharing != nil {
+		out.FairSharing = &v1beta2.FairSharing{


I think we should only do it if enabled in v1beta1. Otherwise if the admin disables in v1beta1, then we will use it in 0.15

please also there is a test which would pick up the initial bug

kannon92 · 2025-11-07T16:32:06Z

apis/config/v1beta1/configuration_conversion.go

 	return autoConvert_v1beta1_Configuration_To_v1beta2_Configuration(in, out, s)
 }

+func Convert_v1beta2_Configuration_To_v1beta1_Configuration(in *v1beta2.Configuration, out *Configuration, s conversionapi.Scope) error {


Do we really need conversions for config apis?

The config API is not a CRD so I don't quite understand how a conversion would actually happen.

This is mainly translated to a config map.

So I guess someone specifiy v1beta1 in a helm chart would potentially be converted? But I don't quite understand where that would occur.

They are very useful for smooth transition. For example, currently users have in configMaps v1beta1 version. When they install new Kueue they may still need time to migrate the configMap, so they could start by reading v1beta1, and then convert to v1beta2 as a follow up.

Most of the code is auto generated anyway, so I don't think maintaining is much cost

Yes, if the user is still using v1beta1, we need to convert it to v1beta2, since the codebase uses the v1beta2 version everywhere.

But without a CRD who does the conversion?

I tried installing Kueue with v1beta1, but it didn’t work because the conversions are missing.

The conversion is applied when loading the configuration in main.go. It is not handled by webhooks.

mimowo · 2025-11-07T17:14:14Z

We also deliver some samples commented out configuration, please make surr it is adjusted

mbobrovskyi · 2025-11-07T17:26:09Z

We also deliver some samples commented out configuration, please make surr it is adjusted

Good point. Updated.

mimowo · 2025-11-07T17:31:53Z

/lgtm
/approve
assymung the tests pass

k8s-ci-robot · 2025-11-07T17:32:01Z

LGTM label has been added.

Git tree hash: 267097fbb9adc2903ec4e875da615ec9b5163775

kannon92

Test failure is legit.

https://github.com/kubernetes-sigs/kueue/blob/main/test/e2e/config/certmanager/controller_manager_config.yaml#L15

Probably need to remove this from our cert manager setup.

/hold

mbobrovskyi · 2025-11-10T05:53:29Z

Probably need to remove this from our cert manager setup.

You right. Thanks! Done.

kannon92

/lgtm

/hold cancel

k8s-ci-robot · 2025-11-10T15:49:08Z

LGTM label has been added.

Git tree hash: fa98c44212e701766f73581d0a77bd65f3d62572

mbobrovskyi · 2025-11-10T15:58:46Z

/hold

kannon92 · 2025-11-13T15:09:34Z

@mbobrovskyi Is this ready for review?

I wasn't clear why you put a hold on.

mbobrovskyi · 2025-11-13T16:12:59Z

Oh, we’re still waiting for the final decision from @mimowo and @mwysokin on whether this is the correct way to remove enabled field — that’s why it’s still on hold.

mimowo · 2025-11-13T16:19:57Z

Let me actually merge, we can still revert that change if someone raises objections, so far I haven't heard any. This way we can also avoid conflicts with other PRs
/unhold
/lgtm
/approve

k8s-ci-robot · 2025-11-13T16:20:05Z

LGTM label has been added.

Git tree hash: 886526aeddf0a9146449b6ec4919700d6eaaa3bc

k8s-ci-robot · 2025-11-13T16:20:06Z

[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

…tes-sigs#7583)

k8s-ci-robot requested review from kannon92 and tenzen-y November 7, 2025 15:45

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 7, 2025

kannon92 reviewed Nov 7, 2025

View reviewed changes

apis/config/v1beta2/configuration_types.go Outdated Show resolved Hide resolved

mbobrovskyi force-pushed the cleanup/delete-enable-field-from-FairSharing-API-in-config branch from f843e3f to 7b6d502 Compare November 7, 2025 15:57

k8s-ci-robot requested a review from mimowo November 7, 2025 15:57

mbobrovskyi force-pushed the cleanup/delete-enable-field-from-FairSharing-API-in-config branch from 7b6d502 to 2629f15 Compare November 7, 2025 16:16

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 7, 2025

mimowo reviewed Nov 7, 2025

View reviewed changes

mbobrovskyi force-pushed the cleanup/delete-enable-field-from-FairSharing-API-in-config branch from 2629f15 to 3795ea7 Compare November 7, 2025 16:30

kannon92 reviewed Nov 7, 2025

View reviewed changes

mbobrovskyi force-pushed the cleanup/delete-enable-field-from-FairSharing-API-in-config branch 3 times, most recently from 64f6c2f to 26c0734 Compare November 7, 2025 17:10

k8s-ci-robot assigned mimowo Nov 7, 2025

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

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

kannon92 reviewed Nov 7, 2025

View reviewed changes

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

k8s-ci-robot requested a review from mimowo November 8, 2025 03:41

mbobrovskyi force-pushed the cleanup/delete-enable-field-from-FairSharing-API-in-config branch 2 times, most recently from 0938921 to 0fa88a1 Compare November 9, 2025 07:08

kannon92 reviewed Nov 10, 2025

View reviewed changes

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

k8s-ci-robot assigned kannon92 Nov 10, 2025

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

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

mbobrovskyi force-pushed the cleanup/delete-enable-field-from-FairSharing-API-in-config branch from 04953c8 to 90eeee8 Compare November 12, 2025 07:48

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 12, 2025

k8s-ci-robot requested a review from kannon92 November 12, 2025 07:48

v1beta2: Delete .enable field from FairSharing API in config

34a35cd

mbobrovskyi force-pushed the cleanup/delete-enable-field-from-FairSharing-API-in-config branch from 90eeee8 to 34a35cd Compare November 13, 2025 07:54

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Nov 13, 2025

k8s-ci-robot merged commit 8b6a908 into kubernetes-sigs:main Nov 13, 2025
22 checks passed

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

mbobrovskyi deleted the cleanup/delete-enable-field-from-FairSharing-API-in-config branch November 13, 2025 17:13

mbobrovskyi mentioned this pull request Nov 15, 2025

v1beta2: Delete .enable field from WaitForPodsReady API in config #7628

Merged

Singularity23x0 pushed a commit to Singularity23x0/kueue that referenced this pull request Nov 17, 2025

v1beta2: Delete .enable field from FairSharing API in config (kuberne…

3ff4e09

…tes-sigs#7583)

mimowo mentioned this pull request Nov 19, 2025

v1beta2: Ignore "enable: true" in the configMap for FairSharing and WaitForPodsReady #7767

Closed

mbobrovskyi mentioned this pull request Nov 26, 2025

Automated cherry pick of #7894: Fix unit tests when WorkloadRequestUseMergePatch feature enabled. #7897

Merged

v1beta2: Delete .enable field from FairSharing API in config #7583

v1beta2: Delete .enable field from FairSharing API in config #7583

Uh oh!

Conversation

mbobrovskyi commented Nov 7, 2025 • edited 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 Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-kueue ready!

Uh oh!

Uh oh!

mbobrovskyi commented Nov 7, 2025

Uh oh!

mimowo Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

mimowo Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

kannon92 Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

mimowo Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

mimowo Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

mbobrovskyi Nov 7, 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!

kannon92 Nov 7, 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 Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

mimowo Nov 7, 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!

mimowo commented Nov 7, 2025

Uh oh!

mbobrovskyi commented Nov 7, 2025

Uh oh!

mimowo commented Nov 7, 2025

Uh oh!

k8s-ci-robot commented Nov 7, 2025

Uh oh!

kannon92 left a comment

Choose a reason for hiding this comment

Uh oh!

mbobrovskyi commented Nov 10, 2025

Uh oh!

kannon92 left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Nov 10, 2025

Uh oh!

mbobrovskyi commented Nov 10, 2025

Uh oh!

kannon92 commented Nov 13, 2025

Uh oh!

mbobrovskyi commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mimowo commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Nov 13, 2025

Uh oh!

k8s-ci-robot commented Nov 13, 2025

mbobrovskyi commented Nov 7, 2025 •

edited

Loading

netlify bot commented Nov 7, 2025 •

edited

Loading

mbobrovskyi Nov 7, 2025 •

edited

Loading

kannon92 Nov 7, 2025 •

edited

Loading

mimowo Nov 7, 2025 •

edited

Loading

mbobrovskyi commented Nov 13, 2025 •

edited

Loading

mimowo commented Nov 13, 2025 •

edited

Loading