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
127 stars 98 forks source link

Branch protection rules prevent first PR of a merge-batch from being merged #209

Closed oliver-goetz closed 1 month ago

oliver-goetz commented 4 months ago

What happened: When tide is creating a new merge-batch it starts the required tests for the batch, but also trigger those tests for its member with the lowest PR number (PR 9200 in my screenshot below).

Screenshot 2024-02-28 at 16 14 50

tide starts merging the PRs directly when the tests for the merge-batch are completed. If there are branch-protection rules those rules prevent the PR with the lowest number (PR 9200) from being merged until its individual tests succeeded. This results in a race between both test runs. When the merge-batch test run wins, all PRs except the first are merged and the first PR becomes part of the next merge-batch. It is likely that the merge-batch test run wins the race, because it is created first.

Screenshot 2024-02-28 at 16 47 21

As you can see in tide history, poor PR 9200 already lost a couple of races and is still not merged yet.

Screenshot 2024-02-28 at 17 56 15

While I'm writing this issue, 9200 lost another race 😢

What you expected to happen: I except that all PRs of a merge-batch are merged by tide if the tests succeed.

I could imagine two approaches to solve the issue:

  1. tide could wait until all tests of the single PR are finished too before it starts merging
  2. tide could still merge directly when the tests of the merge-batch succeed. In this case it could overwrite the contexts of the tests of the single PR

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

Please provide links to example occurrences, if any: You could check the history of PR 9200 in our prow instance: https://prow.gardener.cloud/tide-history?repo=gardener%2Fgardener&pull=9200

Anything else we need to know?: I started implementing approach 2. in PR #135.