Support `tide/priority-merge` label to expedite critical PR's

[stbenjam]

What would you like to be added:

Introduce a tide/priority-merge label to explicitly prioritize a PR above all others. When applied, no other PRs in the repository should be merged until the labeled priority PR's have landed. The PR should not be batch tested with any other PR. In the rare event multiple PR's have the label, they should be processed independently in order of their presence in the queue.

Why is this needed:

We occasionally need to land a change rapidly and tide offers no facilities to let us do that. Occasionally, we will override tests to force a PR to land, but even then another merge could cause tide to retest yet again as overrides are not sticky. Background batching also complicates overrides, leaving critical PRs stuck. While manual intervention (Big Green Button) is possible, a native tide mechanism is preferable.

Useful for:

• large refactors that would end up with merge conflicts if anything else lands between lgtm -> testing completing
• critical fixes like reverts, hotfixes, etc that must land ASAP
• cross-repo dependent PR's that need to land somewhat synchronously (tide retests could delay one by hours)

[petr-muller]

Historically similar functionality existed, caused issues and was eventually removed because of that. So based on that I am cautiously 👎 on this because the benefit over the Big Green Button is fairly small, at least from my PoV.

While manual intervention (Big Green Button) is possible, a native tide mechanism is preferable.

Preferable why? Handling rare, privileged operations with fallback to GH operations over carrying potentially complex code for them (often resulting in confusing scenarios when PRs are not merging) is a pattern that served Prow well.

/area tide
/kind feature

[stbenjam]

Historically similar functionality existed, caused issues and was eventually removed because of that.

Can you link anything to help me understand what that functionality was?

Preferable why? Handling rare, privileged operations

I don't think the 3 cases I listed are rare. At least one of the three happens weekly in OpenShift.

We rarely give folks green button access; giving OWNERs label privileges is much different. You would still need a second party /lgtm to land -- something you don't get when someone has direct merge privileges except by the honor system.

This also gives you the option to let tests run to completion without something landing meanwhile and causing merge conflicts or invalidating the previous testing.

Other PRs should get a tide status that indicates they can't merge until X merges.

[petr-muller]

Went digging, because I remember the conversations, not the actual events.

The priority-problematic system was actually Tide's predecessor and the problems are at least partially described in Tide DD and the reasoning is that Tide optimizes for total merge throughput over the ability to merge a specific PR fast (and the DD actually does not rule out the possibility to add a priority level like you suggest). That's the overall theme I remember from the occasional discussions when Tide priority came up.

There's a bit of discussion on the topic in kubernetes/test-infra#14134 (comment)

There's actually (I did not even know lol) a partial priority mechanism implemented in kubernetes/test-infra#20468 (issue kubernetes/test-infra#18985 usage kubernetes/test-infra#20576)

It does not do everything you want (docs are not great but kubernetes/test-infra#20468 (comment) helps a bit) but it may help OpenShift in the absence of the full feature that you want - I don't think OpenShift instance has this enabled, I have not looked.

I'm not blocking the request, just am personally somewhat -1 about it, but I am not the only approver and I will definitely not stand in the way. I am afraid implementing the ask would be a fairly big lift and would need a bit of work to be done both on the coding and review side. But if someone from OpenShift has capacity to do that then great (and I would help to make that happen)!

[BenTheElder]

I'm also -1, there are various ways you can merge a PR in an emergency but this tool is already rather complex and somewhat buggy (intermittent issues with failure to fallback to serial merges #474 (comment)), doesn't have enough active maintainers, and this is not something Kubernetes would use.

In an urgent situation (say landing a CVE fix quickly) you can trust some small number of people to bypass the merge robot.

During code freeze scenario, we change the merge requirements to require an additional milestone (you could use a label) which has separate permissions, and block other PRs.

Before we even remotely consider something like this, I'd like to see more support for @petr-muller and every day maintenance.

This project also needs proper releasing, other communities are using it but are pulling from our staging images which are not intended to be used by third parties. Eventually this is a problem.

That needs to be undug before we overcomplicate things for third party use cases

#113