[kubernetes] User-selectable cluster version - #1202
[kubernetes] User-selectable cluster version#1202Timofei Larkin (lllamnyp) wants to merge 10 commits into
Conversation
This patch adds a new version field to the kubernetes chart, letting end-users specify the version of kubernetes they want to deploy. [kubernetes] Let users specify desired version of tenant k8s cluster. Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
…ubectl Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
…rabbit Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
WalkthroughThis change introduces a version selection mechanism for the Kubernetes Helm chart, allowing users to specify the Kubernetes minor version via a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm Chart
participant versions.yaml
participant _versions.tpl
User->>Helm Chart: Set .Values.version (e.g., "v1.32")
Helm Chart->>_versions.tpl: include "kubernetes.versionMap" $
_versions.tpl->>versions.yaml: Load version map
_versions.tpl->>_versions.tpl: Validate .Values.version exists
alt Version valid
_versions.tpl-->>Helm Chart: Return mapped patch version (e.g., "v1.32.6")
Helm Chart-->>User: Deploys resources with selected version
else Version invalid
_versions.tpl-->>User: Fail with error listing allowed versions
end
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
⏰ 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)
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 significantly enhances the flexibility of deploying tenant Kubernetes clusters by allowing users to specify the desired Kubernetes version. It establishes a robust version management system within the Helm chart, backed by dynamic schema generation and comprehensive E2E tests for different Kubernetes versions. This change provides better control over the deployed environment and improves test coverage.
Highlights
- Kubernetes Version Selection: I've introduced the capability to explicitly select the Kubernetes version (e.g.,
v1.28,v1.32) when deploying a tenant Kubernetes cluster. This is achieved by adding a newversionparameter to the Helm chart'svalues.yaml. - Centralized Version Management: A new
files/versions.yamlhas been added to centralize the mapping of major.minor Kubernetes versions to their specific patch versions. This file is used by the Helm chart to resolve the full version and by theMakefileto dynamically update thevalues.schema.jsonenum. - Enhanced E2E Testing: The E2E tests for Kubernetes cluster creation have been refactored into a reusable function and expanded to include tests for deploying clusters with both the latest and previous supported Kubernetes versions, ensuring robust validation of the new version selection mechanism.
- Helm Chart and Application Version Updates: The Kubernetes Helm chart version has been bumped to
0.26.0, and the underlying application version (Kubernetes) has been updated to1.32.6.
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 is currently in preview and 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 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 introduces a feature to select the Kubernetes version for a tenant cluster, moving away from a hardcoded version tied to the chart's appVersion. This is achieved by introducing a versions.yaml file as the source of truth, updating Helm templates to use it, and exposing a version parameter in values.yaml.
The e2e tests have been refactored to validate this new functionality by testing both the latest and previous supported Kubernetes versions.
My review focuses on improving the robustness and correctness of the updated e2e test script, and validating the version increments in Chart.yaml.
| kubectl wait machinedeployment kubernetes-${test_name}-md0 -n tenant-test --timeout=1m --for=jsonpath='{.status.replicas}'=2 | ||
|
|
||
| # Get the admin kubeconfig and save it to a file | ||
| kubectl get secret kubernetes-${test_name}-admin-kubeconfig -ojsonpath='{.data.super-admin\.conf}' -n tenant-test | base64 -d > tenantkubeconfig |
There was a problem hiding this comment.
The test writes to a hardcoded filename tenantkubeconfig in the current directory. While bats typically runs tests sequentially, this could lead to race conditions if tests are ever configured to run in parallel. For better robustness, consider using a temporary file created with mktemp and a trap to ensure cleanup.
local tenant_kubeconfig
tenant_kubeconfig=$(mktemp)
trap 'rm -f "${tenant_kubeconfig}"' RETURN
kubectl get secret kubernetes-${test_name}-admin-kubeconfig -ojsonpath='{.data.super-admin\.conf}' -n tenant-test | base64 -d > "${tenant_kubeconfig}"
yq -i ".clusters[0].cluster.server = \"https://localhost:${port}\"" "${tenant_kubeconfig}"
| kubectl wait machinedeployment kubernetes-${test_name}-md0 -n tenant-test --timeout=10m --for=jsonpath='{.status.v1beta2.readyReplicas}'=2 | ||
|
|
||
| # Clean up by deleting the Kubernetes resource | ||
| kubectl -n tenant-test delete kuberneteses.apps.cozystack.io $test_name |
There was a problem hiding this comment.
The $test_name variable is not quoted. It's a shell scripting best practice to always quote variable expansions (e.g., "${test_name}") to prevent unexpected behavior from word splitting and filename expansion (globbing), especially if the variable's value could contain spaces or special characters.
kubectl -n tenant-test delete kuberneteses.apps.cozystack.io "${test_name}"
| # to the chart and its templates, including the app version. | ||
| # Versions are expected to follow Semantic Versioning (https://semver.org/) | ||
| version: 0.25.2 | ||
| version: 0.26.0 |
There was a problem hiding this comment.
The chart version has been incremented. Ensure that this increment follows Semantic Versioning conventions.
| # follow Semantic Versioning. They should reflect the version the application is using. | ||
| # It is recommended to use it with quotes. | ||
| appVersion: 1.32.4 | ||
| appVersion: 1.32.6 |
There was a problem hiding this comment.
The app version has been incremented. Verify that this increment aligns with the application's versioning scheme.
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
packages/apps/kubernetes/files/versions.yaml (2)
1-6: Add a brief header comment to explain how this file is consumed.A one-liner YAML comment (e.g.
# Kubernetes minor→patch version map - consumed by _versions.tpl & Makefile) will make the intent clear for future maintainers skimming the repo.
1-6: Guard against accidental unsorted or duplicate keys.
yq’skeyspreserves the original order; human edits might introduce disorder or duplicates that later leak into the generated enum list.
Consider validating this file in CI (e.g. ayq+sort -ucheck) or load it via a helper that asserts uniqueness and ascending minor order.packages/apps/kubernetes/Chart.yaml (1)
19-25: AutomateappVersion& chart patch bumps fromfiles/versions.yaml.
appVersion: 1.32.6currently mirrors the mapped patch for the defaultv1.32.
When the mapping or default changes, humans must remember to bump bothappVersionand the chartversion. A tiny Go/Make task that:
- Reads the default from
values.yaml- Maps it through
files/versions.yaml- Updates
appVersionand bumpsversionfollowing SemVer ruleswould prevent drift.
packages/apps/kubernetes/Makefile (1)
9-10: UnifyyqCLI style for readability.Line 9 uses
-o=json, while subsequent lines use-o json. Pick one form (the dash or space) consistently to avoid cognitive noise.- yq -o=json -i '.properties.version.enum = (load("files/versions.yaml") | keys)' values.schema.json + yq -o json -i '.properties.version.enum = (load("files/versions.yaml") | keys)' values.schema.jsonpackages/apps/kubernetes/README.md (1)
89-90: Mention the allowed values or link to the source list.The
versionrow lists a default but not the valid range. Add “Allowed values: v1.28 – v1.33 (see files/versions.yaml)” so users aren’t forced to open the repo to discover supported versions.hack/e2e-apps/kubernetes.bats (2)
89-89: Consider adding explicit cleanup for temporary files!The function creates a
tenantkubeconfigfile but doesn't explicitly clean it up. While the function does clean up the Kubernetes resource, consider adding explicit cleanup for temporary files to prevent accumulation during test runs.# Clean up by deleting the Kubernetes resource kubectl -n tenant-test delete kuberneteses.apps.cozystack.io $test_name + + # Clean up temporary files + rm -f tenantkubeconfigAlso applies to: 104-104
95-95: Port forwarding cleanup could be more robust!The background port forwarding process uses a 40-second timeout but may not be reliably cleaned up if the function exits early due to failures. Consider adding a trap or explicit cleanup to ensure the background process is terminated.
+ # Set up trap to clean up port forwarding on exit + trap 'pkill -f "kubectl port-forward.*kubernetes-'"${test_name}"'"' EXIT + # Set up port forwarding to the Kubernetes API server for a 40 second timeout bash -c 'timeout 40s kubectl port-forward service/kubernetes-'"${test_name}"' -n tenant-test '"${port}"':6443 > /dev/null 2>&1 &'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
hack/e2e-apps/kubernetes.bats(2 hunks)packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/Makefile(1 hunks)packages/apps/kubernetes/README.md(1 hunks)packages/apps/kubernetes/files/versions.yaml(1 hunks)packages/apps/kubernetes/templates/_versions.tpl(1 hunks)packages/apps/kubernetes/templates/cluster.yaml(2 hunks)packages/apps/kubernetes/values.schema.json(1 hunks)packages/apps/kubernetes/values.yaml(1 hunks)packages/apps/versions_map(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/apps/kubernetes/Makefile (2)
Learnt from: NickVolynkin
PR: cozystack/cozystack#1196
File: packages/apps/http-cache/Makefile:24-27
Timestamp: 2025-07-14T16:23:12.758Z
Learning: In the cozystack repository, the `readme-generator` tool removes enum contents from values.schema.json files during its operation. Therefore, when using readme-generator in Makefiles, any enum values need to be injected back into the schema using yq commands after readme-generator has run, not before.
Learnt from: NickVolynkin
PR: cozystack/cozystack#1117
File: packages/apps/mysql/Makefile:8-8
Timestamp: 2025-06-26T04:29:24.830Z
Learning: The cozystack project uses yq v4+ on their CI runner, so yq v4 syntax (-o json --indent 4) is compatible and version checks are not needed.
🔇 Additional comments (10)
packages/apps/kubernetes/Makefile (1)
9-9: Keys order in enum will follow YAML insertion order. Confirm that’s wanted.
(load(...) | keys)keeps original order; JSON-schema consumers treat enum as an unordered set, so order is cosmetic, but docs generated from the schema may list versions as they appear here.
If you prefer “latest first” or ascending order regardless of edit sequence, pipe throughsort. Example:... | keys | sort`packages/apps/kubernetes/values.yaml (1)
6-11: Validate that the default minor (v1.32) always exists infiles/versions.yaml.Today it maps to
v1.32.6, but if editors forget to bumpvalues.yamlwhen pruning old minors the chart will break at render time.
A simple test in CI could comparevalues.versionagainst the keys offiles/versions.yaml.packages/apps/kubernetes/values.schema.json (1)
130-142: Well-structured version parameter addition!The new
versionproperty is properly defined with clear documentation, sensible defaults, and appropriate enum constraints. ThevMAJOR.MINORformat is intuitive and the supported version range from v1.28 to v1.33 covers both current and future Kubernetes releases.packages/apps/kubernetes/templates/cluster.yaml (2)
154-154: Consistent version mapping integration!The replacement of direct Chart.AppVersion usage with the
kubernetes.versionMaphelper provides better version management flexibility and consistency across the cluster deployment.
293-293: Consistent version mapping in MachineDeployment!Good consistency applying the same version mapping approach to both control plane and worker node specifications.
packages/apps/kubernetes/templates/_versions.tpl (1)
1-16: Robust version mapping template implementation!This template helper demonstrates excellent practices:
- Efficient caching mechanism prevents repeated file reads
- Comprehensive validation with informative error messages
- Clear logic flow and proper error handling
- Well-structured template function following Helm conventions
The approach of loading versions from a YAML file and mapping them through a template provides flexible version management.
packages/apps/versions_map (2)
58-58: Appropriate version bump for Kubernetes package!The update to version 0.26.0 HEAD correctly reflects the new version management features introduced in the Kubernetes Helm chart.
140-140: Routine version updates!The commit hash updates for tenant, virtual-machine, and vm-instance packages appear to be standard maintenance updates.
Also applies to: 159-159, 177-177
hack/e2e-apps/kubernetes.bats (2)
3-106: Excellent test refactoring with parameterization!The conversion to a reusable function with dynamic version selection greatly improves test maintainability and coverage. The use of yq to select versions from the versions.yaml file is clever and ensures tests stay synchronized with supported versions.
108-113: Great test coverage with multiple versions!Testing both the latest and previous versions provides excellent coverage of the version mapping functionality. The port differentiation (59991 vs 59992) prevents conflicts when tests run concurrently.
|
Closed in favor #1191 |
What this PR does
This patch adds a new version field to the kubernetes chart, letting end-users specify the version of kubernetes they want to deploy.
Release note
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores