Skip to content

Commit c9cd7e6

Browse files
waveywaveswaveywaves
committed
fix: /ok-to-test /retest pipelineruns should not be created if last sha successful
when executing /ok-to-test or /retest gitops commands, check whether the last pipelinerun created for the same SHA was successful. If the run was successful, skip new pipelinerun creation
1 parent 1b4ded8 commit c9cd7e6

File tree

5 files changed

+218
-13
lines changed

5 files changed

+218
-13
lines changed

‎pkg/matcher/annotation_matcher.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,16 @@ import (
1212
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
1313
apipac "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
1414
"github.com/openshift-pipelines/pipelines-as-code/pkg/events"
15+
"github.com/openshift-pipelines/pipelines-as-code/pkg/formatting"
1516
"github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments"
1617
"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
1718
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
1819
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
1920
"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
2021
tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
2122
"go.uber.org/zap"
23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
"knative.dev/pkg/apis"
2225
)
2326

2427
const (
@@ -353,12 +356,45 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger
353356
}
354357

355358
if len(matchedPRs) > 0 {
359+
if existingPR := checkForExistingSuccessfulPipelineRun(ctx, logger, cs, event, repo); existingPR != nil {
360+
return []Match{{PipelineRun: existingPR, Repo: repo}}, nil
361+
}
356362
return matchedPRs, nil
357363
}
358364

359365
return nil, fmt.Errorf("%s", buildAvailableMatchingAnnotationErr(event, pruns))
360366
}
361367

