From dc832d5fc590edb193794b32ba6dbcadaa34c22d Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Thu, 17 Apr 2025 13:09:38 +0000 Subject: [PATCH 01/19] Introduce FairSharing at admission time --- apis/config/v1beta1/configuration_types.go | 20 + apis/config/v1beta1/defaults.go | 5 + apis/config/v1beta1/zz_generated.deepcopy.go | 29 ++ apis/kueue/v1alpha1/zz_generated.deepcopy.go | 2 +- apis/kueue/v1beta1/clusterqueue_types.go | 4 + apis/kueue/v1beta1/fairsharing_types.go | 32 +- apis/kueue/v1beta1/localqueue_types.go | 10 + apis/kueue/v1beta1/zz_generated.deepcopy.go | 56 ++- .../crd/kueue.x-k8s.io_clusterqueues.yaml | 29 ++ .../templates/crd/kueue.x-k8s.io_cohorts.yaml | 22 + .../crd/kueue.x-k8s.io_localqueues.yaml | 65 +++ .../v1beta1/admissionfairsharingstatus.go | 52 +++ .../kueue/v1beta1/admissionscope.go | 42 ++ .../kueue/v1beta1/clusterqueuespec.go | 9 + .../kueue/v1beta1/fairsharingstatus.go | 11 +- .../kueue/v1beta1/localqueuespec.go | 9 + .../kueue/v1beta1/localqueuestatus.go | 9 + client-go/applyconfiguration/utils.go | 4 + cmd/kueue/main.go | 1 + .../bases/kueue.x-k8s.io_clusterqueues.yaml | 29 ++ .../crd/bases/kueue.x-k8s.io_cohorts.yaml | 22 + .../crd/bases/kueue.x-k8s.io_localqueues.yaml | 65 +++ pkg/cache/cache.go | 13 + pkg/cache/cache_test.go | 3 +- pkg/cache/clusterqueue.go | 6 +- pkg/cache/localqueue.go | 17 + pkg/controller/core/core.go | 3 +- pkg/controller/core/localqueue_controller.go | 142 +++++- .../core/localqueue_controller_test.go | 424 +++++++++++++++++- pkg/queue/cluster_queue.go | 47 +- pkg/queue/cluster_queue_test.go | 272 ++++++++++- pkg/queue/manager.go | 23 +- pkg/resources/resource.go | 8 + .../preemption/fairsharing/strategy.go | 4 + pkg/scheduler/preemption/preemption.go | 2 + pkg/scheduler/preemption/preemption_test.go | 12 + pkg/util/resource/resource.go | 14 + pkg/util/testing/wrappers.go | 46 ++ pkg/workload/workload.go | 22 + .../en/docs/reference/kueue-config.v1beta1.md | 51 ++- .../en/docs/reference/kueue.v1beta1.md | 102 +++++ .../admission-fair-sharing-setup.yaml | 42 ++ .../admission-fs/lq-a-simple-job.yaml | 20 + .../admission-fs/lq-b-simple-job.yaml | 20 + .../fairsharing/fair_sharing_test.go | 73 +++ .../scheduler/fairsharing/suite_test.go | 16 +- 46 files changed, 1852 insertions(+), 57 deletions(-) create mode 100644 client-go/applyconfiguration/kueue/v1beta1/admissionfairsharingstatus.go create mode 100644 client-go/applyconfiguration/kueue/v1beta1/admissionscope.go create mode 100644 site/static/examples/admission-fs/admission-fair-sharing-setup.yaml create mode 100644 site/static/examples/admission-fs/lq-a-simple-job.yaml create mode 100644 site/static/examples/admission-fs/lq-b-simple-job.yaml diff --git a/apis/config/v1beta1/configuration_types.go b/apis/config/v1beta1/configuration_types.go index 6c29d467e21..3bfc63238a7 100644 --- a/apis/config/v1beta1/configuration_types.go +++ b/apis/config/v1beta1/configuration_types.go @@ -447,6 +447,7 @@ type PreemptionStrategy string const ( LessThanOrEqualToFinalShare PreemptionStrategy = "LessThanOrEqualToFinalShare" LessThanInitialShare PreemptionStrategy = "LessThanInitialShare" + NoPreemption PreemptionStrategy = "NoPreemption" ) type FairSharing struct { @@ -469,6 +470,25 @@ type FairSharing struct { // This strategy doesn't depend on the share usage of the workload being preempted. // As a result, the strategy chooses to preempt workloads with the lowest priority and // newest start time first. + // - NoPreemption: Never preempt a workload. // The default strategy is ["LessThanOrEqualToFinalShare", "LessThanInitialShare"]. PreemptionStrategies []PreemptionStrategy `json:"preemptionStrategies,omitempty"` + + // admissionFairSharing indicates configuration of FairSharing with the `AdmissionTime` mode on + // +optional + AdmissionFairSharing *AdmissionFairSharing `json:"admissionFairSharing,omitempty"` +} + +type AdmissionFairSharing struct { + // usageHalfLifeTime indicates the time after which the current usage will decay by a half + // If set to 0, usage will be reset to 0. + UsageHalfLifeTime metav1.Duration `json:"usageHalfLifeTime,omitempty"` + + // usageSamplingInterval indicates how often Kueue updates consumedResources in FairSharingStatus + UsageSamplingInterval metav1.Duration `json:"usageSamplingInterval,omitempty"` + + // resourceWeights assigns weights to resources which then are used to calculate LocalQueue/ClusterQueue/Cohort's + // resource usage and order Workloads. + // Defaults to 1. + ResourceWeights map[corev1.ResourceName]float64 `json:"resourceWeights,omitempty"` } diff --git a/apis/config/v1beta1/defaults.go b/apis/config/v1beta1/defaults.go index 924bb6f28c2..d9d75fa1510 100644 --- a/apis/config/v1beta1/defaults.go +++ b/apis/config/v1beta1/defaults.go @@ -210,6 +210,11 @@ func SetDefaults_Configuration(cfg *Configuration) { if fs := cfg.FairSharing; fs != nil && fs.Enable && len(fs.PreemptionStrategies) == 0 { fs.PreemptionStrategies = []PreemptionStrategy{LessThanOrEqualToFinalShare, LessThanInitialShare} } + if fs := cfg.FairSharing; fs != nil && fs.Enable && fs.AdmissionFairSharing != nil { + if fs.AdmissionFairSharing.UsageSamplingInterval.Duration == 0 { + fs.AdmissionFairSharing.UsageSamplingInterval = metav1.Duration{Duration: 5 * time.Minute} + } + } if cfg.Resources != nil { for idx := range cfg.Resources.Transformations { diff --git a/apis/config/v1beta1/zz_generated.deepcopy.go b/apis/config/v1beta1/zz_generated.deepcopy.go index 92f28d9c2b0..10bc383d7ed 100644 --- a/apis/config/v1beta1/zz_generated.deepcopy.go +++ b/apis/config/v1beta1/zz_generated.deepcopy.go @@ -28,6 +28,30 @@ import ( timex "time" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AdmissionFairSharing) DeepCopyInto(out *AdmissionFairSharing) { + *out = *in + out.UsageHalfLifeTime = in.UsageHalfLifeTime + out.UsageSamplingInterval = in.UsageSamplingInterval + if in.ResourceWeights != nil { + in, out := &in.ResourceWeights, &out.ResourceWeights + *out = make(map[corev1.ResourceName]float64, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AdmissionFairSharing. +func (in *AdmissionFairSharing) DeepCopy() *AdmissionFairSharing { + if in == nil { + return nil + } + out := new(AdmissionFairSharing) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClientConnection) DeepCopyInto(out *ClientConnection) { *out = *in @@ -263,6 +287,11 @@ func (in *FairSharing) DeepCopyInto(out *FairSharing) { *out = make([]PreemptionStrategy, len(*in)) copy(*out, *in) } + if in.AdmissionFairSharing != nil { + in, out := &in.AdmissionFairSharing, &out.AdmissionFairSharing + *out = new(AdmissionFairSharing) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FairSharing. diff --git a/apis/kueue/v1alpha1/zz_generated.deepcopy.go b/apis/kueue/v1alpha1/zz_generated.deepcopy.go index 201a05c7d75..a44e38a74f2 100644 --- a/apis/kueue/v1alpha1/zz_generated.deepcopy.go +++ b/apis/kueue/v1alpha1/zz_generated.deepcopy.go @@ -117,7 +117,7 @@ func (in *CohortStatus) DeepCopyInto(out *CohortStatus) { if in.FairSharing != nil { in, out := &in.FairSharing, &out.FairSharing *out = new(v1beta1.FairSharingStatus) - **out = **in + (*in).DeepCopyInto(*out) } } diff --git a/apis/kueue/v1beta1/clusterqueue_types.go b/apis/kueue/v1beta1/clusterqueue_types.go index 71a6690a53b..a4076ce88a2 100644 --- a/apis/kueue/v1beta1/clusterqueue_types.go +++ b/apis/kueue/v1beta1/clusterqueue_types.go @@ -137,6 +137,10 @@ type ClusterQueueSpec struct { // if FairSharing is enabled in the Kueue configuration. // +optional FairSharing *FairSharing `json:"fairSharing,omitempty"` + + // admissionScope indicates whether ClusterQueue uses the Admission Fair Sharing + // +optional + AdmissionScope *AdmissionScope `json:"admissionScope,omitempty"` } // AdmissionChecksStrategy defines a strategy for a AdmissionCheck. diff --git a/apis/kueue/v1beta1/fairsharing_types.go b/apis/kueue/v1beta1/fairsharing_types.go index ccbcb907da4..4b4313c9adf 100644 --- a/apis/kueue/v1beta1/fairsharing_types.go +++ b/apis/kueue/v1beta1/fairsharing_types.go @@ -16,7 +16,11 @@ limitations under the License. package v1beta1 -import "k8s.io/apimachinery/pkg/api/resource" +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // FairSharing contains the properties of the ClusterQueue or Cohort, // when participating in FairSharing. @@ -49,4 +53,30 @@ type FairSharingStatus struct { // weight of zero and is borrowing, this will return // 9223372036854775807, the maximum possible share value. WeightedShare int64 `json:"weightedShare"` + + // admissionFairSharingStatus represents information relevant to the Admission Fair Sharing + AdmissionFairSharingStatus *AdmissionFairSharingStatus `json:"admissionFairSharingStatus,omitempty"` } + +type AdmissionFairSharingStatus struct { + // ConsumedResources represents the aggregated usage of resources over time, + // with decaying function applied. + // The value is populated if usage consumption functionality is enabled in Kueue config. + ConsumedResources corev1.ResourceList `json:"consumedResources,omitempty"` + + // LastUpdate is the time when share and consumed resources were updated. + LastUpdate metav1.Time `json:"lastUpdate,omitempty"` +} + +type AdmissionScope struct { + AdmissionMode AdmissionMode `json:"admissionMode,omitempty"` +} + +type AdmissionMode string + +const ( + // FairSharing based on usage, with QueuingStrategy as defined in CQ. + UsageBasedFairSharing AdmissionMode = "UsageBasedFairSharing" + + NoFairSharing AdmissionMode = "NoFairSharing" +) diff --git a/apis/kueue/v1beta1/localqueue_types.go b/apis/kueue/v1beta1/localqueue_types.go index 1d11d167bfd..7aa2c40dca4 100644 --- a/apis/kueue/v1beta1/localqueue_types.go +++ b/apis/kueue/v1beta1/localqueue_types.go @@ -48,6 +48,12 @@ type LocalQueueSpec struct { // +kubebuilder:validation:Enum=None;Hold;HoldAndDrain // +kubebuilder:default="None" StopPolicy *StopPolicy `json:"stopPolicy,omitempty"` + + // fairSharing defines the properties of the LocalQueue when + // participating in FairSharing. The values are only relevant + // if FairSharing is enabled in the Kueue configuration. + // +optional + FairSharing *FairSharing `json:"fairSharing,omitempty"` } type LocalQueueFlavorStatus struct { @@ -149,6 +155,10 @@ type LocalQueueStatus struct { // +kubebuilder:validation:MaxItems=16 // +optional Flavors []LocalQueueFlavorStatus `json:"flavors,omitempty"` + + // FairSharing contains the information about the current status of fair sharing. + // +optional + FairSharingStatus FairSharingStatus `json:"fairSharingStatus,omitempty"` } const ( diff --git a/apis/kueue/v1beta1/zz_generated.deepcopy.go b/apis/kueue/v1beta1/zz_generated.deepcopy.go index ce07893883c..868649bfc11 100644 --- a/apis/kueue/v1beta1/zz_generated.deepcopy.go +++ b/apis/kueue/v1beta1/zz_generated.deepcopy.go @@ -234,6 +234,44 @@ func (in *AdmissionChecksStrategy) DeepCopy() *AdmissionChecksStrategy { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AdmissionFairSharingStatus) DeepCopyInto(out *AdmissionFairSharingStatus) { + *out = *in + if in.ConsumedResources != nil { + in, out := &in.ConsumedResources, &out.ConsumedResources + *out = make(corev1.ResourceList, len(*in)) + for key, val := range *in { + (*out)[key] = val.DeepCopy() + } + } + in.LastUpdate.DeepCopyInto(&out.LastUpdate) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AdmissionFairSharingStatus. +func (in *AdmissionFairSharingStatus) DeepCopy() *AdmissionFairSharingStatus { + if in == nil { + return nil + } + out := new(AdmissionFairSharingStatus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AdmissionScope) DeepCopyInto(out *AdmissionScope) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AdmissionScope. +func (in *AdmissionScope) DeepCopy() *AdmissionScope { + if in == nil { + return nil + } + out := new(AdmissionScope) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BorrowWithinCohort) DeepCopyInto(out *BorrowWithinCohort) { *out = *in @@ -414,6 +452,11 @@ func (in *ClusterQueueSpec) DeepCopyInto(out *ClusterQueueSpec) { *out = new(FairSharing) (*in).DeepCopyInto(*out) } + if in.AdmissionScope != nil { + in, out := &in.AdmissionScope, &out.AdmissionScope + *out = new(AdmissionScope) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterQueueSpec. @@ -458,7 +501,7 @@ func (in *ClusterQueueStatus) DeepCopyInto(out *ClusterQueueStatus) { if in.FairSharing != nil { in, out := &in.FairSharing, &out.FairSharing *out = new(FairSharingStatus) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -495,6 +538,11 @@ func (in *FairSharing) DeepCopy() *FairSharing { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FairSharingStatus) DeepCopyInto(out *FairSharingStatus) { *out = *in + if in.AdmissionFairSharingStatus != nil { + in, out := &in.AdmissionFairSharingStatus, &out.AdmissionFairSharingStatus + *out = new(AdmissionFairSharingStatus) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FairSharingStatus. @@ -725,6 +773,11 @@ func (in *LocalQueueSpec) DeepCopyInto(out *LocalQueueSpec) { *out = new(StopPolicy) **out = **in } + if in.FairSharing != nil { + in, out := &in.FairSharing, &out.FairSharing + *out = new(FairSharing) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LocalQueueSpec. @@ -768,6 +821,7 @@ func (in *LocalQueueStatus) DeepCopyInto(out *LocalQueueStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + in.FairSharingStatus.DeepCopyInto(&out.FairSharingStatus) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LocalQueueStatus. diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_clusterqueues.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_clusterqueues.yaml index 0d696cdd870..874ef8b7b12 100644 --- a/charts/kueue/templates/crd/kueue.x-k8s.io_clusterqueues.yaml +++ b/charts/kueue/templates/crd/kueue.x-k8s.io_clusterqueues.yaml @@ -115,6 +115,13 @@ spec: type: object type: array type: object + admissionScope: + description: admissionScope indicates whether ClusterQueue uses the + Admission Fair Sharing + properties: + admissionMode: + type: string + type: object cohort: description: |- cohort that this ClusterQueue belongs to. CQs that belong to the @@ -590,6 +597,28 @@ spec: when participating in Fair Sharing. This is recorded only when Fair Sharing is enabled in the Kueue configuration. properties: + admissionFairSharingStatus: + description: admissionFairSharingStatus represents information + relevant to the Admission Fair Sharing + properties: + consumedResources: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + ConsumedResources represents the aggregated usage of resources over time, + with decaying function applied. + The value is populated if usage consumption functionality is enabled in Kueue config. + type: object + lastUpdate: + description: LastUpdate is the time when share and consumed + resources were updated. + format: date-time + type: string + type: object weightedShare: description: |- WeightedShare represents the maximum of the ratios of usage diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_cohorts.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_cohorts.yaml index 873176e192c..b0cacc6f259 100644 --- a/charts/kueue/templates/crd/kueue.x-k8s.io_cohorts.yaml +++ b/charts/kueue/templates/crd/kueue.x-k8s.io_cohorts.yaml @@ -252,6 +252,28 @@ spec: when participating in Fair Sharing. The is recorded only when Fair Sharing is enabled in the Kueue configuration. properties: + admissionFairSharingStatus: + description: admissionFairSharingStatus represents information + relevant to the Admission Fair Sharing + properties: + consumedResources: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + ConsumedResources represents the aggregated usage of resources over time, + with decaying function applied. + The value is populated if usage consumption functionality is enabled in Kueue config. + type: object + lastUpdate: + description: LastUpdate is the time when share and consumed + resources were updated. + format: date-time + type: string + type: object weightedShare: description: |- WeightedShare represents the maximum of the ratios of usage diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml index 35e96fd8423..0feaa9e04d3 100644 --- a/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml +++ b/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml @@ -80,6 +80,31 @@ spec: x-kubernetes-validations: - message: field is immutable rule: self == oldSelf + fairSharing: + description: |- + fairSharing defines the properties of the LocalQueue when + participating in FairSharing. The values are only relevant + if FairSharing is enabled in the Kueue configuration. + properties: + weight: + anyOf: + - type: integer + - type: string + default: 1 + description: |- + weight gives a comparative advantage to this ClusterQueue + or Cohort when competing for unused resources in the + Cohort. The share is based on the dominant resource usage + above nominal quotas for each resource, divided by the + weight. Admission prioritizes scheduling workloads from + ClusterQueues and Cohorts with the lowest share and + preempting workloads from the ClusterQueues and Cohorts + with the highest share. A zero weight implies infinite + share value, meaning that this Node will always be at + disadvantage against other ClusterQueues and Cohorts. + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: object stopPolicy: default: None description: |- @@ -168,6 +193,46 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map + fairSharingStatus: + description: FairSharing contains the information about the current + status of fair sharing. + properties: + admissionFairSharingStatus: + description: admissionFairSharingStatus represents information + relevant to the Admission Fair Sharing + properties: + consumedResources: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + ConsumedResources represents the aggregated usage of resources over time, + with decaying function applied. + The value is populated if usage consumption functionality is enabled in Kueue config. + type: object + lastUpdate: + description: LastUpdate is the time when share and consumed + resources were updated. + format: date-time + type: string + type: object + weightedShare: + description: |- + WeightedShare represents the maximum of the ratios of usage + above nominal quota to the lendable resources in the + Cohort, among all the resources provided by the Node, and + divided by the weight. If zero, it means that the usage of + the Node is below the nominal quota. If the Node has a + weight of zero and is borrowing, this will return + 9223372036854775807, the maximum possible share value. + format: int64 + type: integer + required: + - weightedShare + type: object flavorUsage: description: |- flavorsUsage are the used quotas, by flavor currently in use by the diff --git a/client-go/applyconfiguration/kueue/v1beta1/admissionfairsharingstatus.go b/client-go/applyconfiguration/kueue/v1beta1/admissionfairsharingstatus.go new file mode 100644 index 00000000000..5eb784ce6f9 --- /dev/null +++ b/client-go/applyconfiguration/kueue/v1beta1/admissionfairsharingstatus.go @@ -0,0 +1,52 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +// Code generated by applyconfiguration-gen. DO NOT EDIT. + +package v1beta1 + +import ( + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// AdmissionFairSharingStatusApplyConfiguration represents a declarative configuration of the AdmissionFairSharingStatus type for use +// with apply. +type AdmissionFairSharingStatusApplyConfiguration struct { + ConsumedResources *v1.ResourceList `json:"consumedResources,omitempty"` + LastUpdate *metav1.Time `json:"lastUpdate,omitempty"` +} + +// AdmissionFairSharingStatusApplyConfiguration constructs a declarative configuration of the AdmissionFairSharingStatus type for use with +// apply. +func AdmissionFairSharingStatus() *AdmissionFairSharingStatusApplyConfiguration { + return &AdmissionFairSharingStatusApplyConfiguration{} +} + +// WithConsumedResources sets the ConsumedResources field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the ConsumedResources field is set to the value of the last call. +func (b *AdmissionFairSharingStatusApplyConfiguration) WithConsumedResources(value v1.ResourceList) *AdmissionFairSharingStatusApplyConfiguration { + b.ConsumedResources = &value + return b +} + +// WithLastUpdate sets the LastUpdate field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the LastUpdate field is set to the value of the last call. +func (b *AdmissionFairSharingStatusApplyConfiguration) WithLastUpdate(value metav1.Time) *AdmissionFairSharingStatusApplyConfiguration { + b.LastUpdate = &value + return b +} diff --git a/client-go/applyconfiguration/kueue/v1beta1/admissionscope.go b/client-go/applyconfiguration/kueue/v1beta1/admissionscope.go new file mode 100644 index 00000000000..6d23affc278 --- /dev/null +++ b/client-go/applyconfiguration/kueue/v1beta1/admissionscope.go @@ -0,0 +1,42 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +// Code generated by applyconfiguration-gen. DO NOT EDIT. + +package v1beta1 + +import ( + kueuev1beta1 "sigs.k8s.io/kueue/apis/kueue/v1beta1" +) + +// AdmissionScopeApplyConfiguration represents a declarative configuration of the AdmissionScope type for use +// with apply. +type AdmissionScopeApplyConfiguration struct { + AdmissionMode *kueuev1beta1.AdmissionMode `json:"admissionMode,omitempty"` +} + +// AdmissionScopeApplyConfiguration constructs a declarative configuration of the AdmissionScope type for use with +// apply. +func AdmissionScope() *AdmissionScopeApplyConfiguration { + return &AdmissionScopeApplyConfiguration{} +} + +// WithAdmissionMode sets the AdmissionMode field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the AdmissionMode field is set to the value of the last call. +func (b *AdmissionScopeApplyConfiguration) WithAdmissionMode(value kueuev1beta1.AdmissionMode) *AdmissionScopeApplyConfiguration { + b.AdmissionMode = &value + return b +} diff --git a/client-go/applyconfiguration/kueue/v1beta1/clusterqueuespec.go b/client-go/applyconfiguration/kueue/v1beta1/clusterqueuespec.go index 41a155d831d..62a02cc5ba4 100644 --- a/client-go/applyconfiguration/kueue/v1beta1/clusterqueuespec.go +++ b/client-go/applyconfiguration/kueue/v1beta1/clusterqueuespec.go @@ -35,6 +35,7 @@ type ClusterQueueSpecApplyConfiguration struct { AdmissionChecksStrategy *AdmissionChecksStrategyApplyConfiguration `json:"admissionChecksStrategy,omitempty"` StopPolicy *kueuev1beta1.StopPolicy `json:"stopPolicy,omitempty"` FairSharing *FairSharingApplyConfiguration `json:"fairSharing,omitempty"` + AdmissionScope *AdmissionScopeApplyConfiguration `json:"admissionScope,omitempty"` } // ClusterQueueSpecApplyConfiguration constructs a declarative configuration of the ClusterQueueSpec type for use with @@ -129,3 +130,11 @@ func (b *ClusterQueueSpecApplyConfiguration) WithFairSharing(value *FairSharingA b.FairSharing = value return b } + +// WithAdmissionScope sets the AdmissionScope field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the AdmissionScope field is set to the value of the last call. +func (b *ClusterQueueSpecApplyConfiguration) WithAdmissionScope(value *AdmissionScopeApplyConfiguration) *ClusterQueueSpecApplyConfiguration { + b.AdmissionScope = value + return b +} diff --git a/client-go/applyconfiguration/kueue/v1beta1/fairsharingstatus.go b/client-go/applyconfiguration/kueue/v1beta1/fairsharingstatus.go index da3a5b0004f..1d716ce2e14 100644 --- a/client-go/applyconfiguration/kueue/v1beta1/fairsharingstatus.go +++ b/client-go/applyconfiguration/kueue/v1beta1/fairsharingstatus.go @@ -20,7 +20,8 @@ package v1beta1 // FairSharingStatusApplyConfiguration represents a declarative configuration of the FairSharingStatus type for use // with apply. type FairSharingStatusApplyConfiguration struct { - WeightedShare *int64 `json:"weightedShare,omitempty"` + WeightedShare *int64 `json:"weightedShare,omitempty"` + AdmissionFairSharingStatus *AdmissionFairSharingStatusApplyConfiguration `json:"admissionFairSharingStatus,omitempty"` } // FairSharingStatusApplyConfiguration constructs a declarative configuration of the FairSharingStatus type for use with @@ -36,3 +37,11 @@ func (b *FairSharingStatusApplyConfiguration) WithWeightedShare(value int64) *Fa b.WeightedShare = &value return b } + +// WithAdmissionFairSharingStatus sets the AdmissionFairSharingStatus field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the AdmissionFairSharingStatus field is set to the value of the last call. +func (b *FairSharingStatusApplyConfiguration) WithAdmissionFairSharingStatus(value *AdmissionFairSharingStatusApplyConfiguration) *FairSharingStatusApplyConfiguration { + b.AdmissionFairSharingStatus = value + return b +} diff --git a/client-go/applyconfiguration/kueue/v1beta1/localqueuespec.go b/client-go/applyconfiguration/kueue/v1beta1/localqueuespec.go index 8af741e9aed..c65b91f2bda 100644 --- a/client-go/applyconfiguration/kueue/v1beta1/localqueuespec.go +++ b/client-go/applyconfiguration/kueue/v1beta1/localqueuespec.go @@ -26,6 +26,7 @@ import ( type LocalQueueSpecApplyConfiguration struct { ClusterQueue *kueuev1beta1.ClusterQueueReference `json:"clusterQueue,omitempty"` StopPolicy *kueuev1beta1.StopPolicy `json:"stopPolicy,omitempty"` + FairSharing *FairSharingApplyConfiguration `json:"fairSharing,omitempty"` } // LocalQueueSpecApplyConfiguration constructs a declarative configuration of the LocalQueueSpec type for use with @@ -49,3 +50,11 @@ func (b *LocalQueueSpecApplyConfiguration) WithStopPolicy(value kueuev1beta1.Sto b.StopPolicy = &value return b } + +// WithFairSharing sets the FairSharing field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the FairSharing field is set to the value of the last call. +func (b *LocalQueueSpecApplyConfiguration) WithFairSharing(value *FairSharingApplyConfiguration) *LocalQueueSpecApplyConfiguration { + b.FairSharing = value + return b +} diff --git a/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go b/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go index 16f7ec661a7..f203d0b0261 100644 --- a/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go +++ b/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go @@ -31,6 +31,7 @@ type LocalQueueStatusApplyConfiguration struct { FlavorsReservation []LocalQueueFlavorUsageApplyConfiguration `json:"flavorsReservation,omitempty"` FlavorUsage []LocalQueueFlavorUsageApplyConfiguration `json:"flavorUsage,omitempty"` Flavors []LocalQueueFlavorStatusApplyConfiguration `json:"flavors,omitempty"` + FairSharingStatus *FairSharingStatusApplyConfiguration `json:"fairSharingStatus,omitempty"` } // LocalQueueStatusApplyConfiguration constructs a declarative configuration of the LocalQueueStatus type for use with @@ -114,3 +115,11 @@ func (b *LocalQueueStatusApplyConfiguration) WithFlavors(values ...*LocalQueueFl } return b } + +// WithFairSharingStatus sets the FairSharingStatus field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the FairSharingStatus field is set to the value of the last call. +func (b *LocalQueueStatusApplyConfiguration) WithFairSharingStatus(value *FairSharingStatusApplyConfiguration) *LocalQueueStatusApplyConfiguration { + b.FairSharingStatus = value + return b +} diff --git a/client-go/applyconfiguration/utils.go b/client-go/applyconfiguration/utils.go index 9b153cd165f..8e20e0dc743 100644 --- a/client-go/applyconfiguration/utils.go +++ b/client-go/applyconfiguration/utils.go @@ -59,6 +59,10 @@ func ForKind(kind schema.GroupVersionKind) interface{} { return &kueuev1beta1.AdmissionCheckStatusApplyConfiguration{} case v1beta1.SchemeGroupVersion.WithKind("AdmissionCheckStrategyRule"): return &kueuev1beta1.AdmissionCheckStrategyRuleApplyConfiguration{} + case v1beta1.SchemeGroupVersion.WithKind("AdmissionFairSharingStatus"): + return &kueuev1beta1.AdmissionFairSharingStatusApplyConfiguration{} + case v1beta1.SchemeGroupVersion.WithKind("AdmissionScope"): + return &kueuev1beta1.AdmissionScopeApplyConfiguration{} case v1beta1.SchemeGroupVersion.WithKind("BorrowWithinCohort"): return &kueuev1beta1.BorrowWithinCohortApplyConfiguration{} case v1beta1.SchemeGroupVersion.WithKind("ClusterQueue"): diff --git a/cmd/kueue/main.go b/cmd/kueue/main.go index e132a090c84..979f3878e5b 100644 --- a/cmd/kueue/main.go +++ b/cmd/kueue/main.go @@ -217,6 +217,7 @@ func main() { } if cfg.FairSharing != nil { cacheOptions = append(cacheOptions, cache.WithFairSharing(cfg.FairSharing.Enable)) + queueOptions = append(queueOptions, queue.WithFairSharing(cfg.FairSharing)) } cCache := cache.New(mgr.GetClient(), cacheOptions...) queues := queue.NewManager(mgr.GetClient(), cCache, queueOptions...) diff --git a/config/components/crd/bases/kueue.x-k8s.io_clusterqueues.yaml b/config/components/crd/bases/kueue.x-k8s.io_clusterqueues.yaml index 6d98229eced..e5d39f68d4d 100644 --- a/config/components/crd/bases/kueue.x-k8s.io_clusterqueues.yaml +++ b/config/components/crd/bases/kueue.x-k8s.io_clusterqueues.yaml @@ -100,6 +100,13 @@ spec: type: object type: array type: object + admissionScope: + description: admissionScope indicates whether ClusterQueue uses the + Admission Fair Sharing + properties: + admissionMode: + type: string + type: object cohort: description: |- cohort that this ClusterQueue belongs to. CQs that belong to the @@ -575,6 +582,28 @@ spec: when participating in Fair Sharing. This is recorded only when Fair Sharing is enabled in the Kueue configuration. properties: + admissionFairSharingStatus: + description: admissionFairSharingStatus represents information + relevant to the Admission Fair Sharing + properties: + consumedResources: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + ConsumedResources represents the aggregated usage of resources over time, + with decaying function applied. + The value is populated if usage consumption functionality is enabled in Kueue config. + type: object + lastUpdate: + description: LastUpdate is the time when share and consumed + resources were updated. + format: date-time + type: string + type: object weightedShare: description: |- WeightedShare represents the maximum of the ratios of usage diff --git a/config/components/crd/bases/kueue.x-k8s.io_cohorts.yaml b/config/components/crd/bases/kueue.x-k8s.io_cohorts.yaml index ac89defd6ae..812c377a56c 100644 --- a/config/components/crd/bases/kueue.x-k8s.io_cohorts.yaml +++ b/config/components/crd/bases/kueue.x-k8s.io_cohorts.yaml @@ -237,6 +237,28 @@ spec: when participating in Fair Sharing. The is recorded only when Fair Sharing is enabled in the Kueue configuration. properties: + admissionFairSharingStatus: + description: admissionFairSharingStatus represents information + relevant to the Admission Fair Sharing + properties: + consumedResources: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + ConsumedResources represents the aggregated usage of resources over time, + with decaying function applied. + The value is populated if usage consumption functionality is enabled in Kueue config. + type: object + lastUpdate: + description: LastUpdate is the time when share and consumed + resources were updated. + format: date-time + type: string + type: object weightedShare: description: |- WeightedShare represents the maximum of the ratios of usage diff --git a/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml b/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml index c0ff3951c5d..f0b7e462878 100644 --- a/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml +++ b/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml @@ -65,6 +65,31 @@ spec: x-kubernetes-validations: - message: field is immutable rule: self == oldSelf + fairSharing: + description: |- + fairSharing defines the properties of the LocalQueue when + participating in FairSharing. The values are only relevant + if FairSharing is enabled in the Kueue configuration. + properties: + weight: + anyOf: + - type: integer + - type: string + default: 1 + description: |- + weight gives a comparative advantage to this ClusterQueue + or Cohort when competing for unused resources in the + Cohort. The share is based on the dominant resource usage + above nominal quotas for each resource, divided by the + weight. Admission prioritizes scheduling workloads from + ClusterQueues and Cohorts with the lowest share and + preempting workloads from the ClusterQueues and Cohorts + with the highest share. A zero weight implies infinite + share value, meaning that this Node will always be at + disadvantage against other ClusterQueues and Cohorts. + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: object stopPolicy: default: None description: |- @@ -153,6 +178,46 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map + fairSharingStatus: + description: FairSharing contains the information about the current + status of fair sharing. + properties: + admissionFairSharingStatus: + description: admissionFairSharingStatus represents information + relevant to the Admission Fair Sharing + properties: + consumedResources: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + ConsumedResources represents the aggregated usage of resources over time, + with decaying function applied. + The value is populated if usage consumption functionality is enabled in Kueue config. + type: object + lastUpdate: + description: LastUpdate is the time when share and consumed + resources were updated. + format: date-time + type: string + type: object + weightedShare: + description: |- + WeightedShare represents the maximum of the ratios of usage + above nominal quota to the lendable resources in the + Cohort, among all the resources provided by the Node, and + divided by the weight. If zero, it means that the usage of + the Node is below the nominal quota. If the Node has a + weight of zero and is borrowing, this will return + 9223372036854775807, the maximum possible share value. + format: int64 + type: integer + required: + - weightedShare + type: object flavorUsage: description: |- flavorsUsage are the used quotas, by flavor currently in use by the diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index a92572f1fcc..96cf81ae3b7 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -501,6 +501,19 @@ func (c *Cache) DeleteLocalQueue(q *kueue.LocalQueue) { cq.deleteLocalQueue(q) } +func (c *Cache) GetCacheLocalQueue(cqName kueue.ClusterQueueReference, lq *kueue.LocalQueue) (*LocalQueue, error) { + c.Lock() + defer c.Unlock() + cq := c.hm.ClusterQueue(cqName) + if cq == nil { + return nil, ErrCqNotFound + } + if cacheLq, ok := cq.localQueues[queueKey(lq)]; ok { + return cacheLq, nil + } + return nil, errQNotFound +} + func (c *Cache) UpdateLocalQueue(oldQ, newQ *kueue.LocalQueue) error { if oldQ.Spec.ClusterQueue == newQ.Spec.ClusterQueue { return nil diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 3f1295b3d6c..e150e52fea5 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "sync" "testing" "github.com/google/go-cmp/cmp" @@ -2736,7 +2737,7 @@ func TestCacheQueueOperations(t *testing.T) { cacheQueues[qKey] = cacheQ } } - if diff := cmp.Diff(tc.wantLocalQueues, cacheQueues, cmp.AllowUnexported(LocalQueue{}), cmpopts.EquateEmpty()); diff != "" { + if diff := cmp.Diff(tc.wantLocalQueues, cacheQueues, cmp.AllowUnexported(LocalQueue{}), cmpopts.EquateEmpty(), cmpopts.IgnoreTypes(sync.RWMutex{})); diff != "" { t.Errorf("Unexpected localQueues (-want,+got):\n%s", diff) } }) diff --git a/pkg/cache/clusterqueue.go b/pkg/cache/clusterqueue.go index c571f5341ce..fc69749f25d 100644 --- a/pkg/cache/clusterqueue.go +++ b/pkg/cache/clusterqueue.go @@ -485,7 +485,7 @@ func (c *clusterQueue) updateWorkloadUsage(wi *workload.Info, m int64) { updateFlavorUsage(frUsage, lq.totalReserved, m) lq.reservingWorkloads += int(m) if admitted { - updateFlavorUsage(frUsage, lq.admittedUsage, m) + lq.UpdateAdmittedUsage(frUsage, m) lq.admittedWorkloads += int(m) } if features.Enabled(features.LocalQueueMetrics) { @@ -529,7 +529,7 @@ func (c *clusterQueue) addLocalQueue(q *kueue.LocalQueue) error { updateFlavorUsage(frq, qImpl.totalReserved, 1) qImpl.reservingWorkloads++ if workload.IsAdmitted(wl.Obj) { - updateFlavorUsage(frq, qImpl.admittedUsage, 1) + qImpl.UpdateAdmittedUsage(frq, 1) qImpl.admittedWorkloads++ } } @@ -566,6 +566,8 @@ func (c *clusterQueue) flavorInUse(flavor kueue.ResourceFlavorReference) bool { func (q *LocalQueue) resetFlavorsAndResources(cqUsage resources.FlavorResourceQuantities, cqAdmittedUsage resources.FlavorResourceQuantities) { // Clean up removed flavors or resources. + q.Lock() + defer q.Unlock() q.totalReserved = resetUsage(q.totalReserved, cqUsage) q.admittedUsage = resetUsage(q.admittedUsage, cqAdmittedUsage) } diff --git a/pkg/cache/localqueue.go b/pkg/cache/localqueue.go index 5c1ad1d7720..0faab60c597 100644 --- a/pkg/cache/localqueue.go +++ b/pkg/cache/localqueue.go @@ -17,14 +17,31 @@ limitations under the License. package cache import ( + "sync" + + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/kueue/pkg/queue" "sigs.k8s.io/kueue/pkg/resources" ) type LocalQueue struct { + sync.RWMutex key queue.LocalQueueReference reservingWorkloads int admittedWorkloads int totalReserved resources.FlavorResourceQuantities admittedUsage resources.FlavorResourceQuantities } + +func (lq *LocalQueue) GetAdmittedUsage() corev1.ResourceList { + lq.RLock() + defer lq.RUnlock() + return lq.admittedUsage.FlattenFlavors().ToResourceList() +} + +func (lq *LocalQueue) UpdateAdmittedUsage(usage resources.FlavorResourceQuantities, op int64) { + lq.Lock() + defer lq.Unlock() + updateFlavorUsage(usage, lq.admittedUsage, op) +} diff --git a/pkg/controller/core/core.go b/pkg/controller/core/core.go index 5658abed3e9..d1954b4e626 100644 --- a/pkg/controller/core/core.go +++ b/pkg/controller/core/core.go @@ -43,7 +43,8 @@ func SetupControllers(mgr ctrl.Manager, qManager *queue.Manager, cc *cache.Cache if err := acRec.SetupWithManager(mgr, cfg); err != nil { return "AdmissionCheck", err } - qRec := NewLocalQueueReconciler(mgr.GetClient(), qManager, cc) + qRec := NewLocalQueueReconciler(mgr.GetClient(), qManager, cc, + WithFairSharingConfig(cfg.FairSharing)) if err := qRec.SetupWithManager(mgr, cfg); err != nil { return "LocalQueue", err } diff --git a/pkg/controller/core/localqueue_controller.go b/pkg/controller/core/localqueue_controller.go index f55fa3f9e53..1806d1fa810 100644 --- a/pkg/controller/core/localqueue_controller.go +++ b/pkg/controller/core/localqueue_controller.go @@ -18,8 +18,10 @@ package core import ( "context" + "math" "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -27,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" + "k8s.io/utils/clock" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" @@ -60,13 +63,43 @@ const ( clusterQueueIsInactiveReason = "ClusterQueueIsInactive" ) +type LocalQueueReconcilerOptions struct { + admissionFSConfig *config.AdmissionFairSharing + clock clock.Clock +} + +// LocalQueueReconcilerOption configures the reconciler. +type LocalQueueReconcilerOption func(*LocalQueueReconcilerOptions) + +func WithFairSharingConfig(cfg *config.FairSharing) LocalQueueReconcilerOption { + return func(o *LocalQueueReconcilerOptions) { + if cfg == nil || !cfg.Enable || cfg.AdmissionFairSharing == nil { + o.admissionFSConfig = nil + } else { + o.admissionFSConfig = cfg.AdmissionFairSharing + } + } +} + +func WithClock(c clock.Clock) LocalQueueReconcilerOption { + return func(o *LocalQueueReconcilerOptions) { + o.clock = c + } +} + +var defaultLQOptions = LocalQueueReconcilerOptions{ + clock: realClock, +} + // LocalQueueReconciler reconciles a LocalQueue object type LocalQueueReconciler struct { - client client.Client - log logr.Logger - queues *queue.Manager - cache *cache.Cache - wlUpdateCh chan event.GenericEvent + client client.Client + log logr.Logger + queues *queue.Manager + cache *cache.Cache + wlUpdateCh chan event.GenericEvent + admissionFSConfig *config.AdmissionFairSharing + clock clock.Clock } var _ reconcile.Reconciler = (*LocalQueueReconciler)(nil) @@ -76,13 +109,20 @@ func NewLocalQueueReconciler( client client.Client, queues *queue.Manager, cache *cache.Cache, + opts ...LocalQueueReconcilerOption, ) *LocalQueueReconciler { + options := defaultLQOptions + for _, opt := range opts { + opt(&options) + } return &LocalQueueReconciler{ - log: ctrl.Log.WithName("localqueue-reconciler"), - queues: queues, - cache: cache, - client: client, - wlUpdateCh: make(chan event.GenericEvent, updateChBuffer), + log: ctrl.Log.WithName("localqueue-reconciler"), + queues: queues, + cache: cache, + client: client, + wlUpdateCh: make(chan event.GenericEvent, updateChBuffer), + admissionFSConfig: options.admissionFSConfig, + clock: options.clock, } } @@ -105,8 +145,8 @@ func (r *LocalQueueReconciler) NotifyWorkloadUpdate(oldWl, newWl *kueue.Workload // +kubebuilder:rbac:groups=kueue.x-k8s.io,resources=localqueues/finalizers,verbs=update func (r *LocalQueueReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - var queueObj kueue.LocalQueue - if err := r.client.Get(ctx, req.NamespacedName, &queueObj); err != nil { + var lq kueue.LocalQueue + if err := r.client.Get(ctx, req.NamespacedName, &lq); err != nil { // we'll ignore not-found errors, since there is nothing to do. return ctrl.Result{}, client.IgnoreNotFound(err) } @@ -114,25 +154,45 @@ func (r *LocalQueueReconciler) Reconcile(ctx context.Context, req ctrl.Request) log := ctrl.LoggerFrom(ctx) log.V(2).Info("Reconcile LocalQueue") - if ptr.Deref(queueObj.Spec.StopPolicy, kueue.None) != kueue.None { - err := r.UpdateStatusIfChanged(ctx, &queueObj, metav1.ConditionFalse, StoppedReason, localQueueIsInactiveMsg) + if ptr.Deref(lq.Spec.StopPolicy, kueue.None) != kueue.None { + err := r.UpdateStatusIfChanged(ctx, &lq, metav1.ConditionFalse, StoppedReason, localQueueIsInactiveMsg) return ctrl.Result{}, client.IgnoreNotFound(err) } var cq kueue.ClusterQueue - err := r.client.Get(ctx, client.ObjectKey{Name: string(queueObj.Spec.ClusterQueue)}, &cq) - if err != nil { + if err := r.client.Get(ctx, client.ObjectKey{Name: string(lq.Spec.ClusterQueue)}, &cq); err != nil { if apierrors.IsNotFound(err) { - err = r.UpdateStatusIfChanged(ctx, &queueObj, metav1.ConditionFalse, "ClusterQueueDoesNotExist", clusterQueueIsInactiveMsg) + err = r.UpdateStatusIfChanged(ctx, &lq, metav1.ConditionFalse, "ClusterQueueDoesNotExist", clusterQueueIsInactiveMsg) } return ctrl.Result{}, client.IgnoreNotFound(err) } if meta.IsStatusConditionTrue(cq.Status.Conditions, kueue.ClusterQueueActive) { - err = r.UpdateStatusIfChanged(ctx, &queueObj, metav1.ConditionTrue, "Ready", "Can submit new workloads to localQueue") - return ctrl.Result{}, client.IgnoreNotFound(err) + if err := r.UpdateStatusIfChanged(ctx, &lq, metav1.ConditionTrue, "Ready", "Can submit new workloads to localQueue"); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + } else { + if err := r.UpdateStatusIfChanged(ctx, &lq, metav1.ConditionFalse, clusterQueueIsInactiveReason, clusterQueueIsInactiveMsg); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } } - err = r.UpdateStatusIfChanged(ctx, &queueObj, metav1.ConditionFalse, clusterQueueIsInactiveReason, clusterQueueIsInactiveMsg) - return ctrl.Result{}, client.IgnoreNotFound(err) + + if r.admissionFSConfig != nil { + if err := r.initializeAdmissionFsStatus(ctx, &lq); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + recheckAfter := r.clock.Now().Sub(lq.Status.FairSharingStatus.AdmissionFairSharingStatus.LastUpdate.Time) + if recheckAfter < r.admissionFSConfig.UsageSamplingInterval.Duration { + return ctrl.Result{RequeueAfter: recheckAfter}, nil + } + if err := r.reconcileConsumedUsage(ctx, &lq, lq.Spec.ClusterQueue); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + if err := r.queues.HeapifyClusterQueue(&cq, lq.Name); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: r.admissionFSConfig.UsageSamplingInterval.Duration}, nil + } + return ctrl.Result{}, nil } func (r *LocalQueueReconciler) Create(e event.TypedCreateEvent[*kueue.LocalQueue]) bool { @@ -204,6 +264,46 @@ func (r *LocalQueueReconciler) Update(e event.TypedUpdateEvent[*kueue.LocalQueue return true } +func (r *LocalQueueReconciler) initializeAdmissionFsStatus(ctx context.Context, lq *kueue.LocalQueue) error { + if lq.Status.FairSharingStatus.AdmissionFairSharingStatus == nil { + lq.Status.FairSharingStatus.AdmissionFairSharingStatus = &kueue.AdmissionFairSharingStatus{ + LastUpdate: metav1.NewTime(r.clock.Now()), + } + return r.client.Status().Update(ctx, lq) + } + return nil +} + +func (r *LocalQueueReconciler) reconcileConsumedUsage(ctx context.Context, lq *kueue.LocalQueue, cqName kueue.ClusterQueueReference) error { + halfLifeTime := r.admissionFSConfig.UsageHalfLifeTime.Seconds() + + // reset usage to 0 if halfLife is 0 + if halfLifeTime == 0 { + return r.updateAdmissionFsStatus(ctx, lq, corev1.ResourceList{}) + } + cacheLq, err := r.cache.GetCacheLocalQueue(cqName, lq) + if err != nil { + return err + } + // calculate alpha rate + oldUsage := lq.Status.FairSharingStatus.AdmissionFairSharingStatus.ConsumedResources + newUsage := cacheLq.GetAdmittedUsage() + timeSinceLastUpdate := r.clock.Now().Sub(lq.Status.FairSharingStatus.AdmissionFairSharingStatus.LastUpdate.Time).Seconds() + alpha := 1.0 - math.Pow(0.5, timeSinceLastUpdate/halfLifeTime) + // calculate weighted average of old and new usage + scaledNewUsage := resource.MulByFloat(newUsage, alpha) + scaledOldUsage := resource.MulByFloat(oldUsage, 1-alpha) + sum := resource.MergeResourceListKeepSum(scaledOldUsage, scaledNewUsage) + // update status + return r.updateAdmissionFsStatus(ctx, lq, sum) +} + +func (r *LocalQueueReconciler) updateAdmissionFsStatus(ctx context.Context, lq *kueue.LocalQueue, consumedResources corev1.ResourceList) error { + lq.Status.FairSharingStatus.AdmissionFairSharingStatus.ConsumedResources = consumedResources + lq.Status.FairSharingStatus.AdmissionFairSharingStatus.LastUpdate = metav1.NewTime(r.clock.Now()) + return r.client.Status().Update(ctx, lq) +} + func localQueueReferenceFromLocalQueue(lq *kueue.LocalQueue) metrics.LocalQueueReference { return metrics.LocalQueueReference{ Name: kueue.LocalQueueName(lq.Name), diff --git a/pkg/controller/core/localqueue_controller_test.go b/pkg/controller/core/localqueue_controller_test.go index 001c7bb4925..113782ecf53 100644 --- a/pkg/controller/core/localqueue_controller_test.go +++ b/pkg/controller/core/localqueue_controller_test.go @@ -19,15 +19,21 @@ package core import ( "context" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + testingclock "k8s.io/utils/clock/testing" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/reconcile" + config "sigs.k8s.io/kueue/apis/config/v1beta1" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" "sigs.k8s.io/kueue/pkg/cache" "sigs.k8s.io/kueue/pkg/queue" @@ -36,11 +42,14 @@ import ( ) func TestLocalQueueReconcile(t *testing.T) { + clock := testingclock.NewFakeClock(time.Now().Truncate(time.Second)) cases := map[string]struct { clusterQueue *kueue.ClusterQueue localQueue *kueue.LocalQueue wantLocalQueue *kueue.LocalQueue wantError error + fsConfig *config.FairSharing + runningWls []kueue.Workload }{ "local queue with Hold StopPolicy": { clusterQueue: utiltesting.MakeClusterQueue("test-cluster-queue"). @@ -112,6 +121,404 @@ func TestLocalQueueReconcile(t *testing.T) { Obj(), wantError: nil, }, + "local queue decaying usage decays if there is no running workloads": { + clusterQueue: utiltesting.MakeClusterQueue("cq"). + Active(metav1.ConditionTrue). + Obj(), + localQueue: utiltesting.MakeLocalQueue("test-queue", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + LastUpdate: metav1.NewTime(clock.Now().Add(-5 * time.Minute)), + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("8")}, + }, + }). + Obj(), + wantLocalQueue: utiltesting.MakeLocalQueue("test-queue", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("4"), + }, + }, + }). + Obj(), + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, + }, + }, + }, + "local queue decaying usage sums the previous state and running workloads": { + clusterQueue: utiltesting.MakeClusterQueue("cq"). + Active(metav1.ConditionTrue). + Obj(), + localQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + LastUpdate: metav1.NewTime(clock.Now().Add(-5 * time.Minute)), + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("8")}, + }, + }). + Obj(), + wantLocalQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + ReservingWorkloads(1). + AdmittedWorkloads(1). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("6"), + }, + }, + }). + Obj(), + runningWls: []kueue.Workload{ + *utiltesting.MakeWorkload("wl", "default"). + Queue("lq"). + Request(corev1.ResourceCPU, "4"). + SimpleReserveQuota("cq", "rf", clock.Now()). + Admitted(true). + Obj(), + }, + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, + }, + }, + }, + "local queue decaying usage sums the usage from different flavors and resources": { + clusterQueue: utiltesting.MakeClusterQueue("cq"). + Active(metav1.ConditionTrue). + Obj(), + localQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + LastUpdate: metav1.NewTime(clock.Now().Add(-5 * time.Minute)), + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("8"), + "GPU": resource.MustParse("16"), + }, + }, + }). + Obj(), + wantLocalQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + ReservingWorkloads(3). + AdmittedWorkloads(3). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("6"), + "GPU": resource.MustParse("10"), + }, + }, + }). + Obj(), + runningWls: []kueue.Workload{ + *utiltesting.MakeWorkload("wl-1", "default"). + Queue("lq"). + Request(corev1.ResourceCPU, "2"). + SimpleReserveQuota("cq", "rf-1", clock.Now()). + Admitted(true). + Obj(), + *utiltesting.MakeWorkload("wl-2", "default"). + Queue("lq"). + Request(corev1.ResourceCPU, "2"). + SimpleReserveQuota("cq", "rf-2", clock.Now()). + Admitted(true). + Obj(), + *utiltesting.MakeWorkload("wl-3", "default"). + Queue("lq"). + Request("GPU", "4"). + SimpleReserveQuota("cq", "rf-3", clock.Now()). + Admitted(true). + Obj(), + }, + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, + }, + }, + }, + "local queue decaying usage sums the previous state and running workloads half time twice larger than sampling": { + clusterQueue: utiltesting.MakeClusterQueue("cq"). + Active(metav1.ConditionTrue). + Obj(), + localQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + LastUpdate: metav1.NewTime(clock.Now().Add(-5 * time.Minute)), + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("8")}, + }, + }). + Obj(), + wantLocalQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + ReservingWorkloads(1). + AdmittedWorkloads(1). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("6827m"), + }, + }, + }). + Obj(), + runningWls: []kueue.Workload{ + *utiltesting.MakeWorkload("wl", "default"). + Queue("lq"). + Request(corev1.ResourceCPU, "4"). + SimpleReserveQuota("cq", "rf", clock.Now()). + Admitted(true). + Obj(), + }, + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 10 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, + }, + }, + }, + "local queue decaying usage sums the previous state and running workloads with long half time": { + clusterQueue: utiltesting.MakeClusterQueue("cq"). + Active(metav1.ConditionTrue). + Obj(), + localQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + LastUpdate: metav1.NewTime(clock.Now().Add(-5 * time.Minute)), + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("8")}, + }, + }). + Obj(), + wantLocalQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("7980m"), + }, + }, + }). + Obj(), + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 24 * time.Hour}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, + }, + }, + }, + "local queue decaying usage sums the previous state and running GPU workloads half time twice larger than sampling": { + clusterQueue: utiltesting.MakeClusterQueue("cq"). + Active(metav1.ConditionTrue). + Obj(), + localQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + LastUpdate: metav1.NewTime(clock.Now().Add(-5 * time.Minute)), + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + "GPU": resource.MustParse("8")}, + }, + }). + Obj(), + wantLocalQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + ReservingWorkloads(1). + AdmittedWorkloads(1). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + "GPU": resource.MustParse("6827m"), + }, + }, + }). + Obj(), + runningWls: []kueue.Workload{ + *utiltesting.MakeWorkload("wl", "default"). + Queue("lq"). + Request("GPU", "4"). + SimpleReserveQuota("cq", "rf", clock.Now()). + Admitted(true). + Obj(), + }, + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 10 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, + }, + }, + }, + "local queue decaying usage resets to 0 when half life is 0": { + clusterQueue: utiltesting.MakeClusterQueue("cq"). + Active(metav1.ConditionTrue). + Obj(), + localQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + LastUpdate: metav1.NewTime(clock.Now().Add(-5 * time.Minute)), + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + "GPU": resource.MustParse("8")}, + }, + }). + Obj(), + wantLocalQueue: utiltesting.MakeLocalQueue("lq", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + ReservingWorkloads(1). + AdmittedWorkloads(1). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{}, + }). + Obj(), + runningWls: []kueue.Workload{ + *utiltesting.MakeWorkload("wl", "default"). + Queue("lq"). + Request("GPU", "4"). + SimpleReserveQuota("cq", "rf", clock.Now()). + Admitted(true). + Obj(), + }, + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 0 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, + }, + }, + }, + "local queue decaying usage is not reconciled if not enough time has passed": { + clusterQueue: utiltesting.MakeClusterQueue("cq"). + Active(metav1.ConditionTrue). + Obj(), + localQueue: utiltesting.MakeLocalQueue("test-queue", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + LastUpdate: metav1.NewTime(clock.Now().Add(-4 * time.Minute)), + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("8")}, + }, + }). + Obj(), + wantLocalQueue: utiltesting.MakeLocalQueue("test-queue", "default"). + ClusterQueue("cq"). + Active(metav1.ConditionTrue). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + LastUpdate: metav1.NewTime(clock.Now().Add(-4 * time.Minute)), + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("8")}, + }, + }). + Obj(), + wantError: nil, + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, + }, + }, + }, } for name, tc := range cases { @@ -126,11 +533,23 @@ func TestLocalQueueReconcile(t *testing.T) { WithInterceptorFuncs(interceptor.Funcs{SubResourcePatch: utiltesting.TreatSSAAsStrategicMerge}). Build() + ctxWithLogger, _ := utiltesting.ContextWithLog(t) cqCache := cache.New(cl) + if err := cqCache.AddClusterQueue(ctxWithLogger, tc.clusterQueue); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + _ = cqCache.AddLocalQueue(tc.localQueue) + for _, wl := range tc.runningWls { + cqCache.AddOrUpdateWorkload(&wl) + } qManager := queue.NewManager(cl, cqCache) - ctxWithLogger, _ := utiltesting.ContextWithLog(t) + if err := qManager.AddClusterQueue(ctxWithLogger, tc.clusterQueue); err != nil { + t.Fatalf("Unexpected error: %v", err) + } _ = qManager.AddLocalQueue(ctxWithLogger, tc.localQueue) - reconciler := NewLocalQueueReconciler(cl, qManager, cqCache) + reconciler := NewLocalQueueReconciler(cl, qManager, cqCache, + WithClock(clock), + WithFairSharingConfig(tc.fsConfig)) ctx, ctxCancel := context.WithCancel(ctxWithLogger) defer ctxCancel() @@ -156,6 +575,7 @@ func TestLocalQueueReconcile(t *testing.T) { cmpopts.EquateEmpty(), util.IgnoreConditionTimestamps, util.IgnoreObjectMetaResourceVersion, + cmpopts.IgnoreFields(kueue.AdmissionFairSharingStatus{}, "LastUpdate"), } if diff := cmp.Diff(tc.wantLocalQueue, gotLocalQueue, cmpOpts...); diff != "" { t.Errorf("Workloads after reconcile (-want,+got):\n%s", diff) diff --git a/pkg/queue/cluster_queue.go b/pkg/queue/cluster_queue.go index 944a75259a0..f692905613f 100644 --- a/pkg/queue/cluster_queue.go +++ b/pkg/queue/cluster_queue.go @@ -28,8 +28,10 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/clock" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + config "sigs.k8s.io/kueue/apis/config/v1beta1" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" "sigs.k8s.io/kueue/pkg/hierarchy" "sigs.k8s.io/kueue/pkg/util/heap" @@ -89,8 +91,9 @@ func workloadKey(i *workload.Info) string { return workload.Key(i.Obj) } -func newClusterQueue(cq *kueue.ClusterQueue, wo workload.Ordering) (*ClusterQueue, error) { - cqImpl := newClusterQueueImpl(wo, realClock) +func newClusterQueue(ctx context.Context, client client.Client, cq *kueue.ClusterQueue, wo workload.Ordering, fsConfig *config.FairSharing) (*ClusterQueue, error) { + enableAdmissionFs, fsResWeights := isAdmissionFsEnabled(cq, fsConfig) + cqImpl := newClusterQueueImpl(ctx, client, wo, realClock, fsResWeights, enableAdmissionFs) err := cqImpl.Update(cq) if err != nil { return nil, err @@ -98,8 +101,18 @@ func newClusterQueue(cq *kueue.ClusterQueue, wo workload.Ordering) (*ClusterQueu return cqImpl, nil } -func newClusterQueueImpl(wo workload.Ordering, clock clock.Clock) *ClusterQueue { - lessFunc := queueOrderingFunc(wo) +func isAdmissionFsEnabled(cq *kueue.ClusterQueue, fsConfig *config.FairSharing) (bool, map[corev1.ResourceName]float64) { + enableAdmissionFs, fsResWeights := false, make(map[corev1.ResourceName]float64) + if fsConfig != nil && fsConfig.Enable && fsConfig.AdmissionFairSharing != nil && + cq.Spec.AdmissionScope != nil && cq.Spec.AdmissionScope.AdmissionMode == kueue.UsageBasedFairSharing { + enableAdmissionFs = true + fsResWeights = fsConfig.AdmissionFairSharing.ResourceWeights + } + return enableAdmissionFs, fsResWeights +} + +func newClusterQueueImpl(ctx context.Context, client client.Client, wo workload.Ordering, clock clock.Clock, fsResWeights map[corev1.ResourceName]float64, enableAdmissionFs bool) *ClusterQueue { + lessFunc := queueOrderingFunc(ctx, client, wo, fsResWeights, enableAdmissionFs) return &ClusterQueue{ heap: *heap.New(workloadKey, lessFunc), inadmissibleWorkloads: make(map[string]*workload.Info), @@ -171,6 +184,14 @@ func (c *ClusterQueue) PushOrUpdate(wInfo *workload.Info) { c.heap.PushOrUpdate(wInfo) } +func (c *ClusterQueue) Heapify(lqName string) { + for _, wl := range c.heap.List() { + if string(wl.Obj.Spec.QueueName) == lqName { + c.heap.PushOrUpdate(wl) + } + } +} + // backoffWaitingTimeExpired returns true if the current time is after the requeueAt // and Requeued condition not present or equal True. func (c *ClusterQueue) backoffWaitingTimeExpired(wInfo *workload.Info) bool { @@ -410,8 +431,24 @@ func (c *ClusterQueue) RequeueIfNotPresent(wInfo *workload.Info, reason RequeueR // to sort workloads. The function sorts workloads based on their priority. // When priorities are equal, it uses the workload's creation or eviction // time. -func queueOrderingFunc(wo workload.Ordering) func(a, b *workload.Info) bool { +func queueOrderingFunc(ctx context.Context, c client.Client, wo workload.Ordering, fsResWeights map[corev1.ResourceName]float64, enableAdmissionFs bool) func(a, b *workload.Info) bool { + log := ctrl.LoggerFrom(ctx) + return func(a, b *workload.Info) bool { + if enableAdmissionFs { + lqAUsage, errA := a.LqUsage(ctx, c, fsResWeights) + lqBUsage, errB := b.LqUsage(ctx, c, fsResWeights) + log.V(3).Info("Resource usage from LocalQueue", "LocalQueue", a.Obj.Spec.QueueName, "Usage", lqAUsage) + log.V(3).Info("Resource usage from LocalQueue", "LocalQueue", b.Obj.Spec.QueueName, "Usage", lqBUsage) + switch { + case errA != nil: + log.Error(errA, "Error fetching LocalQueue from informer") + case errB != nil: + log.Error(errB, "Error fetching LocalQueue from informer") + case lqAUsage != lqBUsage: + return lqAUsage < lqBUsage + } + } p1 := utilpriority.Priority(a.Obj) p2 := utilpriority.Priority(b.Obj) diff --git a/pkg/queue/cluster_queue_test.go b/pkg/queue/cluster_queue_test.go index a6b566c80c7..2901de4df8d 100644 --- a/pkg/queue/cluster_queue_test.go +++ b/pkg/queue/cluster_queue_test.go @@ -17,12 +17,14 @@ limitations under the License. package queue import ( + "context" "testing" "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" testingclock "k8s.io/utils/clock/testing" @@ -38,6 +40,10 @@ const ( defaultNamespace = "default" ) +const ( + resourceGPU corev1.ResourceName = "example.com/gpu" +) + const ( lowPriority int32 = 0 highPriority int32 = 1000 @@ -149,7 +155,7 @@ func Test_PushOrUpdate(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - cq := newClusterQueueImpl(defaultOrdering, fakeClock) + cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, fakeClock, nil, false) if cq.Pending() != 0 { t.Error("ClusterQueue should be empty") @@ -178,7 +184,7 @@ func Test_PushOrUpdate(t *testing.T) { func Test_Pop(t *testing.T) { now := time.Now() - cq := newClusterQueueImpl(defaultOrdering, testingclock.NewFakeClock(now)) + cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(now), nil, false) wl1 := workload.NewInfo(utiltesting.MakeWorkload("workload-1", defaultNamespace).Creation(now).Obj()) wl2 := workload.NewInfo(utiltesting.MakeWorkload("workload-2", defaultNamespace).Creation(now.Add(time.Second)).Obj()) if cq.Pop() != nil { @@ -200,7 +206,7 @@ func Test_Pop(t *testing.T) { } func Test_Delete(t *testing.T) { - cq := newClusterQueueImpl(defaultOrdering, testingclock.NewFakeClock(time.Now())) + cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) wl1 := utiltesting.MakeWorkload("workload-1", defaultNamespace).Obj() wl2 := utiltesting.MakeWorkload("workload-2", defaultNamespace).Obj() cq.PushOrUpdate(workload.NewInfo(wl1)) @@ -221,7 +227,7 @@ func Test_Delete(t *testing.T) { } func Test_Info(t *testing.T) { - cq := newClusterQueueImpl(defaultOrdering, testingclock.NewFakeClock(time.Now())) + cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) wl := utiltesting.MakeWorkload("workload-1", defaultNamespace).Obj() if info := cq.Info(workload.Key(wl)); info != nil { t.Error("Workload should not exist") @@ -233,7 +239,7 @@ func Test_Info(t *testing.T) { } func Test_AddFromLocalQueue(t *testing.T) { - cq := newClusterQueueImpl(defaultOrdering, testingclock.NewFakeClock(time.Now())) + cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) wl := utiltesting.MakeWorkload("workload-1", defaultNamespace).Obj() queue := &LocalQueue{ items: map[string]*workload.Info{ @@ -251,7 +257,7 @@ func Test_AddFromLocalQueue(t *testing.T) { } func Test_DeleteFromLocalQueue(t *testing.T) { - cq := newClusterQueueImpl(defaultOrdering, testingclock.NewFakeClock(time.Now())) + cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) q := utiltesting.MakeLocalQueue("foo", "").ClusterQueue("cq").Obj() qImpl := newLocalQueue(q) wl1 := utiltesting.MakeWorkload("wl1", "").Queue(kueue.LocalQueueName(q.Name)).Obj() @@ -406,7 +412,7 @@ func TestClusterQueueImpl(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { - cq := newClusterQueueImpl(defaultOrdering, fakeClock) + cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, fakeClock, nil, false) err := cq.Update(utiltesting.MakeClusterQueue("cq"). NamespaceSelector(&metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ @@ -459,7 +465,7 @@ func TestClusterQueueImpl(t *testing.T) { } func TestQueueInadmissibleWorkloadsDuringScheduling(t *testing.T) { - cq := newClusterQueueImpl(defaultOrdering, testingclock.NewFakeClock(time.Now())) + cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) cq.namespaceSelector = labels.Everything() wl := utiltesting.MakeWorkload("workload-1", defaultNamespace).Obj() cl := utiltesting.NewFakeClient(wl, utiltesting.MakeNamespace(defaultNamespace)) @@ -543,7 +549,7 @@ func TestBackoffWaitingTimeExpired(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - cq := newClusterQueueImpl(defaultOrdering, fakeClock) + cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, fakeClock, nil, false) got := cq.backoffWaitingTimeExpired(tc.workloadInfo) if tc.want != got { t.Errorf("Unexpected result from backoffWaitingTimeExpired\nwant: %v\ngot: %v\n", tc.want, got) @@ -600,14 +606,14 @@ func TestBestEffortFIFORequeueIfNotPresent(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { - cq, _ := newClusterQueue( + cq, _ := newClusterQueue(context.TODO(), nil, &kueue.ClusterQueue{ Spec: kueue.ClusterQueueSpec{ QueueingStrategy: kueue.BestEffortFIFO, }, }, workload.Ordering{PodsReadyRequeuingTimestamp: config.EvictionTimestamp}, - ) + nil) wl := utiltesting.MakeWorkload("workload-1", defaultNamespace).Obj() info := workload.NewInfo(wl) info.LastAssignment = tc.lastAssignment @@ -628,7 +634,7 @@ func TestBestEffortFIFORequeueIfNotPresent(t *testing.T) { } func TestFIFOClusterQueue(t *testing.T) { - q, err := newClusterQueue( + q, err := newClusterQueue(context.TODO(), nil, &kueue.ClusterQueue{ Spec: kueue.ClusterQueueSpec{ QueueingStrategy: kueue.StrictFIFO, @@ -636,7 +642,7 @@ func TestFIFOClusterQueue(t *testing.T) { }, workload.Ordering{ PodsReadyRequeuingTimestamp: config.EvictionTimestamp, - }) + }, nil) if err != nil { t.Fatalf("Failed creating ClusterQueue %v", err) } @@ -833,13 +839,14 @@ func TestStrictFIFO(t *testing.T) { // The default ordering: tt.workloadOrdering = &workload.Ordering{PodsReadyRequeuingTimestamp: config.EvictionTimestamp} } - q, err := newClusterQueue( + q, err := newClusterQueue(context.TODO(), nil, &kueue.ClusterQueue{ Spec: kueue.ClusterQueueSpec{ QueueingStrategy: kueue.StrictFIFO, }, }, - *tt.workloadOrdering) + *tt.workloadOrdering, + nil) if err != nil { t.Fatalf("Failed creating ClusterQueue %v", err) } @@ -875,14 +882,14 @@ func TestStrictFIFORequeueIfNotPresent(t *testing.T) { for reason, test := range tests { t.Run(string(reason), func(t *testing.T) { - cq, _ := newClusterQueue( + cq, _ := newClusterQueue(context.TODO(), nil, &kueue.ClusterQueue{ Spec: kueue.ClusterQueueSpec{ QueueingStrategy: kueue.StrictFIFO, }, }, workload.Ordering{PodsReadyRequeuingTimestamp: config.EvictionTimestamp}, - ) + nil) wl := utiltesting.MakeWorkload("workload-1", defaultNamespace).Obj() if ok := cq.RequeueIfNotPresent(workload.NewInfo(wl), reason); !ok { t.Error("failed to requeue nonexistent workload") @@ -899,3 +906,234 @@ func TestStrictFIFORequeueIfNotPresent(t *testing.T) { }) } } + +func TestFsAdmission(t *testing.T) { + wlCmpOpts := []cmp.Option{ + cmpopts.EquateEmpty(), + cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ResourceVersion"), + cmpopts.IgnoreFields(metav1.Condition{}, "ObservedGeneration", "LastTransitionTime"), + } + + cases := map[string]struct { + cq *kueue.ClusterQueue + lqs []kueue.LocalQueue + fsConfig *config.FairSharing + wls []kueue.Workload + wantWl kueue.Workload + }{ + "workloads are ordered by LQ usage, instead of priorities": { + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionMode(kueue.UsageBasedFairSharing). + Obj(), + lqs: []kueue.LocalQueue{ + *utiltesting.MakeLocalQueue("lqA", "default"). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("2"), + }, + }, + }, + ). + Obj(), + *utiltesting.MakeLocalQueue("lqB", "default"). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + ).Obj(), + }, + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{}, + }, + wls: []kueue.Workload{ + *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), + *utiltesting.MakeWorkload("wlB-low", "default").Queue("lqB").Priority(1).Obj(), + }, + wantWl: *utiltesting.MakeWorkload("wlB-low", "default").Queue("lqB").Priority(1).Obj(), + }, + "workloads are ordered by LQ usage with respect to resource weights": { + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionMode(kueue.UsageBasedFairSharing). + Obj(), + lqs: []kueue.LocalQueue{ + *utiltesting.MakeLocalQueue("lqA", "default"). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("1"), + resourceGPU: resource.MustParse("10"), + }, + }, + }, + ). + Obj(), + *utiltesting.MakeLocalQueue("lqB", "default"). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("1000"), + resourceGPU: resource.MustParse("1"), + }, + }, + }, + ).Obj(), + }, + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{ + ResourceWeights: map[corev1.ResourceName]float64{ + corev1.ResourceCPU: 0, + resourceGPU: 1, + }, + }, + }, + wls: []kueue.Workload{ + *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), + *utiltesting.MakeWorkload("wlB-low", "default").Queue("lqB").Priority(1).Obj(), + }, + wantWl: *utiltesting.MakeWorkload("wlB-low", "default").Queue("lqB").Priority(1).Obj(), + }, + "workloads are ordered by LQ usage with respect to LQs' fair sharing weights": { + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionMode(kueue.UsageBasedFairSharing). + Obj(), + lqs: []kueue.LocalQueue{ + *utiltesting.MakeLocalQueue("lqA", "default"). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("10"), + }, + }, + }, + ). + Obj(), + *utiltesting.MakeLocalQueue("lqB", "default"). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("2")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("6"), + }, + }, + }, + ).Obj(), + }, + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{}, + }, + wls: []kueue.Workload{ + *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), + *utiltesting.MakeWorkload("wlB-low", "default").Queue("lqB").Priority(1).Obj(), + }, + wantWl: *utiltesting.MakeWorkload("wlB-low", "default").Queue("lqB").Priority(1).Obj(), + }, + "workloads with the same LQ usage are ordered by priority": { + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionMode(kueue.UsageBasedFairSharing). + Obj(), + lqs: []kueue.LocalQueue{ + *utiltesting.MakeLocalQueue("lqA", "default"). + FairSharing(&kueue.FairSharing{ + Weight: ptr.To(resource.MustParse("1")), + }). + FairSharingStatus( + &kueue.FairSharingStatus{ + AdmissionFairSharingStatus: &kueue.AdmissionFairSharingStatus{ + ConsumedResources: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: resource.MustParse("10"), + }, + }, + }, + ).Obj(), + }, + fsConfig: &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{}, + }, + wls: []kueue.Workload{ + *utiltesting.MakeWorkload("wlA-low", "default").Queue("lqA").Priority(1).Obj(), + *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), + }, + wantWl: *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), + }, + "workloads with NoFairSharing CQ are ordered by priority": { + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionMode(kueue.NoFairSharing). + Obj(), + lqs: []kueue.LocalQueue{ + *utiltesting.MakeLocalQueue("lqA", "default").Obj(), + }, + fsConfig: &config.FairSharing{ + Enable: true, + }, + wls: []kueue.Workload{ + *utiltesting.MakeWorkload("wlA-low", "default").Queue("lqA").Priority(1).Obj(), + *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), + }, + wantWl: *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), + }, + "workloads with no FS config are ordered by priority": { + cq: utiltesting.MakeClusterQueue("cq"). + AdmissionMode(kueue.NoFairSharing). + Obj(), + lqs: []kueue.LocalQueue{ + *utiltesting.MakeLocalQueue("lqA", "default").Obj(), + }, + wls: []kueue.Workload{ + *utiltesting.MakeWorkload("wlA-low", "default").Queue("lqA").Priority(1).Obj(), + *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), + }, + wantWl: *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + builder := utiltesting.NewClientBuilder() + for _, lq := range tc.lqs { + builder = builder.WithObjects(&lq) + } + client := builder.Build() + ctx := context.Background() + + cq, _ := newClusterQueue(ctx, client, tc.cq, defaultOrdering, tc.fsConfig) + for _, wl := range tc.wls { + cq.PushOrUpdate(workload.NewInfo(&wl)) + } + + gotWl := cq.Pop() + if diff := cmp.Diff(tc.wantWl, *gotWl.Obj, wlCmpOpts...); diff != "" { + t.Errorf("Unexpected workloads on top of the heap (-want,+got):\n%s", diff) + } + }) + } +} diff --git a/pkg/queue/manager.go b/pkg/queue/manager.go index 5c8582b746b..1608e1d8014 100644 --- a/pkg/queue/manager.go +++ b/pkg/queue/manager.go @@ -47,6 +47,7 @@ var ( type options struct { podsReadyRequeuingTimestamp config.RequeuingTimestamp workloadInfoOptions []workload.InfoOption + fairSharing *config.FairSharing } // Option configures the manager. @@ -57,6 +58,12 @@ var defaultOptions = options{ workloadInfoOptions: []workload.InfoOption{}, } +func WithFairSharing(cfg *config.FairSharing) Option { + return func(o *options) { + o.fairSharing = cfg + } +} + // WithPodsReadyRequeuingTimestamp sets the timestamp that is used for ordering // workloads that have been requeued due to the PodsReady condition. func WithPodsReadyRequeuingTimestamp(ts config.RequeuingTimestamp) Option { @@ -101,6 +108,8 @@ type Manager struct { hm hierarchy.Manager[*ClusterQueue, *cohort] topologyUpdateWatchers []TopologyUpdateWatcher + + fairSharingConfig *config.FairSharing } func NewManager(client client.Client, checker StatusChecker, opts ...Option) *Manager { @@ -121,6 +130,7 @@ func NewManager(client client.Client, checker StatusChecker, opts ...Option) *Ma hm: hierarchy.NewManager[*ClusterQueue, *cohort](newCohort), topologyUpdateWatchers: make([]TopologyUpdateWatcher, 0), + fairSharingConfig: options.fairSharing, } m.cond.L = &m.RWMutex return m @@ -162,7 +172,7 @@ func (m *Manager) AddClusterQueue(ctx context.Context, cq *kueue.ClusterQueue) e return errClusterQueueAlreadyExists } - cqImpl, err := newClusterQueue(cq, m.workloadOrdering) + cqImpl, err := newClusterQueue(ctx, m.client, cq, m.workloadOrdering, m.fairSharingConfig) if err != nil { return err } @@ -236,6 +246,17 @@ func (m *Manager) UpdateClusterQueue(ctx context.Context, cq *kueue.ClusterQueue return nil } +func (m *Manager) HeapifyClusterQueue(cq *kueue.ClusterQueue, lqName string) error { + m.Lock() + defer m.Unlock() + cqImpl := m.hm.ClusterQueue(kueue.ClusterQueueReference(cq.Name)) + if cqImpl == nil { + return ErrClusterQueueDoesNotExist + } + cqImpl.Heapify(lqName) + return nil +} + func (m *Manager) DeleteClusterQueue(cq *kueue.ClusterQueue) { m.Lock() defer m.Unlock() diff --git a/pkg/resources/resource.go b/pkg/resources/resource.go index af0e691f31c..6a0342123fe 100644 --- a/pkg/resources/resource.go +++ b/pkg/resources/resource.go @@ -43,3 +43,11 @@ func (q FlavorResourceQuantities) MarshalJSON() ([]byte, error) { } return json.Marshal(temp) } + +func (frq FlavorResourceQuantities) FlattenFlavors() Requests { + result := Requests{} + for key, val := range frq { + result[key.Resource] += val + } + return result +} diff --git a/pkg/scheduler/preemption/fairsharing/strategy.go b/pkg/scheduler/preemption/fairsharing/strategy.go index df413792c20..5a9bbfa5175 100644 --- a/pkg/scheduler/preemption/fairsharing/strategy.go +++ b/pkg/scheduler/preemption/fairsharing/strategy.go @@ -41,3 +41,7 @@ func LessThanOrEqualToFinalShare(preemptorNewShare PreemptorNewShare, _ TargetOl func LessThanInitialShare(preemptorNewShare PreemptorNewShare, targetOldShare TargetOldShare, _ TargetNewShare) bool { return int(preemptorNewShare) < int(targetOldShare) } + +func NoPreemption(_ PreemptorNewShare, _ TargetOldShare, _ TargetNewShare) bool { + return false +} diff --git a/pkg/scheduler/preemption/preemption.go b/pkg/scheduler/preemption/preemption.go index d67ea31ed9c..413c3403542 100644 --- a/pkg/scheduler/preemption/preemption.go +++ b/pkg/scheduler/preemption/preemption.go @@ -292,6 +292,8 @@ func parseStrategies(s []config.PreemptionStrategy) []fairsharing.Strategy { strategies[i] = fairsharing.LessThanOrEqualToFinalShare case config.LessThanInitialShare: strategies[i] = fairsharing.LessThanInitialShare + case config.NoPreemption: + strategies[i] = fairsharing.NoPreemption } } return strategies diff --git a/pkg/scheduler/preemption/preemption_test.go b/pkg/scheduler/preemption/preemption_test.go index 06dab3a7494..8453b186928 100644 --- a/pkg/scheduler/preemption/preemption_test.go +++ b/pkg/scheduler/preemption/preemption_test.go @@ -2196,6 +2196,18 @@ func TestFairPreemptions(t *testing.T) { targetCQ: "a", wantPreempted: sets.New(targetKeyReason("/b_low", kueue.InCohortFairSharingReason)), }, + "NoPreemption strategy; no preemption even if off balanced": { + clusterQueues: baseCQs, + strategies: []config.PreemptionStrategy{config.NoPreemption}, + admitted: []kueue.Workload{ + *utiltesting.MakeWorkload("a1", "").Request(corev1.ResourceCPU, "3").SimpleReserveQuota("a", "default", now).Obj(), + *utiltesting.MakeWorkload("b_low", "").Priority(0).Request(corev1.ResourceCPU, "5").SimpleReserveQuota("b", "default", now).Obj(), + *utiltesting.MakeWorkload("b_high", "").Priority(1).Request(corev1.ResourceCPU, "1").SimpleReserveQuota("b", "default", now).Obj(), + }, + incoming: utiltesting.MakeWorkload("a_incoming", "").Request(corev1.ResourceCPU, "1").Obj(), + targetCQ: "a", + wantPreempted: nil, + }, "preempt workload that doesn't transfer the imbalance, even if high priority": { clusterQueues: baseCQs, strategies: []config.PreemptionStrategy{config.LessThanOrEqualToFinalShare}, diff --git a/pkg/util/resource/resource.go b/pkg/util/resource/resource.go index 2f0782aec6e..e9ede6f27cf 100644 --- a/pkg/util/resource/resource.go +++ b/pkg/util/resource/resource.go @@ -103,3 +103,17 @@ func QuantityToFloat(q *resource.Quantity) float64 { } return float64(q.MilliValue()) / 1000 } + +// MulByFloat multiplies every element in q by f. +// It first mutiplies f by 1 000 to mitigate further precision loss, and then leverages +// k8s.io/apimachinery/pkg/api/resource package to provide precision to 3 decimal places +func MulByFloat(q corev1.ResourceList, f float64) corev1.ResourceList { + ret := q.DeepCopy() + scaleFact := f * 1_000 + for k, v := range ret { + scaledV := float64(v.MilliValue()) * scaleFact + scaledV /= 1_000 + ret[k] = *resource.NewMilliQuantity(int64(scaledV), resource.DecimalSI) + } + return ret +} diff --git a/pkg/util/testing/wrappers.go b/pkg/util/testing/wrappers.go index 7c03659aa7c..65fd3057ac8 100644 --- a/pkg/util/testing/wrappers.go +++ b/pkg/util/testing/wrappers.go @@ -659,12 +659,24 @@ func (q *LocalQueueWrapper) StopPolicy(p kueue.StopPolicy) *LocalQueueWrapper { return q } +// FairSharing sets the fair sharing config. +func (q *LocalQueueWrapper) FairSharing(fs *kueue.FairSharing) *LocalQueueWrapper { + q.Spec.FairSharing = fs + return q +} + // PendingWorkloads updates the pendingWorkloads in status. func (q *LocalQueueWrapper) PendingWorkloads(n int32) *LocalQueueWrapper { q.Status.PendingWorkloads = n return q } +// ReservingWorkloads updates the reservingWorkloads in status. +func (q *LocalQueueWrapper) ReservingWorkloads(n int32) *LocalQueueWrapper { + q.Status.ReservingWorkloads = n + return q +} + // AdmittedWorkloads updates the admittedWorkloads in status. func (q *LocalQueueWrapper) AdmittedWorkloads(n int32) *LocalQueueWrapper { q.Status.AdmittedWorkloads = n @@ -683,6 +695,22 @@ func (q *LocalQueueWrapper) Condition(conditionType string, status metav1.Condit return q } +func (q *LocalQueueWrapper) Active(status metav1.ConditionStatus) *LocalQueueWrapper { + apimeta.SetStatusCondition(&q.Status.Conditions, metav1.Condition{ + Type: kueue.LocalQueueActive, + Status: status, + Reason: "Ready", + Message: "Can submit new workloads to localQueue", + }) + return q +} + +// AdmittedWorkloads updates the admittedWorkloads in status. +func (q *LocalQueueWrapper) FairSharingStatus(status *kueue.FairSharingStatus) *LocalQueueWrapper { + q.Status.FairSharingStatus = *status + return q +} + // Generation sets the generation of the LocalQueue. func (q *LocalQueueWrapper) Generation(num int64) *LocalQueueWrapper { q.ObjectMeta.Generation = num @@ -771,6 +799,24 @@ func (c *ClusterQueueWrapper) AdmissionCheckStrategy(acs ...kueue.AdmissionCheck return c } +func (c *ClusterQueueWrapper) AdmissionMode(am kueue.AdmissionMode) *ClusterQueueWrapper { + if c.Spec.AdmissionScope == nil { + c.Spec.AdmissionScope = &kueue.AdmissionScope{} + } + c.Spec.AdmissionScope.AdmissionMode = am + return c +} + +func (c *ClusterQueueWrapper) Active(status metav1.ConditionStatus) *ClusterQueueWrapper { + apimeta.SetStatusCondition(&c.Status.Conditions, metav1.Condition{ + Type: kueue.ClusterQueueActive, + Status: status, + Reason: "By test", + Message: "by test", + }) + return c +} + // GeneratedName sets the prefix for the server to generate unique name. // No name should be given in the MakeClusterQueue for the GeneratedName to work. func (c *ClusterQueueWrapper) GeneratedName(name string) *ClusterQueueWrapper { diff --git a/pkg/workload/workload.go b/pkg/workload/workload.go index 4a87a52123a..561bea26136 100644 --- a/pkg/workload/workload.go +++ b/pkg/workload/workload.go @@ -282,6 +282,28 @@ func dropExcludedResources(input corev1.ResourceList, excludedPrefixes []string) return res } +func (i *Info) LqUsage(ctx context.Context, c client.Client, resWeights map[corev1.ResourceName]float64) (float64, error) { + var lq kueue.LocalQueue + lqKey := client.ObjectKey{Namespace: i.Obj.Namespace, Name: string(i.Obj.Spec.QueueName)} + if err := c.Get(ctx, lqKey, &lq); err != nil { + return 0, err + } + usage := 0.0 + for resName, resVal := range lq.Status.FairSharingStatus.AdmissionFairSharingStatus.ConsumedResources { + if weight, found := resWeights[resName]; found { + usage += weight * resVal.AsApproximateFloat64() + } else { + // if no weight for resource was defined, use default weight of 1 + usage += resVal.AsApproximateFloat64() + } + } + if lq.Spec.FairSharing != nil && lq.Spec.FairSharing.Weight != nil { + // if no weight for lq was defined, use default weight of 1 + usage /= lq.Spec.FairSharing.Weight.AsApproximateFloat64() + } + return usage, nil +} + // IsUsingTAS returns information if the workload is using TAS func (i *Info) IsUsingTAS() bool { return slices.ContainsFunc(i.TotalRequests, diff --git a/site/content/en/docs/reference/kueue-config.v1beta1.md b/site/content/en/docs/reference/kueue-config.v1beta1.md index 009c9242e82..df9b67466f8 100644 --- a/site/content/en/docs/reference/kueue-config.v1beta1.md +++ b/site/content/en/docs/reference/kueue-config.v1beta1.md @@ -15,6 +15,47 @@ description: Generated API reference documentation for Kueue Configuration. +## `AdmissionFairSharing` {#AdmissionFairSharing} + + +**Appears in:** + +- [FairSharing](#FairSharing) + + + + + + + + + + + + + + + + + + +
FieldDescription
usageHalfLifeTime [Required]
+k8s.io/apimachinery/pkg/apis/meta/v1.Duration +		 +

usageHalfLifeTime indicates the time after which the current usage will decay by a half +If set to 0, usage will be reset to 0.

+
usageSamplingInterval [Required]
+k8s.io/apimachinery/pkg/apis/meta/v1.Duration +		 +

usageSamplingInterval indicates how often Kueue updates consumedResources in FairSharingStatus

+
resourceWeights [Required]
+map[ResourceName]float64 +		 +

resourceWeights assigns weights to resources which then are used to calculate LocalQueue/ClusterQueue/Cohort's +resource usage and order Workloads. +Defaults to 1.

+
+ ## `ClientConnection` {#ClientConnection} @@ -481,11 +522,19 @@ as high as possible. with the incoming workload is strictly less than the share of the preemptee CQ. This strategy doesn't depend on the share usage of the workload being preempted. As a result, the strategy chooses to preempt workloads with the lowest priority and -newest start time first. +newest start time first. +
  • NoPreemption: Never preempt a workload. The default strategy is ["LessThanOrEqualToFinalShare", "LessThanInitialShare"].
    • +admissionFairSharing
    +AdmissionFairSharing + + +

    admissionFairSharing indicates configuration of FairSharing with the AdmissionTime mode on

    + + diff --git a/site/content/en/docs/reference/kueue.v1beta1.md b/site/content/en/docs/reference/kueue.v1beta1.md index d83c18bd353..0d04c8c654e 100644 --- a/site/content/en/docs/reference/kueue.v1beta1.md +++ b/site/content/en/docs/reference/kueue.v1beta1.md @@ -577,6 +577,74 @@ If empty, the AdmissionCheck will run for all workloads submitted to the Cluster +## `AdmissionFairSharingStatus` {#kueue-x-k8s-io-v1beta1-AdmissionFairSharingStatus} + + +**Appears in:** + +- [FairSharingStatus](#kueue-x-k8s-io-v1beta1-FairSharingStatus) + + + + + + + + + + + + + + + +
    FieldDescription
    consumedResources [Required]
    +k8s.io/api/core/v1.ResourceList +    		 +

    ConsumedResources represents the aggregated usage of resources over time, +with decaying function applied. +The value is populated if usage consumption functionality is enabled in Kueue config.

    +
    lastUpdate [Required]
    +k8s.io/apimachinery/pkg/apis/meta/v1.Time +    		 +

    LastUpdate is the time when share and consumed resources were updated.

    +
    + +## `AdmissionMode` {#kueue-x-k8s-io-v1beta1-AdmissionMode} + +(Alias of `string`) + +**Appears in:** + +- [AdmissionScope](#kueue-x-k8s-io-v1beta1-AdmissionScope) + + + + + +## `AdmissionScope` {#kueue-x-k8s-io-v1beta1-AdmissionScope} + + +**Appears in:** + +- [ClusterQueueSpec](#kueue-x-k8s-io-v1beta1-ClusterQueueSpec) + + + + + + + + + + + + +
    FieldDescription
    admissionMode [Required]
    +AdmissionMode +    		 + No description provided.
    + ## `BorrowWithinCohort` {#kueue-x-k8s-io-v1beta1-BorrowWithinCohort} @@ -936,6 +1004,13 @@ participating in FairSharing. The values are only relevant if FairSharing is enabled in the Kueue configuration.

    +admissionScope
    +AdmissionScope + + +

    admissionScope indicates whether ClusterQueue uses the Admission Fair Sharing

    + + @@ -1049,6 +1124,8 @@ subdomain in DNS (RFC 1123).

    - [ClusterQueueSpec](#kueue-x-k8s-io-v1beta1-ClusterQueueSpec) +- [LocalQueueSpec](#kueue-x-k8s-io-v1beta1-LocalQueueSpec) +

    FairSharing contains the properties of the ClusterQueue or Cohort, when participating in FairSharing.

    @@ -1088,6 +1165,8 @@ disadvantage against other ClusterQueues and Cohorts.

    - [ClusterQueueStatus](#kueue-x-k8s-io-v1beta1-ClusterQueueStatus) +- [LocalQueueStatus](#kueue-x-k8s-io-v1beta1-LocalQueueStatus) +

    FairSharingStatus contains the information about the current status of Fair Sharing.

    @@ -1110,6 +1189,13 @@ weight of zero and is borrowing, this will return 9223372036854775807, the maximum possible share value.

    +admissionFairSharingStatus [Required]
    +AdmissionFairSharingStatus + + +

    admissionFairSharingStatus represents information relevant to the Admission Fair Sharing

    + + @@ -1440,6 +1526,15 @@ no new reservation being made.

    +fairSharing
    +FairSharing + + +

    fairSharing defines the properties of the LocalQueue when +participating in FairSharing. The values are only relevant +if FairSharing is enabled in the Kueue configuration.

    + + @@ -1513,6 +1608,13 @@ workloads assigned to this LocalQueue.

    flavors lists all currently available ResourceFlavors in specified ClusterQueue.

    +fairSharingStatus
    +FairSharingStatus + + +

    FairSharing contains the information about the current status of fair sharing.

    + + diff --git a/site/static/examples/admission-fs/admission-fair-sharing-setup.yaml b/site/static/examples/admission-fs/admission-fair-sharing-setup.yaml new file mode 100644 index 00000000000..bbf6b732728 --- /dev/null +++ b/site/static/examples/admission-fs/admission-fair-sharing-setup.yaml @@ -0,0 +1,42 @@ +apiVersion: kueue.x-k8s.io/v1beta1 +kind: ResourceFlavor +metadata: + name: "default-flavor" +--- +apiVersion: kueue.x-k8s.io/v1beta1 +kind: ClusterQueue +metadata: + name: "cluster-queue" +spec: + namespaceSelector: {} # match all. + queueingStrategy: "StrictFIFO" + admissionScope: + admissionMode: "UsageBasedFairSharing" + resourceGroups: + - coveredResources: ["cpu"] + flavors: + - name: "default-flavor" + resources: + - name: "cpu" + nominalQuota: 9 +--- +apiVersion: kueue.x-k8s.io/v1beta1 +kind: LocalQueue +metadata: + namespace: "default" + name: "lq-a" +spec: + clusterQueue: "cluster-queue" + fairSharing: + weight: "1" +--- +apiVersion: kueue.x-k8s.io/v1beta1 +kind: LocalQueue +metadata: + namespace: "default" + name: "lq-b" +spec: + clusterQueue: "cluster-queue" + fairSharing: + weight: "1" + diff --git a/site/static/examples/admission-fs/lq-a-simple-job.yaml b/site/static/examples/admission-fs/lq-a-simple-job.yaml new file mode 100644 index 00000000000..bd90beab8db --- /dev/null +++ b/site/static/examples/admission-fs/lq-a-simple-job.yaml @@ -0,0 +1,20 @@ +apiVersion: batch/v1 +kind: Job +metadata: + generateName: sample-job- + namespace: default + labels: + kueue.x-k8s.io/queue-name: lq-a +spec: + parallelism: 1 + suspend: true + template: + spec: + containers: + - name: dummy-job + image: alpine/sleep # A very small and lightweight image + command: ["/bin/sleep", "infinity"] + resources: + requests: + cpu: 3 + restartPolicy: Never diff --git a/site/static/examples/admission-fs/lq-b-simple-job.yaml b/site/static/examples/admission-fs/lq-b-simple-job.yaml new file mode 100644 index 00000000000..0d82bf61931 --- /dev/null +++ b/site/static/examples/admission-fs/lq-b-simple-job.yaml @@ -0,0 +1,20 @@ +apiVersion: batch/v1 +kind: Job +metadata: + generateName: sample-job- + namespace: default + labels: + kueue.x-k8s.io/queue-name: lq-b +spec: + parallelism: 1 + suspend: true + template: + spec: + containers: + - name: dummy-job + image: alpine/sleep # A very small and lightweight image + command: ["/bin/sleep", "infinity"] + resources: + requests: + cpu: 3 + restartPolicy: Never diff --git a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go index 62a663e6464..e8bd946cd4b 100644 --- a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go +++ b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/component-base/metrics/testutil" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" kueuealpha "sigs.k8s.io/kueue/apis/kueue/v1alpha1" @@ -392,6 +393,78 @@ var _ = ginkgo.Describe("Scheduler", func() { util.ExpectReservingActiveWorkloadsMetric(llmQueue, 4) }) }) + + ginkgo.When("Using AdmissionFairSharing", func() { + var ( + cq *kueue.ClusterQueue + lqA *kueue.LocalQueue + lqB *kueue.LocalQueue + lqC *kueue.LocalQueue + ) + + ginkgo.BeforeEach(func() { + cq = testing.MakeClusterQueue("cq"). + ResourceGroup(*testing.MakeFlavorQuotas(defaultFlavor.Name).Resource(corev1.ResourceCPU, "32").Obj()). + Preemption(kueue.ClusterQueuePreemption{WithinClusterQueue: kueue.PreemptionPolicyNever}). + QueueingStrategy(kueue.StrictFIFO). + AdmissionMode(kueue.UsageBasedFairSharing). + Obj() + cqs = append(cqs, cq) + util.MustCreate(ctx, k8sClient, cq) + + lqA = testing.MakeLocalQueue("lq-a", ns.Name). + FairSharing(&kueue.FairSharing{Weight: ptr.To(resource.MustParse("1"))}). + ClusterQueue(cq.Name).Obj() + lqB = testing.MakeLocalQueue("lq-b", ns.Name). + FairSharing(&kueue.FairSharing{Weight: ptr.To(resource.MustParse("1"))}). + ClusterQueue(cq.Name).Obj() + lqC = testing.MakeLocalQueue("lq-c", ns.Name). + FairSharing(&kueue.FairSharing{Weight: ptr.To(resource.MustParse("1"))}). + ClusterQueue(cq.Name).Obj() + lqs = append(lqs, lqA) + lqs = append(lqs, lqB) + lqs = append(lqs, lqC) + util.MustCreate(ctx, k8sClient, lqA) + util.MustCreate(ctx, k8sClient, lqB) + util.MustCreate(ctx, k8sClient, lqC) + }) + + ginkgo.It("should promote a workload from LQ with lower recent usage", func() { + ginkgo.By("Creating a workload") + wl := createWorkload("lq-a", "32") + + ginkgo.By("Admitting the workload") + util.ExpectWorkloadsToBeAdmitted(ctx, k8sClient, wl) + util.ExpectReservingActiveWorkloadsMetric(cq, 1) + + ginkgo.By("Checking that LQ's resource usage is updated", func() { + gomega.Eventually(func(g gomega.Gomega) { + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(lqA), lqA)).Should(gomega.Succeed()) + g.Expect(lqA.Status.FairSharingStatus).ShouldNot(gomega.BeNil()) + g.Expect(lqA.Status.FairSharingStatus.AdmissionFairSharingStatus).ShouldNot(gomega.BeNil()) + g.Expect(lqA.Status.FairSharingStatus.AdmissionFairSharingStatus.ConsumedResources).Should(gomega.HaveLen(1)) + g.Expect(lqA.Status.FairSharingStatus.AdmissionFairSharingStatus.ConsumedResources[corev1.ResourceCPU]). + To(gomega.Equal(resource.MustParse("32"))) + }, util.Timeout, util.Interval).Should(gomega.Succeed()) + }) + + ginkgo.By("Creating two pending workloads") + wlA := createWorkload("lq-a", "32") + wlB := createWorkload("lq-b", "32") + + ginkgo.By("Finish the previous workload", func() { + gomega.Consistently(func(g gomega.Gomega) { + util.FinishWorkloads(ctx, k8sClient, wl) + updatedCQ := &kueue.ClusterQueue{} + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cq), updatedCQ)).To(gomega.Succeed()) + }, util.Timeout, util.Interval).Should(gomega.Succeed()) + }) + + ginkgo.By("Admitting the workload from LQ-b, which has lower recent usage") + util.ExpectWorkloadsToBeAdmitted(ctx, k8sClient, wlB) + util.ExpectWorkloadsToBePending(ctx, k8sClient, wlA) + }) + }) }) func expectCohortWeightedShare(cohortName string, weightedShare int64) { diff --git a/test/integration/singlecluster/scheduler/fairsharing/suite_test.go b/test/integration/singlecluster/scheduler/fairsharing/suite_test.go index 88756e4b234..c220c681203 100644 --- a/test/integration/singlecluster/scheduler/fairsharing/suite_test.go +++ b/test/integration/singlecluster/scheduler/fairsharing/suite_test.go @@ -19,9 +19,11 @@ package fairsharing import ( "context" "testing" + "time" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -68,13 +70,23 @@ var _ = ginkgo.AfterSuite(func() { }) func managerAndSchedulerSetup(ctx context.Context, mgr manager.Manager) { - fairSharing := &config.FairSharing{Enable: true} + fairSharing := &config.FairSharing{ + Enable: true, + AdmissionFairSharing: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{ + Duration: 250 * time.Microsecond, + }, + UsageSamplingInterval: metav1.Duration{ + Duration: 250 * time.Millisecond, + }, + }, + } err := indexer.Setup(ctx, mgr.GetFieldIndexer()) gomega.Expect(err).NotTo(gomega.HaveOccurred()) cCache := cache.New(mgr.GetClient(), cache.WithFairSharing(fairSharing.Enable)) - queues := queue.NewManager(mgr.GetClient(), cCache) + queues := queue.NewManager(mgr.GetClient(), cCache, queue.WithFairSharing(fairSharing)) configuration := &config.Configuration{FairSharing: fairSharing} configuration.Metrics.EnableClusterQueueResources = true From 4d86b1781e5e90ec639c7b3ffe0cef3b036958b5 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Mon, 5 May 2025 17:15:49 +0000 Subject: [PATCH 02/19] Change shape of config API --- apis/config/v1beta1/configuration_types.go | 10 +-- apis/config/v1beta1/defaults.go | 6 +- apis/config/v1beta1/zz_generated.deepcopy.go | 10 +-- cmd/kueue/main.go | 2 +- pkg/controller/core/core.go | 2 +- pkg/controller/core/localqueue_controller.go | 8 +- .../core/localqueue_controller_test.go | 76 +++++++------------ pkg/queue/cluster_queue.go | 9 +-- pkg/queue/cluster_queue_test.go | 42 ++++------ pkg/queue/manager.go | 16 +++- .../preemption/fairsharing/strategy.go | 4 - pkg/scheduler/preemption/preemption.go | 2 - pkg/scheduler/preemption/preemption_test.go | 12 --- .../en/docs/reference/kueue-config.v1beta1.md | 18 ++--- .../scheduler/fairsharing/suite_test.go | 18 ++--- 15 files changed, 92 insertions(+), 143 deletions(-) diff --git a/apis/config/v1beta1/configuration_types.go b/apis/config/v1beta1/configuration_types.go index 3bfc63238a7..acd912a88ac 100644 --- a/apis/config/v1beta1/configuration_types.go +++ b/apis/config/v1beta1/configuration_types.go @@ -95,6 +95,10 @@ type Configuration struct { // FairSharing controls the Fair Sharing semantics across the cluster. FairSharing *FairSharing `json:"fairSharing,omitempty"` + // admissionFairSharing indicates configuration of FairSharing with the `AdmissionTime` mode on + // +optional + AdmissionFairSharing *AdmissionFairSharing `json:"admissionFairSharing,omitempty"` + // Resources provides additional configuration options for handling the resources. Resources *Resources `json:"resources,omitempty"` @@ -447,7 +451,6 @@ type PreemptionStrategy string const ( LessThanOrEqualToFinalShare PreemptionStrategy = "LessThanOrEqualToFinalShare" LessThanInitialShare PreemptionStrategy = "LessThanInitialShare" - NoPreemption PreemptionStrategy = "NoPreemption" ) type FairSharing struct { @@ -470,13 +473,8 @@ type FairSharing struct { // This strategy doesn't depend on the share usage of the workload being preempted. // As a result, the strategy chooses to preempt workloads with the lowest priority and // newest start time first. - // - NoPreemption: Never preempt a workload. // The default strategy is ["LessThanOrEqualToFinalShare", "LessThanInitialShare"]. PreemptionStrategies []PreemptionStrategy `json:"preemptionStrategies,omitempty"` - - // admissionFairSharing indicates configuration of FairSharing with the `AdmissionTime` mode on - // +optional - AdmissionFairSharing *AdmissionFairSharing `json:"admissionFairSharing,omitempty"` } type AdmissionFairSharing struct { diff --git a/apis/config/v1beta1/defaults.go b/apis/config/v1beta1/defaults.go index d9d75fa1510..7feb5734b75 100644 --- a/apis/config/v1beta1/defaults.go +++ b/apis/config/v1beta1/defaults.go @@ -210,9 +210,9 @@ func SetDefaults_Configuration(cfg *Configuration) { if fs := cfg.FairSharing; fs != nil && fs.Enable && len(fs.PreemptionStrategies) == 0 { fs.PreemptionStrategies = []PreemptionStrategy{LessThanOrEqualToFinalShare, LessThanInitialShare} } - if fs := cfg.FairSharing; fs != nil && fs.Enable && fs.AdmissionFairSharing != nil { - if fs.AdmissionFairSharing.UsageSamplingInterval.Duration == 0 { - fs.AdmissionFairSharing.UsageSamplingInterval = metav1.Duration{Duration: 5 * time.Minute} + if afs := cfg.AdmissionFairSharing; afs != nil { + if afs.UsageSamplingInterval.Duration == 0 { + afs.UsageSamplingInterval = metav1.Duration{Duration: 5 * time.Minute} } } diff --git a/apis/config/v1beta1/zz_generated.deepcopy.go b/apis/config/v1beta1/zz_generated.deepcopy.go index 10bc383d7ed..cf24392827f 100644 --- a/apis/config/v1beta1/zz_generated.deepcopy.go +++ b/apis/config/v1beta1/zz_generated.deepcopy.go @@ -142,6 +142,11 @@ func (in *Configuration) DeepCopyInto(out *Configuration) { *out = new(FairSharing) (*in).DeepCopyInto(*out) } + if in.AdmissionFairSharing != nil { + in, out := &in.AdmissionFairSharing, &out.AdmissionFairSharing + *out = new(AdmissionFairSharing) + (*in).DeepCopyInto(*out) + } if in.Resources != nil { in, out := &in.Resources, &out.Resources *out = new(Resources) @@ -287,11 +292,6 @@ func (in *FairSharing) DeepCopyInto(out *FairSharing) { *out = make([]PreemptionStrategy, len(*in)) copy(*out, *in) } - if in.AdmissionFairSharing != nil { - in, out := &in.AdmissionFairSharing, &out.AdmissionFairSharing - *out = new(AdmissionFairSharing) - (*in).DeepCopyInto(*out) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FairSharing. diff --git a/cmd/kueue/main.go b/cmd/kueue/main.go index 979f3878e5b..59771dbe27d 100644 --- a/cmd/kueue/main.go +++ b/cmd/kueue/main.go @@ -217,7 +217,7 @@ func main() { } if cfg.FairSharing != nil { cacheOptions = append(cacheOptions, cache.WithFairSharing(cfg.FairSharing.Enable)) - queueOptions = append(queueOptions, queue.WithFairSharing(cfg.FairSharing)) + queueOptions = append(queueOptions, queue.WithFairSharing(cfg.FairSharing), queue.WithAdmissionFairSharing(cfg.AdmissionFairSharing)) } cCache := cache.New(mgr.GetClient(), cacheOptions...) queues := queue.NewManager(mgr.GetClient(), cCache, queueOptions...) diff --git a/pkg/controller/core/core.go b/pkg/controller/core/core.go index d1954b4e626..3647d00d3d5 100644 --- a/pkg/controller/core/core.go +++ b/pkg/controller/core/core.go @@ -44,7 +44,7 @@ func SetupControllers(mgr ctrl.Manager, qManager *queue.Manager, cc *cache.Cache return "AdmissionCheck", err } qRec := NewLocalQueueReconciler(mgr.GetClient(), qManager, cc, - WithFairSharingConfig(cfg.FairSharing)) + WithAdmissionFairSharingConfig(cfg.AdmissionFairSharing)) if err := qRec.SetupWithManager(mgr, cfg); err != nil { return "LocalQueue", err } diff --git a/pkg/controller/core/localqueue_controller.go b/pkg/controller/core/localqueue_controller.go index 1806d1fa810..d1df7935714 100644 --- a/pkg/controller/core/localqueue_controller.go +++ b/pkg/controller/core/localqueue_controller.go @@ -71,12 +71,12 @@ type LocalQueueReconcilerOptions struct { // LocalQueueReconcilerOption configures the reconciler. type LocalQueueReconcilerOption func(*LocalQueueReconcilerOptions) -func WithFairSharingConfig(cfg *config.FairSharing) LocalQueueReconcilerOption { +func WithAdmissionFairSharingConfig(cfg *config.AdmissionFairSharing) LocalQueueReconcilerOption { return func(o *LocalQueueReconcilerOptions) { - if cfg == nil || !cfg.Enable || cfg.AdmissionFairSharing == nil { - o.admissionFSConfig = nil + if cfg != nil { + o.admissionFSConfig = cfg } else { - o.admissionFSConfig = cfg.AdmissionFairSharing + o.admissionFSConfig = nil } } } diff --git a/pkg/controller/core/localqueue_controller_test.go b/pkg/controller/core/localqueue_controller_test.go index 113782ecf53..4c33f15bef6 100644 --- a/pkg/controller/core/localqueue_controller_test.go +++ b/pkg/controller/core/localqueue_controller_test.go @@ -48,7 +48,7 @@ func TestLocalQueueReconcile(t *testing.T) { localQueue *kueue.LocalQueue wantLocalQueue *kueue.LocalQueue wantError error - fsConfig *config.FairSharing + afsConfig *config.AdmissionFairSharing runningWls []kueue.Workload }{ "local queue with Hold StopPolicy": { @@ -155,12 +155,9 @@ func TestLocalQueueReconcile(t *testing.T) { }, }). Obj(), - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{ - UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, - UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, - }, + afsConfig: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, }, }, "local queue decaying usage sums the previous state and running workloads": { @@ -207,12 +204,9 @@ func TestLocalQueueReconcile(t *testing.T) { Admitted(true). Obj(), }, - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{ - UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, - UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, - }, + afsConfig: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, }, }, "local queue decaying usage sums the usage from different flavors and resources": { @@ -274,12 +268,9 @@ func TestLocalQueueReconcile(t *testing.T) { Admitted(true). Obj(), }, - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{ - UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, - UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, - }, + afsConfig: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, }, }, "local queue decaying usage sums the previous state and running workloads half time twice larger than sampling": { @@ -326,12 +317,9 @@ func TestLocalQueueReconcile(t *testing.T) { Admitted(true). Obj(), }, - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{ - UsageHalfLifeTime: metav1.Duration{Duration: 10 * time.Minute}, - UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, - }, + afsConfig: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 10 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, }, }, "local queue decaying usage sums the previous state and running workloads with long half time": { @@ -368,12 +356,9 @@ func TestLocalQueueReconcile(t *testing.T) { }, }). Obj(), - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{ - UsageHalfLifeTime: metav1.Duration{Duration: 24 * time.Hour}, - UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, - }, + afsConfig: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 24 * time.Hour}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, }, }, "local queue decaying usage sums the previous state and running GPU workloads half time twice larger than sampling": { @@ -420,12 +405,9 @@ func TestLocalQueueReconcile(t *testing.T) { Admitted(true). Obj(), }, - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{ - UsageHalfLifeTime: metav1.Duration{Duration: 10 * time.Minute}, - UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, - }, + afsConfig: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 10 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, }, }, "local queue decaying usage resets to 0 when half life is 0": { @@ -468,12 +450,9 @@ func TestLocalQueueReconcile(t *testing.T) { Admitted(true). Obj(), }, - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{ - UsageHalfLifeTime: metav1.Duration{Duration: 0 * time.Minute}, - UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, - }, + afsConfig: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 0 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, }, }, "local queue decaying usage is not reconciled if not enough time has passed": { @@ -511,12 +490,9 @@ func TestLocalQueueReconcile(t *testing.T) { }). Obj(), wantError: nil, - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{ - UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, - UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, - }, + afsConfig: &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, + UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, }, }, } @@ -549,7 +525,7 @@ func TestLocalQueueReconcile(t *testing.T) { _ = qManager.AddLocalQueue(ctxWithLogger, tc.localQueue) reconciler := NewLocalQueueReconciler(cl, qManager, cqCache, WithClock(clock), - WithFairSharingConfig(tc.fsConfig)) + WithAdmissionFairSharingConfig(tc.afsConfig)) ctx, ctxCancel := context.WithCancel(ctxWithLogger) defer ctxCancel() diff --git a/pkg/queue/cluster_queue.go b/pkg/queue/cluster_queue.go index f692905613f..3fa7eff737f 100644 --- a/pkg/queue/cluster_queue.go +++ b/pkg/queue/cluster_queue.go @@ -91,7 +91,7 @@ func workloadKey(i *workload.Info) string { return workload.Key(i.Obj) } -func newClusterQueue(ctx context.Context, client client.Client, cq *kueue.ClusterQueue, wo workload.Ordering, fsConfig *config.FairSharing) (*ClusterQueue, error) { +func newClusterQueue(ctx context.Context, client client.Client, cq *kueue.ClusterQueue, wo workload.Ordering, fsConfig *config.AdmissionFairSharing) (*ClusterQueue, error) { enableAdmissionFs, fsResWeights := isAdmissionFsEnabled(cq, fsConfig) cqImpl := newClusterQueueImpl(ctx, client, wo, realClock, fsResWeights, enableAdmissionFs) err := cqImpl.Update(cq) @@ -101,12 +101,11 @@ func newClusterQueue(ctx context.Context, client client.Client, cq *kueue.Cluste return cqImpl, nil } -func isAdmissionFsEnabled(cq *kueue.ClusterQueue, fsConfig *config.FairSharing) (bool, map[corev1.ResourceName]float64) { +func isAdmissionFsEnabled(cq *kueue.ClusterQueue, fsConfig *config.AdmissionFairSharing) (bool, map[corev1.ResourceName]float64) { enableAdmissionFs, fsResWeights := false, make(map[corev1.ResourceName]float64) - if fsConfig != nil && fsConfig.Enable && fsConfig.AdmissionFairSharing != nil && - cq.Spec.AdmissionScope != nil && cq.Spec.AdmissionScope.AdmissionMode == kueue.UsageBasedFairSharing { + if fsConfig != nil && cq.Spec.AdmissionScope != nil && cq.Spec.AdmissionScope.AdmissionMode == kueue.UsageBasedFairSharing { enableAdmissionFs = true - fsResWeights = fsConfig.AdmissionFairSharing.ResourceWeights + fsResWeights = fsConfig.ResourceWeights } return enableAdmissionFs, fsResWeights } diff --git a/pkg/queue/cluster_queue_test.go b/pkg/queue/cluster_queue_test.go index 2901de4df8d..c1e23abe557 100644 --- a/pkg/queue/cluster_queue_test.go +++ b/pkg/queue/cluster_queue_test.go @@ -915,11 +915,11 @@ func TestFsAdmission(t *testing.T) { } cases := map[string]struct { - cq *kueue.ClusterQueue - lqs []kueue.LocalQueue - fsConfig *config.FairSharing - wls []kueue.Workload - wantWl kueue.Workload + cq *kueue.ClusterQueue + lqs []kueue.LocalQueue + afsConfig *config.AdmissionFairSharing + wls []kueue.Workload + wantWl kueue.Workload }{ "workloads are ordered by LQ usage, instead of priorities": { cq: utiltesting.MakeClusterQueue("cq"). @@ -954,10 +954,7 @@ func TestFsAdmission(t *testing.T) { }, ).Obj(), }, - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{}, - }, + afsConfig: &config.AdmissionFairSharing{}, wls: []kueue.Workload{ *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), *utiltesting.MakeWorkload("wlB-low", "default").Queue("lqB").Priority(1).Obj(), @@ -999,13 +996,10 @@ func TestFsAdmission(t *testing.T) { }, ).Obj(), }, - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{ - ResourceWeights: map[corev1.ResourceName]float64{ - corev1.ResourceCPU: 0, - resourceGPU: 1, - }, + afsConfig: &config.AdmissionFairSharing{ + ResourceWeights: map[corev1.ResourceName]float64{ + corev1.ResourceCPU: 0, + resourceGPU: 1, }, }, wls: []kueue.Workload{ @@ -1047,10 +1041,7 @@ func TestFsAdmission(t *testing.T) { }, ).Obj(), }, - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{}, - }, + afsConfig: &config.AdmissionFairSharing{}, wls: []kueue.Workload{ *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), *utiltesting.MakeWorkload("wlB-low", "default").Queue("lqB").Priority(1).Obj(), @@ -1076,10 +1067,7 @@ func TestFsAdmission(t *testing.T) { }, ).Obj(), }, - fsConfig: &config.FairSharing{ - Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{}, - }, + afsConfig: &config.AdmissionFairSharing{}, wls: []kueue.Workload{ *utiltesting.MakeWorkload("wlA-low", "default").Queue("lqA").Priority(1).Obj(), *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), @@ -1093,9 +1081,7 @@ func TestFsAdmission(t *testing.T) { lqs: []kueue.LocalQueue{ *utiltesting.MakeLocalQueue("lqA", "default").Obj(), }, - fsConfig: &config.FairSharing{ - Enable: true, - }, + afsConfig: &config.AdmissionFairSharing{}, wls: []kueue.Workload{ *utiltesting.MakeWorkload("wlA-low", "default").Queue("lqA").Priority(1).Obj(), *utiltesting.MakeWorkload("wlA-high", "default").Queue("lqA").Priority(2).Obj(), @@ -1125,7 +1111,7 @@ func TestFsAdmission(t *testing.T) { client := builder.Build() ctx := context.Background() - cq, _ := newClusterQueue(ctx, client, tc.cq, defaultOrdering, tc.fsConfig) + cq, _ := newClusterQueue(ctx, client, tc.cq, defaultOrdering, tc.afsConfig) for _, wl := range tc.wls { cq.PushOrUpdate(workload.NewInfo(&wl)) } diff --git a/pkg/queue/manager.go b/pkg/queue/manager.go index 1608e1d8014..d37063791dd 100644 --- a/pkg/queue/manager.go +++ b/pkg/queue/manager.go @@ -48,6 +48,7 @@ type options struct { podsReadyRequeuingTimestamp config.RequeuingTimestamp workloadInfoOptions []workload.InfoOption fairSharing *config.FairSharing + admissionFairSharing *config.AdmissionFairSharing } // Option configures the manager. @@ -64,6 +65,12 @@ func WithFairSharing(cfg *config.FairSharing) Option { } } +func WithAdmissionFairSharing(cfg *config.AdmissionFairSharing) Option { + return func(o *options) { + o.admissionFairSharing = cfg + } +} + // WithPodsReadyRequeuingTimestamp sets the timestamp that is used for ordering // workloads that have been requeued due to the PodsReady condition. func WithPodsReadyRequeuingTimestamp(ts config.RequeuingTimestamp) Option { @@ -110,6 +117,8 @@ type Manager struct { topologyUpdateWatchers []TopologyUpdateWatcher fairSharingConfig *config.FairSharing + + admissionFairSharingConfig *config.AdmissionFairSharing } func NewManager(client client.Client, checker StatusChecker, opts ...Option) *Manager { @@ -129,8 +138,9 @@ func NewManager(client client.Client, checker StatusChecker, opts ...Option) *Ma workloadInfoOptions: options.workloadInfoOptions, hm: hierarchy.NewManager[*ClusterQueue, *cohort](newCohort), - topologyUpdateWatchers: make([]TopologyUpdateWatcher, 0), - fairSharingConfig: options.fairSharing, + topologyUpdateWatchers: make([]TopologyUpdateWatcher, 0), + fairSharingConfig: options.fairSharing, + admissionFairSharingConfig: options.admissionFairSharing, } m.cond.L = &m.RWMutex return m @@ -172,7 +182,7 @@ func (m *Manager) AddClusterQueue(ctx context.Context, cq *kueue.ClusterQueue) e return errClusterQueueAlreadyExists } - cqImpl, err := newClusterQueue(ctx, m.client, cq, m.workloadOrdering, m.fairSharingConfig) + cqImpl, err := newClusterQueue(ctx, m.client, cq, m.workloadOrdering, m.admissionFairSharingConfig) if err != nil { return err } diff --git a/pkg/scheduler/preemption/fairsharing/strategy.go b/pkg/scheduler/preemption/fairsharing/strategy.go index 5a9bbfa5175..df413792c20 100644 --- a/pkg/scheduler/preemption/fairsharing/strategy.go +++ b/pkg/scheduler/preemption/fairsharing/strategy.go @@ -41,7 +41,3 @@ func LessThanOrEqualToFinalShare(preemptorNewShare PreemptorNewShare, _ TargetOl func LessThanInitialShare(preemptorNewShare PreemptorNewShare, targetOldShare TargetOldShare, _ TargetNewShare) bool { return int(preemptorNewShare) < int(targetOldShare) } - -func NoPreemption(_ PreemptorNewShare, _ TargetOldShare, _ TargetNewShare) bool { - return false -} diff --git a/pkg/scheduler/preemption/preemption.go b/pkg/scheduler/preemption/preemption.go index 413c3403542..d67ea31ed9c 100644 --- a/pkg/scheduler/preemption/preemption.go +++ b/pkg/scheduler/preemption/preemption.go @@ -292,8 +292,6 @@ func parseStrategies(s []config.PreemptionStrategy) []fairsharing.Strategy { strategies[i] = fairsharing.LessThanOrEqualToFinalShare case config.LessThanInitialShare: strategies[i] = fairsharing.LessThanInitialShare - case config.NoPreemption: - strategies[i] = fairsharing.NoPreemption } } return strategies diff --git a/pkg/scheduler/preemption/preemption_test.go b/pkg/scheduler/preemption/preemption_test.go index 8453b186928..06dab3a7494 100644 --- a/pkg/scheduler/preemption/preemption_test.go +++ b/pkg/scheduler/preemption/preemption_test.go @@ -2196,18 +2196,6 @@ func TestFairPreemptions(t *testing.T) { targetCQ: "a", wantPreempted: sets.New(targetKeyReason("/b_low", kueue.InCohortFairSharingReason)), }, - "NoPreemption strategy; no preemption even if off balanced": { - clusterQueues: baseCQs, - strategies: []config.PreemptionStrategy{config.NoPreemption}, - admitted: []kueue.Workload{ - *utiltesting.MakeWorkload("a1", "").Request(corev1.ResourceCPU, "3").SimpleReserveQuota("a", "default", now).Obj(), - *utiltesting.MakeWorkload("b_low", "").Priority(0).Request(corev1.ResourceCPU, "5").SimpleReserveQuota("b", "default", now).Obj(), - *utiltesting.MakeWorkload("b_high", "").Priority(1).Request(corev1.ResourceCPU, "1").SimpleReserveQuota("b", "default", now).Obj(), - }, - incoming: utiltesting.MakeWorkload("a_incoming", "").Request(corev1.ResourceCPU, "1").Obj(), - targetCQ: "a", - wantPreempted: nil, - }, "preempt workload that doesn't transfer the imbalance, even if high priority": { clusterQueues: baseCQs, strategies: []config.PreemptionStrategy{config.LessThanOrEqualToFinalShare}, diff --git a/site/content/en/docs/reference/kueue-config.v1beta1.md b/site/content/en/docs/reference/kueue-config.v1beta1.md index df9b67466f8..7e371d028d6 100644 --- a/site/content/en/docs/reference/kueue-config.v1beta1.md +++ b/site/content/en/docs/reference/kueue-config.v1beta1.md @@ -20,7 +20,6 @@ description: Generated API reference documentation for Kueue Configuration. **Appears in:** -- [FairSharing](#FairSharing) @@ -233,6 +232,13 @@ instead.

    FairSharing controls the Fair Sharing semantics across the cluster.

    +admissionFairSharing
    +AdmissionFairSharing + + +

    admissionFairSharing indicates configuration of FairSharing with the AdmissionTime mode on

    + + resources [Required]
    Resources @@ -522,19 +528,11 @@ as high as possible. with the incoming workload is strictly less than the share of the preemptee CQ. This strategy doesn't depend on the share usage of the workload being preempted. As a result, the strategy chooses to preempt workloads with the lowest priority and -newest start time first. -
  • NoPreemption: Never preempt a workload. +newest start time first. The default strategy is ["LessThanOrEqualToFinalShare", "LessThanInitialShare"].
    • -admissionFairSharing
    -AdmissionFairSharing - - -

    admissionFairSharing indicates configuration of FairSharing with the AdmissionTime mode on

    - - diff --git a/test/integration/singlecluster/scheduler/fairsharing/suite_test.go b/test/integration/singlecluster/scheduler/fairsharing/suite_test.go index c220c681203..0b89670fb89 100644 --- a/test/integration/singlecluster/scheduler/fairsharing/suite_test.go +++ b/test/integration/singlecluster/scheduler/fairsharing/suite_test.go @@ -72,13 +72,13 @@ var _ = ginkgo.AfterSuite(func() { func managerAndSchedulerSetup(ctx context.Context, mgr manager.Manager) { fairSharing := &config.FairSharing{ Enable: true, - AdmissionFairSharing: &config.AdmissionFairSharing{ - UsageHalfLifeTime: metav1.Duration{ - Duration: 250 * time.Microsecond, - }, - UsageSamplingInterval: metav1.Duration{ - Duration: 250 * time.Millisecond, - }, + } + admissionFairSharing := &config.AdmissionFairSharing{ + UsageHalfLifeTime: metav1.Duration{ + Duration: 250 * time.Microsecond, + }, + UsageSamplingInterval: metav1.Duration{ + Duration: 250 * time.Millisecond, }, } @@ -86,9 +86,9 @@ func managerAndSchedulerSetup(ctx context.Context, mgr manager.Manager) { gomega.Expect(err).NotTo(gomega.HaveOccurred()) cCache := cache.New(mgr.GetClient(), cache.WithFairSharing(fairSharing.Enable)) - queues := queue.NewManager(mgr.GetClient(), cCache, queue.WithFairSharing(fairSharing)) + queues := queue.NewManager(mgr.GetClient(), cCache, queue.WithFairSharing(fairSharing), queue.WithAdmissionFairSharing(admissionFairSharing)) - configuration := &config.Configuration{FairSharing: fairSharing} + configuration := &config.Configuration{FairSharing: fairSharing, AdmissionFairSharing: admissionFairSharing} configuration.Metrics.EnableClusterQueueResources = true mgr.GetScheme().Default(configuration) From 36175b1acb965c8c21eef7c9fb59c2420d019222 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Tue, 6 May 2025 07:53:22 +0000 Subject: [PATCH 03/19] Clean up --- pkg/queue/cluster_queue.go | 14 ++++++++------ pkg/util/resource/resource.go | 7 ++----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/pkg/queue/cluster_queue.go b/pkg/queue/cluster_queue.go index 3fa7eff737f..79718b1d975 100644 --- a/pkg/queue/cluster_queue.go +++ b/pkg/queue/cluster_queue.go @@ -437,15 +437,17 @@ func queueOrderingFunc(ctx context.Context, c client.Client, wo workload.Orderin if enableAdmissionFs { lqAUsage, errA := a.LqUsage(ctx, c, fsResWeights) lqBUsage, errB := b.LqUsage(ctx, c, fsResWeights) - log.V(3).Info("Resource usage from LocalQueue", "LocalQueue", a.Obj.Spec.QueueName, "Usage", lqAUsage) - log.V(3).Info("Resource usage from LocalQueue", "LocalQueue", b.Obj.Spec.QueueName, "Usage", lqBUsage) switch { case errA != nil: - log.Error(errA, "Error fetching LocalQueue from informer") + log.V(3).Error(errA, "Error fetching LocalQueue from informer") case errB != nil: - log.Error(errB, "Error fetching LocalQueue from informer") - case lqAUsage != lqBUsage: - return lqAUsage < lqBUsage + log.V(3).Error(errB, "Error fetching LocalQueue from informer") + default: + log.V(3).Info("Resource usage from LocalQueue", "LocalQueue", a.Obj.Spec.QueueName, "Usage", lqAUsage) + log.V(3).Info("Resource usage from LocalQueue", "LocalQueue", b.Obj.Spec.QueueName, "Usage", lqBUsage) + if lqAUsage != lqBUsage { + return lqAUsage < lqBUsage + } } } p1 := utilpriority.Priority(a.Obj) diff --git a/pkg/util/resource/resource.go b/pkg/util/resource/resource.go index e9ede6f27cf..16c28aad897 100644 --- a/pkg/util/resource/resource.go +++ b/pkg/util/resource/resource.go @@ -105,14 +105,11 @@ func QuantityToFloat(q *resource.Quantity) float64 { } // MulByFloat multiplies every element in q by f. -// It first mutiplies f by 1 000 to mitigate further precision loss, and then leverages -// k8s.io/apimachinery/pkg/api/resource package to provide precision to 3 decimal places +// Leverages k8s.io/apimachinery/pkg/api/resource package to provide precision to 3 decimal places func MulByFloat(q corev1.ResourceList, f float64) corev1.ResourceList { ret := q.DeepCopy() - scaleFact := f * 1_000 for k, v := range ret { - scaledV := float64(v.MilliValue()) * scaleFact - scaledV /= 1_000 + scaledV := float64(v.MilliValue()) * f ret[k] = *resource.NewMilliQuantity(int64(scaledV), resource.DecimalSI) } return ret From 647b8b3b5f234daaaef7fe2b7616a877e6b0e587 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Tue, 6 May 2025 10:29:08 +0000 Subject: [PATCH 04/19] Address comments --- cmd/kueue/main.go | 5 ++++- pkg/controller/core/localqueue_controller.go | 2 -- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/kueue/main.go b/cmd/kueue/main.go index 59771dbe27d..9790982977a 100644 --- a/cmd/kueue/main.go +++ b/cmd/kueue/main.go @@ -217,7 +217,10 @@ func main() { } if cfg.FairSharing != nil { cacheOptions = append(cacheOptions, cache.WithFairSharing(cfg.FairSharing.Enable)) - queueOptions = append(queueOptions, queue.WithFairSharing(cfg.FairSharing), queue.WithAdmissionFairSharing(cfg.AdmissionFairSharing)) + queueOptions = append(queueOptions, queue.WithFairSharing(cfg.FairSharing)) + } + if cfg.AdmissionFairSharing != nil { + queueOptions = append(queueOptions, queue.WithAdmissionFairSharing(cfg.AdmissionFairSharing)) } cCache := cache.New(mgr.GetClient(), cacheOptions...) queues := queue.NewManager(mgr.GetClient(), cCache, queueOptions...) diff --git a/pkg/controller/core/localqueue_controller.go b/pkg/controller/core/localqueue_controller.go index d1df7935714..9e467b97aba 100644 --- a/pkg/controller/core/localqueue_controller.go +++ b/pkg/controller/core/localqueue_controller.go @@ -75,8 +75,6 @@ func WithAdmissionFairSharingConfig(cfg *config.AdmissionFairSharing) LocalQueue return func(o *LocalQueueReconcilerOptions) { if cfg != nil { o.admissionFSConfig = cfg - } else { - o.admissionFSConfig = nil } } } From 321dc1628df2d04a0fccf2ba034864b2599dd0be Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Tue, 6 May 2025 12:36:25 +0000 Subject: [PATCH 05/19] Address comments --- apis/config/v1beta1/configuration_types.go | 1 + apis/kueue/v1beta1/fairsharing_types.go | 7 ++-- apis/kueue/v1beta1/localqueue_types.go | 4 +- pkg/controller/core/localqueue_controller.go | 28 +++++++------- pkg/queue/cluster_queue.go | 2 +- pkg/queue/cluster_queue_test.go | 38 +++++++++---------- pkg/workload/workload.go | 9 ++--- .../en/docs/reference/kueue-config.v1beta1.md | 3 +- .../en/docs/reference/kueue.v1beta1.md | 4 +- .../fairsharing/fair_sharing_test.go | 2 +- 10 files changed, 50 insertions(+), 48 deletions(-) diff --git a/apis/config/v1beta1/configuration_types.go b/apis/config/v1beta1/configuration_types.go index acd912a88ac..7a5d84ea17b 100644 --- a/apis/config/v1beta1/configuration_types.go +++ b/apis/config/v1beta1/configuration_types.go @@ -483,6 +483,7 @@ type AdmissionFairSharing struct { UsageHalfLifeTime metav1.Duration `json:"usageHalfLifeTime,omitempty"` // usageSamplingInterval indicates how often Kueue updates consumedResources in FairSharingStatus + // Defaults to 5min. UsageSamplingInterval metav1.Duration `json:"usageSamplingInterval,omitempty"` // resourceWeights assigns weights to resources which then are used to calculate LocalQueue/ClusterQueue/Cohort's diff --git a/apis/kueue/v1beta1/fairsharing_types.go b/apis/kueue/v1beta1/fairsharing_types.go index 4b4313c9adf..bd802785089 100644 --- a/apis/kueue/v1beta1/fairsharing_types.go +++ b/apis/kueue/v1beta1/fairsharing_types.go @@ -75,8 +75,9 @@ type AdmissionScope struct { type AdmissionMode string const ( - // FairSharing based on usage, with QueuingStrategy as defined in CQ. - UsageBasedFairSharing AdmissionMode = "UsageBasedFairSharing" + // AdmissionFairSharing based on usage, with QueuingStrategy as defined in CQ. + UsageBasedAdmissionFairSharing AdmissionMode = "UsageBasedAdmissionFairSharing" - NoFairSharing AdmissionMode = "NoFairSharing" + // AdmissionFairSharing is disabled for this CQ + NoAdmissionFairSharing AdmissionMode = "NoFairSharing" ) diff --git a/apis/kueue/v1beta1/localqueue_types.go b/apis/kueue/v1beta1/localqueue_types.go index 7aa2c40dca4..7fd8935f09f 100644 --- a/apis/kueue/v1beta1/localqueue_types.go +++ b/apis/kueue/v1beta1/localqueue_types.go @@ -50,8 +50,8 @@ type LocalQueueSpec struct { StopPolicy *StopPolicy `json:"stopPolicy,omitempty"` // fairSharing defines the properties of the LocalQueue when - // participating in FairSharing. The values are only relevant - // if FairSharing is enabled in the Kueue configuration. + // participating in AdmissionFairSharing. The values are only relevant + // if AdmissionFairSharing is enabled in the Kueue configuration. // +optional FairSharing *FairSharing `json:"fairSharing,omitempty"` } diff --git a/pkg/controller/core/localqueue_controller.go b/pkg/controller/core/localqueue_controller.go index 9e467b97aba..1a4ee3470a2 100644 --- a/pkg/controller/core/localqueue_controller.go +++ b/pkg/controller/core/localqueue_controller.go @@ -143,8 +143,8 @@ func (r *LocalQueueReconciler) NotifyWorkloadUpdate(oldWl, newWl *kueue.Workload // +kubebuilder:rbac:groups=kueue.x-k8s.io,resources=localqueues/finalizers,verbs=update func (r *LocalQueueReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - var lq kueue.LocalQueue - if err := r.client.Get(ctx, req.NamespacedName, &lq); err != nil { + var queueObj kueue.LocalQueue + if err := r.client.Get(ctx, req.NamespacedName, &queueObj); err != nil { // we'll ignore not-found errors, since there is nothing to do. return ctrl.Result{}, client.IgnoreNotFound(err) } @@ -152,40 +152,40 @@ func (r *LocalQueueReconciler) Reconcile(ctx context.Context, req ctrl.Request) log := ctrl.LoggerFrom(ctx) log.V(2).Info("Reconcile LocalQueue") - if ptr.Deref(lq.Spec.StopPolicy, kueue.None) != kueue.None { - err := r.UpdateStatusIfChanged(ctx, &lq, metav1.ConditionFalse, StoppedReason, localQueueIsInactiveMsg) + if ptr.Deref(queueObj.Spec.StopPolicy, kueue.None) != kueue.None { + err := r.UpdateStatusIfChanged(ctx, &queueObj, metav1.ConditionFalse, StoppedReason, localQueueIsInactiveMsg) return ctrl.Result{}, client.IgnoreNotFound(err) } var cq kueue.ClusterQueue - if err := r.client.Get(ctx, client.ObjectKey{Name: string(lq.Spec.ClusterQueue)}, &cq); err != nil { + if err := r.client.Get(ctx, client.ObjectKey{Name: string(queueObj.Spec.ClusterQueue)}, &cq); err != nil { if apierrors.IsNotFound(err) { - err = r.UpdateStatusIfChanged(ctx, &lq, metav1.ConditionFalse, "ClusterQueueDoesNotExist", clusterQueueIsInactiveMsg) + err = r.UpdateStatusIfChanged(ctx, &queueObj, metav1.ConditionFalse, "ClusterQueueDoesNotExist", clusterQueueIsInactiveMsg) } return ctrl.Result{}, client.IgnoreNotFound(err) } if meta.IsStatusConditionTrue(cq.Status.Conditions, kueue.ClusterQueueActive) { - if err := r.UpdateStatusIfChanged(ctx, &lq, metav1.ConditionTrue, "Ready", "Can submit new workloads to localQueue"); err != nil { + if err := r.UpdateStatusIfChanged(ctx, &queueObj, metav1.ConditionTrue, "Ready", "Can submit new workloads to localQueue"); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } } else { - if err := r.UpdateStatusIfChanged(ctx, &lq, metav1.ConditionFalse, clusterQueueIsInactiveReason, clusterQueueIsInactiveMsg); err != nil { + if err := r.UpdateStatusIfChanged(ctx, &queueObj, metav1.ConditionFalse, clusterQueueIsInactiveReason, clusterQueueIsInactiveMsg); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } } if r.admissionFSConfig != nil { - if err := r.initializeAdmissionFsStatus(ctx, &lq); err != nil { + if err := r.initializeAdmissionFsStatus(ctx, &queueObj); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } - recheckAfter := r.clock.Now().Sub(lq.Status.FairSharingStatus.AdmissionFairSharingStatus.LastUpdate.Time) - if recheckAfter < r.admissionFSConfig.UsageSamplingInterval.Duration { - return ctrl.Result{RequeueAfter: recheckAfter}, nil + sinceLastUpdate := r.clock.Now().Sub(queueObj.Status.FairSharingStatus.AdmissionFairSharingStatus.LastUpdate.Time) + if interval := r.admissionFSConfig.UsageSamplingInterval.Duration; sinceLastUpdate < interval { + return ctrl.Result{RequeueAfter: interval - sinceLastUpdate}, nil } - if err := r.reconcileConsumedUsage(ctx, &lq, lq.Spec.ClusterQueue); err != nil { + if err := r.reconcileConsumedUsage(ctx, &queueObj, queueObj.Spec.ClusterQueue); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } - if err := r.queues.HeapifyClusterQueue(&cq, lq.Name); err != nil { + if err := r.queues.HeapifyClusterQueue(&cq, queueObj.Name); err != nil { return ctrl.Result{}, err } return ctrl.Result{RequeueAfter: r.admissionFSConfig.UsageSamplingInterval.Duration}, nil diff --git a/pkg/queue/cluster_queue.go b/pkg/queue/cluster_queue.go index 79718b1d975..c14fabf2fc6 100644 --- a/pkg/queue/cluster_queue.go +++ b/pkg/queue/cluster_queue.go @@ -103,7 +103,7 @@ func newClusterQueue(ctx context.Context, client client.Client, cq *kueue.Cluste func isAdmissionFsEnabled(cq *kueue.ClusterQueue, fsConfig *config.AdmissionFairSharing) (bool, map[corev1.ResourceName]float64) { enableAdmissionFs, fsResWeights := false, make(map[corev1.ResourceName]float64) - if fsConfig != nil && cq.Spec.AdmissionScope != nil && cq.Spec.AdmissionScope.AdmissionMode == kueue.UsageBasedFairSharing { + if fsConfig != nil && cq.Spec.AdmissionScope != nil && cq.Spec.AdmissionScope.AdmissionMode == kueue.UsageBasedAdmissionFairSharing { enableAdmissionFs = true fsResWeights = fsConfig.ResourceWeights } diff --git a/pkg/queue/cluster_queue_test.go b/pkg/queue/cluster_queue_test.go index c1e23abe557..2d306a9a7d9 100644 --- a/pkg/queue/cluster_queue_test.go +++ b/pkg/queue/cluster_queue_test.go @@ -155,7 +155,7 @@ func Test_PushOrUpdate(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, fakeClock, nil, false) + cq := newClusterQueueImpl(t.Context(), nil, defaultOrdering, fakeClock, nil, false) if cq.Pending() != 0 { t.Error("ClusterQueue should be empty") @@ -184,7 +184,7 @@ func Test_PushOrUpdate(t *testing.T) { func Test_Pop(t *testing.T) { now := time.Now() - cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(now), nil, false) + cq := newClusterQueueImpl(t.Context(), nil, defaultOrdering, testingclock.NewFakeClock(now), nil, false) wl1 := workload.NewInfo(utiltesting.MakeWorkload("workload-1", defaultNamespace).Creation(now).Obj()) wl2 := workload.NewInfo(utiltesting.MakeWorkload("workload-2", defaultNamespace).Creation(now.Add(time.Second)).Obj()) if cq.Pop() != nil { @@ -206,7 +206,7 @@ func Test_Pop(t *testing.T) { } func Test_Delete(t *testing.T) { - cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) + cq := newClusterQueueImpl(t.Context(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) wl1 := utiltesting.MakeWorkload("workload-1", defaultNamespace).Obj() wl2 := utiltesting.MakeWorkload("workload-2", defaultNamespace).Obj() cq.PushOrUpdate(workload.NewInfo(wl1)) @@ -227,7 +227,7 @@ func Test_Delete(t *testing.T) { } func Test_Info(t *testing.T) { - cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) + cq := newClusterQueueImpl(t.Context(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) wl := utiltesting.MakeWorkload("workload-1", defaultNamespace).Obj() if info := cq.Info(workload.Key(wl)); info != nil { t.Error("Workload should not exist") @@ -239,7 +239,7 @@ func Test_Info(t *testing.T) { } func Test_AddFromLocalQueue(t *testing.T) { - cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) + cq := newClusterQueueImpl(t.Context(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) wl := utiltesting.MakeWorkload("workload-1", defaultNamespace).Obj() queue := &LocalQueue{ items: map[string]*workload.Info{ @@ -257,7 +257,7 @@ func Test_AddFromLocalQueue(t *testing.T) { } func Test_DeleteFromLocalQueue(t *testing.T) { - cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) + cq := newClusterQueueImpl(t.Context(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) q := utiltesting.MakeLocalQueue("foo", "").ClusterQueue("cq").Obj() qImpl := newLocalQueue(q) wl1 := utiltesting.MakeWorkload("wl1", "").Queue(kueue.LocalQueueName(q.Name)).Obj() @@ -412,7 +412,7 @@ func TestClusterQueueImpl(t *testing.T) { for name, test := range tests { t.Run(name, func(t *testing.T) { - cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, fakeClock, nil, false) + cq := newClusterQueueImpl(t.Context(), nil, defaultOrdering, fakeClock, nil, false) err := cq.Update(utiltesting.MakeClusterQueue("cq"). NamespaceSelector(&metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ @@ -465,7 +465,7 @@ func TestClusterQueueImpl(t *testing.T) { } func TestQueueInadmissibleWorkloadsDuringScheduling(t *testing.T) { - cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) + cq := newClusterQueueImpl(t.Context(), nil, defaultOrdering, testingclock.NewFakeClock(time.Now()), nil, false) cq.namespaceSelector = labels.Everything() wl := utiltesting.MakeWorkload("workload-1", defaultNamespace).Obj() cl := utiltesting.NewFakeClient(wl, utiltesting.MakeNamespace(defaultNamespace)) @@ -549,7 +549,7 @@ func TestBackoffWaitingTimeExpired(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - cq := newClusterQueueImpl(context.TODO(), nil, defaultOrdering, fakeClock, nil, false) + cq := newClusterQueueImpl(t.Context(), nil, defaultOrdering, fakeClock, nil, false) got := cq.backoffWaitingTimeExpired(tc.workloadInfo) if tc.want != got { t.Errorf("Unexpected result from backoffWaitingTimeExpired\nwant: %v\ngot: %v\n", tc.want, got) @@ -606,7 +606,7 @@ func TestBestEffortFIFORequeueIfNotPresent(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { - cq, _ := newClusterQueue(context.TODO(), nil, + cq, _ := newClusterQueue(t.Context(), nil, &kueue.ClusterQueue{ Spec: kueue.ClusterQueueSpec{ QueueingStrategy: kueue.BestEffortFIFO, @@ -634,7 +634,7 @@ func TestBestEffortFIFORequeueIfNotPresent(t *testing.T) { } func TestFIFOClusterQueue(t *testing.T) { - q, err := newClusterQueue(context.TODO(), nil, + q, err := newClusterQueue(t.Context(), nil, &kueue.ClusterQueue{ Spec: kueue.ClusterQueueSpec{ QueueingStrategy: kueue.StrictFIFO, @@ -839,7 +839,7 @@ func TestStrictFIFO(t *testing.T) { // The default ordering: tt.workloadOrdering = &workload.Ordering{PodsReadyRequeuingTimestamp: config.EvictionTimestamp} } - q, err := newClusterQueue(context.TODO(), nil, + q, err := newClusterQueue(t.Context(), nil, &kueue.ClusterQueue{ Spec: kueue.ClusterQueueSpec{ QueueingStrategy: kueue.StrictFIFO, @@ -882,7 +882,7 @@ func TestStrictFIFORequeueIfNotPresent(t *testing.T) { for reason, test := range tests { t.Run(string(reason), func(t *testing.T) { - cq, _ := newClusterQueue(context.TODO(), nil, + cq, _ := newClusterQueue(t.Context(), nil, &kueue.ClusterQueue{ Spec: kueue.ClusterQueueSpec{ QueueingStrategy: kueue.StrictFIFO, @@ -923,7 +923,7 @@ func TestFsAdmission(t *testing.T) { }{ "workloads are ordered by LQ usage, instead of priorities": { cq: utiltesting.MakeClusterQueue("cq"). - AdmissionMode(kueue.UsageBasedFairSharing). + AdmissionMode(kueue.UsageBasedAdmissionFairSharing). Obj(), lqs: []kueue.LocalQueue{ *utiltesting.MakeLocalQueue("lqA", "default"). @@ -963,7 +963,7 @@ func TestFsAdmission(t *testing.T) { }, "workloads are ordered by LQ usage with respect to resource weights": { cq: utiltesting.MakeClusterQueue("cq"). - AdmissionMode(kueue.UsageBasedFairSharing). + AdmissionMode(kueue.UsageBasedAdmissionFairSharing). Obj(), lqs: []kueue.LocalQueue{ *utiltesting.MakeLocalQueue("lqA", "default"). @@ -1010,7 +1010,7 @@ func TestFsAdmission(t *testing.T) { }, "workloads are ordered by LQ usage with respect to LQs' fair sharing weights": { cq: utiltesting.MakeClusterQueue("cq"). - AdmissionMode(kueue.UsageBasedFairSharing). + AdmissionMode(kueue.UsageBasedAdmissionFairSharing). Obj(), lqs: []kueue.LocalQueue{ *utiltesting.MakeLocalQueue("lqA", "default"). @@ -1050,7 +1050,7 @@ func TestFsAdmission(t *testing.T) { }, "workloads with the same LQ usage are ordered by priority": { cq: utiltesting.MakeClusterQueue("cq"). - AdmissionMode(kueue.UsageBasedFairSharing). + AdmissionMode(kueue.UsageBasedAdmissionFairSharing). Obj(), lqs: []kueue.LocalQueue{ *utiltesting.MakeLocalQueue("lqA", "default"). @@ -1076,7 +1076,7 @@ func TestFsAdmission(t *testing.T) { }, "workloads with NoFairSharing CQ are ordered by priority": { cq: utiltesting.MakeClusterQueue("cq"). - AdmissionMode(kueue.NoFairSharing). + AdmissionMode(kueue.NoAdmissionFairSharing). Obj(), lqs: []kueue.LocalQueue{ *utiltesting.MakeLocalQueue("lqA", "default").Obj(), @@ -1090,7 +1090,7 @@ func TestFsAdmission(t *testing.T) { }, "workloads with no FS config are ordered by priority": { cq: utiltesting.MakeClusterQueue("cq"). - AdmissionMode(kueue.NoFairSharing). + AdmissionMode(kueue.NoAdmissionFairSharing). Obj(), lqs: []kueue.LocalQueue{ *utiltesting.MakeLocalQueue("lqA", "default").Obj(), diff --git a/pkg/workload/workload.go b/pkg/workload/workload.go index 561bea26136..dd7c441d38b 100644 --- a/pkg/workload/workload.go +++ b/pkg/workload/workload.go @@ -290,12 +290,11 @@ func (i *Info) LqUsage(ctx context.Context, c client.Client, resWeights map[core } usage := 0.0 for resName, resVal := range lq.Status.FairSharingStatus.AdmissionFairSharingStatus.ConsumedResources { - if weight, found := resWeights[resName]; found { - usage += weight * resVal.AsApproximateFloat64() - } else { - // if no weight for resource was defined, use default weight of 1 - usage += resVal.AsApproximateFloat64() + weight, found := resWeights[resName] + if !found { + weight = 1 } + usage += weight * resVal.AsApproximateFloat64() } if lq.Spec.FairSharing != nil && lq.Spec.FairSharing.Weight != nil { // if no weight for lq was defined, use default weight of 1 diff --git a/site/content/en/docs/reference/kueue-config.v1beta1.md b/site/content/en/docs/reference/kueue-config.v1beta1.md index 7e371d028d6..af719d5a94e 100644 --- a/site/content/en/docs/reference/kueue-config.v1beta1.md +++ b/site/content/en/docs/reference/kueue-config.v1beta1.md @@ -40,7 +40,8 @@ If set to 0, usage will be reset to 0.

    k8s.io/apimachinery/pkg/apis/meta/v1.Duration -

    usageSamplingInterval indicates how often Kueue updates consumedResources in FairSharingStatus

    +

    usageSamplingInterval indicates how often Kueue updates consumedResources in FairSharingStatus +Defaults to 5min.

    resourceWeights [Required]
    diff --git a/site/content/en/docs/reference/kueue.v1beta1.md b/site/content/en/docs/reference/kueue.v1beta1.md index 0d04c8c654e..89d863494c3 100644 --- a/site/content/en/docs/reference/kueue.v1beta1.md +++ b/site/content/en/docs/reference/kueue.v1beta1.md @@ -1531,8 +1531,8 @@ no new reservation being made.

    fairSharing defines the properties of the LocalQueue when -participating in FairSharing. The values are only relevant -if FairSharing is enabled in the Kueue configuration.

    +participating in AdmissionFairSharing. The values are only relevant +if AdmissionFairSharing is enabled in the Kueue configuration.

    diff --git a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go index e8bd946cd4b..4da71efa1d0 100644 --- a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go +++ b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go @@ -407,7 +407,7 @@ var _ = ginkgo.Describe("Scheduler", func() { ResourceGroup(*testing.MakeFlavorQuotas(defaultFlavor.Name).Resource(corev1.ResourceCPU, "32").Obj()). Preemption(kueue.ClusterQueuePreemption{WithinClusterQueue: kueue.PreemptionPolicyNever}). QueueingStrategy(kueue.StrictFIFO). - AdmissionMode(kueue.UsageBasedFairSharing). + AdmissionMode(kueue.UsageBasedAdmissionFairSharing). Obj() cqs = append(cqs, cq) util.MustCreate(ctx, k8sClient, cq) From 618449ede0d67e6755e0526a050c519f01a66712 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Tue, 6 May 2025 12:56:20 +0000 Subject: [PATCH 06/19] Generate helm charts --- charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml | 4 ++-- config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml index 0feaa9e04d3..47a68b47483 100644 --- a/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml +++ b/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml @@ -83,8 +83,8 @@ spec: fairSharing: description: |- fairSharing defines the properties of the LocalQueue when - participating in FairSharing. The values are only relevant - if FairSharing is enabled in the Kueue configuration. + participating in AdmissionFairSharing. The values are only relevant + if AdmissionFairSharing is enabled in the Kueue configuration. properties: weight: anyOf: diff --git a/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml b/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml index f0b7e462878..5f65666a2e3 100644 --- a/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml +++ b/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml @@ -68,8 +68,8 @@ spec: fairSharing: description: |- fairSharing defines the properties of the LocalQueue when - participating in FairSharing. The values are only relevant - if FairSharing is enabled in the Kueue configuration. + participating in AdmissionFairSharing. The values are only relevant + if AdmissionFairSharing is enabled in the Kueue configuration. properties: weight: anyOf: From 33618a237db1e37910eb13ae1116fe0a688612f9 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Tue, 6 May 2025 15:27:30 +0000 Subject: [PATCH 07/19] Add unit tests that check time left for another reconcile --- .../core/localqueue_controller_test.go | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/pkg/controller/core/localqueue_controller_test.go b/pkg/controller/core/localqueue_controller_test.go index 4c33f15bef6..94d6901f14e 100644 --- a/pkg/controller/core/localqueue_controller_test.go +++ b/pkg/controller/core/localqueue_controller_test.go @@ -44,12 +44,13 @@ import ( func TestLocalQueueReconcile(t *testing.T) { clock := testingclock.NewFakeClock(time.Now().Truncate(time.Second)) cases := map[string]struct { - clusterQueue *kueue.ClusterQueue - localQueue *kueue.LocalQueue - wantLocalQueue *kueue.LocalQueue - wantError error - afsConfig *config.AdmissionFairSharing - runningWls []kueue.Workload + clusterQueue *kueue.ClusterQueue + localQueue *kueue.LocalQueue + wantLocalQueue *kueue.LocalQueue + wantError error + afsConfig *config.AdmissionFairSharing + runningWls []kueue.Workload + wantRequeueAfter *time.Duration }{ "local queue with Hold StopPolicy": { clusterQueue: utiltesting.MakeClusterQueue("test-cluster-queue"). @@ -489,7 +490,7 @@ func TestLocalQueueReconcile(t *testing.T) { }, }). Obj(), - wantError: nil, + wantRequeueAfter: ptr.To(time.Minute), afsConfig: &config.AdmissionFairSharing{ UsageHalfLifeTime: metav1.Duration{Duration: 5 * time.Minute}, UsageSamplingInterval: metav1.Duration{Duration: 5 * time.Minute}, @@ -530,10 +531,15 @@ func TestLocalQueueReconcile(t *testing.T) { ctx, ctxCancel := context.WithCancel(ctxWithLogger) defer ctxCancel() - _, gotError := reconciler.Reconcile( + result, gotError := reconciler.Reconcile( ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(tc.localQueue)}, ) + if tc.wantRequeueAfter != nil { + if diff := cmp.Diff(*tc.wantRequeueAfter, result.RequeueAfter); diff != "" { + t.Errorf("unexpected reconcile requeue after (-want/+got):\n%s", diff) + } + } if diff := cmp.Diff(tc.wantError, gotError); diff != "" { t.Errorf("unexpected reconcile error (-want/+got):\n%s", diff) From 6cfb3d49fbab17c1dff90f5b33c1b84b1509eda9 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Tue, 6 May 2025 15:27:37 +0000 Subject: [PATCH 08/19] Address nits --- apis/config/v1beta1/configuration_types.go | 3 +-- apis/kueue/v1beta1/fairsharing_types.go | 2 +- apis/kueue/v1beta1/localqueue_types.go | 2 +- .../templates/crd/kueue.x-k8s.io_localqueues.yaml | 2 +- .../kueue/v1beta1/localqueuestatus.go | 2 +- cmd/kueue/main.go | 1 - .../crd/bases/kueue.x-k8s.io_localqueues.yaml | 2 +- pkg/queue/manager.go | 10 ---------- site/content/en/docs/reference/kueue-config.v1beta1.md | 4 ++-- site/content/en/docs/reference/kueue.v1beta1.md | 2 +- site/static/examples/admission-fs/lq-a-simple-job.yaml | 2 +- site/static/examples/admission-fs/lq-b-simple-job.yaml | 2 +- 12 files changed, 11 insertions(+), 23 deletions(-) diff --git a/apis/config/v1beta1/configuration_types.go b/apis/config/v1beta1/configuration_types.go index 7a5d84ea17b..f3cf3da912d 100644 --- a/apis/config/v1beta1/configuration_types.go +++ b/apis/config/v1beta1/configuration_types.go @@ -96,7 +96,6 @@ type Configuration struct { FairSharing *FairSharing `json:"fairSharing,omitempty"` // admissionFairSharing indicates configuration of FairSharing with the `AdmissionTime` mode on - // +optional AdmissionFairSharing *AdmissionFairSharing `json:"admissionFairSharing,omitempty"` // Resources provides additional configuration options for handling the resources. @@ -479,7 +478,7 @@ type FairSharing struct { type AdmissionFairSharing struct { // usageHalfLifeTime indicates the time after which the current usage will decay by a half - // If set to 0, usage will be reset to 0. + // If set to 0, usage will be reset to 0 immediately. UsageHalfLifeTime metav1.Duration `json:"usageHalfLifeTime,omitempty"` // usageSamplingInterval indicates how often Kueue updates consumedResources in FairSharingStatus diff --git a/apis/kueue/v1beta1/fairsharing_types.go b/apis/kueue/v1beta1/fairsharing_types.go index bd802785089..c37bfe44c23 100644 --- a/apis/kueue/v1beta1/fairsharing_types.go +++ b/apis/kueue/v1beta1/fairsharing_types.go @@ -79,5 +79,5 @@ const ( UsageBasedAdmissionFairSharing AdmissionMode = "UsageBasedAdmissionFairSharing" // AdmissionFairSharing is disabled for this CQ - NoAdmissionFairSharing AdmissionMode = "NoFairSharing" + NoAdmissionFairSharing AdmissionMode = "NoAdmissionFairSharing" ) diff --git a/apis/kueue/v1beta1/localqueue_types.go b/apis/kueue/v1beta1/localqueue_types.go index 7fd8935f09f..e9ba4aa3986 100644 --- a/apis/kueue/v1beta1/localqueue_types.go +++ b/apis/kueue/v1beta1/localqueue_types.go @@ -158,7 +158,7 @@ type LocalQueueStatus struct { // FairSharing contains the information about the current status of fair sharing. // +optional - FairSharingStatus FairSharingStatus `json:"fairSharingStatus,omitempty"` + FairSharingStatus FairSharingStatus `json:"fairSharing,omitempty"` } const ( diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml index 47a68b47483..087b38174e1 100644 --- a/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml +++ b/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml @@ -193,7 +193,7 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map - fairSharingStatus: + fairSharing: description: FairSharing contains the information about the current status of fair sharing. properties: diff --git a/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go b/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go index f203d0b0261..04b2b146f7c 100644 --- a/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go +++ b/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go @@ -31,7 +31,7 @@ type LocalQueueStatusApplyConfiguration struct { FlavorsReservation []LocalQueueFlavorUsageApplyConfiguration `json:"flavorsReservation,omitempty"` FlavorUsage []LocalQueueFlavorUsageApplyConfiguration `json:"flavorUsage,omitempty"` Flavors []LocalQueueFlavorStatusApplyConfiguration `json:"flavors,omitempty"` - FairSharingStatus *FairSharingStatusApplyConfiguration `json:"fairSharingStatus,omitempty"` + FairSharingStatus *FairSharingStatusApplyConfiguration `json:"fairSharing,omitempty"` } // LocalQueueStatusApplyConfiguration constructs a declarative configuration of the LocalQueueStatus type for use with diff --git a/cmd/kueue/main.go b/cmd/kueue/main.go index 9790982977a..5afd52b49db 100644 --- a/cmd/kueue/main.go +++ b/cmd/kueue/main.go @@ -217,7 +217,6 @@ func main() { } if cfg.FairSharing != nil { cacheOptions = append(cacheOptions, cache.WithFairSharing(cfg.FairSharing.Enable)) - queueOptions = append(queueOptions, queue.WithFairSharing(cfg.FairSharing)) } if cfg.AdmissionFairSharing != nil { queueOptions = append(queueOptions, queue.WithAdmissionFairSharing(cfg.AdmissionFairSharing)) diff --git a/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml b/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml index 5f65666a2e3..79d8db5c203 100644 --- a/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml +++ b/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml @@ -178,7 +178,7 @@ spec: x-kubernetes-list-map-keys: - type x-kubernetes-list-type: map - fairSharingStatus: + fairSharing: description: FairSharing contains the information about the current status of fair sharing. properties: diff --git a/pkg/queue/manager.go b/pkg/queue/manager.go index d37063791dd..452b92fc0da 100644 --- a/pkg/queue/manager.go +++ b/pkg/queue/manager.go @@ -47,7 +47,6 @@ var ( type options struct { podsReadyRequeuingTimestamp config.RequeuingTimestamp workloadInfoOptions []workload.InfoOption - fairSharing *config.FairSharing admissionFairSharing *config.AdmissionFairSharing } @@ -59,12 +58,6 @@ var defaultOptions = options{ workloadInfoOptions: []workload.InfoOption{}, } -func WithFairSharing(cfg *config.FairSharing) Option { - return func(o *options) { - o.fairSharing = cfg - } -} - func WithAdmissionFairSharing(cfg *config.AdmissionFairSharing) Option { return func(o *options) { o.admissionFairSharing = cfg @@ -116,8 +109,6 @@ type Manager struct { topologyUpdateWatchers []TopologyUpdateWatcher - fairSharingConfig *config.FairSharing - admissionFairSharingConfig *config.AdmissionFairSharing } @@ -139,7 +130,6 @@ func NewManager(client client.Client, checker StatusChecker, opts ...Option) *Ma hm: hierarchy.NewManager[*ClusterQueue, *cohort](newCohort), topologyUpdateWatchers: make([]TopologyUpdateWatcher, 0), - fairSharingConfig: options.fairSharing, admissionFairSharingConfig: options.admissionFairSharing, } m.cond.L = &m.RWMutex diff --git a/site/content/en/docs/reference/kueue-config.v1beta1.md b/site/content/en/docs/reference/kueue-config.v1beta1.md index af719d5a94e..5766f0bfd9d 100644 --- a/site/content/en/docs/reference/kueue-config.v1beta1.md +++ b/site/content/en/docs/reference/kueue-config.v1beta1.md @@ -33,7 +33,7 @@ description: Generated API reference documentation for Kueue Configuration.

    usageHalfLifeTime indicates the time after which the current usage will decay by a half -If set to 0, usage will be reset to 0.

    +If set to 0, usage will be reset to 0 immediately.

    usageSamplingInterval [Required]
    @@ -233,7 +233,7 @@ instead.

    FairSharing controls the Fair Sharing semantics across the cluster.

    -admissionFairSharing
    +admissionFairSharing [Required]
    AdmissionFairSharing diff --git a/site/content/en/docs/reference/kueue.v1beta1.md b/site/content/en/docs/reference/kueue.v1beta1.md index 89d863494c3..71309182f14 100644 --- a/site/content/en/docs/reference/kueue.v1beta1.md +++ b/site/content/en/docs/reference/kueue.v1beta1.md @@ -1608,7 +1608,7 @@ workloads assigned to this LocalQueue.

    flavors lists all currently available ResourceFlavors in specified ClusterQueue.

    -fairSharingStatus
    +fairSharing
    FairSharingStatus diff --git a/site/static/examples/admission-fs/lq-a-simple-job.yaml b/site/static/examples/admission-fs/lq-a-simple-job.yaml index bd90beab8db..852ff3a96da 100644 --- a/site/static/examples/admission-fs/lq-a-simple-job.yaml +++ b/site/static/examples/admission-fs/lq-a-simple-job.yaml @@ -12,7 +12,7 @@ spec: spec: containers: - name: dummy-job - image: alpine/sleep # A very small and lightweight image + image: alpine command: ["/bin/sleep", "infinity"] resources: requests: diff --git a/site/static/examples/admission-fs/lq-b-simple-job.yaml b/site/static/examples/admission-fs/lq-b-simple-job.yaml index 0d82bf61931..2c9cb3f495a 100644 --- a/site/static/examples/admission-fs/lq-b-simple-job.yaml +++ b/site/static/examples/admission-fs/lq-b-simple-job.yaml @@ -12,7 +12,7 @@ spec: spec: containers: - name: dummy-job - image: alpine/sleep # A very small and lightweight image + image: alpine command: ["/bin/sleep", "infinity"] resources: requests: From 322aa329c498b20cdfb4ad8ca574cf0c56fbeeb9 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Tue, 6 May 2025 15:40:39 +0000 Subject: [PATCH 09/19] Adjust integration test to the new shape of API --- .../singlecluster/scheduler/fairsharing/suite_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/singlecluster/scheduler/fairsharing/suite_test.go b/test/integration/singlecluster/scheduler/fairsharing/suite_test.go index 0b89670fb89..41cddccead3 100644 --- a/test/integration/singlecluster/scheduler/fairsharing/suite_test.go +++ b/test/integration/singlecluster/scheduler/fairsharing/suite_test.go @@ -86,7 +86,7 @@ func managerAndSchedulerSetup(ctx context.Context, mgr manager.Manager) { gomega.Expect(err).NotTo(gomega.HaveOccurred()) cCache := cache.New(mgr.GetClient(), cache.WithFairSharing(fairSharing.Enable)) - queues := queue.NewManager(mgr.GetClient(), cCache, queue.WithFairSharing(fairSharing), queue.WithAdmissionFairSharing(admissionFairSharing)) + queues := queue.NewManager(mgr.GetClient(), cCache, queue.WithAdmissionFairSharing(admissionFairSharing)) configuration := &config.Configuration{FairSharing: fairSharing, AdmissionFairSharing: admissionFairSharing} configuration.Metrics.EnableClusterQueueResources = true From 91c977556aef6d315cc0f72ff9fc01872efe4985 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Tue, 6 May 2025 15:42:19 +0000 Subject: [PATCH 10/19] Make FS status in LQ a pointer --- apis/kueue/v1beta1/localqueue_types.go | 2 +- pkg/controller/core/localqueue_controller.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/apis/kueue/v1beta1/localqueue_types.go b/apis/kueue/v1beta1/localqueue_types.go index e9ba4aa3986..eef050f942b 100644 --- a/apis/kueue/v1beta1/localqueue_types.go +++ b/apis/kueue/v1beta1/localqueue_types.go @@ -158,7 +158,7 @@ type LocalQueueStatus struct { // FairSharing contains the information about the current status of fair sharing. // +optional - FairSharingStatus FairSharingStatus `json:"fairSharing,omitempty"` + FairSharingStatus *FairSharingStatus `json:"fairSharing,omitempty"` } const ( diff --git a/pkg/controller/core/localqueue_controller.go b/pkg/controller/core/localqueue_controller.go index 1a4ee3470a2..c05de3973f5 100644 --- a/pkg/controller/core/localqueue_controller.go +++ b/pkg/controller/core/localqueue_controller.go @@ -263,6 +263,9 @@ func (r *LocalQueueReconciler) Update(e event.TypedUpdateEvent[*kueue.LocalQueue } func (r *LocalQueueReconciler) initializeAdmissionFsStatus(ctx context.Context, lq *kueue.LocalQueue) error { + if lq.Status.FairSharingStatus == nil { + lq.Status.FairSharingStatus = &kueue.FairSharingStatus{} + } if lq.Status.FairSharingStatus.AdmissionFairSharingStatus == nil { lq.Status.FairSharingStatus.AdmissionFairSharingStatus = &kueue.AdmissionFairSharingStatus{ LastUpdate: metav1.NewTime(r.clock.Now()), From d0835f3e295a2bdac7f8417d9814d2f184ef6d30 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Tue, 6 May 2025 15:45:59 +0000 Subject: [PATCH 11/19] Adjust wrapper --- pkg/util/testing/wrappers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/testing/wrappers.go b/pkg/util/testing/wrappers.go index 65fd3057ac8..b217dc283c9 100644 --- a/pkg/util/testing/wrappers.go +++ b/pkg/util/testing/wrappers.go @@ -707,7 +707,7 @@ func (q *LocalQueueWrapper) Active(status metav1.ConditionStatus) *LocalQueueWra // AdmittedWorkloads updates the admittedWorkloads in status. func (q *LocalQueueWrapper) FairSharingStatus(status *kueue.FairSharingStatus) *LocalQueueWrapper { - q.Status.FairSharingStatus = *status + q.Status.FairSharingStatus = status return q } From ebeb6da8d3ed635f5abe06c5bc9a190d7f8dc9f9 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Tue, 6 May 2025 15:54:26 +0000 Subject: [PATCH 12/19] Rename API field to be consitent with CQ --- apis/kueue/v1beta1/localqueue_types.go | 2 +- apis/kueue/v1beta1/zz_generated.deepcopy.go | 6 +++++- .../kueue/v1beta1/localqueuestatus.go | 10 +++++----- pkg/controller/core/localqueue_controller.go | 18 +++++++++--------- pkg/util/testing/wrappers.go | 2 +- pkg/workload/workload.go | 2 +- .../scheduler/fairsharing/fair_sharing_test.go | 8 ++++---- 7 files changed, 26 insertions(+), 22 deletions(-) diff --git a/apis/kueue/v1beta1/localqueue_types.go b/apis/kueue/v1beta1/localqueue_types.go index eef050f942b..388dbdb77aa 100644 --- a/apis/kueue/v1beta1/localqueue_types.go +++ b/apis/kueue/v1beta1/localqueue_types.go @@ -158,7 +158,7 @@ type LocalQueueStatus struct { // FairSharing contains the information about the current status of fair sharing. // +optional - FairSharingStatus *FairSharingStatus `json:"fairSharing,omitempty"` + FairSharing *FairSharingStatus `json:"fairSharing,omitempty"` } const ( diff --git a/apis/kueue/v1beta1/zz_generated.deepcopy.go b/apis/kueue/v1beta1/zz_generated.deepcopy.go index 868649bfc11..183ec9d0a2b 100644 --- a/apis/kueue/v1beta1/zz_generated.deepcopy.go +++ b/apis/kueue/v1beta1/zz_generated.deepcopy.go @@ -821,7 +821,11 @@ func (in *LocalQueueStatus) DeepCopyInto(out *LocalQueueStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - in.FairSharingStatus.DeepCopyInto(&out.FairSharingStatus) + if in.FairSharing != nil { + in, out := &in.FairSharing, &out.FairSharing + *out = new(FairSharingStatus) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LocalQueueStatus. diff --git a/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go b/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go index 04b2b146f7c..727702d827a 100644 --- a/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go +++ b/client-go/applyconfiguration/kueue/v1beta1/localqueuestatus.go @@ -31,7 +31,7 @@ type LocalQueueStatusApplyConfiguration struct { FlavorsReservation []LocalQueueFlavorUsageApplyConfiguration `json:"flavorsReservation,omitempty"` FlavorUsage []LocalQueueFlavorUsageApplyConfiguration `json:"flavorUsage,omitempty"` Flavors []LocalQueueFlavorStatusApplyConfiguration `json:"flavors,omitempty"` - FairSharingStatus *FairSharingStatusApplyConfiguration `json:"fairSharing,omitempty"` + FairSharing *FairSharingStatusApplyConfiguration `json:"fairSharing,omitempty"` } // LocalQueueStatusApplyConfiguration constructs a declarative configuration of the LocalQueueStatus type for use with @@ -116,10 +116,10 @@ func (b *LocalQueueStatusApplyConfiguration) WithFlavors(values ...*LocalQueueFl return b } -// WithFairSharingStatus sets the FairSharingStatus field in the declarative configuration to the given value +// WithFairSharing sets the FairSharing field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. -// If called multiple times, the FairSharingStatus field is set to the value of the last call. -func (b *LocalQueueStatusApplyConfiguration) WithFairSharingStatus(value *FairSharingStatusApplyConfiguration) *LocalQueueStatusApplyConfiguration { - b.FairSharingStatus = value +// If called multiple times, the FairSharing field is set to the value of the last call. +func (b *LocalQueueStatusApplyConfiguration) WithFairSharing(value *FairSharingStatusApplyConfiguration) *LocalQueueStatusApplyConfiguration { + b.FairSharing = value return b } diff --git a/pkg/controller/core/localqueue_controller.go b/pkg/controller/core/localqueue_controller.go index c05de3973f5..4772806b8d0 100644 --- a/pkg/controller/core/localqueue_controller.go +++ b/pkg/controller/core/localqueue_controller.go @@ -178,7 +178,7 @@ func (r *LocalQueueReconciler) Reconcile(ctx context.Context, req ctrl.Request) if err := r.initializeAdmissionFsStatus(ctx, &queueObj); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } - sinceLastUpdate := r.clock.Now().Sub(queueObj.Status.FairSharingStatus.AdmissionFairSharingStatus.LastUpdate.Time) + sinceLastUpdate := r.clock.Now().Sub(queueObj.Status.FairSharing.AdmissionFairSharingStatus.LastUpdate.Time) if interval := r.admissionFSConfig.UsageSamplingInterval.Duration; sinceLastUpdate < interval { return ctrl.Result{RequeueAfter: interval - sinceLastUpdate}, nil } @@ -263,11 +263,11 @@ func (r *LocalQueueReconciler) Update(e event.TypedUpdateEvent[*kueue.LocalQueue } func (r *LocalQueueReconciler) initializeAdmissionFsStatus(ctx context.Context, lq *kueue.LocalQueue) error { - if lq.Status.FairSharingStatus == nil { - lq.Status.FairSharingStatus = &kueue.FairSharingStatus{} + if lq.Status.FairSharing == nil { + lq.Status.FairSharing = &kueue.FairSharingStatus{} } - if lq.Status.FairSharingStatus.AdmissionFairSharingStatus == nil { - lq.Status.FairSharingStatus.AdmissionFairSharingStatus = &kueue.AdmissionFairSharingStatus{ + if lq.Status.FairSharing.AdmissionFairSharingStatus == nil { + lq.Status.FairSharing.AdmissionFairSharingStatus = &kueue.AdmissionFairSharingStatus{ LastUpdate: metav1.NewTime(r.clock.Now()), } return r.client.Status().Update(ctx, lq) @@ -287,9 +287,9 @@ func (r *LocalQueueReconciler) reconcileConsumedUsage(ctx context.Context, lq *k return err } // calculate alpha rate - oldUsage := lq.Status.FairSharingStatus.AdmissionFairSharingStatus.ConsumedResources + oldUsage := lq.Status.FairSharing.AdmissionFairSharingStatus.ConsumedResources newUsage := cacheLq.GetAdmittedUsage() - timeSinceLastUpdate := r.clock.Now().Sub(lq.Status.FairSharingStatus.AdmissionFairSharingStatus.LastUpdate.Time).Seconds() + timeSinceLastUpdate := r.clock.Now().Sub(lq.Status.FairSharing.AdmissionFairSharingStatus.LastUpdate.Time).Seconds() alpha := 1.0 - math.Pow(0.5, timeSinceLastUpdate/halfLifeTime) // calculate weighted average of old and new usage scaledNewUsage := resource.MulByFloat(newUsage, alpha) @@ -300,8 +300,8 @@ func (r *LocalQueueReconciler) reconcileConsumedUsage(ctx context.Context, lq *k } func (r *LocalQueueReconciler) updateAdmissionFsStatus(ctx context.Context, lq *kueue.LocalQueue, consumedResources corev1.ResourceList) error { - lq.Status.FairSharingStatus.AdmissionFairSharingStatus.ConsumedResources = consumedResources - lq.Status.FairSharingStatus.AdmissionFairSharingStatus.LastUpdate = metav1.NewTime(r.clock.Now()) + lq.Status.FairSharing.AdmissionFairSharingStatus.ConsumedResources = consumedResources + lq.Status.FairSharing.AdmissionFairSharingStatus.LastUpdate = metav1.NewTime(r.clock.Now()) return r.client.Status().Update(ctx, lq) } diff --git a/pkg/util/testing/wrappers.go b/pkg/util/testing/wrappers.go index b217dc283c9..62e60524e04 100644 --- a/pkg/util/testing/wrappers.go +++ b/pkg/util/testing/wrappers.go @@ -707,7 +707,7 @@ func (q *LocalQueueWrapper) Active(status metav1.ConditionStatus) *LocalQueueWra // AdmittedWorkloads updates the admittedWorkloads in status. func (q *LocalQueueWrapper) FairSharingStatus(status *kueue.FairSharingStatus) *LocalQueueWrapper { - q.Status.FairSharingStatus = status + q.Status.FairSharing = status return q } diff --git a/pkg/workload/workload.go b/pkg/workload/workload.go index dd7c441d38b..238146356f0 100644 --- a/pkg/workload/workload.go +++ b/pkg/workload/workload.go @@ -289,7 +289,7 @@ func (i *Info) LqUsage(ctx context.Context, c client.Client, resWeights map[core return 0, err } usage := 0.0 - for resName, resVal := range lq.Status.FairSharingStatus.AdmissionFairSharingStatus.ConsumedResources { + for resName, resVal := range lq.Status.FairSharing.AdmissionFairSharingStatus.ConsumedResources { weight, found := resWeights[resName] if !found { weight = 1 diff --git a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go index 4da71efa1d0..5060162e709 100644 --- a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go +++ b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go @@ -440,10 +440,10 @@ var _ = ginkgo.Describe("Scheduler", func() { ginkgo.By("Checking that LQ's resource usage is updated", func() { gomega.Eventually(func(g gomega.Gomega) { g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(lqA), lqA)).Should(gomega.Succeed()) - g.Expect(lqA.Status.FairSharingStatus).ShouldNot(gomega.BeNil()) - g.Expect(lqA.Status.FairSharingStatus.AdmissionFairSharingStatus).ShouldNot(gomega.BeNil()) - g.Expect(lqA.Status.FairSharingStatus.AdmissionFairSharingStatus.ConsumedResources).Should(gomega.HaveLen(1)) - g.Expect(lqA.Status.FairSharingStatus.AdmissionFairSharingStatus.ConsumedResources[corev1.ResourceCPU]). + g.Expect(lqA.Status.FairSharing).ShouldNot(gomega.BeNil()) + g.Expect(lqA.Status.FairSharing.AdmissionFairSharingStatus).ShouldNot(gomega.BeNil()) + g.Expect(lqA.Status.FairSharing.AdmissionFairSharingStatus.ConsumedResources).Should(gomega.HaveLen(1)) + g.Expect(lqA.Status.FairSharing.AdmissionFairSharingStatus.ConsumedResources[corev1.ResourceCPU]). To(gomega.Equal(resource.MustParse("32"))) }, util.Timeout, util.Interval).Should(gomega.Succeed()) }) From 37fd72dcd79fe5598d5917fba6b9c439a1a667d2 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Wed, 7 May 2025 08:54:43 +0000 Subject: [PATCH 13/19] Clean up comments, small logic fix --- apis/config/v1beta1/configuration_types.go | 2 +- pkg/controller/core/localqueue_controller.go | 5 ++--- site/content/en/docs/reference/kueue-config.v1beta1.md | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/apis/config/v1beta1/configuration_types.go b/apis/config/v1beta1/configuration_types.go index f3cf3da912d..ddaa26aeb3f 100644 --- a/apis/config/v1beta1/configuration_types.go +++ b/apis/config/v1beta1/configuration_types.go @@ -485,7 +485,7 @@ type AdmissionFairSharing struct { // Defaults to 5min. UsageSamplingInterval metav1.Duration `json:"usageSamplingInterval,omitempty"` - // resourceWeights assigns weights to resources which then are used to calculate LocalQueue/ClusterQueue/Cohort's + // resourceWeights assigns weights to resources which then are used to calculate LocalQueue's // resource usage and order Workloads. // Defaults to 1. ResourceWeights map[corev1.ResourceName]float64 `json:"resourceWeights,omitempty"` diff --git a/pkg/controller/core/localqueue_controller.go b/pkg/controller/core/localqueue_controller.go index 4772806b8d0..861ce230e52 100644 --- a/pkg/controller/core/localqueue_controller.go +++ b/pkg/controller/core/localqueue_controller.go @@ -169,9 +169,8 @@ func (r *LocalQueueReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, client.IgnoreNotFound(err) } } else { - if err := r.UpdateStatusIfChanged(ctx, &queueObj, metav1.ConditionFalse, clusterQueueIsInactiveReason, clusterQueueIsInactiveMsg); err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) - } + err := r.UpdateStatusIfChanged(ctx, &queueObj, metav1.ConditionFalse, clusterQueueIsInactiveReason, clusterQueueIsInactiveMsg) + return ctrl.Result{}, client.IgnoreNotFound(err) } if r.admissionFSConfig != nil { diff --git a/site/content/en/docs/reference/kueue-config.v1beta1.md b/site/content/en/docs/reference/kueue-config.v1beta1.md index 5766f0bfd9d..9cdedb9bba2 100644 --- a/site/content/en/docs/reference/kueue-config.v1beta1.md +++ b/site/content/en/docs/reference/kueue-config.v1beta1.md @@ -48,7 +48,7 @@ Defaults to 5min.

    map[ResourceName]float64 -

    resourceWeights assigns weights to resources which then are used to calculate LocalQueue/ClusterQueue/Cohort's +

    resourceWeights assigns weights to resources which then are used to calculate LocalQueue's resource usage and order Workloads. Defaults to 1.

    From f6ac3372b4bdc342d60b68a49ae6500bb081ec8e Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Wed, 7 May 2025 10:29:29 +0000 Subject: [PATCH 14/19] Add feature gate, reduce number of requeste in lq controller --- pkg/controller/core/localqueue_controller.go | 14 ++++++-------- pkg/controller/core/localqueue_controller_test.go | 2 ++ pkg/features/kube_features.go | 9 +++++++++ pkg/queue/manager.go | 4 +++- .../scheduler/fairsharing/fair_sharing_test.go | 2 ++ 5 files changed, 22 insertions(+), 9 deletions(-) diff --git a/pkg/controller/core/localqueue_controller.go b/pkg/controller/core/localqueue_controller.go index 861ce230e52..9ab80f58105 100644 --- a/pkg/controller/core/localqueue_controller.go +++ b/pkg/controller/core/localqueue_controller.go @@ -173,12 +173,10 @@ func (r *LocalQueueReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, client.IgnoreNotFound(err) } - if r.admissionFSConfig != nil { - if err := r.initializeAdmissionFsStatus(ctx, &queueObj); err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) - } + if r.admissionFSConfig != nil && features.Enabled(features.AdmissionFairSharing) { + updated := r.initializeAdmissionFsStatus(ctx, &queueObj) sinceLastUpdate := r.clock.Now().Sub(queueObj.Status.FairSharing.AdmissionFairSharingStatus.LastUpdate.Time) - if interval := r.admissionFSConfig.UsageSamplingInterval.Duration; sinceLastUpdate < interval { + if interval := r.admissionFSConfig.UsageSamplingInterval.Duration; !updated && sinceLastUpdate < interval { return ctrl.Result{RequeueAfter: interval - sinceLastUpdate}, nil } if err := r.reconcileConsumedUsage(ctx, &queueObj, queueObj.Spec.ClusterQueue); err != nil { @@ -261,7 +259,7 @@ func (r *LocalQueueReconciler) Update(e event.TypedUpdateEvent[*kueue.LocalQueue return true } -func (r *LocalQueueReconciler) initializeAdmissionFsStatus(ctx context.Context, lq *kueue.LocalQueue) error { +func (r *LocalQueueReconciler) initializeAdmissionFsStatus(ctx context.Context, lq *kueue.LocalQueue) bool { if lq.Status.FairSharing == nil { lq.Status.FairSharing = &kueue.FairSharingStatus{} } @@ -269,9 +267,9 @@ func (r *LocalQueueReconciler) initializeAdmissionFsStatus(ctx context.Context, lq.Status.FairSharing.AdmissionFairSharingStatus = &kueue.AdmissionFairSharingStatus{ LastUpdate: metav1.NewTime(r.clock.Now()), } - return r.client.Status().Update(ctx, lq) + return true } - return nil + return false } func (r *LocalQueueReconciler) reconcileConsumedUsage(ctx context.Context, lq *kueue.LocalQueue, cqName kueue.ClusterQueueReference) error { diff --git a/pkg/controller/core/localqueue_controller_test.go b/pkg/controller/core/localqueue_controller_test.go index 94d6901f14e..67c2ae21844 100644 --- a/pkg/controller/core/localqueue_controller_test.go +++ b/pkg/controller/core/localqueue_controller_test.go @@ -36,6 +36,7 @@ import ( config "sigs.k8s.io/kueue/apis/config/v1beta1" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" "sigs.k8s.io/kueue/pkg/cache" + "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/queue" utiltesting "sigs.k8s.io/kueue/pkg/util/testing" "sigs.k8s.io/kueue/test/util" @@ -504,6 +505,7 @@ func TestLocalQueueReconcile(t *testing.T) { tc.clusterQueue, tc.localQueue, } + features.SetFeatureGateDuringTest(t, features.AdmissionFairSharing, true) cl := utiltesting.NewClientBuilder(). WithObjects(objs...). WithStatusSubresource(objs...). diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 2c9ca6c1b65..622ce26db36 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -157,6 +157,12 @@ const ( // // Enable hierarchical cohorts HierarchicalCohorts featuregate.Feature = "HierarchicalCohorts" + + // owner: @pbundyra + // kep: https://github.com/kubernetes-sigs/kueue/tree/main/keps/4136-admission-fair-sharing + // + // Enable admission fair sharing + AdmissionFairSharing featuregate.Feature = "AdmissionFairSharing" ) func init() { @@ -243,6 +249,9 @@ var defaultVersionedFeatureGates = map[featuregate.Feature]featuregate.Versioned HierarchicalCohorts: { {Version: version.MustParse("0.11"), Default: true, PreRelease: featuregate.Beta}, }, + AdmissionFairSharing: { + {Version: version.MustParse("0.12"), Default: false, PreRelease: featuregate.Alpha}, + }, } func SetFeatureGateDuringTest(tb testing.TB, f featuregate.Feature, value bool) { diff --git a/pkg/queue/manager.go b/pkg/queue/manager.go index 452b92fc0da..43afbb37160 100644 --- a/pkg/queue/manager.go +++ b/pkg/queue/manager.go @@ -60,7 +60,9 @@ var defaultOptions = options{ func WithAdmissionFairSharing(cfg *config.AdmissionFairSharing) Option { return func(o *options) { - o.admissionFairSharing = cfg + if features.Enabled(features.AdmissionFairSharing) { + o.admissionFairSharing = cfg + } } } diff --git a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go index 5060162e709..d447d8e9660 100644 --- a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go +++ b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go @@ -28,6 +28,7 @@ import ( kueuealpha "sigs.k8s.io/kueue/apis/kueue/v1alpha1" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" + "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/metrics" "sigs.k8s.io/kueue/pkg/util/testing" "sigs.k8s.io/kueue/test/integration/framework" @@ -403,6 +404,7 @@ var _ = ginkgo.Describe("Scheduler", func() { ) ginkgo.BeforeEach(func() { + features.SetFeatureGateDuringTest(ginkgo.GinkgoTB(), features.AdmissionFairSharing, true) cq = testing.MakeClusterQueue("cq"). ResourceGroup(*testing.MakeFlavorQuotas(defaultFlavor.Name).Resource(corev1.ResourceCPU, "32").Obj()). Preemption(kueue.ClusterQueuePreemption{WithinClusterQueue: kueue.PreemptionPolicyNever}). From 8a87692952d3abbb0969c65f6aa296c80743835b Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Wed, 7 May 2025 11:43:05 +0000 Subject: [PATCH 15/19] Switch on feature gate before creating manager --- .../singlecluster/scheduler/fairsharing/fair_sharing_test.go | 2 -- .../singlecluster/scheduler/fairsharing/suite_test.go | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go index d447d8e9660..5060162e709 100644 --- a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go +++ b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go @@ -28,7 +28,6 @@ import ( kueuealpha "sigs.k8s.io/kueue/apis/kueue/v1alpha1" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" - "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/metrics" "sigs.k8s.io/kueue/pkg/util/testing" "sigs.k8s.io/kueue/test/integration/framework" @@ -404,7 +403,6 @@ var _ = ginkgo.Describe("Scheduler", func() { ) ginkgo.BeforeEach(func() { - features.SetFeatureGateDuringTest(ginkgo.GinkgoTB(), features.AdmissionFairSharing, true) cq = testing.MakeClusterQueue("cq"). ResourceGroup(*testing.MakeFlavorQuotas(defaultFlavor.Name).Resource(corev1.ResourceCPU, "32").Obj()). Preemption(kueue.ClusterQueuePreemption{WithinClusterQueue: kueue.PreemptionPolicyNever}). diff --git a/test/integration/singlecluster/scheduler/fairsharing/suite_test.go b/test/integration/singlecluster/scheduler/fairsharing/suite_test.go index 41cddccead3..b6514b283f6 100644 --- a/test/integration/singlecluster/scheduler/fairsharing/suite_test.go +++ b/test/integration/singlecluster/scheduler/fairsharing/suite_test.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/kueue/pkg/controller/core" "sigs.k8s.io/kueue/pkg/controller/core/indexer" workloadjob "sigs.k8s.io/kueue/pkg/controller/jobs/job" + "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/queue" "sigs.k8s.io/kueue/pkg/scheduler" "sigs.k8s.io/kueue/pkg/webhooks" @@ -85,6 +86,7 @@ func managerAndSchedulerSetup(ctx context.Context, mgr manager.Manager) { err := indexer.Setup(ctx, mgr.GetFieldIndexer()) gomega.Expect(err).NotTo(gomega.HaveOccurred()) + _ = features.SetEnable(features.AdmissionFairSharing, true) cCache := cache.New(mgr.GetClient(), cache.WithFairSharing(fairSharing.Enable)) queues := queue.NewManager(mgr.GetClient(), cCache, queue.WithAdmissionFairSharing(admissionFairSharing)) From 51f3507af92b0d85f4e91e045c0ceeb72e1023d4 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Wed, 7 May 2025 12:03:41 +0000 Subject: [PATCH 16/19] Align examples with the latest naming changes --- .../examples/admission-fs/admission-fair-sharing-setup.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/static/examples/admission-fs/admission-fair-sharing-setup.yaml b/site/static/examples/admission-fs/admission-fair-sharing-setup.yaml index bbf6b732728..1a47838e44f 100644 --- a/site/static/examples/admission-fs/admission-fair-sharing-setup.yaml +++ b/site/static/examples/admission-fs/admission-fair-sharing-setup.yaml @@ -11,7 +11,7 @@ spec: namespaceSelector: {} # match all. queueingStrategy: "StrictFIFO" admissionScope: - admissionMode: "UsageBasedFairSharing" + admissionMode: "UsageBasedAdmissionFairSharing" resourceGroups: - coveredResources: ["cpu"] flavors: From 72aa2c054d19e602ffa8c769d96e076fd0718eec Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Wed, 7 May 2025 14:40:51 +0000 Subject: [PATCH 17/19] Ensure test is not flaky, small refactor --- pkg/queue/cluster_queue.go | 6 ++++-- .../scheduler/fairsharing/fair_sharing_test.go | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/queue/cluster_queue.go b/pkg/queue/cluster_queue.go index c14fabf2fc6..a3b75917501 100644 --- a/pkg/queue/cluster_queue.go +++ b/pkg/queue/cluster_queue.go @@ -92,7 +92,7 @@ func workloadKey(i *workload.Info) string { } func newClusterQueue(ctx context.Context, client client.Client, cq *kueue.ClusterQueue, wo workload.Ordering, fsConfig *config.AdmissionFairSharing) (*ClusterQueue, error) { - enableAdmissionFs, fsResWeights := isAdmissionFsEnabled(cq, fsConfig) + enableAdmissionFs, fsResWeights := afsResourceWeights(cq, fsConfig) cqImpl := newClusterQueueImpl(ctx, client, wo, realClock, fsResWeights, enableAdmissionFs) err := cqImpl.Update(cq) if err != nil { @@ -101,7 +101,7 @@ func newClusterQueue(ctx context.Context, client client.Client, cq *kueue.Cluste return cqImpl, nil } -func isAdmissionFsEnabled(cq *kueue.ClusterQueue, fsConfig *config.AdmissionFairSharing) (bool, map[corev1.ResourceName]float64) { +func afsResourceWeights(cq *kueue.ClusterQueue, fsConfig *config.AdmissionFairSharing) (bool, map[corev1.ResourceName]float64) { enableAdmissionFs, fsResWeights := false, make(map[corev1.ResourceName]float64) if fsConfig != nil && cq.Spec.AdmissionScope != nil && cq.Spec.AdmissionScope.AdmissionMode == kueue.UsageBasedAdmissionFairSharing { enableAdmissionFs = true @@ -184,6 +184,8 @@ func (c *ClusterQueue) PushOrUpdate(wInfo *workload.Info) { } func (c *ClusterQueue) Heapify(lqName string) { + c.rwm.Lock() + defer c.rwm.Unlock() for _, wl := range c.heap.List() { if string(wl.Obj.Spec.QueueName) == lqName { c.heap.PushOrUpdate(wl) diff --git a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go index 5060162e709..711836d4d87 100644 --- a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go +++ b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go @@ -443,8 +443,8 @@ var _ = ginkgo.Describe("Scheduler", func() { g.Expect(lqA.Status.FairSharing).ShouldNot(gomega.BeNil()) g.Expect(lqA.Status.FairSharing.AdmissionFairSharingStatus).ShouldNot(gomega.BeNil()) g.Expect(lqA.Status.FairSharing.AdmissionFairSharingStatus.ConsumedResources).Should(gomega.HaveLen(1)) - g.Expect(lqA.Status.FairSharing.AdmissionFairSharingStatus.ConsumedResources[corev1.ResourceCPU]). - To(gomega.Equal(resource.MustParse("32"))) + usage := lqA.Status.FairSharing.AdmissionFairSharingStatus.ConsumedResources[corev1.ResourceCPU] + g.Expect(usage.MilliValue()).To(gomega.BeNumerically(">=", 0)) }, util.Timeout, util.Interval).Should(gomega.Succeed()) }) From ea99cd2ad862aea1c1546e7f29c294fd9257c814 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Wed, 7 May 2025 16:58:12 +0000 Subject: [PATCH 18/19] Small refactor, fix naming, improve logging, add another feature gate check --- pkg/controller/core/localqueue_controller.go | 4 +--- pkg/queue/cluster_queue.go | 15 ++++++++------- .../scheduler/fairsharing/fair_sharing_test.go | 9 ++------- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/pkg/controller/core/localqueue_controller.go b/pkg/controller/core/localqueue_controller.go index 9ab80f58105..6b68815aee5 100644 --- a/pkg/controller/core/localqueue_controller.go +++ b/pkg/controller/core/localqueue_controller.go @@ -73,9 +73,7 @@ type LocalQueueReconcilerOption func(*LocalQueueReconcilerOptions) func WithAdmissionFairSharingConfig(cfg *config.AdmissionFairSharing) LocalQueueReconcilerOption { return func(o *LocalQueueReconcilerOptions) { - if cfg != nil { - o.admissionFSConfig = cfg - } + o.admissionFSConfig = cfg } } diff --git a/pkg/queue/cluster_queue.go b/pkg/queue/cluster_queue.go index a3b75917501..4f203bbda16 100644 --- a/pkg/queue/cluster_queue.go +++ b/pkg/queue/cluster_queue.go @@ -33,6 +33,7 @@ import ( config "sigs.k8s.io/kueue/apis/config/v1beta1" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" + "sigs.k8s.io/kueue/pkg/features" "sigs.k8s.io/kueue/pkg/hierarchy" "sigs.k8s.io/kueue/pkg/util/heap" utilpriority "sigs.k8s.io/kueue/pkg/util/priority" @@ -91,8 +92,8 @@ func workloadKey(i *workload.Info) string { return workload.Key(i.Obj) } -func newClusterQueue(ctx context.Context, client client.Client, cq *kueue.ClusterQueue, wo workload.Ordering, fsConfig *config.AdmissionFairSharing) (*ClusterQueue, error) { - enableAdmissionFs, fsResWeights := afsResourceWeights(cq, fsConfig) +func newClusterQueue(ctx context.Context, client client.Client, cq *kueue.ClusterQueue, wo workload.Ordering, afsConfig *config.AdmissionFairSharing) (*ClusterQueue, error) { + enableAdmissionFs, fsResWeights := afsResourceWeights(cq, afsConfig) cqImpl := newClusterQueueImpl(ctx, client, wo, realClock, fsResWeights, enableAdmissionFs) err := cqImpl.Update(cq) if err != nil { @@ -101,11 +102,11 @@ func newClusterQueue(ctx context.Context, client client.Client, cq *kueue.Cluste return cqImpl, nil } -func afsResourceWeights(cq *kueue.ClusterQueue, fsConfig *config.AdmissionFairSharing) (bool, map[corev1.ResourceName]float64) { +func afsResourceWeights(cq *kueue.ClusterQueue, afsConfig *config.AdmissionFairSharing) (bool, map[corev1.ResourceName]float64) { enableAdmissionFs, fsResWeights := false, make(map[corev1.ResourceName]float64) - if fsConfig != nil && cq.Spec.AdmissionScope != nil && cq.Spec.AdmissionScope.AdmissionMode == kueue.UsageBasedAdmissionFairSharing { + if afsConfig != nil && cq.Spec.AdmissionScope != nil && cq.Spec.AdmissionScope.AdmissionMode == kueue.UsageBasedAdmissionFairSharing && features.Enabled(features.AdmissionFairSharing) { enableAdmissionFs = true - fsResWeights = fsConfig.ResourceWeights + fsResWeights = afsConfig.ResourceWeights } return enableAdmissionFs, fsResWeights } @@ -441,9 +442,9 @@ func queueOrderingFunc(ctx context.Context, c client.Client, wo workload.Orderin lqBUsage, errB := b.LqUsage(ctx, c, fsResWeights) switch { case errA != nil: - log.V(3).Error(errA, "Error fetching LocalQueue from informer") + log.V(2).Error(errA, "Error determining LocalQueue usage") case errB != nil: - log.V(3).Error(errB, "Error fetching LocalQueue from informer") + log.V(2).Error(errB, "Error determining LocalQueue usage") default: log.V(3).Info("Resource usage from LocalQueue", "LocalQueue", a.Obj.Spec.QueueName, "Usage", lqAUsage) log.V(3).Info("Resource usage from LocalQueue", "LocalQueue", b.Obj.Spec.QueueName, "Usage", lqBUsage) diff --git a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go index 711836d4d87..def3927354b 100644 --- a/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go +++ b/test/integration/singlecluster/scheduler/fairsharing/fair_sharing_test.go @@ -452,13 +452,8 @@ var _ = ginkgo.Describe("Scheduler", func() { wlA := createWorkload("lq-a", "32") wlB := createWorkload("lq-b", "32") - ginkgo.By("Finish the previous workload", func() { - gomega.Consistently(func(g gomega.Gomega) { - util.FinishWorkloads(ctx, k8sClient, wl) - updatedCQ := &kueue.ClusterQueue{} - g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cq), updatedCQ)).To(gomega.Succeed()) - }, util.Timeout, util.Interval).Should(gomega.Succeed()) - }) + ginkgo.By("Finish the previous workload") + util.FinishWorkloads(ctx, k8sClient, wl) ginkgo.By("Admitting the workload from LQ-b, which has lower recent usage") util.ExpectWorkloadsToBeAdmitted(ctx, k8sClient, wlB) From 118ac5c9d83014c230f9af8579c443555522447d Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Wed, 7 May 2025 17:24:57 +0000 Subject: [PATCH 19/19] Fix feature gates in unit tests --- pkg/queue/cluster_queue.go | 1 - pkg/queue/cluster_queue_test.go | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/queue/cluster_queue.go b/pkg/queue/cluster_queue.go index 4f203bbda16..5ac08be6fd7 100644 --- a/pkg/queue/cluster_queue.go +++ b/pkg/queue/cluster_queue.go @@ -435,7 +435,6 @@ func (c *ClusterQueue) RequeueIfNotPresent(wInfo *workload.Info, reason RequeueR // time. func queueOrderingFunc(ctx context.Context, c client.Client, wo workload.Ordering, fsResWeights map[corev1.ResourceName]float64, enableAdmissionFs bool) func(a, b *workload.Info) bool { log := ctrl.LoggerFrom(ctx) - return func(a, b *workload.Info) bool { if enableAdmissionFs { lqAUsage, errA := a.LqUsage(ctx, c, fsResWeights) diff --git a/pkg/queue/cluster_queue_test.go b/pkg/queue/cluster_queue_test.go index 2d306a9a7d9..3f68625bb15 100644 --- a/pkg/queue/cluster_queue_test.go +++ b/pkg/queue/cluster_queue_test.go @@ -32,6 +32,7 @@ import ( config "sigs.k8s.io/kueue/apis/config/v1beta1" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" + "sigs.k8s.io/kueue/pkg/features" utiltesting "sigs.k8s.io/kueue/pkg/util/testing" "sigs.k8s.io/kueue/pkg/workload" ) @@ -1104,6 +1105,7 @@ func TestFsAdmission(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { + features.SetFeatureGateDuringTest(t, features.AdmissionFairSharing, true) builder := utiltesting.NewClientBuilder() for _, lq := range tc.lqs { builder = builder.WithObjects(&lq)