diff --git a/controllers/routingctrl/controller.go b/controllers/routingctrl/controller.go index b7e7b6d..b4b49cf 100644 --- a/controllers/routingctrl/controller.go +++ b/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...) } diff --git a/controllers/routingctrl/delete_resources.go b/controllers/routingctrl/delete_resources.go index c501686..b32414b 100644 --- a/controllers/routingctrl/delete_resources.go +++ b/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) } } diff --git a/controllers/routingctrl/reconcile_resources.go b/controllers/routingctrl/reconcile_resources.go index aca918a..afa2487 100644 --- a/controllers/routingctrl/reconcile_resources.go +++ b/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 { + // Remove all existing routing addresses + metaOptions := []metadata.Option{ + annotations.Remove(annotations.RoutingAddressesExternal("")), + annotations.Remove(annotations.RoutingAddressesPublic("")), + } - if len(publicHosts) > 0 { - metaOptions = append(metaOptions, annotations.RoutingAddressesPublic(strings.Join(publicHosts, ";"))) - } + if len(publicHosts) > 0 { + metaOptions = append(metaOptions, annotations.RoutingAddressesPublic(strings.Join(publicHosts, ";"))) + } - if len(externalHosts) > 0 { - metaOptions = append(metaOptions, annotations.RoutingAddressesExternal(strings.Join(externalHosts, ";"))) - } + if len(externalHosts) > 0 { + metaOptions = append(metaOptions, annotations.RoutingAddressesExternal(strings.Join(externalHosts, ";"))) + } + + metadata.ApplyMetaOptions(target, metaOptions...) + + return nil + }) - metadata.ApplyMetaOptions(target, metaOptions...) + 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 add finalizer to %s (in %s): %w", + target.GroupVersionKind().String(), target.GetNamespace(), err) } } diff --git a/pkg/unstruct/funcs.go b/pkg/unstruct/funcs.go index c587a26..bd3cfde 100644 --- a/pkg/unstruct/funcs.go +++ b/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 {