diff --git a/pkg/webhooks/workload_webhook.go b/pkg/webhooks/workload_webhook.go index 3a7822b242..2e7894b166 100644 --- 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 index 6de4e1dba0..7cc01b26ed 100644 --- 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) {