368+
// checkForExistingSuccessfulPipelineRun checks if there's an existing successful PipelineRun for the same SHA
369+
// when executing /ok-to-test or /retest gitops commands.
370+
func checkForExistingSuccessfulPipelineRun(ctx context.Context, logger *zap.SugaredLogger, cs *params.Run, event *info.Event, repo *apipac.Repository) *tektonv1.PipelineRun {
371+
if (event.EventType == opscomments.RetestAllCommentEventType.String() ||
372+
event.EventType == opscomments.OkToTestCommentEventType.String()) &&
373+
event.SHA != "" {
374+
labelSelector := fmt.Sprintf("%s=%s", keys.SHA, formatting.CleanValueKubernetes(event.SHA))
375+
existingPRs, err := cs.Clients.Tekton.TektonV1().PipelineRuns(repo.GetNamespace()).List(ctx, metav1.ListOptions{
376+
LabelSelector: labelSelector,
377+
})
378+
if err == nil && len(existingPRs.Items) > 0 {
379+
var lastRun tektonv1.PipelineRun
380+
lastRun = existingPRs.Items[0]
381+
382+
for _, pr := range existingPRs.Items {
383+
if pr.CreationTimestamp.After(lastRun.CreationTimestamp.Time) {
384+
lastRun = pr
385+
}
386+
}
387+
388+
if lastRun.Status.GetCondition(apis.ConditionSucceeded).IsTrue() {
389+
logger.Infof("skipping creation of new pipelinerun for sha %s as the last pipelinerun '%s' has already succeeded",
390+
event.SHA, lastRun.Name)
391+
return &lastRun
392+
}
393+
}
394+
}
395+
return nil
396+
}
397+
362398
func buildAvailableMatchingAnnotationErr(event *info.Event, pruns []*tektonv1.PipelineRun) string {
363399
errmsg := "available annotations of the PipelineRuns annotations in .tekton/ dir:"
364400
for _, prun := range pruns {

‎pkg/matcher/annotation_matcher_test.go

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ import (
3030
zapobserver "go.uber.org/zap/zaptest/observer"
3131
"gotest.tools/v3/assert"
3232
"gotest.tools/v3/golden"
33+
corev1 "k8s.io/api/core/v1"
3334
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
35+
"knative.dev/pkg/apis"
36+
knativeduckv1 "knative.dev/pkg/apis/duck/v1"
3437
rtesting "knative.dev/pkg/reconciler/testing"
3538
)
3639

@@ -2570,3 +2573,130 @@ func TestGetName(t *testing.T) {
25702573
})
25712574
}
25722575
}
2576+
2577+
func TestCheckForExistingSuccessfulPipelineRun(t *testing.T) {
2578+
ctx, _ := rtesting.SetupFakeContext(t)
2579+
logger := zap.NewExample().Sugar()
2580+
2581+
repo := &v1alpha1.Repository{
2582+
ObjectMeta: metav1.ObjectMeta{
2583+
Name: "test-repo",
2584+
Namespace: "test-ns",
2585+
},
2586+
}
2587+
2588+
// Create a successful PipelineRun
2589+
pr := &tektonv1.PipelineRun{
2590+
ObjectMeta: metav1.ObjectMeta{
2591+
Name: "test-pr",
2592+
Namespace: "test-ns",
2593+
Labels: map[string]string{
2594+
keys.SHA: "test-sha",
2595+
},
2596+
CreationTimestamp: metav1.Now(),
2597+
},
2598+
Status: tektonv1.PipelineRunStatus{
2599+
Status: knativeduckv1.Status{
2600+
Conditions: knativeduckv1.Conditions{
2601+
apis.Condition{
2602+
Type: apis.ConditionSucceeded,
2603+
Status: corev1.ConditionTrue,
2604+
},
2605+
},
2606+
},
2607+
},
2608+
}
2609+
2610+
// Create a failed PipelineRun with the same SHA but older
2611+
earlierTime := metav1.NewTime(time.Now().Add(-1 * time.Hour))
2612+
failedPR := &tektonv1.PipelineRun{
2613+
ObjectMeta: metav1.ObjectMeta{
2614+
Name: "failed-pr",
2615+
Namespace: "test-ns",
2616+
Labels: map[string]string{
2617+
keys.SHA: "test-sha",
2618+
},
2619+
CreationTimestamp: earlierTime,
2620+
},
2621+
Status: tektonv1.PipelineRunStatus{
2622+
Status: knativeduckv1.Status{
2623+
Conditions: knativeduckv1.Conditions{
2624+
apis.Condition{
2625+
Type: apis.ConditionSucceeded,
2626+
Status: corev1.ConditionFalse,
2627+
},
2628+
},
2629+
},
2630+
},
2631+
}
2632+
2633+
// Setup test clients
2634+
tdata := testclient.Data{
2635+
PipelineRuns: []*tektonv1.PipelineRun{pr, failedPR},
2636+
Repositories: []*v1alpha1.Repository{repo},
2637+
}
2638+
stdata, _ := testclient.SeedTestData(t, ctx, tdata)
2639+
2640+
cs := &params.Run{
2641+
Clients: clients.Clients{
2642+
Log: logger,
2643+
Tekton: stdata.Pipeline,
2644+
Kube: stdata.Kube,
2645+
},
2646+
}
2647+
2648+
tests := []struct {
2649+
name string
2650+
eventType string
2651+
sha string
2652+
wantPR bool
2653+
}{
2654+
{
2655+
name: "Retest command with matching SHA should find successful PR",
2656+
eventType: opscomments.RetestAllCommentEventType.String(),
2657+
sha: "test-sha",
2658+
wantPR: true,
2659+
},
2660+
{
2661+
name: "Ok-to-test command with matching SHA should find successful PR",
2662+
eventType: opscomments.OkToTestCommentEventType.String(),
2663+
sha: "test-sha",
2664+
wantPR: true,
2665+
},
2666+
{
2667+
name: "Retest command with non-matching SHA should not find PR",
2668+
eventType: opscomments.RetestAllCommentEventType.String(),
2669+
sha: "other-sha",
2670+
wantPR: false,
2671+
},
2672+
{
2673+
name: "Different event type should not find PR",
2674+
eventType: opscomments.TestAllCommentEventType.String(),
2675+
sha: "test-sha",
2676+
wantPR: false,
2677+
},
2678+
}
2679+
2680+
for _, tt := range tests {
2681+
t.Run(tt.name, func(t *testing.T) {
2682+
event := &info.Event{
2683+
EventType: tt.eventType,
2684+
SHA: tt.sha,
2685+
}
2686+
2687+
foundPR := checkForExistingSuccessfulPipelineRun(ctx, logger, cs, event, repo)
2688+
2689+
if tt.wantPR && foundPR == nil {
2690+
t.Errorf("Expected to find a successful PipelineRun, but got nil")
2691+
}
2692+
2693+
if !tt.wantPR && foundPR != nil {
2694+
t.Errorf("Expected not to find a PipelineRun, but found %s", foundPR.Name)
2695+
}
2696+
2697+
if tt.wantPR && foundPR != nil {
2698+
assert.Equal(t, "test-pr", foundPR.Name)
2699+
}
2700+
})
2701+
}
2702+
}

