fix: filter out successful pipelineruns for /ok-to-test /retest

waveywaves · waveywaves · commit fa2a5957fec0 · 2025-04-16T19:37:06.000+05:30
When running /ok-to-test or /retest all the pipelineruns are
rerun over again. These pipeline runs instead should be skipped.
When parsing for pipelineruns created for a Repository, we
check whether any of the matched pipelineruns have been triggered
by either the ok-to-test or retest event types. If they have, then
we filter them out of the matched pipelineruns.
diff --git a/pkg/pipelineascode/match.go b/pkg/pipelineascode/match.go
@@ -18,6 +18,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/templates"
 	tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"go.uber.org/zap"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func (p *PacRun) matchRepoPR(ctx context.Context) ([]matcher.Match, *v1alpha1.Repository, error) {
@@ -165,6 +166,90 @@ is that what you want? make sure you use -n when generating the secret, eg: echo
 	return repo, nil
 }
 
+// isAlreadyExecuted checks if the pipeline has already been executed for the given SHA and PR number.
+func (p *PacRun) isAlreadyExecuted(ctx context.Context, match matcher.Match) (bool, error) {
+	// Get existing PipelineRuns for this repository that match the current SHA and PR number
+	labelSelector := fmt.Sprintf("%s=%s,%s=%d,%s=%s",
+		apipac.SHA, p.event.SHA,
+		apipac.PullRequest, p.event.PullRequestNumber,
+		apipac.Repository, match.Repo.Name)
+
+	existingPRs, err := p.run.Clients.Tekton.TektonV1().PipelineRuns(match.Repo.Namespace).List(ctx, metav1.ListOptions{
+		LabelSelector: labelSelector,
+	})
+	if err != nil {
+		return false, fmt.Errorf("failed to get existing pipelineruns: %w", err)
+	}
+
+	// check for any successful runs for this specific pipeline
+	targetPRName := strings.TrimSuffix(match.PipelineRun.GetGenerateName(), "-")
+	if targetPRName == "" {
+		targetPRName = match.PipelineRun.GetName()
+	}
+
+	var latestPR *tektonv1.PipelineRun
+	var latestTime *metav1.Time
+
+	// Find the latest pipeline run for this specific pipeline
+	for i, pr := range existingPRs.Items {
+		// Skip pipeline runs that are still running or not done
+		if !pr.IsDone() || pr.Status.CompletionTime == nil {
+			continue
+		}
+
+		// if it's the same pipeline
+		existingPRName := ""
+		if originalPRName, ok := pr.GetAnnotations()[apipac.OriginalPRName]; ok {
+			existingPRName = originalPRName
+		} else {
+			continue
+		}
+
+		// Make sure we're looking at the correct pipeline
+		if existingPRName == targetPRName {
+			// First matching pipeline or pipeline with a newer completion time
+			if latestPR == nil || pr.Status.CompletionTime.After(latestTime.Time) {
+				latestPR = &existingPRs.Items[i]
+				latestTime = pr.Status.CompletionTime
+			}
+		}
+	}
+
+	// Only skip if the latest pipeline was successful
+	if latestPR != nil && latestPR.Status.GetCondition("Succeeded").IsTrue() {
+		msg := fmt.Sprintf("Skipping pipeline run %s as it has already completed successfully for SHA %s on PR #%d",
+			targetPRName, p.event.SHA, p.event.PullRequestNumber)
+		p.eventEmitter.EmitMessage(match.Repo, zap.InfoLevel, "RepositorySkippingPipelineRun", msg)
+		return true, nil
+	}
+
+	return false, nil
+}
+
+// filterAlreadySuccessfulPipelines filters out pipeline runs that have already been executed successfully.
+func (p *PacRun) filterAlreadySuccessfulPipelines(ctx context.Context, matchedPRs []matcher.Match) []matcher.Match {
+	filteredMatches := []matcher.Match{}
+	for _, match := range matchedPRs {
+		alreadyExecuted, err := p.isAlreadyExecuted(ctx, match)
+		if err != nil {
+			prName := match.PipelineRun.GetGenerateName()
+			if prName == "" {
+				prName = match.PipelineRun.GetName()
+			}
+			msg := fmt.Sprintf("Error checking if pipeline %s was already executed: %v",
+				prName, err)
+			p.eventEmitter.EmitMessage(match.Repo, zap.WarnLevel, "RepositoryCheckExecution", msg)
+			filteredMatches = append(filteredMatches, match)
+			continue
+		}
+
+		if !alreadyExecuted {
+			filteredMatches = append(filteredMatches, match)
+		}
+	}
+	return filteredMatches
+}
+
 // getPipelineRunsFromRepo fetches pipelineruns from git repository and prepare them for creation.
 func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Repository) ([]matcher.Match, error) {
 	provenance := "source"
@@ -262,6 +347,11 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
 			}
 			return nil, nil
 		}
