Automate halting merging in core with a CI check #11707
Assignees
Labels
ci-cd
CI, CD, testing, build issues
Comments
18 tasks
This was referenced Nov 25, 2024
mx-psi
pushed a commit
that referenced
this issue
Nov 27, 2024
#### Description In preparation for the merge queue potentially being enabled,, this PR adds the `merge_group` trigger to CI workflows that run in `main` pull requests. #### Link to tracking issue Updates #11707
mx-psi
added a commit
that referenced
this issue
Nov 28, 2024
#### Description As stated in the [Release Procedure document](https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/release.md#releasing-opentelemetry-collector), all merging in Core should be halted while the "Prepare release" PR is open. This PR adds a CI job checking the existence of such a PR, allowing us to enforce these freezes automatically. To make things simple, I decided to distinguish the "Prepare release" PR from others by using a new `prepare:merge-freeze` label. I modified the "Prepare release" action to include that label on the automatically created PR, but in rare cases where maintainers might create the PR themselves, the label can be added manually. Triagers will need to be careful not to accidentally add this label to any random PR to avoid artificial freezes. Because, in the current state, this CI check is run on each commit of a PR, and at no other time, there is a chance for: - False positives: If a commit is pushed during a freeze, and the freeze is later rescinded, the PR will remain unmergeable until a new commit is pushed. A simple solution is to re-run the check manually. - False negatives: If a commit is pushed outside of a freeze, then a freeze is enacted, the PR will remain mergeable despite the freeze. This is more problematic. To solve this second problem and make this check truly useful, two changes in the repository configuration are necessary: - the merge queue should be enabled, allowing us to run checks again just before merging; - this new CI check should be added to the list of required checks for the main branch. This will allow us to automatically cancel the merging of PRs during a freeze. A later PR on the community repository should take care of these configuration changes. Note that another solution to handle outdated CI checks would be to use a tool like [MergeFreeze](https://www.mergefreeze.com), but reading [this document](https://github.com/open-telemetry/community/blob/main/docs/using-github-extensions.md), it sounds like it would be a harder sell with the community than enabling the merge queue. #### Link to tracking issue Updates #11707 #### Testing Simple tests were made on a separate repository to check that enabling the merge queue fixes the false negatives. --------- Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
HongChenTW
pushed a commit
to HongChenTW/opentelemetry-collector
that referenced
this issue
Dec 19, 2024
…ry#11764) #### Description In preparation for the merge queue potentially being enabled,, this PR adds the `merge_group` trigger to CI workflows that run in `main` pull requests. #### Link to tracking issue Updates open-telemetry#11707
HongChenTW
pushed a commit
to HongChenTW/opentelemetry-collector
that referenced
this issue
Dec 19, 2024
#### Description As stated in the [Release Procedure document](https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/release.md#releasing-opentelemetry-collector), all merging in Core should be halted while the "Prepare release" PR is open. This PR adds a CI job checking the existence of such a PR, allowing us to enforce these freezes automatically. To make things simple, I decided to distinguish the "Prepare release" PR from others by using a new `prepare:merge-freeze` label. I modified the "Prepare release" action to include that label on the automatically created PR, but in rare cases where maintainers might create the PR themselves, the label can be added manually. Triagers will need to be careful not to accidentally add this label to any random PR to avoid artificial freezes. Because, in the current state, this CI check is run on each commit of a PR, and at no other time, there is a chance for: - False positives: If a commit is pushed during a freeze, and the freeze is later rescinded, the PR will remain unmergeable until a new commit is pushed. A simple solution is to re-run the check manually. - False negatives: If a commit is pushed outside of a freeze, then a freeze is enacted, the PR will remain mergeable despite the freeze. This is more problematic. To solve this second problem and make this check truly useful, two changes in the repository configuration are necessary: - the merge queue should be enabled, allowing us to run checks again just before merging; - this new CI check should be added to the list of required checks for the main branch. This will allow us to automatically cancel the merging of PRs during a freeze. A later PR on the community repository should take care of these configuration changes. Note that another solution to handle outdated CI checks would be to use a tool like [MergeFreeze](https://www.mergefreeze.com), but reading [this document](https://github.com/open-telemetry/community/blob/main/docs/using-github-extensions.md), it sounds like it would be a harder sell with the community than enabling the merge queue. #### Link to tracking issue Updates open-telemetry#11707 #### Testing Simple tests were made on a separate repository to check that enabling the merge queue fixes the false negatives. --------- Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
mx-psi
added a commit
to open-telemetry/opentelemetry-collector-contrib
that referenced
this issue
Jan 15, 2025
#### Description As stated in the [Release Procedure document](https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/release.md#releasing-opentelemetry-collector-contrib), all merging in Contrib should be halted while the "Prepare release" PR is open. This PR adds a CI check which fails if such a release PR is currently open. This is the same as the CI check introduced in Core as part of [this issue](open-telemetry/opentelemetry-collector#11707) ([Initial PR](open-telemetry/opentelemetry-collector#11744), [bug](open-telemetry/opentelemetry-collector#11808), [fix](open-telemetry/opentelemetry-collector#11849), [bug 2](open-telemetry/opentelemetry-collector#11906), [fix 2](open-telemetry/opentelemetry-collector#11936), [fix 3](open-telemetry/opentelemetry-collector#12045), [fix 4](open-telemetry/opentelemetry-collector#12051)). Like its predecessor, this will only be fully effective once the merge queue is enabled on this repo (see #36788 for progress on that), enabling us to do proper last-minute checks instead of relying on the freeze status at the time of the latest PR update. Similarly, for proper enforcement, this check will need to be marked as required in the main branch protections (I will create an issue on the community repo to that effect once this PR is merged). #### Link to tracking issue Updates #36848 #### Testing Considering the multiple iterations this code went through on Core, it should now work properly without adaptation. However, considering the multiple iterations this code went through on Core, we should expect the unexpected, especially when it comes to the interaction with the merge queue, so release managers will need to be aware of this change, in case it breaks the release process. --------- Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com> Co-authored-by: Moritz Wiesinger <moritz.wiesinger@dynatrace.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We want to automate halting merges with either a bot or some sort of lockfile that is checked by CI
The text was updated successfully, but these errors were encountered: