ci(helm): add chart release workflows#113
Conversation
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds Helm chart documentation and schema generation, pull-request validation, push/manual OCI releases to GHCR, and expanded cross-platform repository ignore rules. ChangesHelm chart automation
Repository maintenance
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant ChartVersion
participant HelmChartReleaser
participant GHCR
GitHubActions->>ChartVersion: Read current and previous chart versions
ChartVersion->>GitHubActions: Return version_changed
GitHubActions->>HelmChartReleaser: Package chart and update gh-pages
HelmChartReleaser->>GHCR: Push OCI chart artifact
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Welcome @spencercjh! It looks like this is your first PR to Project-HAMi/ascend-device-plugin 🎉 |
bd64f4b to
9e01f3c
Compare
|
/assign archlitchi |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-helm-release.yaml:
- Around line 11-20: Add a workflow-level permissions default near the existing
env/concurrency configuration in the Helm release workflow, setting the GitHub
Actions token to read-only (contents: read). Keep the explicit elevated
permissions in release jobs unchanged.
In @.github/workflows/helm-lint.yaml:
- Around line 41-42: Update the actions/checkout step named “Checkout” in the
Helm lint workflow to set persist-credentials to false before make
verify-helm-chart runs, while leaving the existing checkout behavior and local
git diff functionality unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bec4860b-270d-4dd1-b877-1618e733c8ca
📒 Files selected for processing (8)
.github/workflows/build-helm-release.yaml.github/workflows/helm-lint.yaml.gitignoreMakefilecharts/ascend-device-plugin/README.mdcharts/ascend-device-plugin/README.md.gotmplcharts/ascend-device-plugin/values.schema.jsoncharts/ascend-device-plugin/values.yaml
|
It's almost copied from https://github.com/dynamia-ai/ppu-vgpu-device-plugin |
|
/assign DSFans2014 |
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: spencercjh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
11dd4cb to
e7a3bfb
Compare
Signed-off-by: Spencer Cai <jiahao.cai@dynamia.ai>
Signed-off-by: Spencer Cai <jiahao.cai@dynamia.ai>
Signed-off-by: Spencer Cai <jiahao.cai@dynamia.ai>
e7a3bfb to
e97da30
Compare
| workflow_dispatch: | ||
| push: | ||
| branches: | ||
| - main |
There was a problem hiding this comment.
release the chart if a new tag created
There was a problem hiding this comment.
Implemented v* tag-triggered releases in aa8cc74.
One warning is that repository tags and Chart.yaml.version are independent, so a new repository tag may point to a chart version already published by an earlier main branch version bump. The workflow therefore treats chart artifacts as immutable: chart-releaser uses skip_existing, and GHCR checks the exact OCI chart version before pushing. Existing versions are skipped instead of overwritten; publishing new chart content still requires updating Chart.yaml.version.
There was a problem hiding this comment.
Can we modify this version via parameters during helm packaging?
There was a problem hiding this comment.
Theoretically, of course, it’s possible, but I think that when someone modifies the contents of a chart and wants to publish it for others to use, they should explicitly update the version in Chart.yaml. When templates change frequently, it’s perfectly normal for the patch version in the chart version to be very high. Of course, the maintainer can require restrictions on version increments and decide for themselves what the chart version should be.
I think implementing version updates by passing the version number into a GitHub workflow would make the whole process quite complicated. We’d have to modify the file first, commit and push it, and then perform the Helm packaging and release.
There was a problem hiding this comment.
When templates change frequently, it’s perfectly normal for the patch version in the chart version to be very high.
Do you mean that the chart version should be independent from the entire project?
I think implementing version updates by passing the version number into a GitHub workflow would make the whole process quite complicated.
Getting the tag version in the workflow isn't complicated. Also, creating a new tag already means releasing a new version. If we require the release owner to update the version manually, it's easy to forget and end up with inconsistent version information.
There was a problem hiding this comment.
Yes, the chart version can be independent from the application or repository version. However, I am not insisting that this project must use independent versioning. If the maintainers prefer to keep the repository tag, appVersion, and chart version aligned, that is a valid project policy.
I also agree that relying entirely on the release owner to remember updating Chart.yaml is error-prone. My concern is only that silently overriding the chart version during packaging may create a difference between the checked-in Chart.yaml and the published artifact.
I think CI validation would solve the problem more clearly.
For example, on a v1.3.1 tag, the workflow could verify that:
tag version = 1.3.1
Chart.yaml version = 1.3.1
Chart.yaml appVersion = v1.3.1
If they do not match, the release should fail with a clear error asking the release owner to update Chart.yaml.
This gives us both properties:
- The project can keep the chart and application versions strongly aligned.
- Forgetting to update the version cannot result in an incorrect or missing release.
In other words, automation should prevent an inconsistent release, rather than silently changing the version of the packaged artifact.
If the project later needs to publish a chart-only fix without releasing a new application version, we can reconsider independent chart versioning at that point. But that is a separate versioning-policy decision.
The issue regarding chart versions has actually already come to light in HAMi. HAMi’s charts are already quite complex, and when we discover bugs in chart templates or default values, there’s currently no way to release a quick fix because it must be tied to the entire HAMi release. This causes users to miss out on many minor chart fixes, forcing them to fork the source code and release their own charts.
In the real world, there are numerous well-known open-source projects that use independent chart versions.
| Project | Chart version | appVersion |
|---|---|---|
| kube-prometheus-stack | 87.16.1 | v0.92.1 |
| Argo CD | 10.1.3 | v3.4.5 |
| Grafana Loki | 7.1.0 | 3.6.8 |
| Traefik | 41.0.2 | v3.7.7 |
| ingress-nginx | 4.15.1 | 1.15.1 |
| ExternalDNS | 1.21.1 | 0.21.0 |
| Bitnami PostgreSQL | 17.1.0 | 17.6.0 |
There was a problem hiding this comment.
The chart version is part of the source and release metadata. Choosing and updating it correctly is one of the responsibilities of maintaining and releasing the chart, just like updating release notes or documenting compatibility changes. The maintainer should still decide whether a chart change requires a patch, minor, or major version bump.
Automation can help us enforce that responsibility. For example, CI can fail when a release tag is created but the expected fields in Chart.yaml were not updated. However, that validation should be a guardrail, not a replacement for maintaining the version correctly.
Automatically injecting the Git tag during packaging may prevent one kind of mistake, but it also hides the fact that the version recorded in the repository is stale. It makes the published artifact differ from the checked-in source and treats the symptom rather than fixing the release process.
So I would separate these two questions:
Should this project keep the chart version aligned with the application version?
That is a valid project policy.
How should we prevent maintainers from forgetting to update the version?
CI should detect the inconsistency and fail the release.
Regardless of whether the two versions are aligned or independent, correctly maintaining the chart version is something we should do properly, rather than something we should avoid through packaging-time overrides.
There was a problem hiding this comment.
I'll open an issue on HAMi in a few days to elaborate on the above points again.
Signed-off-by: Spencer Cai <jiahao.cai@dynamia.ai>
Most of this Pull Request was generated or revised with the assistance of AI tools(Codex with GPT-5.6 Sol High). I have reviewed the resulting content and take full responsibility for its accuracy, security, licensing compliance, and inclusion in this project.
Summary
Validation
make verify-helm-chartwith Helmv3.20.0actionlint .github/workflows/helm-lint.yaml .github/workflows/build-helm-release.yamlgit verify-commit HEADNotes
gh-pagesupdates.Summary by CodeRabbit
values.schema.jsonto define and validate configurable chart values..gitignorewith broader Linux/Go/macOS/IDE/worktree ignore patterns.