fix(KONFLUX-10903): add retry logic for PipelineRun finalizer addition

add RetryOnConflict wrapper to EnsurePipelineIsFinalized handle etcd
timeout errors when adding finalizers.Modified AddFinalizerToPipelineRun
to return unwrapped errors so RetryOnConflict can properly detect and
retry transient errors and conflicts.

Signed-off-by: Kasem Alem <[email protected]>

Author: kasemAlem

helpers/integration.go

@@ -444,7 +444,10 @@ func AddFinalizerToPipelineRun(ctx context.Context, adapterClient client.Client,
 	if ok := controllerutil.AddFinalizer(pipelineRun, finalizer); ok {
 		err := adapterClient.Patch(ctx, pipelineRun, patch)
 		if err != nil {
-			return fmt.Errorf("error occurred while patching the updated PipelineRun after finalizer addition: %w", err)
+			logger.Error(err, "error occurred while patching the updated PipelineRun after finalizer addition",
+				"pipelineRun.Name", pipelineRun.Name)
+			// don't return wrapped err, so we can use RetryOnConflict
+			return err
 		}
 
 		logger.LogAuditEvent("Added Finalizer to the PipelineRun", pipelineRun, LogActionUpdate, "finalizer", finalizer)

internal/controller/buildpipeline/buildpipeline_adapter.go

@@ -193,12 +193,32 @@ func (a *Adapter) EnsurePipelineIsFinalized() (controller.OperationResult, error
 		return controller.ContinueProcessing()
 	}
 
-	// if pipelinerun has been deleted, do not add finalilzer
+	// if pipelinerun has been deleted, do not add finalizer
 	if a.pipelineRun.GetDeletionTimestamp() != nil {
 		return controller.ContinueProcessing()
 	}
 
-	err := h.AddFinalizerToPipelineRun(a.context, a.client, a.logger, a.pipelineRun, h.IntegrationPipelineRunFinalizer)
+	err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		var err error
+		// Refetch the PipelineRun to get the latest version before patching
+		a.pipelineRun, err = a.loader.GetPipelineRun(a.context, a.client, a.pipelineRun.Name, a.pipelineRun.Namespace)
+		if err != nil {
+			return err
+		}
+
+		// if pipelinerun has been deleted, do not add finalizer
+		if a.pipelineRun.GetDeletionTimestamp() != nil {
+			return nil
+		}
+
+		// Check again if finalizer was already added (might have been added by another reconcile)
+		if controllerutil.ContainsFinalizer(a.pipelineRun, h.IntegrationPipelineRunFinalizer) {
+			return nil
+		}
+
+		err = h.AddFinalizerToPipelineRun(a.context, a.client, a.logger, a.pipelineRun, h.IntegrationPipelineRunFinalizer)
+		return err
+	})
 	if err != nil {
 		// if IsNotFound error, do not log error or requeue
 		if errors.IsNotFound(err) {

internal/controller/buildpipeline/buildpipeline_adapter_test.go

@@ -1087,9 +1087,25 @@ var _ = Describe("Pipeline Adapter", Ordered, func() {
 
 				// Ensure PLR has finalizer
 				Eventually(func() bool {
-					return slices.Contains(buildPipelineRun.ObjectMeta.Finalizers, helpers.IntegrationPipelineRunFinalizer)
+					// Refetch the PipelineRun from the cluster to get the updated finalizer
+					existingBuildPLR := new(tektonv1.PipelineRun)
+					err := k8sClient.Get(ctx, types.NamespacedName{
+						Namespace: buildPipelineRun.Namespace,
+						Name:      buildPipelineRun.Name,
+					}, existingBuildPLR)
+					if err != nil {
+						return false
+					}
+					return slices.Contains(existingBuildPLR.ObjectMeta.Finalizers, helpers.IntegrationPipelineRunFinalizer)
 				}, time.Second*10).Should(BeTrue())
 
+				// Refetch the PipelineRun to get the latest ResourceVersion before updating status
+				err = k8sClient.Get(ctx, types.NamespacedName{
+					Namespace: buildPipelineRun.Namespace,
+					Name:      buildPipelineRun.Name,
+				}, buildPipelineRun)
+				Expect(err).ToNot(HaveOccurred())
+
 				// Update build PLR as completed
 				buildPipelineRun.Status = tektonv1.PipelineRunStatus{
 					PipelineRunStatusFields: tektonv1.PipelineRunStatusFields{

internal/controller/buildpipeline/buildpipeline_controller.go

@@ -87,7 +87,16 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	})
 	if err != nil {
 		if errors.IsNotFound(err) {
-			if err := helpers.RemoveFinalizerFromPipelineRun(ctx, r.Client, logger, pipelineRun, helpers.IntegrationPipelineRunFinalizer); err != nil {
+			// Use retry logic to handle etcd timeouts when removing finalizer
+			err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
+				// Refetch the PipelineRun to get the latest version before removing finalizer
+				err := r.Get(ctx, req.NamespacedName, pipelineRun)
+				if err != nil {
+					return err
+				}
+				return helpers.RemoveFinalizerFromPipelineRun(ctx, r.Client, logger, pipelineRun, helpers.IntegrationPipelineRunFinalizer)
+			})
+			if err != nil {
 				return ctrl.Result{}, err
 			}
 		}