Skip to content

Commit 2f78eb8

Browse files
chmouelvdemeester
authored andcommitted
fix: Fix cancel in progress to act same pr/branch
We need to ensure that we only cancel the pipeline run that is running on the same branch as the current PR or push event. This ensures that we don't cancel the wrong pipeline run. On PullRequest events, we only cancel the pipeline run that is running on the same Pull Request number. On Push events, we only cancel the pipeline run that is running on the same SourceBranch (which is the same as HeadBranch on push) This avoids the issue when you have multiple pull request running on a repo and we don't cancel the wrong ones that are running for a different PR. Signed-off-by: Chmouel Boudjnah <[email protected]>
1 parent 1b75dfb commit 2f78eb8

File tree

5 files changed

+477
-64
lines changed

5 files changed

+477
-64
lines changed

docs/content/docs/guide/running.md

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -114,17 +114,23 @@ You can choose to cancel a PipelineRun that is currently in progress. This can
114114
be done by adding the annotation `pipelinesascode.tekton.dev/cancel-in-progress:
115115
"true"` in the PipelineRun definition.
116116
117-
This feature only works if the PipelineRun is in progress. If the PipelineRun
118-
has already completed or has been cancelled, it will be skipped. (For persistent
119-
settings, refer to the [max-keep-run annotation]({{< relref
120-
"/docs/guide/cleanups.md" >}}) instead.)
121-
122-
The cancellation occurs after the latest PipelineRun has been successfully
123-
created and started. This annotation cannot be used to ensure that only one
124-
PipelineRun is active at any time.
125-
126-
Currently, `cancel-in-progress` cannot be used in conjunction with [concurrency
127-
limit]({{< relref "/docs/guide/repositorycrd.md#concurrency" >}}).
117+
This feature is effective only when the `PipelineRun` is in progress. If the
118+
`PipelineRun` has already completed or been cancelled, the cancellation will
119+
have no effect. (see the [max-keep-run annotation]({{< relref
120+
"/docs/guide/cleanups.md" >}}) instead to clean old `PipelineRuns`.)
121+
122+
The cancellation only applies to `PipelineRuns` within the scope of the current
123+
`PullRequest` or the targeted branch for Push events. For example, if two
124+
`PullRequests` each have a `PipelineRun` with the same name and the
125+
cancel-in-progress annotation, only the `PipelineRun` in the specific PullRequest
126+
will be cancelled. This prevents interference between separate PullRequests.
127+
128+
The cancellation of the older `PipelineRuns` will be executed only after the
129+
latest `PipelineRun` has been created and started successfully. This annotation
130+
cannot guarantee that only one `PipelineRun` will be active at any given time.
131+
132+
Currently, `cancel-in-progress` cannot be used in conjunction with the [concurrency
133+
limit]({{< relref "/docs/guide/repositorycrd.md#concurrency" >}}) setting.
128134

129135
### Cancelling a PipelineRun with a GitOps command
130136

pkg/pipelineascode/cancel_pipelineruns.go

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,17 @@ import (
66
"strconv"
77
"sync"
88

9-
"github.com/openshift-pipelines/pipelines-as-code/pkg/action"
10-
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
11-
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
12-
"github.com/openshift-pipelines/pipelines-as-code/pkg/formatting"
13-
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
149
tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
1510
"go.uber.org/zap"
1611
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1712
"k8s.io/apimachinery/pkg/labels"
1813
"k8s.io/apimachinery/pkg/selection"
14+
15+
"github.com/openshift-pipelines/pipelines-as-code/pkg/action"
16+
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
17+
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
18+
"github.com/openshift-pipelines/pipelines-as-code/pkg/formatting"
19+
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
1920
)
2021

2122
var cancelMergePatch = map[string]interface{}{
@@ -38,19 +39,25 @@ func (p *PacRun) cancelInProgress(ctx context.Context, matchPR *tektonv1.Pipelin
3839
return nil
3940
}
4041

41-
if repo.Spec.ConcurrencyLimit != nil && *repo.Spec.ConcurrencyLimit > 0 {
42-
return fmt.Errorf("cancel in progress is not supported with concurrency limit")
43-
}
44-
4542
prName, ok := matchPR.GetAnnotations()[keys.OriginalPRName]
4643
if !ok {
4744
return nil
4845
}
49-
labelSelector := getLabelSelector(map[string]string{
46+
47+
if repo.Spec.ConcurrencyLimit != nil && *repo.Spec.ConcurrencyLimit > 0 {
48+
return fmt.Errorf("cancel in progress is not supported with concurrency limit")
49+
}
50+
51+
labelMap := map[string]string{
5052
keys.URLRepository: formatting.CleanValueKubernetes(p.event.Repository),
5153
keys.OriginalPRName: prName,
52-
})
53-
54+
keys.EventType: p.event.TriggerTarget.String(),
55+
}
56+
if p.event.TriggerTarget == triggertype.PullRequest {
57+
labelMap[keys.PullRequest] = strconv.Itoa(p.event.PullRequestNumber)
58+
}
59+
labelSelector := getLabelSelector(labelMap)
60+
p.run.Clients.Log.Infof("cancel-in-progress: selecting pipelineRuns to cancel with labels: %v", labelSelector)
5461
prs, err := p.run.Clients.Tekton.TektonV1().PipelineRuns(matchPR.GetNamespace()).List(ctx, metav1.ListOptions{
5562
LabelSelector: labelSelector,
5663
})
@@ -62,10 +69,27 @@ func (p *PacRun) cancelInProgress(ctx context.Context, matchPR *tektonv1.Pipelin
6269
if pr.GetName() == matchPR.GetName() {
6370
continue
6471
}
72+
if sourceBranch, ok := pr.GetAnnotations()[keys.SourceBranch]; ok {
73+
// NOTE(chmouel): Every PR has their own branch and so is every push to different branch
74+
// it means we only cancel pipelinerun of the same name that runs to
75+
// the unique branch. Note: HeadBranch is the branch from where the PR
76+
// comes from in git jargon.
77+
if sourceBranch != p.event.HeadBranch {
78+
p.logger.Infof("cancel-in-progress: skipping pipelinerun %v/%v as it is not from the same branch, annotation source-branch: %s event headbranch: %s", pr.GetNamespace(), pr.GetName(), sourceBranch, p.event.HeadBranch)
79+
continue
80+
}
81+
}
82+
83+
if pr.IsPending() {
84+
p.logger.Infof("cancel-in-progress: skipping pipelinerun %v/%v as it is pending", pr.GetNamespace(), pr.GetName())
85+
}
86+
6587
if pr.IsDone() {
88+
p.logger.Infof("cancel-in-progress: skipping pipelinerun %v/%v as it is done", pr.GetNamespace(), pr.GetName())
6689
continue
6790
}
6891
if pr.IsCancelled() || pr.IsGracefullyCancelled() || pr.IsGracefullyStopped() {
92+
p.logger.Infof("cancel-in-progress: skipping pipelinerun %v/%v as it is already in %v state", pr.GetNamespace(), pr.GetName(), pr.Spec.Status)
6993
continue
7094
}
7195

0 commit comments

Comments
 (0)