Skip to content

Commit

Permalink
Add CEL rules to ClusterQueue (kubernetes-sigs#1972)
Browse files Browse the repository at this point in the history 
* Add CEL rules to ClusterQueue

* Set default values with CRD validations

* Update message for validation rule

* Remove unecessary checks from crd validations

* Remove test case
  • Loading branch information
@IrvingMg @vsoch
IrvingMg authored and vsoch committed Apr 18, 2024
1 parent d1d424e commit 93933eb
Show file tree
Hide file tree
Showing 10 changed files with 349 additions and 159 deletions.
10 changes: 10 additions & 0 deletions apis/kueue/v1beta1/clusterqueue_types.go
View file
Expand Up @@ -23,6 +23,7 @@ import (
)

// ClusterQueueSpec defines the desired state of ClusterQueue
// +kubebuilder:validation:XValidation:rule="!has(self.cohort) && has(self.resourceGroups) ? self.resourceGroups.all(rg, rg.flavors.all(f, f.resources.all(r, !has(r.borrowingLimit)))) : true", message="borrowingLimit must be nil when cohort is empty"
type ClusterQueueSpec struct {
// resourceGroups describes groups of resources.
// Each resource group defines the list of resources and a list of flavors
Expand All @@ -48,6 +49,8 @@ type ClusterQueueSpec struct {
//
// Validation of a cohort name is equivalent to that of object names:
// subdomain in DNS (RFC 1123).
// +kubebuilder:validation:MaxLength=253
// +kubebuilder:validation:Pattern="^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"
Cohort string `json:"cohort,omitempty"`

// QueueingStrategy indicates the queueing strategy of the workloads
Expand All @@ -74,6 +77,7 @@ type ClusterQueueSpec struct {

// flavorFungibility defines whether a workload should try the next flavor
// before borrowing or preempting in the flavor being evaluated.
// +kubebuilder:default={}
FlavorFungibility *FlavorFungibility `json:"flavorFungibility,omitempty"`

// preemption describes policies to preempt Workloads from this ClusterQueue
Expand All @@ -91,6 +95,7 @@ type ClusterQueueSpec struct {
// The preemption algorithm tries to find a minimal set of Workloads to
// preempt to accomomdate the pending Workload, preempting Workloads with
// lower priority first.
// +kubebuilder:default={}
Preemption *ClusterQueuePreemption `json:"preemption,omitempty"`

// admissionChecks lists the AdmissionChecks required by this ClusterQueue
Expand Down Expand Up @@ -134,6 +139,7 @@ const (
Hold StopPolicy = "Hold"
)

// +kubebuilder:validation:XValidation:rule="self.flavors.all(x, size(x.resources) == size(self.coveredResources))", message="flavors must have the same number of resources as the coveredResources"
type ResourceGroup struct {
// coveredResources is the list of resources covered by the flavors in this
// group.
Expand Down Expand Up @@ -217,6 +223,8 @@ type ResourceQuota struct {
}

// ResourceFlavorReference is the name of the ResourceFlavor.
// +kubebuilder:validation:MaxLength=253
// +kubebuilder:validation:Pattern="^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"
type ResourceFlavorReference string

// ClusterQueueStatus defines the observed state of ClusterQueue
Expand Down Expand Up @@ -362,6 +370,7 @@ type FlavorFungibility struct {

// ClusterQueuePreemption contains policies to preempt Workloads from this
// ClusterQueue or the ClusterQueue's cohort.
// +kubebuilder:validation:XValidation:rule="!(self.reclaimWithinCohort == 'Never' && has(self.borrowWithinCohort) && self.borrowWithinCohort.policy != 'Never')", message="reclaimWithinCohort=Never and borrowWithinCohort.Policy!=Never"
type ClusterQueuePreemption struct {
// reclaimWithinCohort determines whether a pending Workload can preempt
// Workloads from other ClusterQueues in the cohort that are using more than
Expand All @@ -381,6 +390,7 @@ type ClusterQueuePreemption struct {

// borrowWithinCohort provides configuration to allow preemption within
// cohort while borrowing.
// +kubebuilder:default={}
BorrowWithinCohort *BorrowWithinCohort `json:"borrowWithinCohort,omitempty"`

// withinClusterQueue determines whether a pending Workload that doesn't fit
Expand Down
23 changes: 23 additions & 0 deletions charts/kueue/templates/crd/kueue.x-k8s.io_clusterqueues.yaml
View file
Expand Up @@ -99,8 +99,11 @@ spec:
Validation of a cohort name is equivalent to that of object names:
subdomain in DNS (RFC 1123).
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
flavorFungibility:
default: {}
description: |-
flavorFungibility defines whether a workload should try the next flavor
before borrowing or preempting in the flavor being evaluated.
Expand Down Expand Up @@ -185,6 +188,7 @@ spec:
type: object
x-kubernetes-map-type: atomic
preemption:
default: {}
description: |-
preemption describes policies to preempt Workloads from this ClusterQueue
or the ClusterQueue's cohort.
Expand All @@ -206,6 +210,7 @@ spec:
lower priority first.
properties:
borrowWithinCohort:
default: {}
description: |-
borrowWithinCohort provides configuration to allow preemption within
cohort while borrowing.
Expand Down Expand Up @@ -274,6 +279,10 @@ spec:
- LowerOrNewerEqualPriority
type: string
type: object
x-kubernetes-validations:
- message: reclaimWithinCohort=Never and borrowWithinCohort.Policy!=Never
rule: '!(self.reclaimWithinCohort == ''Never'' && has(self.borrowWithinCohort)
&& self.borrowWithinCohort.policy != ''Never'')'
queueingStrategy:
default: BestEffortFIFO
description: |-
Expand Down Expand Up @@ -330,6 +339,8 @@ spec:
name of this flavor. The name should match the .metadata.name of a
ResourceFlavor. If a matching ResourceFlavor does not exist, the
ClusterQueue will have an Active condition set to False.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
resources:
description: |-
Expand Down Expand Up @@ -417,6 +428,10 @@ spec:
- coveredResources
- flavors
type: object
x-kubernetes-validations:
- message: flavors must have the same number of resources as the
coveredResources
rule: self.flavors.all(x, size(x.resources) == size(self.coveredResources))
maxItems: 16
type: array
x-kubernetes-list-type: atomic
Expand All @@ -439,6 +454,10 @@ spec:
- HoldAndDrain
type: string
type: object
x-kubernetes-validations:
- message: borrowingLimit must be nil when cohort is empty
rule: '!has(self.cohort) && has(self.resourceGroups) ? self.resourceGroups.all(rg,
rg.flavors.all(f, f.resources.all(r, !has(r.borrowingLimit)))) : true'
status:
description: ClusterQueueStatus defines the observed state of ClusterQueue
properties:
Expand Down Expand Up @@ -531,6 +550,8 @@ spec:
properties:
name:
description: name of the flavor.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
resources:
description: resources lists the quota usage for the resources
Expand Down Expand Up @@ -583,6 +604,8 @@ spec:
properties:
name:
description: name of the flavor.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
resources:
description: resources lists the quota usage for the resources
Expand Down
4 changes: 4 additions & 0 deletions charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml
View file
Expand Up @@ -172,6 +172,8 @@ spec:
properties:
name:
description: name of the flavor.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
resources:
description: resources lists the quota usage for the resources
Expand Down Expand Up @@ -213,6 +215,8 @@ spec:
properties:
name:
description: name of the flavor.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
resources:
description: resources lists the quota usage for the resources
Expand Down
2 changes: 2 additions & 0 deletions charts/kueue/templates/crd/kueue.x-k8s.io_workloads.yaml
View file
Expand Up @@ -7843,6 +7843,8 @@ spec:
additionalProperties:
description: ResourceFlavorReference is the name of the
ResourceFlavor.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
description: Flavors are the flavors assigned to the workload
for each resource.
Expand Down
23 changes: 23 additions & 0 deletions config/components/crd/bases/kueue.x-k8s.io_clusterqueues.yaml
View file
Expand Up @@ -84,8 +84,11 @@ spec:
Validation of a cohort name is equivalent to that of object names:
subdomain in DNS (RFC 1123).
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
flavorFungibility:
default: {}
description: |-
flavorFungibility defines whether a workload should try the next flavor
before borrowing or preempting in the flavor being evaluated.
Expand Down Expand Up @@ -170,6 +173,7 @@ spec:
type: object
x-kubernetes-map-type: atomic
preemption:
default: {}
description: |-
preemption describes policies to preempt Workloads from this ClusterQueue
or the ClusterQueue's cohort.
Expand All @@ -191,6 +195,7 @@ spec:
lower priority first.
properties:
borrowWithinCohort:
default: {}
description: |-
borrowWithinCohort provides configuration to allow preemption within
cohort while borrowing.
Expand Down Expand Up @@ -259,6 +264,10 @@ spec:
- LowerOrNewerEqualPriority
type: string
type: object
x-kubernetes-validations:
- message: reclaimWithinCohort=Never and borrowWithinCohort.Policy!=Never
rule: '!(self.reclaimWithinCohort == ''Never'' && has(self.borrowWithinCohort)
&& self.borrowWithinCohort.policy != ''Never'')'
queueingStrategy:
default: BestEffortFIFO
description: |-
Expand Down Expand Up @@ -315,6 +324,8 @@ spec:
name of this flavor. The name should match the .metadata.name of a
ResourceFlavor. If a matching ResourceFlavor does not exist, the
ClusterQueue will have an Active condition set to False.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
resources:
description: |-
Expand Down Expand Up @@ -402,6 +413,10 @@ spec:
- coveredResources
- flavors
type: object
x-kubernetes-validations:
- message: flavors must have the same number of resources as the
coveredResources
rule: self.flavors.all(x, size(x.resources) == size(self.coveredResources))
maxItems: 16
type: array
x-kubernetes-list-type: atomic
Expand All @@ -424,6 +439,10 @@ spec:
- HoldAndDrain
type: string
type: object
x-kubernetes-validations:
- message: borrowingLimit must be nil when cohort is empty
rule: '!has(self.cohort) && has(self.resourceGroups) ? self.resourceGroups.all(rg,
rg.flavors.all(f, f.resources.all(r, !has(r.borrowingLimit)))) : true'
status:
description: ClusterQueueStatus defines the observed state of ClusterQueue
properties:
Expand Down Expand Up @@ -516,6 +535,8 @@ spec:
properties:
name:
description: name of the flavor.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
resources:
description: resources lists the quota usage for the resources
Expand Down Expand Up @@ -568,6 +589,8 @@ spec:
properties:
name:
description: name of the flavor.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
resources:
description: resources lists the quota usage for the resources
Expand Down
4 changes: 4 additions & 0 deletions config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml
View file
Expand Up @@ -157,6 +157,8 @@ spec:
properties:
name:
description: name of the flavor.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
resources:
description: resources lists the quota usage for the resources
Expand Down Expand Up @@ -198,6 +200,8 @@ spec:
properties:
name:
description: name of the flavor.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
resources:
description: resources lists the quota usage for the resources
Expand Down
2 changes: 2 additions & 0 deletions config/components/crd/bases/kueue.x-k8s.io_workloads.yaml
View file
Expand Up @@ -7828,6 +7828,8 @@ spec:
additionalProperties:
description: ResourceFlavorReference is the name of the
ResourceFlavor.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
description: Flavors are the flavors assigned to the workload
for each resource.
Expand Down
39 changes: 1 addition & 38 deletions pkg/webhooks/clusterqueue_webhook.go
View file
Expand Up @@ -63,23 +63,6 @@ func (w *ClusterQueueWebhook) Default(ctx context.Context, obj runtime.Object) e
if !controllerutil.ContainsFinalizer(cq, kueue.ResourceInUseFinalizerName) {
controllerutil.AddFinalizer(cq, kueue.ResourceInUseFinalizerName)
}
if cq.Spec.Preemption == nil {
cq.Spec.Preemption = &kueue.ClusterQueuePreemption{
WithinClusterQueue: kueue.PreemptionPolicyNever,
ReclaimWithinCohort: kueue.PreemptionPolicyNever,
}
}
if cq.Spec.Preemption.BorrowWithinCohort == nil {
cq.Spec.Preemption.BorrowWithinCohort = &kueue.BorrowWithinCohort{
Policy: kueue.BorrowWithinCohortPolicyNever,
}
}
if cq.Spec.FlavorFungibility == nil {
cq.Spec.FlavorFungibility = &kueue.FlavorFungibility{
WhenCanBorrow: kueue.Borrow,
WhenCanPreempt: kueue.TryNextFlavor,
}
}
return nil
}

Expand Down Expand Up @@ -116,15 +99,9 @@ func ValidateClusterQueue(cq *kueue.ClusterQueue) field.ErrorList {
path := field.NewPath("spec")

var allErrs field.ErrorList
if len(cq.Spec.Cohort) != 0 {
allErrs = append(allErrs, validateNameReference(cq.Spec.Cohort, path.Child("cohort"))...)
}
allErrs = append(allErrs, validateResourceGroups(cq.Spec.ResourceGroups, cq.Spec.Cohort, path.Child("resourceGroups"))...)
allErrs = append(allErrs,
validation.ValidateLabelSelector(cq.Spec.NamespaceSelector, validation.LabelSelectorValidationOptions{}, path.Child("namespaceSelector"))...)
if cq.Spec.Preemption != nil {
allErrs = append(allErrs, validatePreemption(cq.Spec.Preemption, path.Child("preemption"))...)
}
return allErrs
}

Expand All @@ -134,16 +111,6 @@ func ValidateClusterQueueUpdate(newObj, oldObj *kueue.ClusterQueue) field.ErrorL
return allErrs
}

func validatePreemption(preemption *kueue.ClusterQueuePreemption, path *field.Path) field.ErrorList {
var allErrs field.ErrorList
if preemption.ReclaimWithinCohort == kueue.PreemptionPolicyNever &&
preemption.BorrowWithinCohort != nil &&
preemption.BorrowWithinCohort.Policy != kueue.BorrowWithinCohortPolicyNever {
allErrs = append(allErrs, field.Invalid(path, preemption, "reclaimWithinCohort=Never and borrowWithinCohort.Policy!=Never"))
}
return allErrs
}

func validateResourceGroups(resourceGroups []kueue.ResourceGroup, cohort string, path *field.Path) field.ErrorList {
var allErrs field.ErrorList
seenResources := sets.New[corev1.ResourceName]()
Expand Down Expand Up @@ -174,10 +141,7 @@ func validateResourceGroups(resourceGroups []kueue.ResourceGroup, cohort string,
}

func validateFlavorQuotas(flavorQuotas kueue.FlavorQuotas, coveredResources []corev1.ResourceName, cohort string, path *field.Path) field.ErrorList {
allErrs := validateNameReference(string(flavorQuotas.Name), path.Child("name"))
if len(flavorQuotas.Resources) != len(coveredResources) {
allErrs = append(allErrs, field.Invalid(path.Child("resources"), field.OmitValueType{}, "must have the same number of resources as the coveredResources"))
}
var allErrs field.ErrorList

for i, rq := range flavorQuotas.Resources {
if i >= len(coveredResources) {
Expand All @@ -191,7 +155,6 @@ func validateFlavorQuotas(flavorQuotas kueue.FlavorQuotas, coveredResources []co
if rq.BorrowingLimit != nil {
borrowingLimitPath := path.Child("borrowingLimit")
allErrs = append(allErrs, validateResourceQuantity(*rq.BorrowingLimit, borrowingLimitPath)...)
allErrs = append(allErrs, validateLimit(*rq.BorrowingLimit, cohort, borrowingLimitPath)...)
}
if features.Enabled(features.LendingLimit) && rq.LendingLimit != nil {
lendingLimitPath := path.Child("lendingLimit")
Expand Down

0 comments on commit 93933eb

Please sign in to comment.