Helm Chart: ensure CRD upgrades - #168
Conversation
Move CRDs to templates and move to an in-tree subchart, ensuring that CRD upgrades happen on chart upgrades.
|
All contributors have signed the CLA ✍️ ✅ |
|
@darkmuggle you're going to have to accept the CLA to merge this :-) |
lukebond
left a comment
lukebond
left a comment
There was a problem hiding this comment.
a handful of minor issues i'll leave you to address, but otherwise this is great, thanks so much!
and congrats on your first contribution :)
| echo Version: $VERSION | ||
| echo "VERSION=$VERSION" >> "$GITHUB_OUTPUT" | ||
| - name: Push CRDs helm chart | ||
| uses: appany/helm-oci-chart-releaser@v0.3.0 |
There was a problem hiding this comment.
the helm releaser uses the helm that happens to be in the path, in this case coming from ubuntu-latest. that works today but to future proof and be more deterministic, i suggest using azure/setup-helm... as you have above (:26)
| Deployment, install the CRD chart on its own and disable the bundled copy: | ||
|
|
||
| ```bash | ||
| helm upgrade --install restate-operator-crds \ |
There was a problem hiding this comment.
think this should include --version <version> as you have done in the release note, line 9. if they followed both blindly they might get the latest CRD schema but pin the operator to an older version.
|
|
||
| ## Upgrading | ||
|
|
||
| If your CRDs came in through the old `crds/` directory (2.8.1 or earlier), Helm doesn't consider them managed yet, so the first upgrade needs a one-time ownership hand-off: either pass `--take-ownership` to the CRD chart, or label/annotate the three CRDs with `app.kubernetes.io/managed-by=Helm` plus the `meta.helm.sh/release-name`/`-namespace` metadata. ArgoCD does this for you on sync. Either way, no custom resources are deleted. |
There was a problem hiding this comment.
--take-ownership is helm >= 3.17.0. you give the fallback for older versions (label/annotate) but would be helpful to be more explicit.
| version files together to the new version: | ||
| `v<version>` tag builds and publishes the docker image and **both** helm charts | ||
| (`restate-operator-helm` and `restate-operator-crds`) under that version. It bumps | ||
| nothing. In the release commit you must therefore bump all three version files |
There was a problem hiding this comment.
the nittiest of nits: i think it's 4, not 3 (Cargo.toml, Cargo.lock, operator Chart.yaml version + dependency constraint, crds Chart.yaml), or you could drop the number and save the bother. i know this is not your doing in this PR but it came up green so i read it!
|
I have read the CLA Document and I hereby sign the CLA. |
|
recheck |
- release.yml: install Helm via azure/setup-helm in the CRDs push job instead of relying on the runner's PATH helm - README.md: pin both chart installs with --version <version> - release note: note --take-ownership requires Helm >= 3.17.0 - AGENTS.md: correct version-file count (four, not three)
d7e47ba to
a5d5418
Compare
3 participants
Move CRDs to templates and move to an in-tree subchart, ensuring that CRD upgrades happen on chart upgrades.
Resolves #167