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

[oliver-goetz]

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).

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.

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

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'm happy to implement a solution if we can agree on an approach.

[oliver-goetz]

/sig testing

[petr-muller]

Ouch, that's painful. Nice report 👍

Personally I like the second approach:

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

When a batch passes, Tide could abort the individual jobs for the single PR and backfill their results from the jobs run within a batch. Not sure how feasible that is, but it feels better than waiting for the single PR (which may take more time and even fail in the presence of flakes)

[oliver-goetz]

Okay 👍
I did not find the time yet, but maybe I can do something about it the next weeks 😅

[oliver-goetz]

/assign

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[BenTheElder]

unfortunately github doesn't support migrating issues across organizations, this appears to be active? @oliver-goetz can you re-file this against kubernetes-sigs/prow, we're trying to consolidate prow issues there as part of the migration.

I dropped a ping in #prow slack to nudge re: reviewing your PR

[oliver-goetz]

Thanks @BenTheElder 😄
I just re-filed this issue: kubernetes-sigs/prow#209

/close

[k8s-ci-robot]

@oliver-goetz: Closing this issue.

In response to this:

Thanks @BenTheElder 😄
I just re-filed this issue: kubernetes-sigs/prow#209

/close

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-sigs/prow repository.

[BenTheElder]

Thanks!