fix: Set state only after status report

Set the state annotation and labels in pipelineascode only after
the state was reported to the SCM.

This is achieved by patching the PLR with the state label/annotation
after it was created.

This change is needed in order to avoid a case where the watcher will report
a status before PAC.

With this change the issue can't happen, since the watcher only reconciles
PLRs that has the state annotation, so the first reconcile of the PLRs
will happen only after PAC already sent the initial status to the SCM.

In addition, simplify the condition which checks if the "queued" state
annotation/label should be set. We can only check if the PLR is pending,
it doesn't matter if the reason is because of PAC's concurrency control
or an external controller.

Signed-off-by: Gal Ben Haim <[email protected]>

Author: gbenhaim

pkg/kubeinteraction/labels.go

@@ -55,7 +55,6 @@ func AddLabelsAndAnnotations(event *info.Event, pipelineRun *tektonv1.PipelineRu
 		keys.SourceBranch:  event.HeadBranch,
 		keys.Repository:    repo.GetName(),
 		keys.GitProvider:   providerConfig.Name,
-		keys.State:         StateStarted,
 		keys.ControllerInfo: fmt.Sprintf(`{"name":"%s","configmap":"%s","secret":"%s", "gRepo": "%s"}`,
 			paramsinfo.Controller.Name, paramsinfo.Controller.Configmap, paramsinfo.Controller.Secret, paramsinfo.Controller.GlobalRepository),
 	}

pkg/pipelineascode/pipelineascode.go

@@ -176,14 +176,10 @@ func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.Pi
 		p.logger.Errorf("Error adding labels/annotations to PipelineRun '%s' in namespace '%s': %v", match.PipelineRun.GetName(), match.Repo.GetNamespace(), err)
 	}
 
-	// if concurrency is defined then start the pipelineRun in pending state and
-	// state as queued
+	// if concurrency is defined then start the pipelineRun in pending state
 	if match.Repo.Spec.ConcurrencyLimit != nil && *match.Repo.Spec.ConcurrencyLimit != 0 {
 		// pending status
 		match.PipelineRun.Spec.Status = tektonv1.PipelineRunSpecStatusPending
-		// pac state as queued
-		match.PipelineRun.Labels[keys.State] = kubeinteraction.StateQueued
-		match.PipelineRun.Annotations[keys.State] = kubeinteraction.StateQueued
 	}
 
 	// Create the actual pipelineRun
@@ -240,26 +236,31 @@ func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.Pi
 		OriginalPipelineRunName: pr.GetAnnotations()[keys.OriginalPRName],
 	}
 
+	// Patch the pipelineRun with the appropriate annotations and labels.
+	// Set the state so the watcher will continue with reconciling the pipelineRun
+	// The watcher reconciles only pipelineRuns that has the state annotation.
 	patchAnnotations := map[string]string{}
 	patchLabels := map[string]string{}
 	whatPatching := ""
 	// if pipelineRun is in pending state then report status as queued
+	// The pipelineRun can be pending because of PAC's concurrency limit or because of an external mutatingwebhook
 	if pr.Spec.Status == tektonv1.PipelineRunSpecStatusPending {
 		status.Status = queuedStatus
 		if status.Text, err = mt.MakeTemplate(p.vcx.GetTemplate(provider.QueueingPipelineType)); err != nil {
 			return nil, fmt.Errorf("cannot create message template: %w", err)
 		}
-		// If the PipelineRun is in the "queued" state, add the appropriate label and annotation.
-		// These are later used by the watcher to determine whether the PipelineRun status
-		// should be reported back to the Git provider. We do add the `state` annotations and label when
-		// concurrency is enabled but this would happen when PipelineRun's status has been changed by
-		// the other controller and PaC is not aware of that change.
 		whatPatching = "annotations.state and labels.state"
 		patchAnnotations[keys.State] = kubeinteraction.StateQueued
 		patchLabels[keys.State] = kubeinteraction.StateQueued
 	} else {
+		// Mark that the start will be reported to the Git provider
 		patchAnnotations[keys.SCMReportingPLRStarted] = "true"
-		whatPatching = fmt.Sprintf("annotation.%s", keys.SCMReportingPLRStarted)
+		patchAnnotations[keys.State] = kubeinteraction.StateStarted
+		patchLabels[keys.State] = kubeinteraction.StateStarted
+		whatPatching = fmt.Sprintf(
+			"annotation.%s and annotations.state and labels.state",
+			keys.SCMReportingPLRStarted,
+		)
 	}
 
 	if err := p.vcx.CreateStatus(ctx, p.event, status); err != nil {

pkg/pipelineascode/pipelineascode_test.go

@@ -718,7 +718,7 @@ func TestRun(t *testing.T) {
 						secretName, ok := pr.GetAnnotations()[keys.GitAuthSecret]
 						assert.Assert(t, ok, "Cannot find secret %s on annotations", secretName)
 					}
-					if tt.concurrencyLimit > 0 || pr.Spec.Status == pipelinev1.PipelineRunSpecStatusPending {
+					if pr.Spec.Status == pipelinev1.PipelineRunSpecStatusPending {
 						state, ok := pr.GetAnnotations()[keys.State]
 						assert.Assert(t, ok, "State hasn't been set on PR", state)
 						assert.Equal(t, state, kubeinteraction.StateQueued)