Reconciler: Retry reporting provider status if failed

reconciler will retry to report provider status if failed now
with a backoff time. It will try 3 times. if it still fails for
reporting the final status then it updates the `state` to `failed`.
so that those pipelineruns can be queried.

Signed-off-by: Shivam Mukhade <[email protected]>

Author: Shivam Mukhade

pkg/kubeinteraction/labels.go

@@ -16,6 +16,7 @@ const (
 	StateStarted   = "started"
 	StateQueued    = "queued"
 	StateCompleted = "completed"
+	StateFailed    = "failed"
 )
 
 func AddLabelsAndAnnotations(event *info.Event, pipelineRun *tektonv1beta1.PipelineRun, repo *apipac.Repository, providerinfo *info.ProviderConfig) {

pkg/reconciler/reconciler.go

@@ -44,9 +44,9 @@ var (
 func (r *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) pkgreconciler.Event {
 	logger := logging.FromContext(ctx)
 
-	// if pipelineRun is in completed state then return
+	// if pipelineRun is in completed or failed state then return
 	state, exist := pr.GetLabels()[keys.State]
-	if exist && state == kubeinteraction.StateCompleted {
+	if exist && (state == kubeinteraction.StateCompleted || state == kubeinteraction.StateFailed) {
 		return nil
 	}
 
@@ -113,16 +113,18 @@ func (r *Reconciler) reportFinalStatus(ctx context.Context, logger *zap.SugaredL
 		return repo, fmt.Errorf("cannot clean prs: %w", err)
 	}
 
+	finalState := kubeinteraction.StateCompleted
 	newPr, err := r.postFinalStatus(ctx, logger, provider, event, pr)
 	if err != nil {
-		return repo, fmt.Errorf("cannot post final status: %w", err)
+		logger.Errorf("failed to post final status, moving on: %v", err)
+		finalState = kubeinteraction.StateFailed
 	}
 
 	if err := r.updateRepoRunStatus(ctx, logger, newPr, repo, event); err != nil {
 		return repo, fmt.Errorf("cannot update run status: %w", err)
 	}
 
-	if _, err := r.updatePipelineRunState(ctx, logger, pr, kubeinteraction.StateCompleted); err != nil {
+	if _, err := r.updatePipelineRunState(ctx, logger, pr, finalState); err != nil {
 		return repo, fmt.Errorf("cannot update state: %w", err)
 	}
 
@@ -186,8 +188,11 @@ func (r *Reconciler) updatePipelineRunToInProgress(ctx context.Context, logger *
 		OriginalPipelineRunName: pr.GetLabels()[keys.OriginalPRName],
 	}
 
-	if err := p.CreateStatus(ctx, r.run.Clients.Tekton, event, r.run.Info.Pac, status); err != nil {
-		return fmt.Errorf("cannot create a in_progress status on the provider platform: %w", err)
+	if err := createStatusWithRetry(ctx, logger, r.run.Clients.Tekton, p, event, r.run.Info.Pac, status); err != nil {
+		// if failed to report status for running state, let the pipelineRun continue,
+		// pipelineRun is already started so we will try again once it completes
+		logger.Errorf("failed to report status to running on provider continuing! error: %v", err)
+		return nil
 	}
 
 	logger.Info("updated in_progress status on provider platform for pipelineRun ", pr.GetName())

pkg/reconciler/status.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/google/go-github/v48/github"
 	apipac "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
@@ -16,6 +17,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/secrets"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/sort"
 	tektonv1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
+	"github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
 	"go.uber.org/zap"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -26,6 +28,12 @@ const (
 	failureReasonText       = "%s<br><h4>Failure reason</h4><br>%s"
 )
 
+var backoffSchedule = []time.Duration{
+	1 * time.Second,
+	3 * time.Second,
+	5 * time.Second,
+}
+
 func (r *Reconciler) updateRepoRunStatus(ctx context.Context, logger *zap.SugaredLogger, pr *tektonv1beta1.PipelineRun, repo *pacv1a1.Repository, event *info.Event) error {
 	refsanitized := formatting.SanitizeBranch(event.BaseBranch)
 	repoStatus := pacv1a1.RepositoryRunStatus{
@@ -121,7 +129,21 @@ func (r *Reconciler) postFinalStatus(ctx context.Context, logger *zap.SugaredLog
 		OriginalPipelineRunName: pr.GetLabels()[apipac.OriginalPRName],
 	}
 
-	err = vcx.CreateStatus(ctx, r.run.Clients.Tekton, event, r.run.Info.Pac, status)
+	err = createStatusWithRetry(ctx, logger, r.run.Clients.Tekton, vcx, event, r.run.Info.Pac, status)
 	logger.Infof("pipelinerun %s has a status of '%s'", pr.Name, status.Conclusion)
 	return pr, err
 }
+
+func createStatusWithRetry(ctx context.Context, logger *zap.SugaredLogger, tekton versioned.Interface, vcx provider.Interface, event *info.Event, opts *info.PacOpts, status provider.StatusOpts) error {
+	var finalError error
+	for _, backoff := range backoffSchedule {
+		err := vcx.CreateStatus(ctx, tekton, event, opts, status)
+		if err == nil {
+			return nil
+		}
+		logger.Infof("failed to create status, error: %v, retrying in %v", err, backoff)
+		time.Sleep(backoff)
+		finalError = err
+	}
+	return fmt.Errorf("failed to report status: %w", finalError)
+}

pkg/reconciler/status_test.go

@@ -0,0 +1,31 @@
+package reconciler
+
+import (
+	"context"
+	"testing"
+
+	provider2 "github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/test/provider"
+	"go.uber.org/zap"
+	zapobserver "go.uber.org/zap/zaptest/observer"
+	"gotest.tools/v3/assert"
+)
+
+func TestCreateStatusWithRetry(t *testing.T) {
+	observer, _ := zapobserver.New(zap.InfoLevel)
+	fakelogger := zap.New(observer).Sugar()
+	vcx := provider.TestProviderImp{}
+
+	err := createStatusWithRetry(context.TODO(), fakelogger, nil, &vcx, nil, nil, provider2.StatusOpts{})
+	assert.NilError(t, err)
+}
+
+func TestCreateStatusWithRetry_ErrorCase(t *testing.T) {
+	observer, _ := zapobserver.New(zap.InfoLevel)
+	fakelogger := zap.New(observer).Sugar()
+	vcx := provider.TestProviderImp{}
+	vcx.CreateStatusErorring = true
+
+	err := createStatusWithRetry(context.TODO(), fakelogger, nil, &vcx, nil, nil, provider2.StatusOpts{})
+	assert.Error(t, err, "failed to report status: some provider error occurred while reporting status")
+}

pkg/test/provider/testwebvcs.go

@@ -63,7 +63,7 @@ func (v *TestProviderImp) GetTaskURI(ctx context.Context, params *params.Run, ev
 
 func (v *TestProviderImp) CreateStatus(ctx context.Context, _ versioned.Interface, event *info.Event, opts *info.PacOpts, statusOpts provider.StatusOpts) error {
 	if v.CreateStatusErorring {
-		return fmt.Errorf("you want me to error I error for you")
+		return fmt.Errorf("some provider error occurred while reporting status")
 	}
 	return nil
 }