Implement Cohort Webhook

charts/kueue/templates/webhook/webhook.yaml

@@ -599,6 +599,26 @@ webhooks:
         resources:
           - clusterqueues
     sideEffects: None
+  - admissionReviewVersions:
+      - v1
+    clientConfig:
+      service:
+        name: '{{ include "kueue.fullname" . }}-webhook-service'
+        namespace: '{{ .Release.Namespace }}'
+        path: /validate-kueue-x-k8s-io-v1alpha1-cohort
+    failurePolicy: Fail
+    name: vcohort.kb.io
+    rules:
+      - apiGroups:
+          - kueue.x-k8s.io
+        apiVersions:
+          - v1alpha1
+        operations:
+          - CREATE
+          - UPDATE
+        resources:
+          - cohorts
+    sideEffects: None
   - admissionReviewVersions:
       - v1
     clientConfig:

config/components/webhook/manifests.yaml

@@ -555,6 +555,26 @@ webhooks:
     resources:
     - clusterqueues
   sideEffects: None
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /validate-kueue-x-k8s-io-v1alpha1-cohort
+  failurePolicy: Fail
+  name: vcohort.kb.io
+  rules:
+  - apiGroups:
+    - kueue.x-k8s.io
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    - UPDATE
+    resources:
+    - cohorts
+  sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig:

pkg/util/testing/wrappers.go

@@ -600,6 +600,11 @@ func (c *CohortWrapper) Obj() *kueuealpha.Cohort {
 	return &c.Cohort
 }
 