‎test/gitea_test.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import (
3131
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
3232
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings"
3333
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
34-
"github.com/openshift-pipelines/pipelines-as-code/pkg/sort"
3534
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/cctx"
3635
tknpactest "github.com/openshift-pipelines/pipelines-as-code/test/pkg/cli"
3736
tgitea "github.com/openshift-pipelines/pipelines-as-code/test/pkg/gitea"
@@ -439,7 +438,7 @@ func TestGiteaConfigMaxKeepRun(t *testing.T) {
439438
_, err := twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts)
440439
assert.NilError(t, err)
441440

442-
time.Sleep(15 * time.Second) // “Evil does not sleep. It waits.” - Galadriel
441+
time.Sleep(15 * time.Second) // "Evil does not sleep. It waits." - Galadriel
443442

444443
prs, err := topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(context.Background(), metav1.ListOptions{})
445444
assert.NilError(t, err)
@@ -466,12 +465,12 @@ func TestGiteaConfigCancelInProgress(t *testing.T) {
466465
_, f := tgitea.TestPR(t, topts)
467466
defer f()
468467

469-
time.Sleep(3 * time.Second) // “Evil does not sleep. It waits.” - Galadriel
468+
time.Sleep(3 * time.Second) // "Evil does not sleep. It waits." - Galadriel
470469

471470
// wait a bit that the pipelinerun had created
472471
tgitea.PostCommentOnPullRequest(t, topts, "/retest")
473472

474-
time.Sleep(2 * time.Second) // “Evil does not sleep. It waits.” - Galadriel
473+
time.Sleep(2 * time.Second) // "Evil does not sleep. It waits." - Galadriel
475474

476475
targetRef := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("cancel-in-progress")
477476
entries, err := payload.GetEntries(prmap, topts.TargetNS, topts.DefaultBranch, topts.TargetEvent, map[string]string{})
@@ -501,8 +500,7 @@ func TestGiteaConfigCancelInProgress(t *testing.T) {
501500
prs, err := topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(context.Background(), metav1.ListOptions{})
502501
assert.NilError(t, err)
503502

504-
sort.PipelineRunSortByStartTime(prs.Items)
505-
assert.Equal(t, len(prs.Items), 3, "should have 2 pipelineruns, but we have: %d", len(prs.Items))
503+
// Reset counter and count cancelled PipelineRuns from the updated list
506504
cancelledPr := 0
507505
for _, pr := range prs.Items {
508506
if pr.GetStatusCondition().GetCondition(apis.ConditionSucceeded).GetReason() == "Cancelled" {
@@ -518,12 +516,18 @@ func TestGiteaConfigCancelInProgress(t *testing.T) {
518516
tgitea.PostCommentOnPullRequest(t, topts, "/retest")
519517
tgitea.WaitForStatus(t, topts, "heads/"+targetRef, "", false)
520518

519+
// Get a fresh list of PipelineRuns after all tests
520+
prs, err = topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(context.Background(), metav1.ListOptions{})
521+
assert.NilError(t, err)
522+
523+
// Reset counter and count cancelled PipelineRuns from the updated list
524+
cancelledPr = 0
521525
for _, pr := range prs.Items {
522526
if pr.GetStatusCondition().GetCondition(apis.ConditionSucceeded).GetReason() == "Cancelled" {
523527
cancelledPr++
524528
}
525529
}
526-
assert.Equal(t, cancelledPr, 2, "tweo pr should have been canceled")
530+
assert.Equal(t, cancelledPr, 2, "two prs should have been canceled")
527531
}
528532

529533
func TestGiteaConfigCancelInProgressAfterPRClosed(t *testing.T) {

‎test/github_config_maxkeepruns_test.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,27 @@ func TestGithubMaxKeepRuns(t *testing.T) {
2626
g.RunPullRequest(ctx, t)
2727
defer g.TearDown(ctx, t)
2828

29-
g.Cnx.Clients.Log.Infof("Creating /retest in PullRequest")
30-
_, _, err := g.Provider.Client().Issues.CreateComment(ctx, g.Options.Organization, g.Options.Repo, g.PRNumber,
31-
&ghlib.IssueComment{Body: ghlib.Ptr("/retest")})
29+
// Wait for the first pipeline run to be created
30+
time.Sleep(5 * time.Second)
31+
32+
// Get the name of the PipelineRun to retest specifically
33+
prList, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{})
34+
assert.NilError(t, err)
35+
assert.Assert(t, len(prList.Items) > 0, "Expected at least one PipelineRun to be created")
36+
37+
pipelineRunName := ""
38+
for _, pr := range prList.Items {
39+
if originalName, ok := pr.GetAnnotations()[keys.OriginalPRName]; ok {
40+
pipelineRunName = originalName
41+
break
42+
}
43+
}
44+
assert.Assert(t, pipelineRunName != "", "Could not find the original PipelineRun name")
45+
46+
// Use retest with specific PipelineRun name
47+
g.Cnx.Clients.Log.Infof("Creating /retest %s in PullRequest", pipelineRunName)
48+
_, _, err = g.Provider.Client().Issues.CreateComment(ctx, g.Options.Organization, g.Options.Repo, g.PRNumber,
49+
&ghlib.IssueComment{Body: ghlib.Ptr("/retest " + pipelineRunName)})
3250
assert.NilError(t, err)
3351

3452
g.Cnx.Clients.Log.Infof("Wait for the second repository update to be updated")

‎test/gitlab_merge_request_test.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,32 @@ func TestGitlabMergeRequest(t *testing.T) {
104104
assert.Equal(t, "Merge Request", prsNew.Items[i].Annotations[keys.EventType])
105105
}
106106

107-
runcnx.Clients.Log.Infof("Sending /retest comment on MergeRequest %s/-/merge_requests/%d", projectinfo.WebURL, mrID)
107+
// Wait a moment to ensure all PipelineRuns are fully created
108+
time.Sleep(5 * time.Second)
109+
110+
// Query for an existing PipelineRun to retest specifically
111+
prList, err := runcnx.Clients.Tekton.TektonV1().PipelineRuns(targetNS).List(ctx, metav1.ListOptions{})
112+
assert.NilError(t, err)
113+
assert.Assert(t, len(prList.Items) > 0, "Expected at least one PipelineRun to be created")
114+
115+
pipelineRunName := ""
116+
for _, pr := range prList.Items {
117+
if originalName, ok := pr.GetAnnotations()[keys.OriginalPRName]; ok {
118+
pipelineRunName = originalName
119+
break
120+
}
121+
}
122+
assert.Assert(t, pipelineRunName != "", "Could not find the original PipelineRun name")
123+
124+
runcnx.Clients.Log.Infof("Sending /retest %s comment on MergeRequest %s/-/merge_requests/%d", pipelineRunName, projectinfo.WebURL, mrID)
108125
_, _, err = glprovider.Client().Notes.CreateMergeRequestNote(opts.ProjectID, mrID, &clientGitlab.CreateMergeRequestNoteOptions{
109-
Body: clientGitlab.Ptr("/retest"),
126+
Body: clientGitlab.Ptr("/retest " + pipelineRunName),
110127
})
111128
assert.NilError(t, err)
112129

113130
sopt = twait.SuccessOpt{
114131
Title: commitTitle,
115-
OnEvent: opscomments.RetestAllCommentEventType.String(),
132+
OnEvent: opscomments.RetestSingleCommentEventType.String(),
116133
TargetNS: targetNS,
117134
NumberofPRMatch: 5, // this is the max we get in repos status
118135
SHA: "",

0 commit comments

Comments
 (0)