kubernetes-sigs / prow

Prow is a Kubernetes based CI/CD system developed to serve the Kubernetes community. This repository contains Prow source code and Hugo sources for Prow documentation site.
https://docs.prow.k8s.io
Apache License 2.0
129 stars 99 forks source link

cherrypicker plugin can't process several requests in the same comment #292

Closed VannTen closed 2 weeks ago

VannTen commented 1 month ago

I'm reposting here the original report in test-infra: kubernetes/test-infra#29666

Expected behavior:

A comment of the form:

/cherrypick release-X
/cherrypick release-Y

should trigger two answers announcing the cherry-pick for each , or alternatively one message with all the planned cherry-picks. Then when the branch is merged both cherrypick should be created against the target branch (the behavior is consistent, I don't have a link right now but I've noted it on merged PRs as well)

Actual behavior: Only the first cherrypick commands is acknowledged by the bot.

Examples: For instance this comment only triggers a cherry-pick on the first mentioned branch.

This was implemented but has apparently regressed.

xrstf commented 1 month ago

I tried to look into this and am wondering if the cherrypicker would really not have created all of the requested cherrypicks, even though it might have responded with a misleading message.

Because the handlePullRequest function does loop through all target branches and is covered by a unit test. But the handleSingleComment function only ever takes the first match and formulates a response for it.

I must admit I have never tried to ignore that message and just let the plugin do its thing once the PR merges, I also always manually created new GitHub comments for the remaining target branches.

xrstf commented 1 month ago

So I tested this today and it seems I am right: https://github.com/kubermatic/kubermatic/pull/13811 -- the response comment only mentioned the first branch, but ultimately cherrypicker tried to create 2 PRs.

VannTen commented 1 month ago

Good to know o/ I guess the only things that needs changing is the prow message then, I guess we're not the only ones confused by this ^^.

xrstf commented 1 month ago

/kind bug

VannTen commented 2 weeks ago

Fixed by #305 ! (and it works on a recent PR :+1: ) Thanks !