Flavor Fungibility Prefers Fit over NoBorrow #6132

pajakd · 2025-07-22T09:20:46Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Some of the users would like to define a different preference when selecting a flavor in flavor fungibility. Currently we always try to minimize preemptions. Sometimes it means that an incoming high-priority workload would choose a flavor in which it has to borrow (because its nominal quota is in use or borrowed). A workload that is borrowing is at risk of being preempted which may be undesirable for high-priority workloads so it might be better for it to preempt and be scheduled in a flavor that does not require borrowing.

This PR adds a feature gate FlavorFungibilityImplicitPreferenceDefault. When the feature gate is enabled, it infers the preference (borrowing or preemption) based on the FavorFungibilityStrategy. More precisely in two cases:

WhenCanPreempt = Preempt and WhenCanBorrow = TryNextFlavor
WhenCanPreempt = TryNextFlavor and WhenCanBorrow = TryNextFlavor

flavor assigner will prioritize flavors that require preemption over ones that require borrowing. In all other cases, borrowing will be prioritized.

KEP changes will be added in a separate PR.

Which issue(s) this PR fixes:

Fixes #5424

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Flavor Fungibility: Introduces a new mode which allows to prefer preemption over borrowing when choosing a flavor. 
In this mode the preference is decided based on FavorFungibilityStrategy. This behavior is behind the 
FlavorFungibilityImplicitPreferenceDefault Alpha feature gate (disabled by default).

k8s-ci-robot · 2025-07-22T09:20:48Z

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-07-22T09:21:57Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`1ca3056`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/6882307a48ecf50008c9b6fb

mimowo · 2025-07-22T09:35:01Z

@pajakd @gabesaba please prepare also a KEP update for this extension.

apis/kueue/v1beta1/clusterqueue_types.go

pkg/scheduler/flavorassigner/flavorassigner.go

gabesaba · 2025-07-22T10:07:03Z

pkg/scheduler/preemption/preemption_oracle.go

+	for _, candidate := range candidates {
+		revertRemoval := cq.SimulateUsageRemoval(candidate.WorkloadInfo.Usage())
+		defer revertRemoval()
+	}


can we use SimulateWorklaodRemoval?

Yes. But we have to iterate over the candidates anyway because they are of type Target and we need WorkloadInfo. With SimulateWorkloadRemoval we don't have to get the WorkloadInfo.Usage() so it should be better.

gabesaba · 2025-07-22T10:07:43Z

pkg/scheduler/preemption/preemption_oracle.go

 	if len(candidates) == 0 {
-		return preemptioncommon.NoCandidates
+		return preemptioncommon.NoCandidates, borrowAfterPreemptions
 	}


can this clause come before usage removal? on line 50?

We need to calculate the borrowing level before this (even if there are no candidates) because we return it. And before that calculation we have to remove the usage of the candidates (if there are any). So, this is why its here.

Does borrowing level matter if it is NoFit (which is implied by 0 candidates, right?)

It does not matter. If the workload does not fit, the borrowAfterPreemptions will be the maximum distance. Do you suggest it makes more sense to return something else?

I think I finally understood what you mean. Updated the code.

I take it back. Changing the borrowing for NoCandiates makes some integrations tests to fail. I reverted the change. I'll investigate why is that but in the meantime can it stay as it is?

Once investigated please also add a unit test which demonstrates why this needs to be the wait it is. So that it is much easier to avoid the trap in the future.

pkg/webhooks/clusterqueue_webhook.go

apis/kueue/v1beta1/clusterqueue_types.go

pkg/scheduler/flavorassigner/flavorassigner.go

mimowo · 2025-07-22T10:15:58Z

/hold
I have some higher level concerns as it does not match my understanding of the solution from initial discussions. If we want this to be included still in 0.13 please prepare KEP update with the story and justification for the changes, in particular addressing:

pajakd · 2025-07-22T11:11:57Z

@pajakd @gabesaba please prepare also a KEP update for this extension.

Proposed KEP changes in #6133

Co-authored-by: gabesaba <[email protected]>

mimowo · 2025-07-24T09:14:08Z

@pajakd please mark all addressed comments as resolved and update the release note

pkg/features/kube_features.go

Co-authored-by: Michał Woźniak <[email protected]>

gabesaba · 2025-07-24T13:36:09Z

/lgtm

thank you!

k8s-ci-robot · 2025-07-24T13:36:17Z

LGTM label has been added.

Details

Git tree hash: a68c4aa2a68c2fd06ffe2058e2b15a0a81d171ad

mimowo · 2025-07-24T14:11:29Z

pkg/scheduler/flavorassigner/flavorassigner.go

+func preferToAvoidBorrowing(fungiblityConfig kueue.FlavorFungibility) bool {
+	return (fungiblityConfig.WhenCanBorrow == kueue.TryNextFlavor && fungiblityConfig.WhenCanPreempt == kueue.Preempt) ||
+		(fungiblityConfig.WhenCanBorrow == kueue.TryNextFlavor && fungiblityConfig.WhenCanPreempt == kueue.TryNextFlavor)
+}


non-blocking nit: Since now the function simplifies to fungiblityConfig.WhenCanBorrow == kueue.TryNextFlavor can you inline?

