fix: Fixed PipelineRun cancelling on PR closed

[zakisk]

issue: when a PipelineRun is running on PR and it doesn't have cancel-in-progress annotation specified, on the PR close PAC is cancelling the PipelinRun.

solution: in code on PR close, it wasn't checked before that a PipelineRun has cancel-in-progress annotation or not and PAC just cancel PipelineRuns regardless of feature enabled. Now cancel-in-progress annotation is added to PipelineRun labels so that it can be filtered using labelSelector.

https://issues.redhat.com/browse/SRVKP-7456

Changes

Submitter Checklist

• 📝 Ensure your commit message is clear and informative. Refer to the How to write a git commit message guide. Include the commit message in the PR body rather than linking to an external site (e.g., Jira ticket).

• ♽ Run make test lint before submitting a PR to avoid unnecessary CI processing. Consider installing pre-commit and running pre-commit install in the repository root for an efficient workflow.

• ✨ We use linters to maintain clean and consistent code. Run make lint before submitting a PR. Some linters offer a --fix mode, executable with make fix-linters (ensure markdownlint and golangci-lint are installed).

• 📖 Document any user-facing features or changes in behavior.

• 🧪 While 100% coverage isn't required, we encourage unit tests for code changes where possible.

• 🎁 If feasible, add an end-to-end test. See README for details.

• 🔎 Address any CI test flakiness before merging, or provide a valid reason to bypass it (e.g., token rate limitations).

• If adding a provider feature, fill in the following details:

  • GitHub App
  • GitHub Webhook
  • Gitea/Forgejo
  • GitLab
  • Bitbucket Cloud
  • Bitbucket Data Center

  (update the provider documentation accordingly)

[chmouel]

i think this loop would not do what expected....

[zakisk]

by this I expect that after PR close it should check PipelineRun is running for 10 secs (on 1 sec interval) to confirm that PR close doesn't cancel the PipelineRun.

[zakisk]

if the status of the PipelineRun is not changed in 10 secs then it means PipelineRun is not cancelled and running as expected.

[chmouel]

ah yeah but the thing is a bit fragile tho?

what about something likethis instead

	found := false
	var prReason string

	for range 10 {
		prs, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{})
		if err != nil {
			t.Logf("failed to list PipelineRuns: %v", err)
			time.Sleep(1 * time.Second)
			continue // try again
		}
		if len(prs.Items) != 1 {
			t.Logf("expected 1 PipelineRun, got %d", len(prs.Items))
			time.Sleep(1 * time.Second)
			continue // try again
		}
		// Check all conditions, get the right one
		conditions := prs.Items[0].Status.GetConditions()
		for _, c := range conditions {
			if c.Type == apis.ConditionSucceeded {
				prReason = string(c.Reason)
				if prReason == string(tektonv1.PipelineRunReasonRunning) {
					found = true
					break
				}
			}
		}
		if found {
			break
		}
		time.Sleep(1 * time.Second)
	}
	assert.Equal(t, true, found, fmt.Sprintf("PipelineRun never reached 'Running' state, last reason: %v", prReason))

It now correctly iterates conditions using a range loop instead of assuming [0].
It explicitly checks for c.Type == apis.ConditionSucceeded.
The if found { break } wil make the loop to exit as soon as the desired state is detected, which is more efficient than the original loop which always ran 10 times.
Adds continue on list errors or count mismatches, allowing retries within the 10 attempts.

i haven't tested it fully

[zakisk]

sounds good! but I want to observe for 10 secs that PipelineRun is still running and not getting cancelled. on found we may not break the loop. (will update accordingly)

[Copilot]

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

[zakisk]

/test linters