chore: requeue on update conflict

- requeue when update fails with a conflict error instead of returning the error to reduce the occurence of update conflict error "the object has been modified"
syntasso · Oct 17, 2024 · 821d4bb · 821d4bb
1 parent 15c8fb2
commit 821d4bb
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 8 deletions.
diff --git a/controllers/promise_controller.go b/controllers/promise_controller.go
@@ -151,7 +151,7 @@ func (r *PromiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	if value, found := promise.Labels[v1alpha1.PromiseVersionLabel]; found {
 		if promise.Status.Version != value {
 			promise.Status.Version = value
-			return ctrl.Result{}, r.Client.Status().Update(ctx, promise)
+			return r.updatePromiseStatus(ctx, promise)
 		}
 	}
 
@@ -162,10 +162,10 @@ func (r *PromiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 
 	scheduledReconciliation := promise.Status.LastAvailableTime != nil && time.Since(promise.Status.LastAvailableTime.Time) > DefaultReconciliationInterval
 	if (requirementsChanged || scheduledReconciliation) && originalStatus == v1alpha1.PromiseStatusAvailable {
-		err := r.Client.Status().Update(ctx, promise)
-		if err != nil {
-			return ctrl.Result{}, err
+		if result, statusUpdateErr := r.updatePromiseStatus(ctx, promise); statusUpdateErr != nil || !result.IsZero() {
+			return result, statusUpdateErr
 		}
+
 		logger.Info("Requeueing: requirements changed or scheduled reconciliation")
 		return ctrl.Result{}, nil
 	}
@@ -261,7 +261,7 @@ func (r *PromiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 			}
 			logger.Info("updating observed generation", "from", promise.Status.ObservedGeneration, "to", promise.GetGeneration())
 			promise.Status.ObservedGeneration = promise.GetGeneration()
-			return ctrl.Result{}, r.Client.Status().Update(ctx, promise)
+			return r.updatePromiseStatus(ctx, promise)
 		}
 	} else {
 		logger.Info("Promise only contains dependencies, skipping creation of API and dynamic controller")
@@ -274,7 +274,7 @@ func (r *PromiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	logger.Info("Promise status being set to Available")
 	promise.Status.Status = v1alpha1.PromiseStatusAvailable
 	promise.Status.LastAvailableTime = &metav1.Time{Time: time.Now()}
-	return ctrl.Result{}, r.Client.Status().Update(ctx, promise)
+	return r.updatePromiseStatus(ctx, promise)
 }
 
 func (r *PromiseReconciler) nextReconciliation(promise *v1alpha1.Promise, logger logr.Logger) (ctrl.Result, error) {
@@ -1079,3 +1079,12 @@ func (r *PromiseReconciler) markRequiredPromiseAsRequired(ctx context.Context, v
 		r.Log.Error(err, "error updating promise required by promise", "promise", promise.GetName(), "required promise", requiredPromise.GetName())
 	}
 }
+
+func (r *PromiseReconciler) updatePromiseStatus(ctx context.Context, promise *v1alpha1.Promise) (ctrl.Result, error) {
+	err := r.Client.Status().Update(ctx, promise)
+	if errors.IsConflict(err) {
+		r.Log.Info("failed to update Promise status due to update conflict, requeue...")
+		return fastRequeue, nil
+	}
+	return ctrl.Result{}, err
+}
diff --git a/controllers/work_controller.go b/controllers/work_controller.go
@@ -81,7 +81,10 @@ func (r *WorkReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 
 	logger.Info("Requesting scheduling for Work")
 	unscheduledWorkloadGroupIDs, err := r.Scheduler.ReconcileWork(work)
-	if err != nil {
+	if errors.IsConflict(err) {
+		logger.Info("failed to schedule Work due to update conflict, requeue...")
+		return fastRequeue, nil
+	} else if err != nil {
 		//TODO remove this error checking
 		//temp fix until resolved: https://syntasso.slack.com/archives/C044T9ZFUMN/p1674058648965449
 		logger.Error(err, "Error scheduling Work, will retry...")

diff --git a/controllers/workplacement_controller.go b/controllers/workplacement_controller.go
@@ -25,6 +25,7 @@ import (
 	"github.com/go-logr/logr"
 	"gopkg.in/yaml.v2"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -122,7 +123,11 @@ func (r *WorkPlacementReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		workPlacement.Status.VersionID = versionID
 		logger.Info("Updating version status", "versionID", versionID)
 		err = r.Client.Status().Update(ctx, workPlacement)
-		if err != nil {
+		if kerrors.IsConflict(err) {
+			r.VersionCache[workPlacement.GetUniqueID()] = versionID
+			r.Log.Info("failed to update WorkPlacement status due to update conflict, requeue...")
+			return fastRequeue, nil
+		} else if err != nil {
 			r.VersionCache[workPlacement.GetUniqueID()] = versionID
 			logger.Error(err, "Error updating WorkPlacement status")
 			return ctrl.Result{}, err