[kubernetes] No default nodegroups in values - #1276
[kubernetes] No default nodegroups in values#1276Timofei Larkin (lllamnyp) wants to merge 1 commit into
Conversation
Since nodegroups are defined as key-value pairs in the values, there's no easy way to remove the example md0 nodegroup that was provided there for purposes of documentation. Commenting it out from values.yaml keeps the documentation in place, but doesn't force the node group onto users. Release note: [kubernetes] Remove potentially undesirable node group from default values of kubernetes chart. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
WalkthroughThe changes increment the Kubernetes chart version from 0.26.0 to 0.26.1, update the HEAD pointer and version mapping in versions_map, and modify the default Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HelmChart
participant KubernetesApp
User->>HelmChart: Deploy/upgrade chart (v0.26.1)
HelmChart->>KubernetesApp: Apply values.yaml (nodeGroups: {})
KubernetesApp-->>HelmChart: No default node groups created
HelmChart-->>User: Deployment complete
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 Timofei Larkin (@lllamnyp), 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 refines the default configuration of the Kubernetes Helm chart by removing a pre-defined example nodegroup from the values.yaml file. This change ensures that users are not implicitly forced to deploy an md0 nodegroup, providing a cleaner starting point for new deployments while retaining the example configuration as commented-out documentation.
Highlights
- Configuration Cleanup: The default
md0nodegroup entry has been removed frompackages/apps/kubernetes/values.yaml, making thenodeGroupssection an empty map by default. This prevents users from implicitly deploying an example nodegroup. - Documentation Preservation: The previously defined
md0nodegroup configuration has been commented out inpackages/apps/kubernetes/values.yaml, preserving it as an example for users without forcing its deployment. - Chart Version Bump: The chart version in
packages/apps/kubernetes/Chart.yamlhas been incremented from0.26.0to0.26.1. - Version Mapping Update: The
packages/apps/versions_mapfile has been updated to reflect the new chart version0.26.1and to correctly map the previous0.26.0version to 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 (@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 (@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 improves the user experience of the Kubernetes Helm chart by removing the default node group. The changes are clear and align with the stated goal. The indentation of the commented-out example has been improved for better usability.
| # md0: | ||
| # minReplicas: 0 | ||
| # maxReplicas: 10 | ||
| # instanceType: "u1.medium" | ||
| # ephemeralStorage: 20Gi | ||
| # roles: | ||
| # - ingress-nginx | ||
| # | ||
| # resources: | ||
| # cpu: "" | ||
| # memory: "" | ||
| # | ||
| # ## List of GPUs to attach (WARN: NVIDIA driver requires at least 4 GiB of RAM) | ||
| # ## e.g: | ||
| # ## instanceType: "u1.xlarge" | ||
| # ## gpus: | ||
| # ## - name: nvidia.com/AD102GL_L40S | ||
| # gpus: [] |
There was a problem hiding this comment.
The indentation of the commented-out nodeGroups example is slightly off, which can make it more difficult for users to uncomment and use this block. The commented block should have indentation that matches the valid YAML structure, so users can simply uncomment the block to use it.
# md0:
# minReplicas: 0
# maxReplicas: 10
# instanceType: "u1.medium"
# ephemeralStorage: 20Gi
# roles:
# - ingress-nginx
#
# resources:
# cpu: ""
# memory: ""
#
# ## List of GPUs to attach (WARN: NVIDIA driver requires at least 4 GiB of RAM)
# ## e.g:
# ## instanceType: "u1.xlarge"
# ## gpus:
# ## - name: nvidia.com/AD102GL_L40S
# gpus: []
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/apps/kubernetes/values.yaml (1)
19-25: Trim YAML trailing spaces to silence lint warnings
yamllintflags two lines in this commented example block for trailing whitespace (lines 21 & 25 in the linter report).
Minor, but it will fail strict CI runs.-# roles:␠ -# - ingress-nginx␠ -# -# resources:␠ -# cpu: ""␠ -# memory: ""␠ +# roles: +# - ingress-nginx +# +# resources: +# cpu: "" +# memory: ""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: lllamnyp
PR: cozystack/cozystack#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.
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.
🪛 YAMLlint (1.37.1)
packages/apps/kubernetes/values.yaml
[error] 21-21: trailing spaces
(trailing-spaces)
[error] 25-25: trailing spaces
(trailing-spaces)
⏰ 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 (3)
packages/apps/kubernetes/values.yaml (1)
13-14: Explicitly re-confirm that an emptynodeGroupsset is acceptableChanging the default to
{}means a freshly deployed tenant cluster will have no worker nodes until users supply their own node-group stanza.
Double-check that:
- All docs/tutorials that relied on
md0still read correctly.- Provisioning workflows (e.g. CI demos) create a node-group explicitly elsewhere.
If that behaviour is intentional, nothing further to do.
packages/apps/kubernetes/Chart.yaml (1)
19-19: Version bump looks goodChart version incremented to
0.26.1with no other metadata changes — LGTM.packages/apps/versions_map (1)
64-65: Recorded commit 68a4709 exists in history – no action requiredVerification command
git rev-parse --quiet --verify 68a47097^{commit}returned68a47097c19f96378a3892d184dedb010c9e118b, confirming the commit is present. Downstream reproducibility remains intact.
|
I'd like to proper testing of this first, to confirm that it will not destroy already existing clusters on update. |
|
Stale |
Since nodegroups are defined as key-value pairs in the values, there's no easy way to remove the example md0 nodegroup that was provided there for purposes of documentation. Commenting it out from values.yaml keeps the documentation in place, but doesn't force the node group onto users.
Release note:
[kubernetes] Remove potentially undesirable node group from default values of kubernetes chart.
Summary by CodeRabbit
Chores
Refactor