FlavorFungability: replace FlavorFungibilityImplicitPreferenceDefault feature gate with API #7316
+358
−33
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -380,8 +380,16 @@ const ( | |
| TryNextFlavor FlavorFungibilityPolicy = "TryNextFlavor" | ||
| ) | ||
|
|
||
| type FlavorFungibilityPreference string | ||
|
|
||
| const ( | ||
| BorrowingOverPreemption FlavorFungibilityPreference = "BorrowingOverPreemption" | ||
| PreemptionOverBorrowing FlavorFungibilityPreference = "PreemptionOverBorrowing" | ||
| ) | ||
|
|
||
| // FlavorFungibility determines whether a workload should try the next flavor | ||
| // before borrowing or preempting in current flavor. | ||
| // +kubebuilder:validation:XValidation:rule="!has(self.preference) || (self.whenCanBorrow == 'TryNextFlavor' && self.whenCanPreempt == 'TryNextFlavor')",message="preference can only be set when both whenCanBorrow and whenCanPreempt are TryNextFlavor" | ||
| type FlavorFungibility struct { | ||
| // whenCanBorrow determines whether a workload should try the next flavor | ||
| // before borrowing in current flavor. The possible values are: | ||
|
|
@@ -406,6 +414,15 @@ type FlavorFungibility struct { | |
| // +kubebuilder:default="TryNextFlavor" | ||
| // +optional | ||
| WhenCanPreempt FlavorFungibilityPolicy `json:"whenCanPreempt,omitempty"` | ||
| // preference selects the order between minimizing preemption and avoiding borrowing | ||
| // when both policies are set to TryNextFlavor. The possible values are: | ||
| // | ||
| // - `BorrowingOverPreemption` (default): prefer lower borrowing distance before preferring lower preemption mode. | ||
| // - `PreemptionOverBorrowing`: prefer lower preemption mode before preferring lower borrowing distance. | ||
| // | ||
| // +kubebuilder:validation:Enum={BorrowingOverPreemption,PreemptionOverBorrowing} | ||
| // +optional | ||
| Preference *FlavorFungibilityPreference `json:"preference,omitempty"` | ||
| } | ||
|
|
||
| // ClusterQueuePreemption contains policies to preempt Workloads from this | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -192,6 +192,19 @@ spec: | |
| flavorFungibility defines whether a workload should try the next flavor | ||
| before borrowing or preempting in the flavor being evaluated. | ||
| properties: | ||
| preference: | ||
| description: |- | ||
| preference selects the order between minimizing preemption and avoiding borrowing | ||
| when both policies are set to TryNextFlavor. The possible values are: | ||
|
|
||
| - `BorrowingOverPreemption` (default): prefer lower borrowing distance before preferring lower preemption mode. | ||
| - `PreemptionOverBorrowing`: prefer lower preemption mode before preferring lower borrowing distance. | ||
|
|
||
| Deprecated values from earlier releases are not supported; please use the names above. | ||
| enum: | ||
| - BorrowingOverPreemption | ||
| - PreemptionOverBorrowing | ||
| type: string | ||
| whenCanBorrow: | ||
| default: MayStopSearch | ||
| description: |- | ||
|
|
@@ -224,6 +237,9 @@ spec: | |
| - Preempt | ||
| type: string | ||
| type: object | ||
| x-kubernetes-validations: | ||
| - message: preference can only be set when both whenCanBorrow and whenCanPreempt are TryNextFlavor | ||
| rule: '!has(self.preference) || (self.whenCanBorrow == ''TryNextFlavor'' && self.whenCanPreempt == ''TryNextFlavor'')' | ||
| namespaceSelector: | ||
| description: |- | ||
| namespaceSelector defines which namespaces are allowed to submit workloads to | ||
|
|
@@ -940,6 +956,17 @@ spec: | |
| flavorFungibility defines whether a workload should try the next flavor | ||
| before borrowing or preempting in the flavor being evaluated. | ||
| properties: | ||
| preference: | ||
| description: |- | ||
| preference selects the order between minimizing preemption and avoiding borrowing | ||
| when both policies are set to TryNextFlavor. The possible values are: | ||
|
|
||
| - `BorrowingOverPreemption` (default): prefer lower borrowing distance before preferring lower preemption mode. | ||
| - `PreemptionOverBorrowing`: prefer lower preemption mode before preferring lower borrowing distance. | ||
| enum: | ||
| - BorrowingOverPreemption | ||
| - PreemptionOverBorrowing | ||
| type: string | ||
| whenCanBorrow: | ||
| default: MayStopSearch | ||
| description: |- | ||
|
|
@@ -968,6 +995,9 @@ spec: | |
| - TryNextFlavor | ||
| type: string | ||
| type: object | ||
| x-kubernetes-validations: | ||
| - message: preference can only be set when both whenCanBorrow and whenCanPreempt are TryNextFlavor | ||
| rule: '!has(self.preference) || (self.whenCanBorrow == ''TryNextFlavor'' && self.whenCanPreempt == ''TryNextFlavor'')' | ||
| namespaceSelector: | ||
| description: |- | ||
| namespaceSelector defines which namespaces are allowed to submit workloads to | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -163,6 +163,19 @@ spec: | |
| flavorFungibility defines whether a workload should try the next flavor | ||
| before borrowing or preempting in the flavor being evaluated. | ||
| properties: | ||
| preference: | ||
| description: |- | ||
| preference selects the order between minimizing preemption and avoiding borrowing | ||
| when both policies are set to TryNextFlavor. The possible values are: | ||
|
|
||
| - `BorrowingOverPreemption` (default): prefer lower borrowing distance before preferring lower preemption mode. | ||
| - `PreemptionOverBorrowing`: prefer lower preemption mode before preferring lower borrowing distance. | ||
|
|
||
| Deprecated values from earlier releases are not supported; please use the names above. | ||
| enum: | ||
| - BorrowingOverPreemption | ||
| - PreemptionOverBorrowing | ||
| type: string | ||
| whenCanBorrow: | ||
| default: MayStopSearch | ||
| description: |- | ||
|
|
@@ -195,6 +208,11 @@ spec: | |
| - Preempt | ||
| type: string | ||
| type: object | ||
| x-kubernetes-validations: | ||
| - message: preference can only be set when both whenCanBorrow and | ||
| whenCanPreempt are TryNextFlavor | ||
| rule: '!has(self.preference) || (self.whenCanBorrow == ''TryNextFlavor'' | ||
| && self.whenCanPreempt == ''TryNextFlavor'')' | ||
| namespaceSelector: | ||
| description: |- | ||
| namespaceSelector defines which namespaces are allowed to submit workloads to | ||
|
|
@@ -929,6 +947,17 @@ spec: | |
| flavorFungibility defines whether a workload should try the next flavor | ||
| before borrowing or preempting in the flavor being evaluated. | ||
| properties: | ||
| preference: | ||
| description: |- | ||
| preference selects the order between minimizing preemption and avoiding borrowing | ||
| when both policies are set to TryNextFlavor. The possible values are: | ||
|
|
||
| - `BorrowingOverPreemption` (default): prefer lower borrowing distance before preferring lower preemption mode. | ||
| - `PreemptionOverBorrowing`: prefer lower preemption mode before preferring lower borrowing distance. | ||
| enum: | ||
| - BorrowingOverPreemption | ||
| - PreemptionOverBorrowing | ||
| type: string | ||
| whenCanBorrow: | ||
| default: MayStopSearch | ||
| description: |- | ||
|
|
@@ -957,6 +986,11 @@ spec: | |
| - TryNextFlavor | ||
| type: string | ||
| type: object | ||
| x-kubernetes-validations: | ||
| - message: preference can only be set when both whenCanBorrow and | ||
| whenCanPreempt are TryNextFlavor | ||
| rule: '!has(self.preference) || (self.whenCanBorrow == ''TryNextFlavor'' | ||
| && self.whenCanPreempt == ''TryNextFlavor'')' | ||
| namespaceSelector: | ||
| description: |- | ||
| namespaceSelector defines which namespaces are allowed to submit workloads to | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -160,8 +160,8 @@ const ( | |
| // owner: @pajakd | ||
| // kep: https://github.com/kubernetes-sigs/kueue/tree/main/keps/582-preempt-based-on-flavor-order | ||
| // | ||
| // In flavor fungibility, the preference whether to preempt or borrow is inferred from flavor fungibility policy. | ||
| // Deprecated: planned to be removed in v0.17. Will remain enabled by default through v0.16. | ||
| FlavorFungibilityImplicitPreferenceDefault featuregate.Feature = "FlavorFungibilityImplicitPreferenceDefault" | ||
|
|
||
| // owner: @alaypatel07 | ||
|
|
@@ -297,6 +297,9 @@ var defaultVersionedFeatureGates = map[featuregate.Feature]featuregate.Versioned | |
| }, | ||
| FlavorFungibilityImplicitPreferenceDefault: { | ||
| {Version: version.MustParse("0.13"), Default: false, PreRelease: featuregate.Alpha}, | ||
| // TODO: what to do with feature gate? | ||
| // {Version: version.MustParse("0.15"), Default: true, PreRelease: featuregate.Beta}, | ||
| // {Version: version.MustParse("0.16"), Default: true, PreRelease: featuregate.Deprecated}, // remove in 0.17 | ||
|
Comment on lines
+300
to
+302
Contributor
There was a problem hiding this comment. // {Version: version.MustParse("0.15"), Default: false, PreRelease: featuregate.Deprecated}, // remove in 0.16 |
||
| }, | ||
| DynamicResourceAllocation: { | ||
| {Version: version.MustParse("0.14"), Default: false, PreRelease: featuregate.Alpha}, | ||
|
|
@@ -327,6 +330,11 @@ func Enabled(f featuregate.Feature) bool { | |
| return utilfeature.DefaultFeatureGate.Enabled(f) | ||
| } | ||
|
|
||
| // Disabled is helper for `!utilfeature.DefaultFeatureGate.Enabled()` | ||
| func Disabled(f featuregate.Feature) bool { | ||
| return !Enabled(f) | ||
| } | ||
|
Comment on lines
+333
to
+336
Contributor
There was a problem hiding this comment. Why just not use !Enabled(f)?
Contributor
There was a problem hiding this comment. yes, I don't see value in the extra helper |
||
|
|
||
| // SetEnable helper function that can be used to set the enabled value of a feature gate, | ||
| // it should only be used in integration test pending the merge of | ||
| // https://github.com/kubernetes/kubernetes/pull/118346 | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -379,25 +379,39 @@ func isPreferred(a, b granularMode, fungibilityConfig kueue.FlavorFungibility) b | |
| return true | ||
| } | ||
|
|
||
|
Contributor
There was a problem hiding this comment. /hold |
||
| borrowingOverPreemption := func() bool { | ||
| if a.preemptionMode != b.preemptionMode { | ||
| return a.preemptionMode > b.preemptionMode | ||
| } | ||
| return a.borrowingLevel.betterThan(b.borrowingLevel) | ||
| } | ||
| preemptionOverBorrowing := func() bool { | ||
| if a.borrowingLevel != b.borrowingLevel { | ||
| return a.borrowingLevel.betterThan(b.borrowingLevel) | ||
| } | ||
| return a.preemptionMode > b.preemptionMode | ||
| } | ||
|
|
||
| if fungibilityConfig.Preference != nil { | ||
| switch *fungibilityConfig.Preference { | ||
| case kueue.BorrowingOverPreemption: | ||
| return preemptionOverBorrowing() | ||
| case kueue.PreemptionOverBorrowing: | ||
| return borrowingOverPreemption() | ||
| } | ||
| } | ||
|
|
||
| // BorrowingOverPreemption preference | ||
| if features.Disabled(features.FlavorFungibilityImplicitPreferenceDefault) { | ||
| return borrowingOverPreemption() | ||
| } | ||
|
|
||
| // PreemptionOverBorrowing preference | ||
| if fungibilityConfig.WhenCanBorrow == kueue.TryNextFlavor { | ||
| return preemptionOverBorrowing() | ||
| } | ||
| // BorrowingOverPreemption preference | ||
| return borrowingOverPreemption() | ||
| } | ||
|
|
||
| func fromPreemptionPossibility(preemptionPossibility preemptioncommon.PreemptionPossibility) preemptionMode { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure aligned with the KEP