From cabf085e6c6083b2ac514d62d6a305e4afec896d Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Fri, 10 Jan 2025 13:42:53 +0000 Subject: [PATCH] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! General improvements --- chart/Chart.yaml | 4 +- chart/values.yaml | 2 +- .../clusterimageexport_controller.go | 76 ++++++++++++++----- 3 files changed, 58 insertions(+), 24 deletions(-) diff --git a/chart/Chart.yaml b/chart/Chart.yaml index 9677e0d..8b9c67a 100644 --- a/chart/Chart.yaml +++ b/chart/Chart.yaml @@ -10,9 +10,9 @@ description: | type: application -version: 0.2.34 +version: 0.2.35 -appVersion: "0.2.34" +appVersion: "0.2.35" home: https://github.com/lukaszraczylo/kubernetes-images-sync-operator diff --git a/chart/values.yaml b/chart/values.yaml index 45df93d..a51d9d8 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -12,7 +12,7 @@ sa: - ALL image: repository: ghcr.io/lukaszraczylo/kubernetes-images-sync-operator - tag: 0.2.34 + tag: 0.2.35 resources: limits: cpu: 500m diff --git a/internal/controller/raczylo.com/clusterimageexport_controller.go b/internal/controller/raczylo.com/clusterimageexport_controller.go index 18f46d3..cf3b8de 100644 --- a/internal/controller/raczylo.com/clusterimageexport_controller.go +++ b/internal/controller/raczylo.com/clusterimageexport_controller.go @@ -12,6 +12,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/util/retry" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -81,25 +82,29 @@ func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.R } } - // Reset status if the ClusterImageExport is in a completed state - if clusterImageExport.Status.Progress == shared.STATUS_SUCCESS || clusterImageExport.Status.Progress == shared.STATUS_FAILED { - // Reset the status to allow reprocessing - clusterImageExport.Status.Progress = "" - if err := r.Status().Update(ctx, clusterImageExport); err != nil { - l.Error(err, "unable to reset ClusterImageExport status") - return ctrl.Result{}, err + // Handle status updates with retries + if err := r.updateStatusWithRetry(ctx, clusterImageExport, func(export *raczylocomv1.ClusterImageExport) error { + // Reset status if the ClusterImageExport is in a completed state + if export.Status.Progress == shared.STATUS_SUCCESS || export.Status.Progress == shared.STATUS_FAILED { + export.Status.Progress = "" + return nil } - // Requeue to process with reset status - return ctrl.Result{Requeue: true}, nil + + // Set to PENDING if empty + if export.Status.Progress == "" { + export.Status.Progress = shared.STATUS_PENDING + return nil + } + + return nil + }); err != nil { + l.Error(err, "unable to update ClusterImageExport status") + return ctrl.Result{}, err } - // If the status is empty, set it to PENDING + // If we just reset the status, requeue to process with reset status if clusterImageExport.Status.Progress == "" { - clusterImageExport.Status.Progress = shared.STATUS_PENDING - if err := r.Status().Update(ctx, clusterImageExport); err != nil { - l.Error(err, "unable to update ClusterImageExport status") - return ctrl.Result{}, err - } + return ctrl.Result{Requeue: true}, nil } // Proceed with the rest of the reconciliation logic @@ -120,8 +125,11 @@ func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.R } } - clusterImageExport.Status.Progress = shared.STATUS_RUNNING - if err := r.Status().Update(ctx, clusterImageExport); err != nil { + // Update status to RUNNING with retry + if err := r.updateStatusWithRetry(ctx, clusterImageExport, func(export *raczylocomv1.ClusterImageExport) error { + export.Status.Progress = shared.STATUS_RUNNING + return nil + }); err != nil { l.Error(err, "unable to update ClusterImageExport status to RUNNING") return ctrl.Result{}, err } @@ -137,8 +145,10 @@ func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.R if err == nil { // ClusterImage exists, check its status if clusterImage.Status.Progress == shared.STATUS_FAILED { - clusterImageExport.Status.Progress = shared.STATUS_FAILED - if err := r.Status().Update(ctx, clusterImageExport); err != nil { + if err := r.updateStatusWithRetry(ctx, clusterImageExport, func(export *raczylocomv1.ClusterImageExport) error { + export.Status.Progress = shared.STATUS_FAILED + return nil + }); err != nil { l.Error(err, "unable to update ClusterImageExport status to FAILED") return ctrl.Result{}, err } @@ -193,8 +203,10 @@ func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.R } if allCompleted { - clusterImageExport.Status.Progress = shared.STATUS_SUCCESS - if err := r.Status().Update(ctx, clusterImageExport); err != nil { + if err := r.updateStatusWithRetry(ctx, clusterImageExport, func(export *raczylocomv1.ClusterImageExport) error { + export.Status.Progress = shared.STATUS_SUCCESS + return nil + }); err != nil { l.Error(err, "unable to update ClusterImageExport status to SUCCESS") return ctrl.Result{}, err } @@ -203,6 +215,28 @@ func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{Requeue: !allCompleted}, nil } +// updateStatusWithRetry attempts to update the status of a ClusterImageExport with retries +func (r *ClusterImageExportReconciler) updateStatusWithRetry(ctx context.Context, clusterImageExport *raczylocomv1.ClusterImageExport, updateFn func(*raczylocomv1.ClusterImageExport) error) error { + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + // Get the latest version of the resource + latest := &raczylocomv1.ClusterImageExport{} + if err := r.Get(ctx, client.ObjectKey{ + Namespace: clusterImageExport.Namespace, + Name: clusterImageExport.Name, + }, latest); err != nil { + return err + } + + // Apply the update function to the latest version + if err := updateFn(latest); err != nil { + return err + } + + // Update the status + return r.Status().Update(ctx, latest) + }) +} + func (r *ClusterImageExportReconciler) checkAllClusterImagesCompleted(ctx context.Context, clusterImageExport *raczylocomv1.ClusterImageExport) (bool, error) { clusterImageList := &raczylocomv1.ClusterImageList{} if err := r.List(ctx, clusterImageList, client.InNamespace(clusterImageExport.Namespace), client.MatchingFields{"spec.exportName": clusterImageExport.Name}); err != nil {