[webhook][workload] Validate MaximumExecutionTime

kubernetes-sigs · Oct 3, 2024 · c3b99a3 · c3b99a3
1 parent 7a23a76
commit c3b99a3
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 0 deletions.
diff --git a/pkg/webhooks/workload_webhook.go b/pkg/webhooks/workload_webhook.go
@@ -22,6 +22,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	apivalidation "k8s.io/apimachinery/pkg/api/validation"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -111,6 +112,7 @@ func ValidateWorkload(obj *kueue.Workload) field.ErrorList {
 	if variableCountPodSets > 1 {
 		allErrs = append(allErrs, field.Invalid(specPath.Child("podSets"), variableCountPodSets, "at most one podSet can use minCount"))
 	}
+	allErrs = append(allErrs, validateMaximumExecutionTime(obj.Spec.MaximumExecutionTime, specPath.Child("maximumExecutionTime"))...)
 
 	statusPath := field.NewPath("status")
 	if workload.HasQuotaReservation(obj) {
@@ -183,6 +185,13 @@ func validatePodSetUpdates(acs *kueue.AdmissionCheckState, obj *kueue.Workload,
 	return allErrs
 }
 
+func validateMaximumExecutionTime(met *metav1.Duration, path *field.Path) field.ErrorList {
+	if met != nil && *&met.Duration <= 0 {
+		return field.ErrorList{field.Invalid(path, *met, "should be grater then 0 if specified")}
+	}
+	return nil
+}
+
 func validateImmutablePodSetUpdates(newObj, oldObj *kueue.Workload, basePath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
 	newAcs := slices.ToRefMap(newObj.Status.AdmissionChecks, func(f *kueue.AdmissionCheckState) string { return f.Name })
@@ -275,6 +284,8 @@ func ValidateWorkloadUpdate(newObj, oldObj *kueue.Workload) field.ErrorList {
 	if workload.HasQuotaReservation(oldObj) {
 		allErrs = append(allErrs, apivalidation.ValidateImmutableField(newObj.Spec.PodSets, oldObj.Spec.PodSets, specPath.Child("podSets"))...)
 	}
+	allErrs = append(allErrs, validateMaximumExecutionTimeUpdate(newObj, oldObj, specPath.Child("maximumExecutionTime"))...)
+
 	if workload.HasQuotaReservation(newObj) && workload.HasQuotaReservation(oldObj) {
 		allErrs = append(allErrs, validateReclaimablePodsUpdate(newObj, oldObj, field.NewPath("status", "reclaimablePods"))...)
 	}
@@ -331,3 +342,12 @@ func validateReclaimablePodsUpdate(newObj, oldObj *kueue.Workload, basePath *fie
 	}
 	return ret
 }
+
+func validateMaximumExecutionTimeUpdate(newWl, oldWl *kueue.Workload, path *field.Path) field.ErrorList {
+	admitted := workload.IsAdmitted(oldWl) || workload.IsAdmitted(newWl)
+	changed := ptr.Deref(oldWl.Spec.MaximumExecutionTime, metav1.Duration{}) != ptr.Deref(newWl.Spec.MaximumExecutionTime, metav1.Duration{})
+	if admitted && changed {
+		return apivalidation.ValidateImmutableField(newWl.Spec.MaximumExecutionTime, oldWl.Spec.MaximumExecutionTime, path)
+	}
+	return nil
+}
diff --git a/pkg/webhooks/workload_webhook_test.go b/pkg/webhooks/workload_webhook_test.go
@@ -237,6 +237,15 @@ func TestValidateWorkload(t *testing.T) {
 				field.Invalid(podSetsPath, nil, ""),
 			},
 		},
+
+		"valid maximum execution time": {
+			workload: testingutil.MakeWorkload(testWorkloadName, testWorkloadNamespace).MaximumExecutionTime(time.Microsecond).Obj(),
+		},
+
+		"invalid maximum execution time": {
+			workload: testingutil.MakeWorkload(testWorkloadName, testWorkloadNamespace).MaximumExecutionTime(0).Obj(),
+			wantErr:  field.ErrorList{field.Invalid(specPath.Child("maximumExecutionTime"), 0, "")},
+		},
 	}
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {
@@ -392,6 +401,28 @@ func TestValidateWorkloadUpdate(t *testing.T) {
 				State:              kueue.CheckStateReady,
 			}).Obj(),
 		},
+		"can add maximum execution time when not admitted": {
+			before: testingutil.MakeWorkload(testWorkloadName, testWorkloadNamespace).Obj(),
+			after:  testingutil.MakeWorkload(testWorkloadName, testWorkloadNamespace).MaximumExecutionTime(time.Second).Obj(),
+		},
+		"can update maximum execution time when not admitted": {
+			before: testingutil.MakeWorkload(testWorkloadName, testWorkloadNamespace).MaximumExecutionTime(2 * time.Second).Obj(),
+			after:  testingutil.MakeWorkload(testWorkloadName, testWorkloadNamespace).MaximumExecutionTime(time.Second).Obj(),
+		},
+		"cannot add maximum execution time when admitted": {
+			before: testingutil.MakeWorkload(testWorkloadName, testWorkloadNamespace).Obj(),
+			after: testingutil.MakeWorkload(testWorkloadName, testWorkloadNamespace).
+				ReserveQuota(testingutil.MakeAdmission("cq").Obj()).Admitted(true).
+				MaximumExecutionTime(time.Second).Obj(),
+			wantErr: field.ErrorList{field.Invalid(field.NewPath("spec", "maximumExecutionTime"), metav1.Duration{Duration: time.Second}, "")},
+		},
+		"cannot update maximum execution time when admitted": {
+			before: testingutil.MakeWorkload(testWorkloadName, testWorkloadNamespace).MaximumExecutionTime(2 * time.Second).Obj(),
+			after: testingutil.MakeWorkload(testWorkloadName, testWorkloadNamespace).
+				ReserveQuota(testingutil.MakeAdmission("cq").Obj()).Admitted(true).
+				MaximumExecutionTime(time.Second).Obj(),
+			wantErr: field.ErrorList{field.Invalid(field.NewPath("spec", "maximumExecutionTime"), metav1.Duration{Duration: time.Second}, "")},
+		},
 	}
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {