Skip to content

Commit 30fddf4

Browse files
chmoueltheakshaypant
authored andcommitted
fix(gitlab): fallback to commit status on /retest
Added a fallback mechanism to retrieve commit statuses from the git provider when the corresponding PipelineRuns are missing from the cluster due to pruning. This ensures that the retest logic correctly identifies previously successful templates and avoids re-running them unnecessarily. Issues Related (partial): #2580 Jira: https://issues.redhat.com/browse/SRVKP-11112 Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
1 parent ae69110 commit 30fddf4

File tree

20 files changed

+913
-12
lines changed

20 files changed

+913
-12
lines changed

docs/content/docs/guides/gitops-commands/_index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ failure is not with your PR but seems to be an infrastructure issue.
2525
- Previously **failed** PipelineRuns exist for the same commit, OR
2626
- No PipelineRun has run for the same commit yet
2727

28-
If a successful PipelineRun already exists for the same commit, `/retest` **skips** it to avoid unnecessary duplication.
28+
If a successful PipelineRun already exists for the same commit, `/retest` **skips** it to avoid unnecessary duplication. When PipelineRuns have been pruned from the cluster (for example, by a retention policy), Pipelines-as-Code falls back to querying the Git provider's commit status API to determine which pipelines previously succeeded, ensuring only failed pipelines are re-run even when the original PipelineRun objects no longer exist.
2929

3030
**When to use it:** To force a rerun regardless of previous status, use:
3131

docs/content/docs/guides/repository-crd/comment-settings.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,15 @@ in the target repository, because GitLab only creates pipeline entries for commi
4141
that actually trigger CI in that specific project. If either status update
4242
succeeds, Pipelines-as-Code does not post a comment on the Merge Request.
4343

44+
Successful commit status reporting is also important for the `/retest` command.
45+
When PipelineRuns are pruned from the cluster, Pipelines-as-Code queries
46+
GitLab's commit status API to identify which pipelines previously succeeded,
47+
ensuring only failed pipelines are re-run. If commit statuses were never
48+
reported (for example, because both status updates failed and only a comment was
49+
posted), Pipelines-as-Code cannot determine prior results and will fail to
50+
re-run any pipelines when `/retest` unless you retest individual pipelineruns
51+
with `/test <pipeline-name>`.
52+
4453
When a status update succeeds, you can see the status in the GitLab UI in the `Pipelines` tab, as
4554
shown in the following example:
4655

