Add retry-on-conflict for hypervisor status patches

internal/controller/aggregates_controller.go

@@ -23,7 +23,6 @@ import (
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -64,7 +63,6 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, nil
 	}
 
-	base := hv.DeepCopy()
 	desiredAggregateNames, desiredCondition := ac.determineDesiredState(hv)
 
 	// Extract current aggregate names for comparison
@@ -73,39 +71,51 @@ func (ac *AggregatesController) Reconcile(ctx context.Context, req ctrl.Request)
 		currentAggregateNames[i] = agg.Name
 	}
 
+	var newAggregates []kvmv1.Aggregate
+	aggregatesChanged := false
 	if !slicesEqualUnordered(desiredAggregateNames, currentAggregateNames) {
 		// Apply aggregates to OpenStack and update status
 		aggregates, err := openstack.ApplyAggregates(ctx, ac.computeClient, hv.Name, desiredAggregateNames)
 		if err != nil {
-			// Set error condition
+			// Set error condition with retry on conflict
 			condition := metav1.Condition{
 				Type:    kvmv1.ConditionTypeAggregatesUpdated,
 				Status:  metav1.ConditionFalse,
 				Reason:  kvmv1.ConditionReasonFailed,
 				Message: fmt.Errorf("failed to apply aggregates: %w", err).Error(),
 			}
 
-			if meta.SetStatusCondition(&hv.Status.Conditions, condition) {
-				if err2 := ac.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
-					k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(AggregatesControllerName)); err2 != nil {
-					return ctrl.Result{}, errors.Join(err, err2)
-				}
+			if err2 := utils.PatchHypervisorStatusWithRetry(ctx, ac.Client, hv.Name, AggregatesControllerName, func(h *kvmv1.Hypervisor) {
+				meta.SetStatusCondition(&h.Status.Conditions, condition)
+			}); err2 != nil {
+				return ctrl.Result{}, errors.Join(err, err2)
 			}
 			return ctrl.Result{}, err
 		}
 
-		hv.Status.Aggregates = aggregates
+		newAggregates = aggregates
+		aggregatesChanged = true
 	}
 
-	// Set the condition based on the determined desired state
-	meta.SetStatusCondition(&hv.Status.Conditions, desiredCondition)
-
-	if equality.Semantic.DeepEqual(base, hv) {
+	// Skip the round-trip when nothing would change
+	existing := meta.FindStatusCondition(hv.Status.Conditions, desiredCondition.Type)
+	conditionUnchanged := existing != nil &&
+		existing.Status == desiredCondition.Status &&
+		existing.Reason == desiredCondition.Reason &&
+		existing.Message == desiredCondition.Message
+	if !aggregatesChanged && conditionUnchanged {
 		return ctrl.Result{}, nil
 	}
 
-	return ctrl.Result{}, ac.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
-		k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(AggregatesControllerName))
+	// Patch status with retry on conflict
+	err := utils.PatchHypervisorStatusWithRetry(ctx, ac.Client, hv.Name, AggregatesControllerName, func(h *kvmv1.Hypervisor) {
+		if aggregatesChanged {
+			h.Status.Aggregates = newAggregates
+		}
+		meta.SetStatusCondition(&h.Status.Conditions, desiredCondition)
+	})
+
+	return ctrl.Result{}, err
 }
 
 // determineDesiredState returns the desired aggregates and the corresponding condition

internal/controller/hypervisor_controller.go

@@ -38,6 +38,7 @@ import (
 
 	kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
 	"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/global"
+	"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils"
 )
 
 const (
@@ -121,8 +122,16 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request)
 		}
 
 		if !equality.Semantic.DeepEqual(hypervisor, base) {
-			return ctrl.Result{}, hv.Status().Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(base,
-				k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorControllerName))
+			// Capture values to apply - only mutate fields this controller owns
+			newInternalIP := hypervisor.Status.InternalIP
+			terminatingCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeTerminating)
+
+			return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, hv.Client, hypervisor.Name, HypervisorControllerName, func(h *kvmv1.Hypervisor) {
+				h.Status.InternalIP = newInternalIP
+				if terminatingCondition != nil {
+					meta.SetStatusCondition(&h.Status.Conditions, *terminatingCondition)
+				}
+			})
 		}
 
 		syncLabelsAndAnnotations(nodeLabels, hypervisor, node)

internal/controller/hypervisor_instance_ha_controller.go

@@ -30,6 +30,7 @@ import (
 	logger "sigs.k8s.io/controller-runtime/pkg/log"
 
 	kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
+	"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils"
 )
 
 const (
@@ -99,17 +100,17 @@ func (r *HypervisorInstanceHaController) Reconcile(ctx context.Context, req ctrl
 		}
 
 		if err := disableInstanceHA(hv); err != nil {
-			meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
+			condition := metav1.Condition{
 				Type:    kvmv1.ConditionTypeHaEnabled,
 				Status:  metav1.ConditionUnknown,
 				Message: err.Error(),
 				Reason:  kvmv1.ConditionReasonFailed,
-			})
-
-			if patchErr := r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(old, k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorInstanceHaControllerName)); patchErr != nil {
-				return ctrl.Result{}, errors.Join(err, patchErr)
 			}
-			return ctrl.Result{}, err
+
+			patchErr := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, HypervisorInstanceHaControllerName, func(h *kvmv1.Hypervisor) {
+				meta.SetStatusCondition(&h.Status.Conditions, condition)
+			})
+			return ctrl.Result{}, errors.Join(err, patchErr)
 		}
 	} else {
 		if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
@@ -123,25 +124,31 @@ func (r *HypervisorInstanceHaController) Reconcile(ctx context.Context, req ctrl
 		}
 
 		if err := enableInstanceHA(hv); err != nil {
-			meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
+			condition := metav1.Condition{
 				Type:    kvmv1.ConditionTypeHaEnabled,
 				Status:  metav1.ConditionUnknown,
 				Message: err.Error(),
 				Reason:  kvmv1.ConditionReasonFailed,
-			})
-
-			if patchErr := r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(old, k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorInstanceHaControllerName)); patchErr != nil {
-				return ctrl.Result{}, errors.Join(err, patchErr)
 			}
