fix: cancel-in-progress with retest comment

fix cancel-in-progress on /retest /test /ok-to-test or any gitops
comments variable.

Since when we set the trigger event to the test comment like retest-all
instead of pull_request on the label of the pipelinerun we can't rely on
that to group the pipelinerun for cancellation.

We now have a helper function to do a label select to any pipelinerun
that has the gitops comments as event_type or pull_request

Signed-off-by: Chmouel Boudjnah <[email protected]>

Author: chmouel

pkg/opscomments/comments.go

@@ -5,11 +5,12 @@ import (
 	"regexp"
 	"strings"
 
+	"go.uber.org/zap"
+
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/events"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
-	"go.uber.org/zap"
 )
 
 var (
@@ -122,6 +123,20 @@ func IsAnyOpsEventType(eventType string) bool {
 		eventType == OnCommentEventType.String()
 }
 
+// AnyOpsKubeLabelInSelector will output a Kubernetes label out of all possible
+// CommentEvent Type for selection.
+func AnyOpsKubeLabelInSelector() string {
+	return fmt.Sprintf("%s,%s,%s,%s,%s,%s,%s,%s",
+		TestSingleCommentEventType.String(),
+		TestAllCommentEventType.String(),
+		RetestAllCommentEventType.String(),
+		RetestSingleCommentEventType.String(),
+		CancelCommentSingleEventType.String(),
+		CancelCommentAllEventType.String(),
+		OkToTestCommentEventType.String(),
+		OnCommentEventType.String())
+}
+
 func GetPipelineRunFromTestComment(comment string) string {
 	if strings.Contains(comment, testComment) {
 		return getNameFromComment(testComment, comment)

pkg/opscomments/comments_test.go

@@ -1,18 +1,20 @@
 package opscomments
 
 import (
+	"strings"
 	"testing"
 
+	"go.uber.org/zap"
+	zapobserver "go.uber.org/zap/zaptest/observer"
+	"gotest.tools/v3/assert"
+	rtesting "knative.dev/pkg/reconciler/testing"
+
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/events"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
 	testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients"
 	testnewrepo "github.com/openshift-pipelines/pipelines-as-code/pkg/test/repository"
-	"go.uber.org/zap"
-	zapobserver "go.uber.org/zap/zaptest/observer"
-	"gotest.tools/v3/assert"
-	rtesting "knative.dev/pkg/reconciler/testing"
 )
 
 func TestLabelsBackwardCompat(t *testing.T) {
@@ -727,3 +729,7 @@ func TestGetPipelineRunAndBranchNameFromCancelComment(t *testing.T) {
 		})
 	}
 }
+
+func TestAnyOpsKubeLabelInSelector(t *testing.T) {
+	assert.Assert(t, strings.Contains(AnyOpsKubeLabelInSelector(), RetestSingleCommentEventType.String()))
+}

pkg/pipelineascode/cancel_pipelineruns.go

@@ -16,6 +16,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/formatting"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
 )
 
