Add CEL rules to ClusterQueue

kubernetes-sigs · Apr 11, 2024 · 8cd72b7 · 8cd72b7
1 parent b9c346a
commit 8cd72b7
Show file tree

Hide file tree

Showing 10 changed files with 364 additions and 142 deletions.
diff --git a/apis/kueue/v1beta1/clusterqueue_types.go b/apis/kueue/v1beta1/clusterqueue_types.go
@@ -23,6 +23,7 @@ import (
 )
 
 // ClusterQueueSpec defines the desired state of ClusterQueue
+// +kubebuilder:validation:XValidation:rule="(!has(self.cohort) || size(self.cohort) == 0) && has(self.resourceGroups) ? self.resourceGroups.all(rg, rg.flavors.all(f, f.resources.all(r, !has(r.borrowingLimit)))) : true", message="resourceGroups[0].flavors[0].resources[0].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
@@ -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
@@ -134,6 +137,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.
@@ -217,6 +221,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
@@ -362,6 +368,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' && self.borrowWithinCohort != null &&  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

diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_clusterqueues.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_clusterqueues.yaml
@@ -99,6 +99,8 @@ 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:
                 description: |-
@@ -274,6 +276,10 @@ spec:
                     - LowerOrNewerEqualPriority
                     type: string
                 type: object
+                x-kubernetes-validations:
+                - message: reclaimWithinCohort=Never and borrowWithinCohort.Policy!=Never
+                  rule: '!(self.reclaimWithinCohort == ''Never'' && self.borrowWithinCohort
+                    != null &&  self.borrowWithinCohort.policy != ''Never'')'
               queueingStrategy:
                 default: BestEffortFIFO
                 description: |-
@@ -330,6 +336,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: |-
@@ -417,6 +425,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
@@ -439,6 +451,12 @@ spec:
                 - HoldAndDrain
                 type: string
             type: object
+            x-kubernetes-validations:
+            - message: resourceGroups[0].flavors[0].resources[0].borrowingLimit must
+                be nil when cohort is empty
+              rule: '(!has(self.cohort) || size(self.cohort) == 0) && 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:
@@ -531,6 +549,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
@@ -583,6 +603,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

diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml
@@ -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
@@ -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

diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_workloads.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_workloads.yaml
@@ -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.

diff --git a/config/components/crd/bases/kueue.x-k8s.io_clusterqueues.yaml b/config/components/crd/bases/kueue.x-k8s.io_clusterqueues.yaml
@@ -84,6 +84,8 @@ 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:
                 description: |-
@@ -259,6 +261,10 @@ spec:
                     - LowerOrNewerEqualPriority
                     type: string
                 type: object
+                x-kubernetes-validations:
+                - message: reclaimWithinCohort=Never and borrowWithinCohort.Policy!=Never
+                  rule: '!(self.reclaimWithinCohort == ''Never'' && self.borrowWithinCohort
+                    != null &&  self.borrowWithinCohort.policy != ''Never'')'
               queueingStrategy:
                 default: BestEffortFIFO
                 description: |-
@@ -315,6 +321,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: |-
@@ -402,6 +410,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
@@ -424,6 +436,12 @@ spec:
                 - HoldAndDrain
                 type: string
             type: object
+            x-kubernetes-validations:
+            - message: resourceGroups[0].flavors[0].resources[0].borrowingLimit must
+                be nil when cohort is empty
+              rule: '(!has(self.cohort) || size(self.cohort) == 0) && 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:
@@ -516,6 +534,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
@@ -568,6 +588,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

diff --git a/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml b/config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml
@@ -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
@@ -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

diff --git a/config/components/crd/bases/kueue.x-k8s.io_workloads.yaml b/config/components/crd/bases/kueue.x-k8s.io_workloads.yaml
@@ -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.

diff --git a/pkg/webhooks/clusterqueue_webhook.go b/pkg/webhooks/clusterqueue_webhook.go
@@ -116,15 +116,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
 }
 
@@ -134,16 +128,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]()
@@ -174,10 +158,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) {
@@ -191,7 +172,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")