k8s make volumesnapshot crd name shorter #1284
Conversation
WalkthroughThis change updates the Helm chart name for the volume snapshot CRD from a longer to a shorter form across relevant files, adjusts the associated namespaces, increments the Kubernetes chart version, and updates the version mapping to reflect the new version and commit hash. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant KubernetesApp
participant HelmRelease
participant HelmChart
User->>KubernetesApp: Deploy/Upgrade app (v0.26.2)
KubernetesApp->>HelmRelease: Reference new chart name and namespaces
HelmRelease->>HelmChart: Use cozy-vsnap-crd
HelmChart-->>HelmRelease: Deploy CRDs with updated chart name
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Summary of Changes
Hello @klinch0, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request aims to shorten the naming convention for the Kubernetes volumesnapshot Custom Resource Definition (CRD) related components. The primary goal is to simplify the Helm chart name and associated namespaces, making them more concise and easier to manage within the Kubernetes ecosystem.
Highlights
- Helm Chart Renaming: The Helm chart name for the
volumesnapshotCRD has been shortened fromcozy-volumesnapshot-crd-for-tenant-k8stocozy-vsnap-crd-k8sin its ownChart.yamland in thehelmreleasesdefinition. - Namespace Updates: The
targetNamespaceandstorageNamespacereferences within thevolumesnapshot_crd.yamlHelmRelease definition have been updated to reflect the new, shorter naming convention (cozy-vsnap-crd-k8s). - Version Bumps: The main
kubernetesapplication Helm chart version has been incremented to0.26.2, and theversions_mapfile has been updated accordingly to reflect this new version and its commit hash.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request aims to shorten the name of the volumesnapshot CRD helm chart and associated resources. The changes update the chart name and namespaces accordingly.
I've identified one high-severity issue: the directory containing the chart definition has not been renamed to match the new chart name. This will likely cause build failures or incorrect behavior due to a script that derives the chart name from its directory. This needs to be addressed for the change to be effective.
| @@ -1,3 +1,3 @@ | |||
| apiVersion: v2 | |||
| name: cozy-volumesnapshot-crd-for-tenant-k8s | |||
| name: cozy-vsnap-crd-k8s | |||
There was a problem hiding this comment.
While you've correctly updated the chart name here, the directory containing this Chart.yaml file is still named volumesnapshot-crd-for-tenant-k8s.
For consistency and to align with Helm best practices, the directory name should match the chart name. In this case, the directory should be renamed to vsnap-crd-k8s.
This is also important because the fix-chartnames target in packages/system/Makefile automatically sets the chart name based on its directory name. Without renaming the directory, your change to the chart name here will be overwritten by the build script, causing a mismatch with the chart name expected in packages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml.
Please rename the directory:
packages/system/volumesnapshot-crd-for-tenant-k8s -> packages/system/vsnap-crd-k8s
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/system/volumesnapshot-crd-for-tenant-k8s/Chart.yaml (1)
2-2: Directory / chart name mismatch may confuse tooling.The chart’s
name:has been shortened tocozy-vsnap-crd-k8s, but the directory is stillvolumesnapshot-crd-for-tenant-k8s/. Most Helm tools cope, yet some internal scripts that assume the folder = name (e.g.helm package ./${name}) will now producecozy-vsnap-crd-k8s-*.tgzfrom a differently-named folder, which can break upload / index paths.Consider renaming the directory (or updating any packaging scripts) to keep them in sync and avoid brittle path logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml(2 hunks)packages/apps/versions_map(1 hunks)packages/system/volumesnapshot-crd-for-tenant-k8s/Chart.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: lllamnyp
PR: cozystack/cozystack#1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is `kuberneteses.apps.cozystack.io`, not `kubernetes.apps.cozystack.io`. This is defined in the API schema even though it's not grammatically perfect.
packages/system/volumesnapshot-crd-for-tenant-k8s/Chart.yaml (2)
Learnt from: lllamnyp
PR: #1233
File: packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml:33-35
Timestamp: 2025-07-23T09:15:09.658Z
Learning: The ./charts/robotlb/ directory in the hetzner-robotlb package contains vendored code, and the team generally avoids modifying vendored code to maintain clean separation from upstream dependencies.
Learnt from: lllamnyp
PR: #1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is kuberneteses.apps.cozystack.io, not kubernetes.apps.cozystack.io. This is defined in the API schema even though it's not grammatically perfect.
packages/apps/versions_map (1)
Learnt from: kvaps
PR: #1206
File: packages/apps/versions_map:30-31
Timestamp: 2025-07-16T13:17:07.580Z
Learning: In packages/apps/versions_map, using HEAD for certain versions (like ferretdb 1.0.0) is intentional and part of the core logic, not an oversight that needs to be fixed for reproducibility.
🔇 Additional comments (3)
packages/apps/kubernetes/Chart.yaml (1)
19-19: Version bump looks correct.
0.26.2follows SemVer and is reflected inversions_map. No further action.packages/apps/versions_map (1)
65-66: Mapping update consistent with project policy.Replacing
HEADfor 0.26.1 with a commit hash and adding 0.26.2→HEAD follows the repository’s established pattern (see learning #1206).packages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml (1)
13-13: Chart ref updated – good.Chart reference now matches the new shorter name.
packages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml
Outdated
Show resolved
Hide resolved
a541360 to
6966286
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/system/volumesnapshot-crd-for-tenant-k8s/Makefile (1)
1-3: Add standard.PHONYtargets to silence lint warnings and avoid accidental file/dir clashes
checkmakeflags the absence ofall,clean, andtest. Declaring them as phony (and wiring a minimal default) improves usability and keeps CI quiet.+# Default phony targets -------------------------------------------------------- +.PHONY: all clean test update + +# `all` is conventional; delegate to the existing update recipe +all: update + +# Clean target removes generated artefacts +clean: + rm -rf templates + +# Placeholder for future tests +test: + @echo "No tests for $(NAME) yet" + export NAME=vsnap-crd-k8s export NAMESPACE=cozy-$(NAME)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/templates/helmreleases/csi.yaml(1 hunks)packages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml(2 hunks)packages/apps/versions_map(1 hunks)packages/system/volumesnapshot-crd-for-tenant-k8s/Chart.yaml(1 hunks)packages/system/volumesnapshot-crd-for-tenant-k8s/Makefile(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/apps/versions_map
- packages/apps/kubernetes/templates/helmreleases/csi.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/system/volumesnapshot-crd-for-tenant-k8s/Chart.yaml
- packages/apps/kubernetes/Chart.yaml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: lllamnyp
PR: cozystack/cozystack#1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is `kuberneteses.apps.cozystack.io`, not `kubernetes.apps.cozystack.io`. This is defined in the API schema even though it's not grammatically perfect.
🪛 YAMLlint (1.37.1)
packages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
🪛 checkmake (0.2.2)
packages/system/volumesnapshot-crd-for-tenant-k8s/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (2)
packages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml (2)
4-13: Renaming looks good – confirms previous review feedback
metadata.name,spec.releaseName, andchart.spec.chartnow follow the shortervsnap-crd-k8spattern, mitigating the 63-char limit risk.
No further action needed here. 👍
24-25: No residual references to old namespace foundFile: packages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml
Lines: 24–25targetNamespace: cozy-vsnap-crd-k8s storageNamespace: cozy-vsnap-crd-k8sI ran:
rg -n "volumesnapshot-crd-for-tenant-k8s|cozy-volumesnapshot-crd-for-tenant-k8s"across the repo and got zero matches, confirming there are no lingering references to the old
cozy-volumesnapshot-crd-for-tenant-k8snamespace.
Signed-off-by: kklinch0 <kklinch0@gmail.com>
6966286 to
efb2c63
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/system/volumesnapshot-crd-for-tenant-k8s/Makefile (1)
6-12: Markupdateas a.PHONYtarget
updateproduces side effects but has no file output. Declaring it phony avoids stale-timestamp surprises:+ .PHONY: update update: rm -rf templates mkdir templatespackages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml (1)
24-25: Optional: rename the template file for clarityThe file itself is still
volumesnapshot_crd.yaml. Renaming it tovsnap_crd.yaml(and adjusting any patch-generation scripts) keeps file paths aligned with resource names.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/templates/helmreleases/csi.yaml(1 hunks)packages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml(2 hunks)packages/apps/versions_map(1 hunks)packages/system/volumesnapshot-crd-for-tenant-k8s/Chart.yaml(1 hunks)packages/system/volumesnapshot-crd-for-tenant-k8s/Makefile(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/apps/versions_map
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/apps/kubernetes/Chart.yaml
- packages/system/volumesnapshot-crd-for-tenant-k8s/Chart.yaml
- packages/apps/kubernetes/templates/helmreleases/csi.yaml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: lllamnyp
PR: cozystack/cozystack#1130
File: hack/e2e-apps/kubernetes.bats:101-101
Timestamp: 2025-07-11T06:11:25.438Z
Learning: In cozystack, the plural form for the Kubernetes custom resource is `kuberneteses.apps.cozystack.io`, not `kubernetes.apps.cozystack.io`. This is defined in the API schema even though it's not grammatically perfect.
🪛 YAMLlint (1.37.1)
packages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
🪛 checkmake (0.2.2)
packages/system/volumesnapshot-crd-for-tenant-k8s/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🔇 Additional comments (2)
packages/system/volumesnapshot-crd-for-tenant-k8s/Makefile (1)
1-2: ✅ Verified cleanup ofvolumesnapshot-crd-for-tenant-k8sA repository-wide ripgrep search (excluding
node_modules,dist,build, and.git) returned no matches for the old identifier. No further action required.packages/apps/kubernetes/templates/helmreleases/volumesnapshot_crd.yaml (1)
4-14: No leftover references tocozy-volumesnapshot-crd-for-tenant-k8sorvolumesnapshot-crd-for-tenant-k8sfoundRan a repository-wide search and confirmed all occurrences of the old chart and release names have been removed. Ready to merge.
<!-- Thank you for making a contribution! Here are some tips for you: - Start the PR title with the [label] of Cozystack component: - For system components: [platform], [system], [linstor], [cilium], [kube-ovn], [dashboard], [cluster-api], etc. - For managed apps: [apps], [tenant], [kubernetes], [postgres], [virtual-machine] etc. - For development and maintenance: [tests], [ci], [docs], [maintenance]. - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same [label] as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note - k8s make volumesnapshot crd name shorter ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated chart version for the Kubernetes application. * Changed Helm chart and namespace references to use a new, shorter name. * Updated version mapping to reflect the latest Kubernetes package version. * Renamed the Helm chart for volume snapshot resources to a shorter name. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What this PR does
Release note
Summary by CodeRabbit