-			return ctrl.Result{}, err
+
+			patchErr := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, HypervisorInstanceHaControllerName, func(h *kvmv1.Hypervisor) {
+				meta.SetStatusCondition(&h.Status.Conditions, condition)
+			})
+			return ctrl.Result{}, errors.Join(err, patchErr)
 		}
 	}
 
 	if equality.Semantic.DeepEqual(hv, old) {
 		return ctrl.Result{}, nil
 	}
 
-	return ctrl.Result{}, r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(old, k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorInstanceHaControllerName))
+	// Only set the HaEnabled condition this controller owns
+	haCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled)
+	return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, HypervisorInstanceHaControllerName, func(h *kvmv1.Hypervisor) {
+		if haCondition != nil {
+			meta.SetStatusCondition(&h.Status.Conditions, *haCondition)
+		}
+	})
 }
 
 // SetupWithManager sets up the controller with the Manager.

internal/controller/hypervisor_maintenance_controller.go

@@ -39,6 +39,7 @@ import (
 
 	kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
 	"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/openstack"
+	"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils"
 )
 
 const (
@@ -82,8 +83,22 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c
 		return ctrl.Result{}, nil
 	}
 
-	return ctrl.Result{}, hec.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(old,
-		k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(MaintenanceControllerName))
+	// Capture only the fields this controller owns
+	disabledCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled)
+	evictingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting)
+	evicted := hv.Status.Evicted
+
+	return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, hec.Client, hv.Name, HypervisorMaintenanceControllerName, func(h *kvmv1.Hypervisor) {
+		if disabledCondition != nil {
+			meta.SetStatusCondition(&h.Status.Conditions, *disabledCondition)
+		}
+		if evictingCondition != nil {
+			meta.SetStatusCondition(&h.Status.Conditions, *evictingCondition)
+		} else {
+			meta.RemoveStatusCondition(&h.Status.Conditions, kvmv1.ConditionTypeEvicting)
+		}
+		h.Status.Evicted = evicted
+	})
 }
 
 func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.Context, hv *kvmv1.Hypervisor) error {

internal/controller/hypervisor_taint_controller.go

@@ -30,6 +30,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
+	"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils"
 )
 
 const (
@@ -84,8 +85,13 @@ func (r *HypervisorTaintController) Reconcile(ctx context.Context, req ctrl.Requ
 		return ctrl.Result{}, nil
 	}
 
-	return ctrl.Result{}, r.Status().Patch(ctx, hypervisor, k8sclient.MergeFromWithOptions(before,
-		k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(HypervisorTaintControllerName))
+	// Only set the Tainted condition this controller owns
+	taintedCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeTainted)
+	return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hypervisor.Name, HypervisorTaintControllerName, func(h *kvmv1.Hypervisor) {
+		if taintedCondition != nil {
+			meta.SetStatusCondition(&h.Status.Conditions, *taintedCondition)
+		}
+	})
 }
 
 func (r *HypervisorTaintController) SetupWithManager(mgr ctrl.Manager) error {

internal/controller/offboarding_controller.go

@@ -35,6 +35,7 @@ import (
 
 	kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1"
 	"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/openstack"
+	"github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils"
 )
 
 const (
@@ -146,30 +147,30 @@ func (r *HypervisorOffboardingReconciler) Reconcile(ctx context.Context, req ctr
 }
 
 func (r *HypervisorOffboardingReconciler) setOffboardingCondition(ctx context.Context, hv *kvmv1.Hypervisor, message string) (ctrl.Result, error) {
-	base := hv.DeepCopy()
-	meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
-		Type:    kvmv1.ConditionTypeOffboarded,
-		Status:  metav1.ConditionFalse,
-		Reason:  "Offboarding",
-		Message: message,
+	err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OffboardingControllerName, func(h *kvmv1.Hypervisor) {
+		meta.SetStatusCondition(&h.Status.Conditions, metav1.Condition{
+			Type:    kvmv1.ConditionTypeOffboarded,
+			Status:  metav1.ConditionFalse,
+			Reason:  "Offboarding",
+			Message: message,
+		})
 	})
-	if err := r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
-		k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(OffboardingControllerName)); err != nil {
+	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("cannot update hypervisor status due to %w", err)
 	}
 	return ctrl.Result{}, nil
 }
 
 func (r *HypervisorOffboardingReconciler) markOffboarded(ctx context.Context, hv *kvmv1.Hypervisor) error {
-	base := hv.DeepCopy()
-	meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{
-		Type:    kvmv1.ConditionTypeOffboarded,
-		Status:  metav1.ConditionTrue,
-		Reason:  "Offboarded",
-		Message: "Offboarding successful",
+	err := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, OffboardingControllerName, func(h *kvmv1.Hypervisor) {
+		meta.SetStatusCondition(&h.Status.Conditions, metav1.Condition{
+			Type:    kvmv1.ConditionTypeOffboarded,
+			Status:  metav1.ConditionTrue,
+			Reason:  "Offboarded",
+			Message: "Offboarding successful",
+		})
 	})
-	if err := r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base,
-		k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(OffboardingControllerName)); err != nil {
+	if err != nil {
 		return fmt.Errorf("cannot update hypervisor status due to %w", err)
 	}
 	return nil