Test-infra: hook: re-evaluate cherry-pick labels when a base branch changes for a PR

[olegch]

What happened:

A PR was first created on a release branch release-1.16 and then rebased to master.
Base branch on the PR was also changed to master, but k8s-robot still thinks that it is a cherry-pick into a release branch.

What you expected to happen:

k8s-robot to take into account the new value of PR's base branch.

How to reproduce it (as minimally and precisely as possible):

Create a PR from a commit on release branch, then rebase the fix branch to master, and change the base branch of the PR to master

Please provide links to example occurrences, if any:

https://github.com/kubernetes/kubernetes/pull/88198#issuecomment-586979860

Anything else we need to know?:

[stevekuznetsov]

@olegch what specific symptoms are causing you to think that the robots still consider this a cherry-pick to a release branch?

[olegch]

The robot continues to set "do-not-merge/cherry-pick-not-approved" label, although I am not that familiar with how the robot works to be absolutely sure.

[stevekuznetsov]

Ah, I see!

[stevekuznetsov]

/retitle hook: re-evaluate cherry-pick labels when a base branch changes for a PR

[stevekuznetsov]

/area prow/hook

[stevekuznetsov]

The cherrypickunapproved plugin needs to be edited to handle the case where a pull request is edited. The trigger plugin already handles such a case and causes new jobs to run, the code that does it is here:

https://github.com/kubernetes/test-infra/blob/8ab6e0b2c0ff4014b8261034a2024f54b2802bae/prow/plugins/trigger/pull-request.go#L95-L116

This is the logic in the cherrypickunapproved plugin that needs to be changed:

https://github.com/kubernetes/test-infra/blob/8ab6e0b2c0ff4014b8261034a2024f54b2802bae/prow/plugins/cherrypickunapproved/cherrypick-unapproved.go#L83-L89

/good-first-issue

[k8s-ci-robot]

@stevekuznetsov:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

The cherrypickunapproved plugin needs to be edited to handle the case where a pull request is edited. The trigger plugin already handles such a case and causes new jobs to run, the code that does it is here:

https://github.com/kubernetes/test-infra/blob/8ab6e0b2c0ff4014b8261034a2024f54b2802bae/prow/plugins/trigger/pull-request.go#L95-L116

This is the logic in the cherrypickunapproved plugin that needs to be changed:

https://github.com/kubernetes/test-infra/blob/8ab6e0b2c0ff4014b8261034a2024f54b2802bae/prow/plugins/cherrypickunapproved/cherrypick-unapproved.go#L83-L89

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[ardakuyumcu]

Is the expected behavior to simply remove the do-not-merge/cherry-pick-not-approved label from the PR in this case? I'm interested in helping here.

[odinuge]

Is the expected behavior to simply remove the do-not-merge/cherry-pick-not-approved label from the PR in this case? I'm interested in helping here.

Yes! When the PR is targeting a branch that is not a release branch, the label should be removed.

[ardakuyumcu]

I can take a stab at this.

/assign

[ardakuyumcu]

I proposed https://github.com/kubernetes/test-infra/pull/16382 before addressing this issue to see if we can leverage the label information included in the pull request event payload.

[ardakuyumcu]

Do we care to handle the opposite case where the base of a PR changes from a non-release branch (e.g. master) to a release branch? I imagine the behavior in that case would be the same as the behavior of the plugin when a new release PR is opened.

[stevekuznetsov]

Yes, handling that as well would be perfect!