From 0f86c4593438de30e28601cae3b793c236ca0810 Mon Sep 17 00:00:00 2001 From: Patryk Bundyra Date: Mon, 28 Oct 2024 11:38:29 +0000 Subject: [PATCH] Reduce number of patch requests --- pkg/controller/core/workload_controller.go | 4 +--- pkg/workload/admissionchecks.go | 14 ++++--------- pkg/workload/workload.go | 24 +++++++++++++--------- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/pkg/controller/core/workload_controller.go b/pkg/controller/core/workload_controller.go index 1ba1594bc7..a0a179b541 100644 --- a/pkg/controller/core/workload_controller.go +++ b/pkg/controller/core/workload_controller.go @@ -380,12 +380,10 @@ func (r *WorkloadReconciler) reconcileCheckBasedEviction(ctx context.Context, wl // at this point we know a Workload has at least one Retry AdmissionCheck message := "At least one admission check is false" workload.SetEvictedCondition(wl, kueue.WorkloadEvictedByAdmissionCheck, message) + workload.SetAllChecksToPending(wl, "AdmissionCheck pending after retry") if err := workload.ApplyAdmissionStatus(ctx, r.client, wl, true); err != nil { return false, client.IgnoreNotFound(err) } - if err := workload.SetAllChecksToPending(ctx, r.client, wl); err != nil { - return false, client.IgnoreNotFound(err) - } cqName, _ := r.queues.ClusterQueueForWorkload(wl) workload.ReportEvictedWorkload(r.recorder, wl, cqName, kueue.WorkloadEvictedByAdmissionCheck, message) return true, nil diff --git a/pkg/workload/admissionchecks.go b/pkg/workload/admissionchecks.go index 87cf3fc258..5d4c5cd9a5 100644 --- a/pkg/workload/admissionchecks.go +++ b/pkg/workload/admissionchecks.go @@ -17,13 +17,11 @@ limitations under the License. package workload import ( - "context" "time" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" - "sigs.k8s.io/controller-runtime/pkg/client" kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1" ) @@ -89,20 +87,16 @@ func FindAdmissionCheck(checks []kueue.AdmissionCheckState, checkName string) *k } // SetAllChecksToPending sets all AdmissionChecks to Pending -func SetAllChecksToPending(ctx context.Context, c client.Client, w *kueue.Workload) error { - wlCopy := BaseSSAWorkload(w) - message := "AdmissionCheck pending after retry" - checks := make([]kueue.AdmissionCheckState, len(w.Status.AdmissionChecks)) - for i := range w.Status.AdmissionChecks { +func SetAllChecksToPending(w *kueue.Workload, message string) { + checks := w.Status.AdmissionChecks + for i := range checks { checks[i] = kueue.AdmissionCheckState{ - Name: w.Status.AdmissionChecks[i].Name, + Name: checks[i].Name, State: kueue.CheckStatePending, LastTransitionTime: metav1.NewTime(time.Now()), Message: message, } } - wlCopy.Status.AdmissionChecks = checks - return ApplyStatusPatch(ctx, c, wlCopy) } // SetAdmissionCheckState - adds or updates newCheck in the provided checks list. diff --git a/pkg/workload/workload.go b/pkg/workload/workload.go index 8cfd0ab09c..11ffe99976 100644 --- a/pkg/workload/workload.go +++ b/pkg/workload/workload.go @@ -613,8 +613,7 @@ func PropagateResourceRequests(w *kueue.Workload, info *Info) bool { // AdmissionStatusPatch creates a new object based on the input workload that contains // the admission and related conditions. The object can be used in Server-Side-Apply. // If strict is true, resourceVersion will be part of the patch. -func AdmissionStatusPatch(w *kueue.Workload, strict bool) *kueue.Workload { - wlCopy := BaseSSAWorkload(w) +func AdmissionStatusPatch(w *kueue.Workload, wlCopy *kueue.Workload, strict bool) { wlCopy.Status.Admission = w.Status.Admission.DeepCopy() wlCopy.Status.RequeueState = w.Status.RequeueState.DeepCopy() if wlCopy.Status.Admission != nil { @@ -634,15 +633,25 @@ func AdmissionStatusPatch(w *kueue.Workload, strict bool) *kueue.Workload { wlCopy.ResourceVersion = w.ResourceVersion } wlCopy.Status.AccumulatedPastExexcutionTimeSeconds = w.Status.AccumulatedPastExexcutionTimeSeconds - return wlCopy +} + +func AdmissionChecksStatusPatch(w *kueue.Workload, wlCopy *kueue.Workload) { + if wlCopy.Status.AdmissionChecks == nil && w.Status.AdmissionChecks != nil { + wlCopy.Status.AdmissionChecks = make([]kueue.AdmissionCheckState, 0) + } + for _, ac := range w.Status.AdmissionChecks { + SetAdmissionCheckState(&wlCopy.Status.AdmissionChecks, ac) + } } // ApplyAdmissionStatus updated all the admission related status fields of a workload with SSA. // If strict is true, resourceVersion will be part of the patch, make this call fail if Workload // was changed. func ApplyAdmissionStatus(ctx context.Context, c client.Client, w *kueue.Workload, strict bool) error { - patch := AdmissionStatusPatch(w, strict) - return ApplyAdmissionStatusPatch(ctx, c, patch) + wlCopy := BaseSSAWorkload(w) + AdmissionStatusPatch(w, wlCopy, strict) + AdmissionChecksStatusPatch(w, wlCopy) + return ApplyAdmissionStatusPatch(ctx, c, wlCopy) } // ApplyAdmissionStatusPatch applies the patch of admission related status fields of a workload with SSA. @@ -650,11 +659,6 @@ func ApplyAdmissionStatusPatch(ctx context.Context, c client.Client, patch *kueu return c.Status().Patch(ctx, patch, client.Apply, client.FieldOwner(constants.AdmissionName)) } -// ApplyStatusPatch applies the patch of status fields of a workload with SSA. -func ApplyStatusPatch(ctx context.Context, c client.Client, patch *kueue.Workload) error { - return c.Status().Patch(ctx, patch, client.Apply, client.FieldOwner(constants.WorkloadControllerName)) -} - type Ordering struct { PodsReadyRequeuingTimestamp config.RequeuingTimestamp }