Skip to content

fix(gitlab): trigger PipelineRun only on label change #2122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

PuneetPunamiya
Copy link
Contributor

@PuneetPunamiya PuneetPunamiya commented Jun 11, 2025

  • Before this patch pipeline was triggered on any merge request update, regardless of what changed such as title, description or assigne changes

  • With the current change, pipeline will be triggered only when labels are added or removed

  • Fixes: https://issues.redhat.com/browse/SRVKP-7383

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)

@PuneetPunamiya PuneetPunamiya force-pushed the fix-pipeline-of-mr-retriggered-when-desc-changed branch from 03c7ddf to 6a0a3cd Compare June 12, 2025 07:46
@PuneetPunamiya PuneetPunamiya changed the title fix(gitlab): trigger PipelineRun only on PR update or label change fix(gitlab): trigger PipelineRun only on label change Jun 12, 2025
@PuneetPunamiya PuneetPunamiya force-pushed the fix-pipeline-of-mr-retriggered-when-desc-changed branch from 6a0a3cd to b1d8cb1 Compare June 12, 2025 12:27
@zakisk zakisk force-pushed the fix-pipeline-of-mr-retriggered-when-desc-changed branch from b1d8cb1 to e0fe4ed Compare June 16, 2025 09:30
Before this patch pipeline was triggered on any merge request update,
regardless of what changed such as title, description or assigne changes

With the current change, pipeline will be triggered only when labels
are added or removed

Signed-off-by: PuneetPunamiya <[email protected]>
@PuneetPunamiya PuneetPunamiya force-pushed the fix-pipeline-of-mr-retriggered-when-desc-changed branch from e0fe4ed to f8866d3 Compare June 16, 2025 09:46
Comment on lines +666 to +689
// Send another Push to make an update and make sure we react to it
entries, err = payload.GetEntries(map[string]string{
"hello-world.yaml": "testdata/pipelinerun.yaml",
}, targetNS, projectinfo.DefaultBranch,
triggertype.PullRequest.String(), map[string]string{})
assert.NilError(t, err)
scmOpts.BaseRefName = targetRefName
_ = scm.PushFilesToRefGit(t, scmOpts, entries)

sopt := twait.SuccessOpt{
Title: commitTitle,
OnEvent: "Merge Request",
TargetNS: targetNS,
NumberofPRMatch: 4,
SHA: "",
}
twait.Succeeded(ctx, t, runcnx, opts, sopt)
prsNew, err := runcnx.Clients.Tekton.TektonV1().PipelineRuns(targetNS).List(ctx, metav1.ListOptions{})
assert.NilError(t, err)
assert.Assert(t, len(prsNew.Items) == 4)

for i := 0; i < len(prsNew.Items); i++ {
assert.Equal(t, "Merge Request", prsNew.Items[i].Annotations[keys.EventType])
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think don't need to re-trigger two pipelineruns again, pipelineruns created on merge request open are sufficient.

Comment on lines +691 to +695
runcnx.Clients.Log.Infof("Changing Title on MergeRequest %s/-/merge_requests/%d", projectinfo.WebURL, mrID)
_, _, err = glprovider.Client().MergeRequests.UpdateMergeRequest(opts.ProjectID, mrID, &clientGitlab.UpdateMergeRequestOptions{
Title: clientGitlab.Ptr("test"),
})
assert.NilError(t, err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after updating title, if you immediately check whether PipelineRuns are created or not, it will always be 4 because it takes sometime to create PipelineRun and the test is exiting right after updating the title so I think you should wait for sometime to confirm that no PipelineRun has been created. you can do something similar below:

for i := 0; i < 10; i++ {

changes.Assignees.Previous == nil && changes.Assignees.Current == nil &&
changes.Reviewers.Previous == nil && changes.Reviewers.Current == nil &&
changes.Description.Previous == "" && changes.Description.Current == "" &&
!changes.Draft.Previous && !changes.Draft.Current &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chmouel we don't consider MR's draft state in GitLab, right? should we consider a "Merge Request Event" if an MR is converted to "ready" from draft??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants