feat(routing): use PatchWithRetry

controllers/routingctrl/controller.go

@@ -98,8 +98,6 @@ func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		errs = append(errs, reconciler(ctx, sourceRes))
 	}
 
-	errs = append(errs, unstruct.Patch(ctx, r.Client, sourceRes))
-
 	return ctrl.Result{}, errors.Join(errs...)
 }
 

controllers/routingctrl/delete_resources.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/opendatahub-io/odh-platform/pkg/metadata/labels"
 	"github.com/opendatahub-io/odh-platform/pkg/routing"
+	"github.com/opendatahub-io/odh-platform/pkg/unstruct"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	k8slabels "k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -88,10 +89,13 @@ func (r *Controller) deleteOwnedResources(ctx context.Context,
 // removeFinalizer is called after a successful cleanup, it removes the finalizer from the resource in the cluster.
 func removeFinalizer(ctx context.Context, cli client.Client, sourceRes *unstructured.Unstructured) error {
 	if controllerutil.ContainsFinalizer(sourceRes, finalizerName) {
-		controllerutil.RemoveFinalizer(sourceRes, finalizerName)
+		if err := unstruct.PatchWithRetry(ctx, cli, sourceRes, func() error {
+			controllerutil.RemoveFinalizer(sourceRes, finalizerName)
 
-		if err := cli.Update(ctx, sourceRes); err != nil {
-			return fmt.Errorf("failed to remove finalizer: %w", err)
+			return nil
+		}); err != nil {
+			return fmt.Errorf("failed to remove finalizer from %s (in %s): %w",
+				sourceRes.GroupVersionKind().String(), sourceRes.GetNamespace(), err)
 		}
 	}
 

controllers/routingctrl/reconcile_resources.go

@@ -23,12 +23,8 @@ func (r *Controller) createRoutingResources(ctx context.Context, target *unstruc
 
 	if len(exportModes) == 0 {
 		r.log.Info("No export mode found for target")
-		metadata.ApplyMetaOptions(target,
-			annotations.Remove(annotations.RoutingAddressesExternal("")),
-			annotations.Remove(annotations.RoutingAddressesPublic("")),
-		)
 
-		return nil
+		return r.propagateHostsToWatchedCR(ctx, target, nil, nil)
 	}
 
 	r.log.Info("Reconciling resources for target", "target", target)
@@ -97,33 +93,46 @@ func (r *Controller) exportService(ctx context.Context, target *unstructured.Uns
 		}
 	}
 
-	return r.propagateHostsToWatchedCR(target, publicHosts, externalHosts)
+	return r.propagateHostsToWatchedCR(ctx, target, publicHosts, externalHosts)
 }
 
-func (r *Controller) propagateHostsToWatchedCR(target *unstructured.Unstructured, publicHosts, externalHosts []string) error {
-	// Remove all existing routing addresses
-	metaOptions := []metadata.Option{
-		annotations.Remove(annotations.RoutingAddressesExternal("")),
-		annotations.Remove(annotations.RoutingAddressesPublic("")),
-	}
+func (r *Controller) propagateHostsToWatchedCR(ctx context.Context, target *unstructured.Unstructured, publicHosts, externalHosts []string) error {
+	err := unstruct.PatchWithRetry(ctx, r.Client, target, func() error {
+		// Always remove the annotations first
+		annotations.Remove(annotations.RoutingAddressesExternal(""))(target)
+		annotations.Remove(annotations.RoutingAddressesPublic(""))(target)
 
-	if len(publicHosts) > 0 {
-		metaOptions = append(metaOptions, annotations.RoutingAddressesPublic(strings.Join(publicHosts, ";")))
-	}
+		var metaOptions []metadata.Option
 
-	if len(externalHosts) > 0 {
-		metaOptions = append(metaOptions, annotations.RoutingAddressesExternal(strings.Join(externalHosts, ";")))
-	}
+		if len(publicHosts) > 0 {
+			metaOptions = append(metaOptions, annotations.RoutingAddressesPublic(strings.Join(publicHosts, ";")))
+		}
+
+		if len(externalHosts) > 0 {
+			metaOptions = append(metaOptions, annotations.RoutingAddressesExternal(strings.Join(externalHosts, ";")))
+		}
 
-	metadata.ApplyMetaOptions(target, metaOptions...)
+		metadata.ApplyMetaOptions(target, metaOptions...)
+
+		return nil
+	})
+
+	if err != nil {
+		return fmt.Errorf("failed to propagate hosts to watched CR %s/%s: %w", target.GetNamespace(), target.GetName(), err)
+	}
 
 	return nil
 }
 
 func (r *Controller) ensureResourceHasFinalizer(ctx context.Context, target *unstructured.Unstructured) error {
-	if controllerutil.AddFinalizer(target, finalizerName) {
-		if err := unstruct.Patch(ctx, r.Client, target); err != nil {
-			return fmt.Errorf("failed to patch finalizer to resource %s/%s: %w", target.GetNamespace(), target.GetName(), err)
+	if !controllerutil.ContainsFinalizer(target, finalizerName) {
+		if err := unstruct.PatchWithRetry(ctx, r.Client, target, func() error {
+			controllerutil.AddFinalizer(target, finalizerName)
+
+			return nil
+		}); err != nil {
+			return fmt.Errorf("failed to patch finalizer to %s (in %s): %w",
+				target.GroupVersionKind().String(), target.GetNamespace(), err)
 		}
 	}
 

pkg/unstruct/funcs.go

@@ -10,6 +10,7 @@ import (
 	k8stypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
 func Apply(ctx context.Context, cli client.Client, objects []*unstructured.Unstructured, metaOptions ...metadata.Option) error {
@@ -63,23 +64,13 @@ func IsMarkedForDeletion(target *unstructured.Unstructured) bool {
 	return !target.GetDeletionTimestamp().IsZero()
 }
 
-// Patch updates the specified Kubernetes resource by applying changes from the provided target object.
-// In case of conflicts, it will retry using default strategy.
-func Patch(ctx context.Context, cli client.Client, target *unstructured.Unstructured) error {
-	err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
-		currentRes := &unstructured.Unstructured{}
-		currentRes.SetGroupVersionKind(target.GroupVersionKind())
+// PatchWithRetry applies changes to the specified resource using the provided mutate function.
+// It uses a retry mechanism to handle potential conflicts during the update process.
+func PatchWithRetry(ctx context.Context, cli client.Client, target *unstructured.Unstructured, mutate controllerutil.MutateFn) error {
+	err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+		_, err := controllerutil.CreateOrPatch(ctx, cli, target, mutate)
 
-		if err := cli.Get(ctx, client.ObjectKeyFromObject(target), currentRes); err != nil {
-			return fmt.Errorf("failed re-fetching resource: %w", err)
-		}
-
-		patch := client.MergeFrom(currentRes)
-		if errPatch := cli.Patch(ctx, target, patch); errPatch != nil {
-			return fmt.Errorf("failed to patch: %w", errPatch)
-		}
-
-		return nil
+		return err //nolint:wrapcheck // Return unwrapped error per RetryOnConflict godoc
 	})
 
 	if err != nil {