Skip to content

Commit

Permalink
use CreateOrPatch, only update resource when needed
Browse files Browse the repository at this point in the history
  • Loading branch information
cam-garrison committed Sep 4, 2024
1 parent 78a6003 commit 3c70e9d
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 55 deletions.
2 changes: 0 additions & 2 deletions controllers/routingctrl/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}

Expand Down
10 changes: 7 additions & 3 deletions controllers/routingctrl/delete_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
}

Expand Down
49 changes: 28 additions & 21 deletions controllers/routingctrl/reconcile_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -97,38 +93,49 @@ 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.ContainsFinalizer(target, finalizerName) {
if err := unstruct.PatchMutate(ctx, r.Client, target, func() error {
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 resource %s/%s: %w", target.GetNamespace(), target.GetName(), err)
return fmt.Errorf("failed to patch finalizer to %s (in %s): %w",
target.GroupVersionKind().String(), target.GetNamespace(), err)
}
}

return nil
}

Expand Down
33 changes: 4 additions & 29 deletions pkg/unstruct/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,38 +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 PatchMutate(ctx context.Context, cli client.Client, target *unstructured.Unstructured, mutate controllerutil.MutateFn) error {
// 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)
return err
})

if err != nil {
return fmt.Errorf("failed to patchmutate resource metadata with retry: %w", err)
}

return nil
}

// 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.DefaultBackoff, func() error {
currentRes := &unstructured.Unstructured{}
currentRes.SetGroupVersionKind(target.GroupVersionKind())

if err := cli.Get(ctx, client.ObjectKeyFromObject(target), currentRes); err != nil {
return err //nolint:wrapcheck // Return unwrapped error for retry logic
}

patch := client.MergeFrom(target)
if errPatch := cli.Patch(ctx, currentRes, patch); errPatch != nil {
return errPatch //nolint:wrapcheck // Return unwrapped error for retry logic
}

return nil
return err //nolint:wrapcheck // Return unwrapped error per RetryOnConflict godoc
})

if err != nil {
Expand Down

0 comments on commit 3c70e9d

Please sign in to comment.