mimowo · 2025-07-24T14:12:08Z

/remove-kind api-change

mimowo · 2025-07-24T14:20:03Z

/approve
Please follow up on the two non-blocking comments:

k8s-ci-robot · 2025-07-24T14:20:14Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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]

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

* Allow to avoid borrowing in flavor fungibility * Update comment * Update comment * Update comment * Typo fix * Fix tests * Fix typo * Update pkg/scheduler/flavorassigner/flavorassigner.go Co-authored-by: gabesaba <[email protected]> * Address comments * Fix a whitespace * Fix tests * Address comments * Avoid -> prefer * Strategy -> policy * Update the defaults and validation * Fix * Allow empty selection policy * Fix whitespaces * Fix unit test * Update apis/kueue/v1beta1/clusterqueue_types.go Co-authored-by: Michał Woźniak <[email protected]> * Fix errors * Revert API + add feature gate * Cleanup * Cleanup * Simplify the preference check * Additional integration tests * Update pkg/features/kube_features.go Co-authored-by: Michał Woźniak <[email protected]> * Earlier check for 0 candidates * Fix tests * Remove reliance on FS * Fix test --------- Co-authored-by: gabesaba <[email protected]> Co-authored-by: Michał Woźniak <[email protected]>

pajakd added 2 commits July 22, 2025 08:15

Allow to avoid borrowing in flavor fungibility

791936e

Update comment

3a10ae7

k8s-ci-robot requested review from gabesaba and kannon92 July 22, 2025 09:20

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 22, 2025

pajakd changed the title ~~Enable borrowing avoidance in flavor fungibility~~ Flavor Fungibility Prefers Fit over NoBorrow Jul 22, 2025

Update comment

e057d64

pajakd marked this pull request as ready for review July 22, 2025 09: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 Jul 22, 2025

k8s-ci-robot requested a review from tenzen-y July 22, 2025 09:33

Update comment

9feb5da

mimowo reviewed Jul 22, 2025

View reviewed changes

apis/kueue/v1beta1/clusterqueue_types.go Outdated Show resolved Hide resolved

Typo fix

4ac26f0

gabesaba reviewed Jul 22, 2025

View reviewed changes

mimowo reviewed Jul 22, 2025

View reviewed changes

pkg/scheduler/flavorassigner/flavorassigner.go Outdated Show resolved Hide resolved

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

pajakd added 2 commits July 22, 2025 10:32

Fix tests

8ec3b4e

Fix typo

8dff86f

pajakd and others added 2 commits July 22, 2025 13:17

Update pkg/scheduler/flavorassigner/flavorassigner.go

1c4cd94

Co-authored-by: gabesaba <[email protected]>

Address comments

8024dca

Additional integration tests

0e004a9

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

mimowo reviewed Jul 24, 2025

View reviewed changes

pkg/features/kube_features.go Show resolved Hide resolved

pajakd and others added 4 commits July 24, 2025 11:17

Update pkg/features/kube_features.go

4c0b919

Co-authored-by: Michał Woźniak <[email protected]>

Earlier check for 0 candidates

f922ada

Fix tests

e505622

Remove reliance on FS

82f1da8

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 Jul 24, 2025

Fix test

1ca3056

k8s-ci-robot assigned gabesaba Jul 24, 2025

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

mimowo reviewed Jul 24, 2025

View reviewed changes

k8s-ci-robot removed the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Jul 24, 2025

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

k8s-ci-robot merged commit 32ed754 into kubernetes-sigs:main Jul 24, 2025
22 checks passed

k8s-ci-robot added this to the v0.13 milestone Jul 24, 2025

pajakd deleted the fungibility_order branch July 25, 2025 06:19

pajakd mentioned this pull request Jul 25, 2025

Follow-up to Flavor Fungibility Prefers Fit over NoBorrow #6181

Merged

mimowo mentioned this pull request Jul 28, 2025

FlavorFungability: update documentation for FlavorFungibilityImplicitPreferenceDefault #6201

Closed

varunsyal mentioned this pull request Sep 26, 2025

Workloads unable to use cohort guarantees (hierarchical cohorts, fair sharing, multiple flavors.) #7015

Closed

pajakd mentioned this pull request Nov 27, 2025

Self nominate pajakd as reviewer #7946

Merged

3 tasks

Flavor Fungibility Prefers Fit over NoBorrow #6132

Flavor Fungibility Prefers Fit over NoBorrow #6132

Uh oh!

Conversation

pajakd commented Jul 22, 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!

k8s-ci-robot commented Jul 22, 2025

Uh oh!

netlify bot commented Jul 22, 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!

mimowo commented Jul 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

pajakd commented Jul 22, 2025

Uh oh!

mimowo commented Jul 24, 2025

Uh oh!

Uh oh!

gabesaba commented Jul 24, 2025

Uh oh!

k8s-ci-robot commented Jul 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mimowo commented Jul 24, 2025

Uh oh!

mimowo commented Jul 24, 2025

Uh oh!

k8s-ci-robot commented Jul 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pajakd commented Jul 22, 2025 •

edited

Loading

netlify bot commented Jul 22, 2025 •

edited

Loading

mimowo commented Jul 22, 2025 •

edited

Loading