-
Notifications
You must be signed in to change notification settings - Fork 494
ci: hackathon release improvements #5049
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
also removes the backport workflow in favor of the release-backport one
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces comprehensive release automation improvements for Alloy, including:
- Custom release-please runner with minor-only versioning strategy (treating breaking changes as minor bumps)
- New Go-based release tools for branch creation, RC creation, backporting, and forwardporting
- Automated GitHub workflows for release management
- Replacement of shell scripts with more robust Go implementations
Key Changes
- Custom Node.js release-please runner that overrides major version bumps to minor bumps for breaking changes
- New Go packages for version handling, GitHub API interactions, and git operations
- Workflow automation for release branch creation, RC tagging, backporting, and PR title validation
- Script renamed from
tools/releasetotools/mkrelease
Reviewed changes
Copilot reviewed 27 out of 30 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/release/release-please-runner/* | Custom Node.js runner implementing minor-breaking versioning strategy |
| tools/release/internal/* | Shared Go packages for version, GitHub client, and git operations |
| tools/release/{create-release-branch,create-rc,backport,forwardport-release-to-main}/* | Go tools for release automation tasks |
| .github/workflows/release-*.yml | New automated workflows for release management |
| .github/workflows/lint-pr-title.yml | PR title linting workflow for conventional commits |
| tools/mkrelease | Renamed script for creating releases with artifacts |
| tools/go.mod | Added golang.org/x/mod dependency for semver handling |
Files not reviewed (1)
- tools/release/release-please-runner/package-lock.json: Language not supported
- also convert the enrich logic to a go tool - update some docs to remove references to modifying the changelog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 37 out of 39 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- tools/release/release-please-runner/package-lock.json: Language not supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 51 out of 53 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- tools/release/release-please-runner/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
tools/release:1
- The deleted
tools/releasescript previously leaked theGITHUB_TOKENin cleartext by running withset -xand passing the token via the-t "${GITHUB_TOKEN}"flag toghr, which would expose the token in shell traces and process command lines in CI logs. An attacker with access to build logs or process listings could have captured this token and used it to push releases or otherwise act with repository write privileges. This PR mitigates the issue by removing this script entirely and replacing it with a new release script that relies onGH_TOKENonly via environment variable and explicitly disables xtrace around token handling.
release-please depends on PR descriptions during overrides. This change prevents PR descriptions from being modified by non-grafana contributors immediately after merge. Maintainers/admins can still modify them as needed to adjust release notes. Closed issues are now locked after 2 weeks of no activity instead of 30 days.
🔍 Dependency ReviewNo upgraded or downgraded Go module dependencies were detected in the provided diff of go.mod files. Notes
|
| comment, or you can ask for a review on the Slack channel | ||
| [#alloy](https://slack.grafana.com). | ||
|
|
||
| ## Updating the changelog |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should document how naming goes into changelog here, or link to the doc if it's documented elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I don't think I mentioned semantic conventions anywhere yet other than the workflow which will point them out if they don't pass.
I expect most of the time that the maintainer who merges each PR will ensure the title is accurate, but the contributor should definitely have some guidance for getting all green checks on their PRs.
| You did it! Now it's time to celebrate by announcing the Alloy release in the following Slack | ||
| channels: | ||
|
|
||
| [#alloy (internal slack)](https://grafanalabs.enterprise.slack.com/archives/CSN5HV0CQ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably shouldn't include internal slack references here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is precedence for this in several Grafana repos, but it's also easy enough for us to find, so I'll remove it.
| * Some of the modules in `opentelemetry-collector` come from a [grafana/opentelemetry-collector](https://github.com/grafana/opentelemetry-collector) fork. | ||
| * This is mostly so that we can include metrics of Collector components with the metrics shown under Alloy's `/metrics` endpoint. | ||
| * All Collector and Collector-Contrib dependencies should be updated at the same time, because they | ||
| - Some of the modules in `opentelemetry-collector` come from a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can definitely revisit this doc in a separate PR. Most of my changes were prettier formatting the file after I added the contents of what was in docs/developer/release/00-ensure-otel-dep-updated.md to it. (The stuff up above this section with some adjusted wording)
| @@ -0,0 +1,184 @@ | |||
| package main | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this based off of the shared github action we have for backport, or something hand rolled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is conceptually inspired by the shared backport GHA, but is much more simplistic and procedural (by design). Its purpose is to facilitate semantic commit-friendly cherry-picking in a very reliable way, and doesn't extend at all beyond that use case. The goal with this (and the other) automated workflow(s) is to keep the scope and flow as narrow and easy to follow as possible.
| } | ||
|
|
||
| // cleanupOldBackportLabels removes backport labels older than n-2. | ||
| func cleanupOldBackportLabels(ctx context.Context, client *gh.Client) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will deleting these labels make it harder to track old backports? I'm not sure I see the value of removing them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleting the labels is a subtle way to enforce that things shouldn't be backported further than n-2. It doesn't prevent you from making the label yourself and using it, but helps establish that paradigm.
For each backport, we get a definitive reference in the original PR that looks like this:
And that can never be mutated as labels come and go.
There's two primary questions I had in mind when designing the workflow from the tracking perspective:
- Given a PR, was it backported? And more specifically, what's its backport status? Those history entries on the issue provide that.
- What (in total) has been backported to a given release branch? That's discoverable via the changelog (or the commit history) since the changelog corresponds 1-to-1 with feat/fix commits.
TODO (pre-merge)
TODO (post-merge)