+
+		// filter out pipelines that have already been executed successfully
+		if p.event.EventType == opscomments.RetestAllCommentEventType.String() || p.event.EventType == opscomments.OkToTestCommentEventType.String() {
+			matchedPRs = p.filterAlreadySuccessfulPipelines(ctx, matchedPRs)
+		}
 	}
 
 	// if the event is a comment event, but we don't have any match from the keys.OnComment then do the ACL checks again
diff --git a/test/github_pullrequest_retest_skip_test.go b/test/github_pullrequest_retest_skip_test.go
@@ -0,0 +1,195 @@
+//go:build e2e
+// +build e2e
+
+package test
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/google/go-github/v70/github"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
+	tgithub "github.com/openshift-pipelines/pipelines-as-code/test/pkg/github"
+	twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait"
+	"gotest.tools/v3/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// TestGithubPullRequestRetestSkipSuccessful tests that a pipeline run is skipped when using /retest
+// if the pipeline has already been executed successfully
+func TestGithubPullRequestRetestSkipSuccessful(t *testing.T) {
+	ctx := context.Background()
+	g := &tgithub.PRTest{
+		Label:     "Github PullRequest Retest Skip Successful",
+		YamlFiles: []string{"testdata/pipelinerun.yaml"}, // Use a simple pipeline
+	}
+	g.RunPullRequest(ctx, t)
+	defer g.TearDown(ctx, t)
+
+	// Wait for the initial pipeline to complete
+	waitOpts := twait.Opts{
+		RepoName:        g.TargetNamespace,
+		Namespace:       g.TargetNamespace,
+		MinNumberStatus: 1,
+		PollTimeout:     twait.DefaultTimeout,
+		TargetSHA:       g.SHA,
+	}
+	_, err := twait.UntilRepositoryUpdated(ctx, g.Cnx.Clients, waitOpts)
+	assert.NilError(t, err)
+
+	// Get the initial number of PipelineRuns
+	initialPRs, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{
+		LabelSelector: keys.SHA + "=" + g.SHA,
+	})
+	assert.NilError(t, err)
+	initialPRCount := len(initialPRs.Items)
+	g.Cnx.Clients.Log.Infof("Found %d initial PipelineRuns", initialPRCount)
+
+	// Send a /retest comment
+	g.Cnx.Clients.Log.Infof("Creating /retest comment in PullRequest")
+	_, _, err = g.Provider.Client.Issues.CreateComment(ctx,
+		g.Options.Organization,
+		g.Options.Repo, g.PRNumber,
+		&github.IssueComment{Body: github.Ptr("/retest")})
+	assert.NilError(t, err)
+
+	// Allow some time for the comment to be processed
+	time.Sleep(10 * time.Second)
+
+	// Get the final number of PipelineRuns
+	finalPRs, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{
+		LabelSelector: keys.SHA + "=" + g.SHA,
+	})
+	assert.NilError(t, err)
+	finalPRCount := len(finalPRs.Items)
+	g.Cnx.Clients.Log.Infof("Found %d final PipelineRuns", finalPRCount)
+
+	// Verify no new PipelineRuns were created
+	assert.Equal(t, initialPRCount, finalPRCount, "Expected no new PipelineRuns to be created as the initial one succeeded")
+
+	// Check that we have logs from the controller indicating the pipeline was skipped
+	// This is a bit tricky in E2E tests, so we'll just check that we still have the successful status
+	repo, err := g.Cnx.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(g.TargetNamespace).Get(ctx, g.TargetNamespace, metav1.GetOptions{})
+	assert.NilError(t, err)
+	assert.Equal(t, string(repo.Status[len(repo.Status)-1].Conditions[0].Status), "True", "Repository status should be successful")
+}
+
+// TestGithubPullRequestRetestRunFailedPipeline tests that a pipeline run is not skipped when using /retest
+// if the most recent execution of the pipeline has failed (even if an earlier one succeeded)
+func TestGithubPullRequestRetestRunFailedPipeline(t *testing.T) {
+	ctx := context.Background()
+	g := &tgithub.PRTest{
+		Label:     "Github PullRequest Retest Run Failed Pipeline",
+		YamlFiles: []string{"testdata/pipelinerun.yaml", "testdata/pipelinerun-error-snippet.yaml"}, // The second one will fail
+	}
+	g.RunPullRequest(ctx, t)
+	defer g.TearDown(ctx, t)
+
+	// Wait for the initial pipelines to complete
+	waitOpts := twait.Opts{
+		RepoName:        g.TargetNamespace,
+		Namespace:       g.TargetNamespace,
+		MinNumberStatus: 1,
+		PollTimeout:     twait.DefaultTimeout,
+		TargetSHA:       g.SHA,
+	}
+	_, err := twait.UntilRepositoryUpdated(ctx, g.Cnx.Clients, waitOpts)
+	assert.NilError(t, err)
+
+	// Get the initial number of PipelineRuns
+	initialPRs, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{
+		LabelSelector: keys.SHA + "=" + g.SHA,
+	})
+	assert.NilError(t, err)
+	initialPRCount := len(initialPRs.Items)
+	g.Cnx.Clients.Log.Infof("Found %d initial PipelineRuns", initialPRCount)
+
+	// Send a /retest comment
+	g.Cnx.Clients.Log.Infof("Creating /retest comment in PullRequest")
+	_, _, err = g.Provider.Client.Issues.CreateComment(ctx,
+		g.Options.Organization,
+		g.Options.Repo, g.PRNumber,
+		&github.IssueComment{Body: github.Ptr("/retest")})
+	assert.NilError(t, err)
+
+	// Wait for new pipelines to be created
+	newWaitOpts := twait.Opts{
+		RepoName:        g.TargetNamespace,
+		Namespace:       g.TargetNamespace,
+		MinNumberStatus: initialPRCount + 1, // At least 1 new pipeline should be created
+		PollTimeout:     twait.DefaultTimeout,
+		TargetSHA:       g.SHA,
+	}
+	err = twait.UntilPipelineRunCreated(ctx, g.Cnx.Clients, newWaitOpts)
+	assert.NilError(t, err)
+
+	// Get the final number of PipelineRuns
+	finalPRs, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{
+		LabelSelector: keys.SHA + "=" + g.SHA,
+	})
+	assert.NilError(t, err)
+	finalPRCount := len(finalPRs.Items)
+	g.Cnx.Clients.Log.Infof("Found %d final PipelineRuns", finalPRCount)
+
+	// Verify at least one new PipelineRun was created (the failed one should be rerun)
+	assert.Assert(t, finalPRCount > initialPRCount, "Expected at least one new PipelineRun to be created as one of the initial ones failed")
+}
+
+// TestGithubPullRequestOkToTestSkipSuccessful tests that a pipeline run is skipped when using /ok-to-test
+// if the pipeline has already been executed successfully
+func TestGithubPullRequestOkToTestSkipSuccessful(t *testing.T) {
+	ctx := context.Background()
+	g := &tgithub.PRTest{
+		Label:     "Github PullRequest Ok-to-test Skip Successful",
+		YamlFiles: []string{"testdata/pipelinerun.yaml"}, // Use a simple pipeline
+	}
+	g.RunPullRequest(ctx, t)
+	defer g.TearDown(ctx, t)
+
+	// Wait for the initial pipeline to complete
+	waitOpts := twait.Opts{
+		RepoName:        g.TargetNamespace,
+		Namespace:       g.TargetNamespace,
+		MinNumberStatus: 1,
+		PollTimeout:     twait.DefaultTimeout,
+		TargetSHA:       g.SHA,
+	}
+	_, err := twait.UntilRepositoryUpdated(ctx, g.Cnx.Clients, waitOpts)
+	assert.NilError(t, err)
+
+	// Get the initial number of PipelineRuns
+	initialPRs, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{
+		LabelSelector: keys.SHA + "=" + g.SHA,
+	})
+	assert.NilError(t, err)
+	initialPRCount := len(initialPRs.Items)
+	g.Cnx.Clients.Log.Infof("Found %d initial PipelineRuns", initialPRCount)
+
+	// Send a /ok-to-test comment
+	g.Cnx.Clients.Log.Infof("Creating /ok-to-test comment in PullRequest")
+	_, _, err = g.Provider.Client.Issues.CreateComment(ctx,
+		g.Options.Organization,
+		g.Options.Repo, g.PRNumber,
+		&github.IssueComment{Body: github.Ptr("/ok-to-test")})
+	assert.NilError(t, err)
+
+	// Allow some time for the comment to be processed
+	time.Sleep(10 * time.Second)
+
+	// Get the final number of PipelineRuns
+	finalPRs, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{
+		LabelSelector: keys.SHA + "=" + g.SHA,
+	})
+	assert.NilError(t, err)
+	finalPRCount := len(finalPRs.Items)
+	g.Cnx.Clients.Log.Infof("Found %d final PipelineRuns", finalPRCount)
+
+	// Verify no new PipelineRuns were created
+	assert.Equal(t, initialPRCount, finalPRCount, "Expected no new PipelineRuns to be created as the initial one succeeded")
+
+	// Check that repository status remains successful
+	repo, err := g.Cnx.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(g.TargetNamespace).Get(ctx, g.TargetNamespace, metav1.GetOptions{})
+	assert.NilError(t, err)
+	assert.Equal(t, string(repo.Status[len(repo.Status)-1].Conditions[0].Status), "True", "Repository status should be successful")
+}
