Add CEL rules to Workload

kubernetes-sigs · Apr 18, 2024 · 7e75cde · 7e75cde
1 parent 768df1e
commit 7e75cde
Show file tree

Hide file tree

Showing 9 changed files with 934 additions and 317 deletions.
diff --git a/apis/kueue/v1beta1/workload_types.go b/apis/kueue/v1beta1/workload_types.go
@@ -22,6 +22,7 @@ import (
 )
 
 // WorkloadSpec defines the desired state of Workload
+// +kubebuilder:validation:XValidation:rule="has(self.priorityClassName) ? has(self.priority) : true", message="priority should not be nil when priorityClassName is set"
 type WorkloadSpec struct {
 	// podSets is a list of sets of homogeneous pods, each described by a Pod spec
 	// and a count.
@@ -36,6 +37,8 @@ type WorkloadSpec struct {
 
 	// queueName is the name of the LocalQueue the Workload is associated with.
 	// queueName cannot be changed while .status.admission is not null.
+	// +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])?)*$"
 	QueueName string `json:"queueName,omitempty"`
 
 	// If specified, indicates the workload's priority.
@@ -44,6 +47,8 @@ type WorkloadSpec struct {
 	// the highest priority. Any other name must be defined by creating a
 	// PriorityClass object with that name. If not specified, the workload
 	// priority will be default or zero if there is no default.
+	// +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])?)*$"
 	PriorityClassName string `json:"priorityClassName,omitempty"`
 
 	// Priority determines the order of access to the resources managed by the
@@ -79,12 +84,15 @@ type Admission struct {
 	// PodSetAssignments hold the admission results for each of the .spec.podSets entries.
 	// +listType=map
 	// +listMapKey=name
+	// +kubebuilder:validation:MaxItems=8
 	PodSetAssignments []PodSetAssignment `json:"podSetAssignments"`
 }
 
 type PodSetAssignment struct {
 	// Name is the name of the podSet. It should match one of the names in .spec.podSets.
 	// +kubebuilder:default=main
+	// +kubebuilder:validation:MaxLength=63
+	// +kubebuilder:validation:Pattern="^[a-z0-9]([-a-z0-9]*[a-z0-9])?$"
 	Name string `json:"name"`
 
 	// Flavors are the flavors assigned to the workload for each resource.
@@ -107,8 +115,11 @@ type PodSetAssignment struct {
 	Count *int32 `json:"count,omitempty"`
 }
 
+// +kubebuilder:validation:XValidation:rule="has(self.minCount) ? self.minCount <= self.count : true", message="minCount should be positive and less or equal to count"
 type PodSet struct {
 	// name is the PodSet name.
+	// +kubebuilder:validation:MaxLength=63
+	// +kubebuilder:validation:Pattern="^[a-z0-9]([-a-z0-9]*[a-z0-9])?$"
 	Name string `json:"name"`
 
 	// template is the Pod template.
@@ -141,6 +152,7 @@ type PodSet struct {
 	// This is an alpha field and requires enabling PartialAdmission feature gate.
 	//
 	// +optional
+	// +kubebuilder:validation:Minimum=1
 	MinCount *int32 `json:"minCount,omitempty"`
 }
 
@@ -149,6 +161,7 @@ type WorkloadStatus struct {
 	// admission holds the parameters of the admission of the workload by a
 	// ClusterQueue. admission can be set back to null, but its fields cannot be
 	// changed once set.
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf", message="field is immutable"
 	Admission *Admission `json:"admission,omitempty"`
 
 	// requeueState holds the re-queue state
@@ -179,6 +192,7 @@ type WorkloadStatus struct {
 	// +optional
 	// +listType=map
 	// +listMapKey=name
+	// +kubebuilder:validation:MaxItems=8
 	ReclaimablePods []ReclaimablePod `json:"reclaimablePods,omitempty"`
 
 	// admissionChecks list all the admission checks required by the workload and the current status
@@ -187,6 +201,7 @@ type WorkloadStatus struct {
 	// +listMapKey=name
 	// +patchStrategy=merge
 	// +patchMergeKey=name
+	// +kubebuilder:validation:MaxItems=8
 	AdmissionChecks []AdmissionCheckState `json:"admissionChecks,omitempty" patchStrategy:"merge" patchMergeKey:"name"`
 }
 
@@ -234,6 +249,7 @@ type AdmissionCheckState struct {
 
 	// +optional
 	// +listType=atomic
+	// +kubebuilder:validation:MaxItems=8
 	PodSetUpdates []PodSetUpdate `json:"podSetUpdates,omitempty"`
 }
 
@@ -257,6 +273,12 @@ type PodSetUpdate struct {
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
 
 	// +optional
+	// +kubebuilder:validation:MaxItems=8
+	// +kubebuilder:validation:XValidation:rule="self.all(x, !has(x.key) ? x.operator == 'Exists' : true)", message="operator must be Exists when 'key' is empty, which means 'match all values and all keys'"
+	// +kubebuilder:validation:XValidation:rule="self.all(x, has(x.tolerationSeconds) ? x.effect == 'NoExecute' : true)", message="effect must be 'NoExecute' when 'tolerationSeconds' is set"
+	// +kubebuilder:validation:XValidation:rule="self.all(x, !has(x.operator) || x.operator in ['Equal', 'Exists'])", message="supported toleration values: 'Equal'(default), 'Exists'"
+	// +kubebuilder:validation:XValidation:rule="self.all(x, has(x.operator) && x.operator == 'Exists' ? !has(x.value) : true)", message="a value must be empty when 'operator' is 'Exists'"
+	// +kubebuilder:validation:XValidation:rule="self.all(x, !has(x.effect) || x.effect in ['NoSchedule', 'PreferNoSchedule', 'NoExecute'])", message="supported taint effect values: 'NoSchedule', 'PreferNoSchedule', 'NoExecute'"
 	Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
 }
 
@@ -336,6 +358,9 @@ const (
 // +kubebuilder:resource:shortName={wl}
 
 // Workload is the Schema for the workloads API
+// +kubebuilder:validation:XValidation:rule="has(self.spec) && has(self.status) && has(self.status.conditions) && self.status.conditions.exists(c, c.type == 'QuotaReserved' && c.status == 'True') && has(self.status.admission) ? size(self.spec.podSets) == size(self.status.admission.podSetAssignments) : true", message="podSetAssignments must have the same number of podSets as the spec"
+// +kubebuilder:validation:XValidation:rule="(has(oldSelf.spec) && has(oldSelf.status) && has(oldSelf.status.conditions) && oldSelf.status.conditions.exists(c, c.type == 'QuotaReserved' && c.status == 'True')) ? (self.spec.podSets == oldSelf.spec.podSets && self.spec.priorityClassSource == oldSelf.spec.priorityClassSource && self.spec.priorityClassName == oldSelf.spec.priorityClassName) : true", message="field is immutable"
+// +kubebuilder:validation:XValidation:rule="(has(oldSelf.spec) && has(oldSelf.status) && has(oldSelf.status.conditions) && oldSelf.status.conditions.exists(c, c.type == 'QuotaReserved' && c.status == 'True')) && (has(self.spec) && has(self.status) && has(self.status.conditions) && self.status.conditions.exists(c, c.type == 'QuotaReserved' && c.status == 'True')) ? self.spec.queueName == oldSelf.spec.queueName : true", message="field is immutable"
 type Workload struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_workloads.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_workloads.yaml
@@ -111,9 +111,12 @@ spec:
 
                         This is an alpha field and requires enabling PartialAdmission feature gate.
                       format: int32
+                      minimum: 1
                       type: integer
                     name:
                       description: name is the PodSet name.
+                      maxLength: 63
+                      pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
                       type: string
                     template:
                       description: |-
@@ -7767,6 +7770,9 @@ spec:
                   - name
                   - template
                   type: object
+                  x-kubernetes-validations:
+                  - message: minCount should be positive and less or equal to count
+                    rule: 'has(self.minCount) ? self.minCount <= self.count : true'
                 maxItems: 8
                 minItems: 1
                 type: array
@@ -7790,6 +7796,8 @@ spec:
                   the highest priority. Any other name must be defined by creating a
                   PriorityClass object with that name. If not specified, the workload
                   priority will be default or zero if there is no default.
+                maxLength: 253
+                pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                 type: string
               priorityClassSource:
                 default: ""
@@ -7806,10 +7814,15 @@ spec:
                 description: |-
                   queueName is the name of the LocalQueue the Workload is associated with.
                   queueName cannot be changed while .status.admission is not null.
+                maxLength: 253
+                pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                 type: string
             required:
             - podSets
             type: object
+            x-kubernetes-validations:
+            - message: priority should not be nil when priorityClassName is set
+              rule: 'has(self.priorityClassName) ? has(self.priority) : true'
           status:
             description: WorkloadStatus defines the observed state of Workload
             properties:
@@ -7853,6 +7866,8 @@ spec:
                           default: main
                           description: Name is the name of the podSet. It should match
                             one of the names in .spec.podSets.
+                          maxLength: 63
+                          pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$
                           type: string
                         resourceUsage:
                           additionalProperties:
@@ -7872,6 +7887,7 @@ spec:
                       required:
                       - name
                       type: object
+                    maxItems: 8
                     type: array
                     x-kubernetes-list-map-keys:
                     - name
@@ -7880,6 +7896,9 @@ spec:
                 - clusterQueue
                 - podSetAssignments
                 type: object
+                x-kubernetes-validations:
+                - message: field is immutable
+                  rule: self == oldSelf
               admissionChecks:
                 description: admissionChecks list all the admission checks required
                   by the workload and the current status
@@ -7962,10 +7981,32 @@ spec:
                                     If the operator is Exists, the value should be empty, otherwise just a regular string.
                                   type: string
                               type: object
+                            maxItems: 8
                             type: array
+                            x-kubernetes-validations:
+                            - message: operator must be Exists when 'key' is empty,
+                                which means 'match all values and all keys'
+                              rule: 'self.all(x, !has(x.key) ? x.operator == ''Exists''
+                                : true)'
+                            - message: effect must be 'NoExecute' when 'tolerationSeconds'
+                                is set
+                              rule: 'self.all(x, has(x.tolerationSeconds) ? x.effect
+                                == ''NoExecute'' : true)'
+                            - message: 'supported toleration values: ''Equal''(default),
+                                ''Exists'''
+                              rule: self.all(x, !has(x.operator) || x.operator in
+                                ['Equal', 'Exists'])
+                            - message: a value must be empty when 'operator' is 'Exists'
+                              rule: 'self.all(x, has(x.operator) && x.operator ==
+                                ''Exists'' ? !has(x.value) : true)'
+                            - message: 'supported taint effect values: ''NoSchedule'',
+                                ''PreferNoSchedule'', ''NoExecute'''
+                              rule: self.all(x, !has(x.effect) || x.effect in ['NoSchedule',
+                                'PreferNoSchedule', 'NoExecute'])
                         required:
                         - name
                         type: object
+                      maxItems: 8
                       type: array
                       x-kubernetes-list-type: atomic
                     state:
@@ -7983,6 +8024,7 @@ spec:
                   - name
                   - state
                   type: object
+                maxItems: 8
                 type: array
                 x-kubernetes-list-map-keys:
                 - name
@@ -8090,6 +8132,7 @@ spec:
                   - count
                   - name
                   type: object
+                maxItems: 8
                 type: array
                 x-kubernetes-list-map-keys:
                 - name
@@ -8117,6 +8160,24 @@ spec:
                 type: object
             type: object
         type: object
+        x-kubernetes-validations:
+        - message: podSetAssignments must have the same number of podSets as the spec
+          rule: 'has(self.spec) && has(self.status) && has(self.status.conditions)
+            && self.status.conditions.exists(c, c.type == ''QuotaReserved'' && c.status
+            == ''True'') && has(self.status.admission) ? size(self.spec.podSets) ==
+            size(self.status.admission.podSetAssignments) : true'
+        - message: field is immutable
+          rule: '(has(oldSelf.spec) && has(oldSelf.status) && has(oldSelf.status.conditions)
+            && oldSelf.status.conditions.exists(c, c.type == ''QuotaReserved'' &&
+            c.status == ''True'')) ? (self.spec.podSets == oldSelf.spec.podSets &&
+            self.spec.priorityClassSource == oldSelf.spec.priorityClassSource && self.spec.priorityClassName
+            == oldSelf.spec.priorityClassName) : true'
+        - message: field is immutable
+          rule: '(has(oldSelf.spec) && has(oldSelf.status) && has(oldSelf.status.conditions)
+            && oldSelf.status.conditions.exists(c, c.type == ''QuotaReserved'' &&
+            c.status == ''True'')) && (has(self.spec) && has(self.status) && has(self.status.conditions)
+            && self.status.conditions.exists(c, c.type == ''QuotaReserved'' && c.status
+            == ''True'')) ? self.spec.queueName == oldSelf.spec.queueName : true'
     served: true
     storage: true
     subresources: