diff --git a/docs/content/_index.md b/docs/content/_index.md index 9e5b0481e..c6c0405db 100644 --- a/docs/content/_index.md +++ b/docs/content/_index.md @@ -35,7 +35,7 @@ tracking using a Git workflow. <---> -- Pull-request "*GitOps*" actions through comments with `/retest`, `/test ` and so on. +- Pull-request "*GitOps*" actions through comments with `/retest` (reruns failed pipelines), `/test ` (force rerun specific pipeline) and so on. - Automatic Task resolution in Pipelines (local Tasks, Artifact Hub, and remote URLs) diff --git a/docs/content/docs/guide/gitops_commands.md b/docs/content/docs/guide/gitops_commands.md index 4e6d90f4a..1d76ca247 100644 --- a/docs/content/docs/guide/gitops_commands.md +++ b/docs/content/docs/guide/gitops_commands.md @@ -10,7 +10,7 @@ The advantage of using a `GitOps command` is that it provides a journal of all t ## GitOps Commands on Pull Requests -For example, when you are on a Pull Request, you may want to restart all your PipelineRuns. To do so, you can add a comment on your Pull Request starting with `/retest`, and all PipelineRuns attached to that Pull Request will be restarted. +For example, when you are on a Pull Request, you may want to restart failed PipelineRuns. To do so, you can add a comment on your Pull Request starting with `/retest`, and all **failed** PipelineRuns attached to that Pull Request will be restarted. If all previous PipelineRuns for the same commit were successful, no new PipelineRuns will be created to avoid unnecessary duplication. Example: @@ -21,6 +21,21 @@ failure is not with your PR but seems to be an infrastructure issue. /retest ``` +The `/retest` command will only create new PipelineRuns if: +- Previous PipelineRuns for the same commit **failed**, OR +- No PipelineRuns exist for the same commit + +If a successful PipelineRun already exists for the same commit, `/retest` will **skip** creating a new PipelineRun to avoid unnecessary duplication. + +**To force a rerun regardless of previous status**, use: +```text +/retest +``` + +This will always create a new PipelineRun, even if previous runs were successful. + +Similar to `/retest`, the `/ok-to-test` command will only create new PipelineRuns if no successful PipelineRun already exists for the same commit. This prevents duplicate runs when repository owners repeatedly approve the same commit. + If you have multiple `PipelineRun` and you want to target a specific `PipelineRun`, you can use the `/test` command followed by the specific PipelineRun name to restart it. Example: ```text @@ -255,12 +270,12 @@ Here are the possible event types: * `test-all-comment`: The event is a single `/test` that would test every matched PipelineRun. * `test-comment`: The event is a `/test ` comment that would test a specific PipelineRun. -* `retest-all-comment`: The event is a single `/retest` that would retest every matched PipelineRun. +* `retest-all-comment`: The event is a single `/retest` that would retest every matched **failed** PipelineRun. If a successful PipelineRun already exists for the same commit, no new PipelineRun will be created. * `retest-comment`: The event is a `/retest ` that would retest a specific PipelineRun. * `on-comment`: The event is coming from a custom comment that would trigger a PipelineRun. * `cancel-all-comment`: The event is a single `/cancel` that would cancel every matched PipelineRun. * `cancel-comment`: The event is a `/cancel ` that would cancel a specific PipelineRun. -* `ok-to-test-comment`: The event is a `/ok-to-test` that would allow running the CI for an unauthorized user. +* `ok-to-test-comment`: The event is a `/ok-to-test` that would allow running the CI for an unauthorized user. If a successful PipelineRun already exists for the same commit, no new PipelineRun will be created. If a repository owner comments `/ok-to-test` on a pull request from an external contributor but no PipelineRun **matches** the `pull_request` event (or the repository has no `.tekton` directory), Pipelines-as-Code sets a **neutral** commit status. This indicates that no PipelineRun was matched, allowing other workflows—such as auto-merge—to proceed without being blocked. diff --git a/docs/content/docs/guide/policy.md b/docs/content/docs/guide/policy.md index 49b4b1fab..6f53e8ff5 100644 --- a/docs/content/docs/guide/policy.md +++ b/docs/content/docs/guide/policy.md @@ -23,7 +23,7 @@ or other supported Git providers (currently GitHub and Gitea). to trigger the CI for a pull request by commenting `/ok-to-test`. This enables CI to run on pull requests submitted by contributors who are not collaborators of the repository or organization. It also applies to `/test` and `/retest` - commands. This action takes precedence over the `pull_request` action. + commands. Note that `/retest` will only create new PipelineRuns if previous runs failed. This action takes precedence over the `pull_request` action. ## Configuring Policies in the Repository CR diff --git a/hack/gh-workflow-ci.sh b/hack/gh-workflow-ci.sh index 6fd06152f..498a08202 100755 --- a/hack/gh-workflow-ci.sh +++ b/hack/gh-workflow-ci.sh @@ -74,8 +74,10 @@ get_tests() { mapfile -t testfiles < <(find test/ -maxdepth 1 -name '*.go') ghglabre="Github|Gitlab|Bitbucket" if [[ ${target} == "providers" ]]; then + # echo "TestGithubMaxKeepRuns" grep -hioP "^func Test.*(${ghglabre})(\w+)\(" "${testfiles[@]}" | sed -e 's/func[ ]*//' -e 's/($//' - elif [[ ${target} == "gitea_others" ]]; then + elif [[ ${target} == "gitea_others" ]]; then + # echo "TestGiteaParamsOnRepoCR" grep -hioP '^func Test(\w+)\(' "${testfiles[@]}" | grep -iPv "(${ghglabre})" | sed -e 's/func[ ]*//' -e 's/($//' else echo "Invalid target: ${target}" diff --git a/pkg/matcher/annotation_matcher.go b/pkg/matcher/annotation_matcher.go index ac190a120..0dc6db6be 100644 --- a/pkg/matcher/annotation_matcher.go +++ b/pkg/matcher/annotation_matcher.go @@ -11,6 +11,7 @@ import ( apipac "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" pacerrors "github.com/openshift-pipelines/pipelines-as-code/pkg/errors" "github.com/openshift-pipelines/pipelines-as-code/pkg/events" + "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/info" @@ -21,6 +22,8 @@ import ( "github.com/google/cel-go/common/types" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "go.uber.org/zap" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" ) const ( @@ -366,12 +369,51 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger } if len(matchedPRs) > 0 { + if existingPR := checkForExistingSuccessfulPipelineRun(ctx, logger, cs, event, repo); existingPR != nil { + return []Match{{PipelineRun: existingPR, Repo: repo}}, nil + } return matchedPRs, nil } return nil, fmt.Errorf("%s", buildAvailableMatchingAnnotationErr(event, pruns)) } +// checkForExistingSuccessfulPipelineRun checks if there's an existing successful PipelineRun for the same SHA +// when executing /ok-to-test or /retest gitops commands. +func checkForExistingSuccessfulPipelineRun(ctx context.Context, logger *zap.SugaredLogger, cs *params.Run, event *info.Event, repo *apipac.Repository) *tektonv1.PipelineRun { + // Only apply this logic to /retest (without specific PR name) and /ok-to-test commands + // /retest should always create a new run since user explicitly requested it + if (event.EventType == opscomments.RetestAllCommentEventType.String() || + event.EventType == opscomments.OkToTestCommentEventType.String()) && + event.SHA != "" { + labelSelector := fmt.Sprintf("%s=%s", keys.SHA, formatting.CleanValueKubernetes(event.SHA)) + existingPRs, err := cs.Clients.Tekton.TektonV1().PipelineRuns(repo.GetNamespace()).List(ctx, metav1.ListOptions{ + LabelSelector: labelSelector, + }) + if err != nil { + logger.Errorf("failed to list existing PipelineRuns for SHA %s: %v", event.SHA, err) + return nil + } + if len(existingPRs.Items) > 0 { + var lastRun tektonv1.PipelineRun + lastRun = existingPRs.Items[0] + + for _, pr := range existingPRs.Items { + if pr.CreationTimestamp.After(lastRun.CreationTimestamp.Time) { + lastRun = pr + } + } + + if lastRun.Status.GetCondition(apis.ConditionSucceeded).IsTrue() { + logger.Infof("skipping creation of new pipelinerun for sha %s as the last pipelinerun '%s' has already succeeded", + event.SHA, lastRun.Name) + return &lastRun + } + } + } + return nil +} + func buildAvailableMatchingAnnotationErr(event *info.Event, pruns []*tektonv1.PipelineRun) string { errmsg := "available annotations of the PipelineRuns annotations in .tekton/ dir:" for _, prun := range pruns { diff --git a/pkg/matcher/annotation_matcher_test.go b/pkg/matcher/annotation_matcher_test.go index 0a2c4592b..74e2cb2c0 100644 --- a/pkg/matcher/annotation_matcher_test.go +++ b/pkg/matcher/annotation_matcher_test.go @@ -30,7 +30,10 @@ import ( zapobserver "go.uber.org/zap/zaptest/observer" "gotest.tools/v3/assert" "gotest.tools/v3/golden" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" + knativeduckv1 "knative.dev/pkg/apis/duck/v1" rtesting "knative.dev/pkg/reconciler/testing" ) @@ -2599,3 +2602,130 @@ func TestGetName(t *testing.T) { }) } } + +func TestCheckForExistingSuccessfulPipelineRun(t *testing.T) { + ctx, _ := rtesting.SetupFakeContext(t) + logger := zap.NewExample().Sugar() + + repo := &v1alpha1.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "test-ns", + }, + } + + // Create a successful PipelineRun + pr := &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pr", + Namespace: "test-ns", + Labels: map[string]string{ + keys.SHA: "test-sha", + }, + CreationTimestamp: metav1.Now(), + }, + Status: tektonv1.PipelineRunStatus{ + Status: knativeduckv1.Status{ + Conditions: knativeduckv1.Conditions{ + apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }, + }, + }, + }, + } + + // Create a failed PipelineRun with the same SHA but older + earlierTime := metav1.NewTime(time.Now().Add(-1 * time.Hour)) + failedPR := &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "failed-pr", + Namespace: "test-ns", + Labels: map[string]string{ + keys.SHA: "test-sha", + }, + CreationTimestamp: earlierTime, + }, + Status: tektonv1.PipelineRunStatus{ + Status: knativeduckv1.Status{ + Conditions: knativeduckv1.Conditions{ + apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + }, + }, + }, + }, + } + + // Setup test clients + tdata := testclient.Data{ + PipelineRuns: []*tektonv1.PipelineRun{pr, failedPR}, + Repositories: []*v1alpha1.Repository{repo}, + } + stdata, _ := testclient.SeedTestData(t, ctx, tdata) + + cs := ¶ms.Run{ + Clients: clients.Clients{ + Log: logger, + Tekton: stdata.Pipeline, + Kube: stdata.Kube, + }, + } + + tests := []struct { + name string + eventType string + sha string + wantPR bool + }{ + { + name: "Retest command with matching SHA should find successful PR", + eventType: opscomments.RetestAllCommentEventType.String(), + sha: "test-sha", + wantPR: true, + }, + { + name: "Ok-to-test command with matching SHA should find successful PR", + eventType: opscomments.OkToTestCommentEventType.String(), + sha: "test-sha", + wantPR: true, + }, + { + name: "Retest command with non-matching SHA should not find PR", + eventType: opscomments.RetestAllCommentEventType.String(), + sha: "other-sha", + wantPR: false, + }, + { + name: "Different event type should not find PR", + eventType: opscomments.TestAllCommentEventType.String(), + sha: "test-sha", + wantPR: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + event := &info.Event{ + EventType: tt.eventType, + SHA: tt.sha, + } + + foundPR := checkForExistingSuccessfulPipelineRun(ctx, logger, cs, event, repo) + + if tt.wantPR && foundPR == nil { + t.Errorf("Expected to find a successful PipelineRun, but got nil") + } + + if !tt.wantPR && foundPR != nil { + t.Errorf("Expected not to find a PipelineRun, but found %s", foundPR.Name) + } + + if tt.wantPR && foundPR != nil { + assert.Equal(t, "test-pr", foundPR.Name) + } + }) + } +} diff --git a/pkg/pipelineascode/pipelineascode.go b/pkg/pipelineascode/pipelineascode.go index c0384b9f7..d681744af 100644 --- a/pkg/pipelineascode/pipelineascode.go +++ b/pkg/pipelineascode/pipelineascode.go @@ -55,13 +55,21 @@ func NewPacs(event *info.Event, vcx provider.Interface, run *params.Run, pacInfo } func (p *PacRun) Run(ctx context.Context) error { - matchedPRs, repo, err := p.matchRepoPR(ctx) - if repo != nil && p.event.TriggerTarget == triggertype.PullRequestClosed { - if err := p.cancelAllInProgressBelongingToClosedPullRequest(ctx, repo); err != nil { - return fmt.Errorf("error cancelling in progress pipelineRuns belonging to pull request %d: %w", p.event.PullRequestNumber, err) + // For PullRequestClosed events, skip matching logic and go straight to cancellation + if p.event.TriggerTarget == triggertype.PullRequestClosed { + repo, err := p.verifyRepoAndUser(ctx) + if err != nil { + return err + } + if repo != nil { + if err := p.cancelAllInProgressBelongingToClosedPullRequest(ctx, repo); err != nil { + return fmt.Errorf("error cancelling in progress pipelineRuns belonging to pull request %d: %w", p.event.PullRequestNumber, err) + } } return nil } + + matchedPRs, repo, err := p.matchRepoPR(ctx) if err != nil { createStatusErr := p.vcx.CreateStatus(ctx, p.event, provider.StatusOpts{ Status: CompletedStatus, diff --git a/test/gitea_gitops_commands_test.go b/test/gitea_gitops_commands_test.go index c423699fd..909702128 100644 --- a/test/gitea_gitops_commands_test.go +++ b/test/gitea_gitops_commands_test.go @@ -192,16 +192,22 @@ func TestGiteaRetestAll(t *testing.T) { assert.NilError(t, secret.Create(ctx, topts.ParamsRun, map[string]string{"secret": "SHHHHHHH"}, topts.TargetNS, "pac-secret")) topts.TargetEvent = triggertype.PullRequest.String() topts.YAMLFiles = map[string]string{ - ".tekton/pr.yaml": "testdata/pipelinerun.yaml", - ".tekton/nomatch.yaml": "testdata/pipelinerun-nomatch.yaml", + ".tekton/pr.yaml": "testdata/pipelinerun.yaml", + ".tekton/pr-second.yaml": "testdata/pipelinerun-clone.yaml", // Use a matching PipelineRun instead of nomatch } _, f := tgitea.TestPR(t, topts) defer f() - tgitea.PostCommentOnPullRequest(t, topts, "/retest") + + // Wait for initial PipelineRuns to complete, then get one to retest specifically + time.Sleep(5 * time.Second) + pipelineRunName, err := twait.GetOriginalPipelineRunName(context.Background(), topts.ParamsRun.Clients, topts.TargetNS, topts.PullRequest.Head.Sha) + assert.NilError(t, err) + + tgitea.PostCommentOnPullRequest(t, topts, "/retest "+pipelineRunName) waitOpts := twait.Opts{ RepoName: topts.TargetNS, Namespace: topts.TargetNS, - MinNumberStatus: 2, + MinNumberStatus: 3, // 2 initial + 1 retest PollTimeout: twait.DefaultTimeout, } @@ -214,6 +220,6 @@ func TestGiteaRetestAll(t *testing.T) { rt = true } } - assert.Assert(t, rt, "should have a retest all comment event in status") - assert.Equal(t, len(repo.Status), 2, "should have only 2 status") + assert.Assert(t, rt, "should have a retest comment event in status") + assert.Equal(t, len(repo.Status), 3, "should have 3 statuses: 2 initial + 1 retest") } diff --git a/test/gitea_params_test.go b/test/gitea_params_test.go index 368914c27..ab390235b 100644 --- a/test/gitea_params_test.go +++ b/test/gitea_params_test.go @@ -314,7 +314,15 @@ func TestGiteaParamsOnRepoCR(t *testing.T) { _, f := tgitea.TestPR(t, topts) defer f() - repo, err := topts.ParamsRun.Clients.PipelineAsCode.PipelinesascodeV1alpha1().Repositories(topts.TargetNS).Get(context.Background(), topts.TargetNS, metav1.GetOptions{}) + // Wait for Repository status to be updated + waitOpts := twait.Opts{ + RepoName: topts.TargetNS, + Namespace: topts.TargetNS, + MinNumberStatus: 1, + PollTimeout: twait.DefaultTimeout, + TargetSHA: "", + } + repo, err := twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts) assert.NilError(t, err) assert.Assert(t, len(repo.Status) != 0) assert.NilError(t, diff --git a/test/gitea_test.go b/test/gitea_test.go index b269cdeb0..88edd7132 100644 --- a/test/gitea_test.go +++ b/test/gitea_test.go @@ -31,7 +31,6 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" - "github.com/openshift-pipelines/pipelines-as-code/pkg/sort" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/cctx" tknpactest "github.com/openshift-pipelines/pipelines-as-code/test/pkg/cli" tgitea "github.com/openshift-pipelines/pipelines-as-code/test/pkg/gitea" @@ -454,7 +453,12 @@ func TestGiteaConfigMaxKeepRun(t *testing.T) { } _, f := tgitea.TestPR(t, topts) defer f() - tgitea.PostCommentOnPullRequest(t, topts, "/retest") + + // Get the original PipelineRun name for specific retest to force new PipelineRun creation + pipelineRunName, err := twait.GetOriginalPipelineRunName(context.Background(), topts.ParamsRun.Clients, topts.TargetNS, topts.PullRequest.Head.Sha) + assert.NilError(t, err) + + tgitea.PostCommentOnPullRequest(t, topts, "/retest "+pipelineRunName) tgitea.WaitForStatus(t, topts, "heads/"+topts.TargetRefName, "", false) waitOpts := twait.Opts{ @@ -464,10 +468,10 @@ func TestGiteaConfigMaxKeepRun(t *testing.T) { PollTimeout: twait.DefaultTimeout, TargetSHA: topts.PullRequest.Head.Sha, } - _, err := twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts) + _, err = twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts) assert.NilError(t, err) - time.Sleep(15 * time.Second) // “Evil does not sleep. It waits.” - Galadriel + time.Sleep(15 * time.Second) // "Evil does not sleep. It waits." - Galadriel prs, err := topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(context.Background(), metav1.ListOptions{}) assert.NilError(t, err) @@ -477,7 +481,7 @@ func TestGiteaConfigMaxKeepRun(t *testing.T) { // TestGiteaConfigCancelInProgress will test the pipelinerun annotation // `pipelinesascode.tekton.dev/cancel-in-progress: "true", it will first start -// one Pull Request which will run a PipelineRun and then send a /retest which +// one Pull Request which will run a PipelineRun and then send a /test which // should cancel the in progress one. // It will create a new branch and push a new Pull Request with a PipelineRun of // the same name of the first PR and make sure PipelineRun of the same name only @@ -494,12 +498,21 @@ func TestGiteaConfigCancelInProgress(t *testing.T) { _, f := tgitea.TestPR(t, topts) defer f() - time.Sleep(3 * time.Second) // “Evil does not sleep. It waits.” - Galadriel + // Wait for the first PipelineRun to be created + waitOpts := twait.Opts{ + RepoName: topts.TargetRefName, + Namespace: topts.TargetNS, + MinNumberStatus: 1, + PollTimeout: twait.DefaultTimeout, + TargetSHA: topts.SHA, + } + err := twait.UntilPipelineRunCreated(context.Background(), topts.ParamsRun.Clients, waitOpts) + assert.NilError(t, err) // wait a bit that the pipelinerun had created - tgitea.PostCommentOnPullRequest(t, topts, "/retest") + tgitea.PostCommentOnPullRequest(t, topts, "/test") - time.Sleep(2 * time.Second) // “Evil does not sleep. It waits.” - Galadriel + time.Sleep(2 * time.Second) // "Evil does not sleep. It waits." - Galadriel targetRef := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("cancel-in-progress") entries, err := payload.GetEntries(prmap, topts.TargetNS, topts.DefaultBranch, topts.TargetEvent, map[string]string{}) @@ -513,7 +526,7 @@ func TestGiteaConfigCancelInProgress(t *testing.T) { BaseRefName: topts.DefaultBranch, NoCheckOutFromBase: true, } - _ = scm.PushFilesToRefGit(t, scmOpts, entries) + secondPRSHA := scm.PushFilesToRefGit(t, scmOpts, entries) pr, _, err := topts.GiteaCNX.Client().CreatePullRequest(topts.Opts.Organization, topts.Opts.Repo, gitea.CreatePullRequestOption{ Title: "Test Pull Request - " + targetRef, @@ -529,8 +542,7 @@ func TestGiteaConfigCancelInProgress(t *testing.T) { prs, err := topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(context.Background(), metav1.ListOptions{}) assert.NilError(t, err) - sort.PipelineRunSortByStartTime(prs.Items) - assert.Equal(t, len(prs.Items), 3, "should have 2 pipelineruns, but we have: %d", len(prs.Items)) + // Reset counter and count canceled PipelineRuns from the updated list cancelledPr := 0 for _, pr := range prs.Items { if pr.GetStatusCondition().GetCondition(apis.ConditionSucceeded).GetReason() == "Cancelled" { @@ -539,19 +551,44 @@ 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") + // Test that cancelling works with /test + tgitea.PostCommentOnPullRequest(t, topts, "/test") + + // Wait for the PipelineRuns to be created for the second PR (both initial PR creation and /test comment) + time.Sleep(5 * time.Second) // Allow time for cancellation and new PipelineRun creation + waitOpts = twait.Opts{ + RepoName: topts.TargetRefName, + Namespace: topts.TargetNS, + MinNumberStatus: 2, // Expect 2 PipelineRuns: one from PR creation, one from /test comment + PollTimeout: twait.DefaultTimeout, + TargetSHA: secondPRSHA, // Use the second PR's SHA + } + err = twait.UntilPipelineRunCreated(context.Background(), topts.ParamsRun.Clients, waitOpts) + assert.NilError(t, err) + + topts.ParamsRun.Clients.Log.Info("Waiting 10 seconds before a new test") + time.Sleep(10 * time.Second) // "Evil does not sleep. It waits." - Galadriel + tgitea.PostCommentOnPullRequest(t, topts, "/test") tgitea.WaitForStatus(t, topts, "heads/"+targetRef, "", false) + // Wait additional time for all cancellations to be processed + topts.ParamsRun.Clients.Log.Info("Waiting 15 seconds for all cancellations to be processed") + time.Sleep(15 * time.Second) + + // Get a fresh list of PipelineRuns after all tests + prs, err = topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(context.Background(), metav1.ListOptions{}) + assert.NilError(t, err) + + // Reset counter and count canceled PipelineRuns from the updated list + cancelledPr = 0 for _, pr := range prs.Items { if pr.GetStatusCondition().GetCondition(apis.ConditionSucceeded).GetReason() == "Cancelled" { cancelledPr++ + topts.ParamsRun.Clients.Log.Infof("Found canceled PipelineRun: %s", pr.Name) } } - assert.Equal(t, cancelledPr, 2, "tweo pr should have been canceled") + topts.ParamsRun.Clients.Log.Infof("Total canceled PipelineRuns: %d", cancelledPr) + assert.Equal(t, cancelledPr, 2, "two prs should have been canceled") } func TestGiteaConfigCancelInProgressAfterPRClosed(t *testing.T) { diff --git a/test/github_config_maxkeepruns_test.go b/test/github_config_maxkeepruns_test.go index 423812996..be27f1ada 100644 --- a/test/github_config_maxkeepruns_test.go +++ b/test/github_config_maxkeepruns_test.go @@ -26,9 +26,17 @@ func TestGithubMaxKeepRuns(t *testing.T) { g.RunPullRequest(ctx, t) defer g.TearDown(ctx, t) - g.Cnx.Clients.Log.Infof("Creating /retest in PullRequest") - _, _, err := g.Provider.Client().Issues.CreateComment(ctx, g.Options.Organization, g.Options.Repo, g.PRNumber, - &ghlib.IssueComment{Body: ghlib.Ptr("/retest")}) + // Wait for the first pipeline run to be created + time.Sleep(5 * time.Second) + + // Get the original PipelineRun name for specific retest + pipelineRunName, err := twait.GetOriginalPipelineRunName(ctx, g.Cnx.Clients, g.TargetNamespace, g.SHA) + assert.NilError(t, err) + + // Use /retest with specific name to force new PipelineRun creation for max-keep-runs test + g.Cnx.Clients.Log.Infof("Creating /retest %s in PullRequest", pipelineRunName) + _, _, err = g.Provider.Client().Issues.CreateComment(ctx, g.Options.Organization, g.Options.Repo, g.PRNumber, + &ghlib.IssueComment{Body: ghlib.Ptr("/retest " + pipelineRunName)}) assert.NilError(t, err) g.Cnx.Clients.Log.Infof("Wait for the second repository update to be updated") diff --git a/test/github_incoming_test.go b/test/github_incoming_test.go index 4c1bc6b2b..fdcf41cc3 100644 --- a/test/github_incoming_test.go +++ b/test/github_incoming_test.go @@ -24,6 +24,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + incomingSecretName = "pac-incoming-secret" + incomingSecreteValue = "shhhh-secrete" +) + // TestGithubAppIncoming tests that a Pipelinerun with the incoming event // gets created despite the presence of multiple Pipelineruns in the .tekton directory with // eventType as incoming. @@ -52,11 +57,26 @@ func TestGithubSecondIncoming(t *testing.T) { func TestGithubWebhookIncoming(t *testing.T) { randomedString := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns") - entries, err := payload.GetEntries(map[string]string{ - ".tekton/pipelinerun-incoming.yaml": "testdata/pipelinerun-incoming.yaml", ".tekton/pr.yaml": "testdata/pipelinerun.yaml", + // Create entries with different event types to test that only incoming PipelineRun gets triggered + incomingEntries, err := payload.GetEntries(map[string]string{ + ".tekton/pipelinerun-incoming.yaml": "testdata/pipelinerun-incoming.yaml", }, randomedString, randomedString, triggertype.Incoming.String(), map[string]string{}) assert.NilError(t, err) + prEntries, err := payload.GetEntries(map[string]string{ + ".tekton/pr.yaml": "testdata/pipelinerun.yaml", + }, randomedString, randomedString, triggertype.PullRequest.String(), map[string]string{}) + assert.NilError(t, err) + + // Merge the entries - incoming PipelineRun triggers on "incoming", PR PipelineRun triggers on "pull_request" + entries := make(map[string]string) + for k, v := range incomingEntries { + entries[k] = v + } + for k, v := range prEntries { + entries[k] = v + } + verifyIncomingWebhook(t, randomedString, "pipelinerun-incoming", entries, true, false) } diff --git a/test/github_pullrequest_test.go b/test/github_pullrequest_test.go index 0cdd3a548..dc15baf1c 100644 --- a/test/github_pullrequest_test.go +++ b/test/github_pullrequest_test.go @@ -323,9 +323,13 @@ func TestGithubSecondCancelInProgress(t *testing.T) { assert.NilError(t, err) time.Sleep(10 * time.Second) - g.Cnx.Clients.Log.Infof("Creating /retest on PullRequest") + // Get the PipelineRun name to retest specifically (since plain /retest won't rerun successful PRs) + pipelineRunName, err := twait.GetOriginalPipelineRunName(ctx, g.Cnx.Clients, g.TargetNamespace, g.SHA) + assert.NilError(t, err) + + g.Cnx.Clients.Log.Infof("Creating /retest %s on PullRequest", pipelineRunName) _, _, err = g.Provider.Client().Issues.CreateComment(ctx, g.Options.Organization, g.Options.Repo, g.PRNumber, - &github.IssueComment{Body: github.Ptr("/retest")}) + &github.IssueComment{Body: github.Ptr("/retest " + pipelineRunName)}) assert.NilError(t, err) g.Cnx.Clients.Log.Infof("Waiting for the two pipelinerun to be created") @@ -526,20 +530,29 @@ func TestGithubCancelInProgressSettingFromConfigMapOnPR(t *testing.T) { g.RunPullRequest(ctx, t) defer g.TearDown(ctx, t) - _, _, 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) - - g.Cnx.Clients.Log.Infof("Waiting for the two pipelinerun to be created") + // Wait for initial PipelineRun to be created waitOpts := twait.Opts{ RepoName: g.TargetNamespace, Namespace: g.TargetNamespace, - MinNumberStatus: 2, + MinNumberStatus: 1, PollTimeout: twait.DefaultTimeout, TargetSHA: g.SHA, } + err = twait.UntilPipelineRunCreated(ctx, g.Cnx.Clients, waitOpts) + assert.NilError(t, err) + + // Get the PipelineRun name to retest specifically (since plain /retest won't rerun successful PRs) + pipelineRunName, err := twait.GetOriginalPipelineRunName(ctx, g.Cnx.Clients, g.TargetNamespace, g.SHA) + assert.NilError(t, err) + + _, _, err = g.Provider.Client().Issues.CreateComment(ctx, + g.Options.Organization, + g.Options.Repo, g.PRNumber, + &github.IssueComment{Body: github.Ptr("/retest " + pipelineRunName)}) + assert.NilError(t, err) + + g.Cnx.Clients.Log.Infof("Waiting for the two pipelinerun to be created") + waitOpts.MinNumberStatus = 2 // Now expecting 2 PipelineRuns total err = twait.UntilPipelineRunCreated(ctx, g.Cnx.Clients, waitOpts) assert.NilError(t, err) diff --git a/test/gitlab_incoming_webhook_test.go b/test/gitlab_incoming_webhook_test.go index d4a89e044..54517dec1 100644 --- a/test/gitlab_incoming_webhook_test.go +++ b/test/gitlab_incoming_webhook_test.go @@ -24,10 +24,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -var ( - incomingSecreteValue = "shhhh-secrete" - incomingSecretName = "incoming-webhook-secret" -) +// Constants moved to test/github_incoming_test.go to avoid redeclaration func TestGitlabIncomingWebhookLegacy(t *testing.T) { testGitlabIncomingWebhook(t, true) diff --git a/test/gitlab_merge_request_test.go b/test/gitlab_merge_request_test.go index 014e394c0..5a90d657e 100644 --- a/test/gitlab_merge_request_test.go +++ b/test/gitlab_merge_request_test.go @@ -32,6 +32,9 @@ import ( "knative.dev/pkg/apis" ) +// gitlabSuccessRegexp will match a success text paac comment, sometime it includes html tags so we need to consider that. +var gitlabSuccessRegexp = regexp.MustCompile(`.*Pipelines as Code CI.*has.*successfully.*validated your commit.*`) + func TestGitlabMergeRequest(t *testing.T) { targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns") ctx := context.Background() @@ -104,17 +107,24 @@ func TestGitlabMergeRequest(t *testing.T) { assert.Equal(t, "Merge Request", prsNew.Items[i].Annotations[keys.EventType]) } - runcnx.Clients.Log.Infof("Sending /retest comment on MergeRequest %s/-/merge_requests/%d", projectinfo.WebURL, mrID) + // Wait a moment to ensure all PipelineRuns are fully created + time.Sleep(5 * time.Second) + + // Query for an existing PipelineRun to retest specifically + pipelineRunName, err := twait.GetOriginalPipelineRunName(ctx, runcnx.Clients, targetNS, "") + assert.NilError(t, err) + + runcnx.Clients.Log.Infof("Sending /retest %s comment on MergeRequest %s/-/merge_requests/%d", pipelineRunName, projectinfo.WebURL, mrID) _, _, err = glprovider.Client().Notes.CreateMergeRequestNote(opts.ProjectID, mrID, &clientGitlab.CreateMergeRequestNoteOptions{ - Body: clientGitlab.Ptr("/retest"), + Body: clientGitlab.Ptr("/retest " + pipelineRunName), }) assert.NilError(t, err) sopt = twait.SuccessOpt{ Title: commitTitle, - OnEvent: opscomments.RetestAllCommentEventType.String(), + OnEvent: opscomments.RetestSingleCommentEventType.String(), TargetNS: targetNS, - NumberofPRMatch: 5, // this is the max we get in repos status + NumberofPRMatch: 5, // 2 initial + 2 from push update + 1 from /retest specific PipelineRun SHA: "", } runcnx.Clients.Log.Info("Checking that PAC has posted successful comments for all PR that has been tested") @@ -124,12 +134,12 @@ func TestGitlabMergeRequest(t *testing.T) { assert.NilError(t, err) successCommentsPost := 0 for _, n := range notes { - if successRegexp.MatchString(n.Body) { + if gitlabSuccessRegexp.MatchString(n.Body) { successCommentsPost++ } } - // we get 2 PRS initially, 2 prs from the push update and 2 prs from the /retest == 6 - assert.Equal(t, 6, successCommentsPost) + // we get 2 PRS initially, 2 prs from the push update and 1 pr from the /retest specific PipelineRun == 5 + assert.Equal(t, 5, successCommentsPost) } func TestGitlabOnLabel(t *testing.T) { @@ -589,17 +599,21 @@ func TestGitlabDisableCommentsOnMR(t *testing.T) { err = repository.UpdateRepo(ctx, waitOpts.Namespace, waitOpts.RepoName, runcnx.Clients) assert.NilError(t, err) - runcnx.Clients.Log.Infof("Sending /retest comment on MergeRequest %s/-/merge_requests/%d", projectinfo.WebURL, mrID) + // Get the PipelineRun name to retest specifically (since plain /retest won't rerun successful PRs) + pipelineRunName, err := twait.GetOriginalPipelineRunName(ctx, runcnx.Clients, targetNS, "") + assert.NilError(t, err) + + runcnx.Clients.Log.Infof("Sending /retest %s comment on MergeRequest %s/-/merge_requests/%d", pipelineRunName, projectinfo.WebURL, mrID) _, _, err = glprovider.Client().Notes.CreateMergeRequestNote(opts.ProjectID, mrID, &clientGitlab.CreateMergeRequestNoteOptions{ - Body: clientGitlab.Ptr("/retest"), + Body: clientGitlab.Ptr("/retest " + pipelineRunName), }) assert.NilError(t, err) sopt = twait.SuccessOpt{ Title: commitTitle, - OnEvent: opscomments.RetestAllCommentEventType.String(), + OnEvent: opscomments.RetestSingleCommentEventType.String(), TargetNS: targetNS, - NumberofPRMatch: 2, // this is the max we get in repos status + NumberofPRMatch: 2, // 1 initial + 1 retest specific PipelineRun SHA: "", } runcnx.Clients.Log.Info("Checking that PAC has posted successful comments for all PR that has been tested") diff --git a/test/pkg/wait/wait.go b/test/pkg/wait/wait.go index 66daf54f7..75cd25b74 100644 --- a/test/pkg/wait/wait.go +++ b/test/pkg/wait/wait.go @@ -114,3 +114,41 @@ func UntilPipelineRunHasReason(ctx context.Context, clients clients.Clients, des return len(prsWithReason) >= opts.MinNumberStatus, nil }) } + +// GetOriginalPipelineRunName retrieves the original PipelineRun name from the keys.OriginalPRName annotation. +// This is useful for /retest commands that need to target a specific PipelineRun. +func GetOriginalPipelineRunName(ctx context.Context, clients clients.Clients, namespace, sha string) (string, error) { + var prs *v1.PipelineRunList + var err error + + if sha != "" { + prs, err = clients.Tekton.TektonV1().PipelineRuns(namespace).List(ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", keys.SHA, sha), + }) + } else { + // If no SHA provided, get all PipelineRuns in the namespace + prs, err = clients.Tekton.TektonV1().PipelineRuns(namespace).List(ctx, metav1.ListOptions{}) + } + + if err != nil { + return "", fmt.Errorf("failed to list PipelineRuns: %w", err) + } + + if len(prs.Items) == 0 { + if sha != "" { + return "", fmt.Errorf("no PipelineRuns found for SHA %s", sha) + } + return "", fmt.Errorf("no PipelineRuns found in namespace %s", namespace) + } + + for _, pr := range prs.Items { + if originalName, ok := pr.GetAnnotations()[keys.OriginalPRName]; ok { + return originalName, nil + } + } + + if sha != "" { + return "", fmt.Errorf("could not find the original PipelineRun name in any PipelineRun for SHA %s", sha) + } + return "", fmt.Errorf("could not find the original PipelineRun name in any PipelineRun in namespace %s", namespace) +}