@@ -51,12 +52,14 @@ func (p *PacRun) cancelInProgress(ctx context.Context, matchPR *tektonv1.Pipelin
 	labelMap := map[string]string{
 		keys.URLRepository:  formatting.CleanValueKubernetes(p.event.Repository),
 		keys.OriginalPRName: prName,
-		keys.EventType:      p.event.TriggerTarget.String(),
 	}
 	if p.event.TriggerTarget == triggertype.PullRequest {
 		labelMap[keys.PullRequest] = strconv.Itoa(p.event.PullRequestNumber)
 	}
 	labelSelector := getLabelSelector(labelMap)
+	if p.event.TriggerTarget == triggertype.PullRequest {
+		labelSelector += fmt.Sprintf(",%s in (pull_request, %s)", keys.EventType, opscomments.AnyOpsKubeLabelInSelector())
+	}
 	p.run.Clients.Log.Infof("cancel-in-progress: selecting pipelineRuns to cancel with labels: %v", labelSelector)
 	prs, err := p.run.Clients.Tekton.TektonV1().PipelineRuns(matchPR.GetNamespace()).List(ctx, metav1.ListOptions{
 		LabelSelector: labelSelector,

pkg/pipelineascode/cancel_pipelineruns_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/formatting"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
@@ -492,7 +493,7 @@ func TestCancelInProgress(t *testing.T) {
 			},
 		},
 		{
-			name: "cancel in progress",
+			name: "match/cancel in progress",
 			event: &info.Event{
 				Repository:        "foo",
 				SHA:               "foosha",
@@ -537,7 +538,58 @@ func TestCancelInProgress(t *testing.T) {
 			wantLog: "cancel-in-progress: cancelling pipelinerun foo/",
 		},
 		{
-			name: "cancel in progress exclude not belonging to same push branch",
+			name: "match/cancel in progress from /retest",
+			event: &info.Event{
+				Repository:        "foo",
+				SHA:               "foosha",
+				HeadBranch:        "head",
+				EventType:         string(triggertype.PullRequest),
+				TriggerTarget:     triggertype.PullRequest,
+				PullRequestNumber: pullReqNumber,
+			},
+			pipelineRuns: []*pipelinev1.PipelineRun{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "pr-foo",
+						Namespace: "foo",
+						Labels:    fooRepoLabels,
+						Annotations: map[string]string{
+							keys.CancelInProgress: "true",
+							keys.OriginalPRName:   "pr-foo",
+							keys.Repository:       "foo",
+							keys.SourceBranch:     "head",
+						},
+					},
+					Spec: pipelinev1.PipelineRunSpec{},
+				},
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "pr-foo-1",
+						Namespace: "foo",
+						Labels: map[string]string{
+							keys.OriginalPRName: "pr-foo",
+							keys.URLRepository:  formatting.CleanValueKubernetes("foo"),
+							keys.SHA:            formatting.CleanValueKubernetes("foosha"),
+							keys.PullRequest:    strconv.Itoa(pullReqNumber),
+							keys.EventType:      string(opscomments.RetestAllCommentEventType),
+						},
+						Annotations: map[string]string{
+							keys.CancelInProgress: "true", keys.OriginalPRName: "pr-foo",
+							keys.Repository:   "foo",
+							keys.SourceBranch: "head",
+						},
+					},
+					Spec: pipelinev1.PipelineRunSpec{},
+				},
+			},
+			repo: fooRepo,
+			cancelledPipelineRuns: map[string]bool{
+				"pr-foo-1": true,
+			},
+			wantLog: "cancel-in-progress: cancelling pipelinerun foo/pr-foo-1",
+		},
+		{
+			name: "match/cancel in progress exclude not belonging to same push branch",
 			event: &info.Event{
 				Repository:    "foo",
 				SHA:           "foosha",
@@ -609,7 +661,7 @@ func TestCancelInProgress(t *testing.T) {
 			wantLog: "skipping pipelinerun foo/pr-foo-1 as it is not from the same branch",
 		},
 		{
-			name: "cancel in progress exclude not belonging to same pr",
+			name: "match/cancel in progress exclude not belonging to same pr",
 			event: &info.Event{
 				Repository:        "foo",
 				SHA:               "foosha",
@@ -684,9 +736,8 @@ func TestCancelInProgress(t *testing.T) {
 			},
 			wantLog: "cancel-in-progress: cancelling pipelinerun foo/",
 		},
-
 		{
-			name: "cancel in progress with concurrency limit",
+			name: "skip/cancel in progress with concurrency limit",
 			event: &info.Event{
 				Repository: "foo",
 				SHA:        "foosha",

test/gitea_test.go

@@ -14,6 +14,14 @@ import (
 
 	"code.gitea.io/sdk/gitea"
 	"github.com/google/go-github/v66/github"
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
+	"github.com/tektoncd/pipeline/pkg/names"
+	yaml "gopkg.in/yaml.v2"
+	"gotest.tools/v3/assert"
+	"gotest.tools/v3/env"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"knative.dev/pkg/apis"
+
 	pacapi "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
 	tknpacdelete "github.com/openshift-pipelines/pipelines-as-code/pkg/cmd/tknpac/deleterepo"
@@ -36,13 +44,6 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/test/pkg/scm"
 	"github.com/openshift-pipelines/pipelines-as-code/test/pkg/secret"
 	twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait"
-	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
-	"github.com/tektoncd/pipeline/pkg/names"
-	"gopkg.in/yaml.v2"
-	"gotest.tools/v3/assert"
-	"gotest.tools/v3/env"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"knative.dev/pkg/apis"
 )
 
 // successRegexp will match a success text paac comment,sometime it includes html tags so we need to consider that.
@@ -441,6 +442,20 @@ func TestGiteaConfigCancelInProgress(t *testing.T) {
 		}
 	}
 	assert.Equal(t, cancelledPr, 1, "only one pr should have been canceled")
+
+	// Test that cancelling works with /retest
+	tgitea.PostCommentOnPullRequest(t, topts, "/retest")
+	topts.ParamsRun.Clients.Log.Info("Waiting 10 seconds before a new retest")
+	time.Sleep(10 * time.Second) // “Evil does not sleep. It waits.” - Galadriel
+	tgitea.PostCommentOnPullRequest(t, topts, "/retest")
+	tgitea.WaitForStatus(t, topts, "heads/"+targetRef, "", false)
+
+	for _, pr := range prs.Items {
+		if pr.GetStatusCondition().GetCondition(apis.ConditionSucceeded).GetReason() == "Cancelled" {
+			cancelledPr++
+		}
+	}
+	assert.Equal(t, cancelledPr, 2, "tweo pr should have been canceled")
 }
 
 func TestGiteaPush(t *testing.T) {