+func (c *CohortWrapper) Parent(parentName string) *CohortWrapper {
+	c.Cohort.Spec.Parent = parentName
+	return c
+}
+
 // ResourceGroup adds a ResourceGroup with flavors.
 func (c *CohortWrapper) ResourceGroup(flavors ...kueue.FlavorQuotas) *CohortWrapper {
 	c.Spec.ResourceGroups = append(c.Spec.ResourceGroups, createResourceGroup(flavors...))

pkg/webhooks/clusterqueue_webhook.go

@@ -82,11 +82,10 @@ func (w *ClusterQueueWebhook) ValidateCreate(ctx context.Context, obj runtime.Ob
 // ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type
 func (w *ClusterQueueWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
 	newCQ := newObj.(*kueue.ClusterQueue)
-	oldCQ := oldObj.(*kueue.ClusterQueue)
 
 	log := ctrl.LoggerFrom(ctx).WithName("clusterqueue-webhook")
 	log.V(5).Info("Validating update", "clusterQueue", klog.KObj(newCQ))
-	allErrs := ValidateClusterQueueUpdate(newCQ, oldCQ)
+	allErrs := ValidateClusterQueueUpdate(newCQ)
 	return nil, allErrs.ToAggregate()
 }
 
@@ -99,7 +98,11 @@ func ValidateClusterQueue(cq *kueue.ClusterQueue) field.ErrorList {
 	path := field.NewPath("spec")
 
 	var allErrs field.ErrorList
-	allErrs = append(allErrs, validateResourceGroups(cq.Spec.ResourceGroups, cq.Spec.Cohort, path.Child("resourceGroups"))...)
+	config := validationConfig{
+		hasParent:                        cq.Spec.Cohort != "",
+		enforceNominalGreaterThanLending: true,
+	}
+	allErrs = append(allErrs, validateResourceGroups(cq.Spec.ResourceGroups, config, path.Child("resourceGroups"))...)
 	allErrs = append(allErrs,
 		validation.ValidateLabelSelector(cq.Spec.NamespaceSelector, validation.LabelSelectorValidationOptions{}, path.Child("namespaceSelector"))...)
 	allErrs = append(allErrs, validateCQAdmissionChecks(&cq.Spec, path)...)
@@ -112,10 +115,8 @@ func ValidateClusterQueue(cq *kueue.ClusterQueue) field.ErrorList {
 	return allErrs
 }
 
-func ValidateClusterQueueUpdate(newObj, _ *kueue.ClusterQueue) field.ErrorList {
-	var allErrs field.ErrorList
-	allErrs = append(allErrs, ValidateClusterQueue(newObj)...)
-	return allErrs
+func ValidateClusterQueueUpdate(newObj *kueue.ClusterQueue) field.ErrorList {
+	return ValidateClusterQueue(newObj)
 }
 
 func validatePreemption(preemption *kueue.ClusterQueuePreemption, path *field.Path) field.ErrorList {
@@ -137,7 +138,7 @@ func validateCQAdmissionChecks(spec *kueue.ClusterQueueSpec, path *field.Path) f
 	return allErrs
 }
 
-func validateResourceGroups(resourceGroups []kueue.ResourceGroup, cohort string, path *field.Path) field.ErrorList {
+func validateResourceGroups(resourceGroups []kueue.ResourceGroup, config validationConfig, path *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
 	seenResources := sets.New[corev1.ResourceName]()
 	seenFlavors := sets.New[kueue.ResourceFlavorReference]()
@@ -155,7 +156,7 @@ func validateResourceGroups(resourceGroups []kueue.ResourceGroup, cohort string,
 		}
 		for j, fqs := range rg.Flavors {
 			path := path.Child("flavors").Index(j)
-			allErrs = append(allErrs, validateFlavorQuotas(fqs, rg.CoveredResources, cohort, path)...)
+			allErrs = append(allErrs, validateFlavorQuotas(fqs, rg.CoveredResources, config, path)...)
 			if seenFlavors.Has(fqs.Name) {
 				allErrs = append(allErrs, field.Duplicate(path.Child("name"), fqs.Name))
 			} else {
@@ -166,7 +167,7 @@ func validateResourceGroups(resourceGroups []kueue.ResourceGroup, cohort string,
 	return allErrs
 }
 
-func validateFlavorQuotas(flavorQuotas kueue.FlavorQuotas, coveredResources []corev1.ResourceName, cohort string, path *field.Path) field.ErrorList {
+func validateFlavorQuotas(flavorQuotas kueue.FlavorQuotas, coveredResources []corev1.ResourceName, config validationConfig, path *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
 
 	for i, rq := range flavorQuotas.Resources {
@@ -180,13 +181,14 @@ func validateFlavorQuotas(flavorQuotas kueue.FlavorQuotas, coveredResources []co
 		allErrs = append(allErrs, validateResourceQuantity(rq.NominalQuota, path.Child("nominalQuota"))...)
 		if rq.BorrowingLimit != nil {
 			borrowingLimitPath := path.Child("borrowingLimit")
+			allErrs = append(allErrs, validateLimit(*rq.BorrowingLimit, config, borrowingLimitPath)...)
 			allErrs = append(allErrs, validateResourceQuantity(*rq.BorrowingLimit, borrowingLimitPath)...)
 		}
 		if features.Enabled(features.LendingLimit) && rq.LendingLimit != nil {
 			lendingLimitPath := path.Child("lendingLimit")
 			allErrs = append(allErrs, validateResourceQuantity(*rq.LendingLimit, lendingLimitPath)...)
-			allErrs = append(allErrs, validateLimit(*rq.LendingLimit, cohort, lendingLimitPath)...)
-			allErrs = append(allErrs, validateLendingLimit(*rq.LendingLimit, rq.NominalQuota, lendingLimitPath)...)
+			allErrs = append(allErrs, validateLimit(*rq.LendingLimit, config, lendingLimitPath)...)
+			allErrs = append(allErrs, validateLendingLimit(*rq.LendingLimit, rq.NominalQuota, config, lendingLimitPath)...)
 		}
 	}
 	return allErrs
@@ -202,18 +204,18 @@ func validateResourceQuantity(value resource.Quantity, fldPath *field.Path) fiel
 }
 
 // validateLimit enforces that BorrowingLimit or LendingLimit must be nil when cohort is empty
-func validateLimit(limit resource.Quantity, cohort string, fldPath *field.Path) field.ErrorList {
+func validateLimit(limit resource.Quantity, config validationConfig, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
-	if len(cohort) == 0 {
+	if !config.hasParent {
 		allErrs = append(allErrs, field.Invalid(fldPath, limit.String(), limitIsEmptyErrorMsg))
 	}
 	return allErrs
 }
 
 // validateLendingLimit enforces that LendingLimit is not greater than NominalQuota
-func validateLendingLimit(lend, nominal resource.Quantity, fldPath *field.Path) field.ErrorList {
+func validateLendingLimit(lend, nominal resource.Quantity, config validationConfig, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
-	if lend.Cmp(nominal) > 0 {
+	if config.enforceNominalGreaterThanLending && lend.Cmp(nominal) > 0 {
 		allErrs = append(allErrs, field.Invalid(fldPath, lend.String(), lendingLimitErrorMsg))
 	}
 	return allErrs

pkg/webhooks/clusterqueue_webhook_test.go

@@ -361,7 +361,7 @@ func TestValidateClusterQueueUpdate(t *testing.T) {
 
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			gotErr := ValidateClusterQueueUpdate(tc.newClusterQueue, tc.oldClusterQueue)
+			gotErr := ValidateClusterQueueUpdate(tc.newClusterQueue)
 			if diff := cmp.Diff(tc.wantErr, gotErr, cmpopts.IgnoreFields(field.Error{}, "Detail", "BadValue")); diff != "" {
 				t.Errorf("ValidateResources() mismatch (-want +got):\n%s", diff)
 			}

pkg/webhooks/cohort_webhook.go

@@ -0,0 +1,77 @@
+/*
+Copyright 2024 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.
+*/
+
+package webhooks
+
+import (
+	"context"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/klog/v2"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	kueuealpha "sigs.k8s.io/kueue/apis/kueue/v1alpha1"
+)
+
+type CohortWebhook struct{}
+
+func setupWebhookForCohort(mgr ctrl.Manager) error {
+	return ctrl.NewWebhookManagedBy(mgr).
+		For(&kueuealpha.Cohort{}).
+		WithValidator(&CohortWebhook{}).
+		Complete()
+}
+
+func (w *CohortWebhook) Default(ctx context.Context, obj runtime.Object) error {
+	return nil
+}
+
+//+kubebuilder:webhook:path=/validate-kueue-x-k8s-io-v1alpha1-cohort,mutating=false,failurePolicy=fail,sideEffects=None,groups=kueue.x-k8s.io,resources=cohorts,verbs=create;update,versions=v1alpha1,name=vcohort.kb.io,admissionReviewVersions=v1
+
+var _ webhook.CustomValidator = &CohortWebhook{}
+
+// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type
+func (w *CohortWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	cohort := obj.(*kueuealpha.Cohort)
+	log := ctrl.LoggerFrom(ctx).WithName("cohort-webhook")
+	log.V(5).Info("Validating Cohort create", "cohort", klog.KObj(cohort))
+	return nil, validateCohort(cohort).ToAggregate()
+}
+
+// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type
+func (w *CohortWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
+	cohort := newObj.(*kueuealpha.Cohort)
+	log := ctrl.LoggerFrom(ctx).WithName("cohort-webhook")
+	log.V(5).Info("Validating Cohort update", "cohort", klog.KObj(cohort))
+	return nil, validateCohort(cohort).ToAggregate()
+}
+
+// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type
+func (w *CohortWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
+	return nil, nil
+}
+
+func validateCohort(cohort *kueuealpha.Cohort) field.ErrorList {
+	path := field.NewPath("spec")
+	config := validationConfig{
+		hasParent:                        cohort.Spec.Parent != "",
+		enforceNominalGreaterThanLending: false,
+	}
+	return validateResourceGroups(cohort.Spec.ResourceGroups, config, path.Child("resourceGroups"))
+}

pkg/webhooks/common.go

@@ -29,3 +29,8 @@ func validateResourceName(name corev1.ResourceName, fldPath *field.Path) field.E
 	}
 	return allErrs
 }
+
+type validationConfig struct {
+	hasParent                        bool
+	enforceNominalGreaterThanLending bool
+}

pkg/webhooks/webhooks.go

@@ -16,7 +16,9 @@ limitations under the License.
 
 package webhooks
 
-import ctrl "sigs.k8s.io/controller-runtime"
+import (
+	ctrl "sigs.k8s.io/controller-runtime"
+)
 
 // Setup sets up the webhooks for core controllers. It returns the name of the
 // webhook that failed to create and an error, if any.
@@ -33,5 +35,9 @@ func Setup(mgr ctrl.Manager) (string, error) {
 		return "ClusterQueue", err
 	}
 
+	if err := setupWebhookForCohort(mgr); err != nil {
+		return "Cohort", err
+	}
+
 	return "", nil
 }