[chore] Fix "Prepare Release" action #11849
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11849 +/- ##
=======================================
Coverage 91.58% 91.58%
=======================================
Files 446 446
Lines 23741 23741
=======================================
Hits 21743 21743
Misses 1623 1623
Partials 375 375 ☔ View full report in Codecov by Sentry. |
atoulme
approved these changes
Dec 11, 2024
songy23
approved these changes
Dec 12, 2024
codeboten
approved these changes
Dec 12, 2024
Merged
via the queue into
open-telemetry:main
with commit Dec 12, 2024
220b583
45 of 60 checks passed
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 17, 2024
#### Description The last attempt at making the Merge freeze check work in release PRs failed (#11906). This PR tries a different approach: it changes the criteria of the Merge freeze check, so that a freeze is enacted iff there is an open PR authored by @opentelemetrybot whose title contains "[chore] Prepare release" (note that if it weren't for the author, this PR would qualify). This PR additionally reverts #11849, so no label is added to the release PR. I also added the `pull_request.enqueued` trigger, taking inspiration [from Merge Freeze](https://docs.mergefreeze.com/github-merge-queue#how-it-works), to see if it could help reject PRs earlier. I tried to make sure the freeze check would be properly skipped for the release PR itself, both in PR checks and in the merge queue, but given the state of Github's documentation, I'm not very confident about this. Notably, these is an edge case where I'm not sure what would happen: what if another PR gets added to the merge queue at the same time as the release PR? How many times would the "merge_group" check run, and with what values for "github.event.merge_group.head_commit"? Would both PR be booted out of the queue (not great)? Would both be accepted (way worse)? Does it depend on the order? #### Link to tracking issue Fixes #11906 and fixed #11808 #### Testing As always with this, it's pretty much impossible to test before merging. Once merged, I strongly recommend we do the following test to make sure that this issue does not block the real release process again: - Create two dummy PRs that change nothing of consequence: the freeze check should pass - Run the "Prepare release" action - Rerun the freeze check on one of the dummy PRs: it should now fail - Approve the second PR and try to merge it: it should be booted out of the merge queue - Close all test PRs This unfortunately does not test whether the release PR gets merged properly, but I don't see how to test until the next release process, unfortunately.
HongChenTW
pushed a commit
to HongChenTW/opentelemetry-collector
that referenced
this pull request
Dec 19, 2024
#### Description The "Prepare Release" action failed during the latest release process, as the OpenTelemetry Bot, which opens the release PR, does not have the permissions to add the new `release:merge-freeze` label. This PR fixes this by creating the PR with the bot token, then adding the label separately using the regular CI token. Since we already use default CI tokens on -contrib to add labels, I assume they have the permission to do that on this repo as well. I deemed this simpler than the alternative solutions mentioned in the tracking issue, though I'm not certain it will work. Note that the tracking issue is a release blocker. #### Link to tracking issue Fixes open-telemetry#11808 #### Testing I'm not sure how I could test this without getting it merged first unfortunately. Once merged, we could try running the action, then closing the resulting PR and issue.
HongChenTW
pushed a commit
to HongChenTW/opentelemetry-collector
that referenced
this pull request
Dec 19, 2024
…etry#11936) #### Description The last attempt at making the Merge freeze check work in release PRs failed (open-telemetry#11906). This PR tries a different approach: it changes the criteria of the Merge freeze check, so that a freeze is enacted iff there is an open PR authored by @opentelemetrybot whose title contains "[chore] Prepare release" (note that if it weren't for the author, this PR would qualify). This PR additionally reverts open-telemetry#11849, so no label is added to the release PR. I also added the `pull_request.enqueued` trigger, taking inspiration [from Merge Freeze](https://docs.mergefreeze.com/github-merge-queue#how-it-works), to see if it could help reject PRs earlier. I tried to make sure the freeze check would be properly skipped for the release PR itself, both in PR checks and in the merge queue, but given the state of Github's documentation, I'm not very confident about this. Notably, these is an edge case where I'm not sure what would happen: what if another PR gets added to the merge queue at the same time as the release PR? How many times would the "merge_group" check run, and with what values for "github.event.merge_group.head_commit"? Would both PR be booted out of the queue (not great)? Would both be accepted (way worse)? Does it depend on the order? #### Link to tracking issue Fixes open-telemetry#11906 and fixed open-telemetry#11808 #### Testing As always with this, it's pretty much impossible to test before merging. Once merged, I strongly recommend we do the following test to make sure that this issue does not block the real release process again: - Create two dummy PRs that change nothing of consequence: the freeze check should pass - Run the "Prepare release" action - Rerun the freeze check on one of the dummy PRs: it should now fail - Approve the second PR and try to merge it: it should be booted out of the merge queue - Close all test PRs This unfortunately does not test whether the release PR gets merged properly, but I don't see how to test until the next release process, unfortunately.
mx-psi
added a commit
to open-telemetry/opentelemetry-collector-contrib
that referenced
this pull request
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The "Prepare Release" action failed during the latest release process, as the OpenTelemetry Bot, which opens the release PR, does not have the permissions to add the new
release:merge-freezelabel. This PR fixes this by creating the PR with the bot token, then adding the label separately using the regular CI token.Since we already use default CI tokens on -contrib to add labels, I assume they have the permission to do that on this repo as well. I deemed this simpler than the alternative solutions mentioned in the tracking issue, though I'm not certain it will work.
Note that the tracking issue is a release blocker.
Link to tracking issue
Fixes #11808
Testing
I'm not sure how I could test this without getting it merged first unfortunately. Once merged, we could try running the action, then closing the resulting PR and issue.