pkg/matcher/annotation_matcher.go

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger
424424
if event.EventType == opscomments.RetestAllCommentEventType.String() ||
425425
event.EventType == opscomments.OkToTestCommentEventType.String() {
426426
logger.Debugf("MatchPipelinerunByAnnotation: filtering successful templates for event_type=%s", event.EventType)
427-
filtered := filterSuccessfulTemplates(ctx, logger, cs, event, repo, matchedPRs)
427+
filtered := filterSuccessfulTemplates(ctx, logger, cs, event, repo, vcx, matchedPRs)
428428
if len(filtered) == 0 {
429429
return nil, ErrNoFailedPipelineToRetest
430430
}
@@ -438,7 +438,7 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger
438438

439439
// filterSuccessfulTemplates filters out templates that already have successful PipelineRuns
440440
// when executing /ok-to-test or /retest gitops commands, implementing per-template checking.
441-
func filterSuccessfulTemplates(ctx context.Context, logger *zap.SugaredLogger, cs *params.Run, event *info.Event, repo *apipac.Repository, matchedPRs []Match) []Match {
441+
func filterSuccessfulTemplates(ctx context.Context, logger *zap.SugaredLogger, cs *params.Run, event *info.Event, repo *apipac.Repository, vcx provider.Interface, matchedPRs []Match) []Match {
442442
if event.SHA == "" {
443443
return matchedPRs
444444
}
@@ -469,7 +469,7 @@ func filterSuccessfulTemplates(ctx context.Context, logger *zap.SugaredLogger, c
469469
continue // Skip PipelineRuns without template identification
470470
}
471471

472-
// Check if this PipelineRun succeeded
472+
// Only completed successful PipelineRuns should suppress a /retest.
473473
if pr.Status.GetCondition(apis.ConditionSucceeded).IsTrue() {
474474
// Keep the most recent successful run for each template
475475
if existing, exists := successfulTemplates[originalPRName]; !exists ||
@@ -479,6 +479,20 @@ func filterSuccessfulTemplates(ctx context.Context, logger *zap.SugaredLogger, c
479479
}
480480
}
481481

482+
// Also check provider commit statuses (covers pruned PipelineRuns)
483+
commitStatuses, err := vcx.GetCommitStatuses(ctx, event)
484+
if err != nil {
485+
logger.Warnf("failed to get commit statuses from provider for SHA %s: %v", event.SHA, err)
486+
} else if len(commitStatuses) > 0 {
487+
appName := cs.Info.GetPacOpts().ApplicationName
488+
for _, cs := range commitStatuses {
489+
originalName := parseOriginalPRName(cs.Name, appName)
490+
if originalName != "" && isSuccessStatus(cs.Status) {
491+
successfulTemplates[originalName] = &tektonv1.PipelineRun{} // placeholder
492+
}
493+
}
494+
}
495+
482496
// Filter out templates that have successful runs
483497
var filteredPRs []Match
484498

@@ -497,6 +511,26 @@ func filterSuccessfulTemplates(ctx context.Context, logger *zap.SugaredLogger, c
497511
return filteredPRs
498512
}
499513

514+
// parseOriginalPRName extracts the original PipelineRun name from a commit status name.
515+
// The commit status name format is "ApplicationName / OriginalPRName".
516+
func parseOriginalPRName(checkName, appName string) string {
517+
prefix := appName + " / "
518+
if strings.HasPrefix(checkName, prefix) {
519+
return strings.TrimPrefix(checkName, prefix)
520+
}
521+
return ""
522+
}
523+
524+
// isSuccessStatus returns true if the status string represents a successful state
525+
// across different git providers.
526+
func isSuccessStatus(status string) bool {
527+
switch strings.ToLower(status) {
528+
case "success", "successful", "completed":
529+
return true
530+
}
531+
return false
532+
}
533+
500534
func buildAvailableMatchingAnnotationErr(event *info.Event, pruns []*tektonv1.PipelineRun) string {
501535
errmsg := "available annotations of the PipelineRuns annotations in .tekton/ dir:"
502536
for _, prun := range pruns {

pkg/matcher/annotation_matcher_test.go

Lines changed: 159 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
ghprovider "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/github"
2626
testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients"
2727
ghtesthelper "github.com/openshift-pipelines/pipelines-as-code/pkg/test/github"
28+
testprovider "github.com/openshift-pipelines/pipelines-as-code/pkg/test/provider"
2829
testnewrepo "github.com/openshift-pipelines/pipelines-as-code/pkg/test/repository"
2930
tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
3031
"go.uber.org/zap"
@@ -3078,6 +3079,33 @@ func TestFilterSuccessfulTemplates(t *testing.T) {
30783079
},
30793080
}
30803081

3082+
// Template E: has a running run - should be kept for /retest
3083+
runningPRE := &tektonv1.PipelineRun{
3084+
ObjectMeta: metav1.ObjectMeta{
3085+
Name: "template-e-running",
3086+
Namespace: "test-ns",
3087+
Labels: map[string]string{
3088+
keys.SHA: "test-sha",
3089+
keys.OriginalPRName: "template-e",
3090+
},
3091+
Annotations: map[string]string{
3092+
keys.OriginalPRName: "template-e",
3093+
},
3094+
CreationTimestamp: now,
3095+
},
3096+
Status: tektonv1.PipelineRunStatus{
3097+
Status: knativeduckv1.Status{
3098+
Conditions: knativeduckv1.Conditions{
3099+
apis.Condition{
3100+
Type: apis.ConditionSucceeded,
3101+
Status: corev1.ConditionUnknown,
3102+
Reason: "Running",
3103+
},
3104+
},
3105+
},
3106+
},
3107+
}
3108+
30813109
// PipelineRun with different SHA - should not interfere
30823110
differentSHAPR := &tektonv1.PipelineRun{
30833111
ObjectMeta: metav1.ObjectMeta{
@@ -3128,7 +3156,7 @@ func TestFilterSuccessfulTemplates(t *testing.T) {
31283156
// Setup test clients
31293157
tdata := testclient.Data{
31303158
PipelineRuns: []*tektonv1.PipelineRun{
3131-
successfulPRA, failedPRB, olderSuccessfulPRD, newerSuccessfulPRD, differentSHAPR, noOriginalNamePR,
3159+
successfulPRA, failedPRB, olderSuccessfulPRD, newerSuccessfulPRD, runningPRE, differentSHAPR, noOriginalNamePR,
31323160
},
31333161
Repositories: []*v1alpha1.Repository{repo},
31343162
}
@@ -3169,8 +3197,9 @@ func TestFilterSuccessfulTemplates(t *testing.T) {
31693197
createMatchedPR("template-b"), // has failed run - should be kept
31703198
createMatchedPR("template-c"), // no previous runs - should be kept
31713199
createMatchedPR("template-d"), // has multiple successful runs - should be filtered
3200+
createMatchedPR("template-e"), // has a running run - should be kept
31723201
},
3173-
expectedNames: []string{"template-b", "template-c"},
3202+
expectedNames: []string{"template-b", "template-c", "template-e"},
31743203
},
31753204
{
31763205
name: "Ok-to-test command filters templates with successful runs",
@@ -3250,7 +3279,7 @@ func TestFilterSuccessfulTemplates(t *testing.T) {
32503279
return
32513280
}
32523281

3253-
filtered := filterSuccessfulTemplates(ctx, logger, cs, event, repo, tt.matchedPRs)
3282+
filtered := filterSuccessfulTemplates(ctx, logger, cs, event, repo, &ghprovider.Provider{}, tt.matchedPRs)
32543283

32553284
// Check that the correct number of templates remain
32563285
assert.Equal(t, len(tt.expectedNames), len(filtered),
@@ -3288,3 +3317,130 @@ func TestFilterSuccessfulTemplates(t *testing.T) {
32883317
})
32893318
}
32903319
}
3320+
3321+
func TestFilterSuccessfulTemplatesFallbackToCommitStatuses(t *testing.T) {
3322+
ctx, _ := rtesting.SetupFakeContext(t)
3323+
observer, _ := zapobserver.New(zap.DebugLevel)
3324+
logger := zap.New(observer).Sugar()
3325+
3326+
repo := &v1alpha1.Repository{
3327+
ObjectMeta: metav1.ObjectMeta{
3328+
Name: "test-repo",
3329+
Namespace: "test-ns",
3330+
},
3331+
}
3332+
3333+
// No PipelineRuns in the cluster (pruned)
3334+
tdata := testclient.Data{
3335+
Repositories: []*v1alpha1.Repository{repo},
3336+
}
3337+
stdata, _ := testclient.SeedTestData(t, ctx, tdata)
3338+
3339+
cs := &params.Run{
3340+
Clients: clients.Clients{
3341+
Log: logger,
3342+
Tekton: stdata.Pipeline,
3343+
Kube: stdata.Kube,
3344+
},
3345+
}
3346+
pac := info.NewPacOpts()
3347+
pac.ApplicationName = "Pipelines as Code CI"
3348+
cs.Info.Pac = pac
3349+
3350+
createMatchedPR := func(name string) Match {
3351+
return Match{
3352+
PipelineRun: &tektonv1.PipelineRun{
3353+
ObjectMeta: metav1.ObjectMeta{
3354+
Name: name,
3355+
},
3356+
},
3357+
}
3358+
}
3359+
3360+
tests := []struct {
3361+
name string
3362+
commitStatuses []provider.CommitStatusInfo
3363+
matchedPRs []Match
3364+
expectedNames []string
3365+
}{
3366+
{
3367+
name: "Fallback filters successful templates from commit statuses",
3368+
commitStatuses: []provider.CommitStatusInfo{
3369+
{Name: "Pipelines as Code CI / template-a", Status: "success"},
3370+
{Name: "Pipelines as Code CI / template-b", Status: "failed"},
3371+
},
3372+
matchedPRs: []Match{
3373+
createMatchedPR("template-a"),
3374+
createMatchedPR("template-b"),
3375+
createMatchedPR("template-c"),
3376+
},
3377+
expectedNames: []string{"template-b", "template-c"},
3378+
},
3379+
{
3380+
name: "Fallback with all successful statuses filters all",
3381+
commitStatuses: []provider.CommitStatusInfo{
3382+
{Name: "Pipelines as Code CI / template-a", Status: "success"},
3383+
{Name: "Pipelines as Code CI / template-b", Status: "successful"},
3384+
},
3385+
matchedPRs: []Match{
3386+
createMatchedPR("template-a"),
3387+
createMatchedPR("template-b"),
3388+
},
3389+
expectedNames: []string{},
3390+
},
3391+
{
3392+
name: "Fallback ignores statuses with different app name prefix",
3393+
commitStatuses: []provider.CommitStatusInfo{
3394+
{Name: "Other App / template-a", Status: "success"},
3395+
{Name: "Pipelines as Code CI / template-b", Status: "success"},
3396+
},
3397+
matchedPRs: []Match{
3398+
createMatchedPR("template-a"),
3399+
createMatchedPR("template-b"),
3400+
},
3401+
expectedNames: []string{"template-a"},
3402+
},
3403+
{
3404+
name: "No commit statuses re-runs all",
3405+
commitStatuses: nil,
3406+
matchedPRs: []Match{
3407+
createMatchedPR("template-a"),
3408+
createMatchedPR("template-b"),
3409+
},
3410+
expectedNames: []string{"template-a", "template-b"},
3411+
},
3412+
}
3413+
3414+
for _, tt := range tests {
3415+
t.Run(tt.name, func(t *testing.T) {
3416+
event := &info.Event{
3417+
EventType: "retest",
3418+
SHA: "test-sha-pruned",
3419+
}
3420+
vcx := &testprovider.TestProviderImp{
3421+
CommitStatuses: tt.commitStatuses,
3422+
}
3423+
3424+
filtered := filterSuccessfulTemplates(ctx, logger, cs, event, repo, vcx, tt.matchedPRs)
3425+
3426+
assert.Equal(t, len(tt.expectedNames), len(filtered),
3427+
"Expected %d templates but got %d", len(tt.expectedNames), len(filtered))
3428+
3429+
var actualNames []string
3430+
for _, match := range filtered {
3431+
actualNames = append(actualNames, getName(match.PipelineRun))
3432+
}
3433+
3434+
for _, expectedName := range tt.expectedNames {
3435+
found := false
3436+
for _, actualName := range actualNames {
3437+
if actualName == expectedName {
3438+
found = true
3439+
break
3440+
}
3441+
}
3442+
assert.Assert(t, found, "Expected template %s not found in %v", expectedName, actualNames)
3443+
}
3444+
})
3445+
}
3446+
}

pkg/provider/bitbucketcloud/bitbucket.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,10 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts
159159
return nil
160160
}
161161

162+
func (v *Provider) GetCommitStatuses(_ context.Context, _ *info.Event) ([]provider.CommitStatusInfo, error) {
163+
return nil, nil
164+
}
165+
162166
func (v *Provider) GetTektonDir(_ context.Context, event *info.Event, path, provenance string) (string, error) {
163167
v.provenance = provenance
164168
repositoryFiles, err := v.getDir(event, path)

pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp
170170
return nil
171171
}
172172

173+
func (v *Provider) GetCommitStatuses(_ context.Context, _ *info.Event) ([]provider.CommitStatusInfo, error) {
174+
return nil, nil
175+
}
176+
173177
func (v *Provider) concatAllYamlFiles(ctx context.Context, objects []string, sha string, runevent *info.Event) (string, error) {
174178
var allTemplates string
175179
for _, value := range objects {

pkg/provider/gitea/gitea.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,10 @@ func (v *Provider) createStatusCommit(ctx context.Context, event *info.Event, pa
360360
return nil
361361
}
362362

363+
func (v *Provider) GetCommitStatuses(_ context.Context, _ *info.Event) ([]provider.CommitStatusInfo, error) {
364+
return nil, nil
365+
}
366+
363367
func (v *Provider) GetTektonDir(_ context.Context, event *info.Event, path, provenance string) (string, error) {
364368
// default set provenance from the SHA
365369
revision := event.SHA

pkg/provider/github/github.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,10 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
330330
return nil
331331
}
332332

333+
func (v *Provider) GetCommitStatuses(_ context.Context, _ *info.Event) ([]provider.CommitStatusInfo, error) {
334+
return nil, nil
335+
}
336+
333337
// GetTektonDir retrieves all YAML files from the .tekton directory and returns them as a single concatenated multi-document YAML file.
334338
func (v *Provider) GetTektonDir(ctx context.Context, runevent *info.Event, path, provenance string) (string, error) {
335339
tektonDirSha := ""

0 commit comments

Comments
 (0)