Fix regression for updating Kamaji #665
Conversation
This fix introduced Kamaji update #633 But helm chart didn't actually updated This affected issue with creating new clusters. Ref clastix/kamaji#623 Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
WalkthroughThe pull request downgrades the Kamaji chart and application versions while updating the dependency constraints. It revises version numbers in both the Chart.yaml and README.md files, and clarifies configuration details in the README and values.yaml. Additionally, the TenantControlPlane CRD is enhanced with new fields and stricter validation rules. A conditional check in the controller template ensures the Changes
Sequence Diagram(s)sequenceDiagram
participant Helm as Helm Chart
participant Values as Values.yaml
participant Controller as Controller Template
participant Pod as Deployment Container
Values->>Controller: Retrieve defaultDatastoreName value
alt Value is non-empty
Controller->>Pod: Pass "--datastore" argument with value
else Value is empty
Controller->>Pod: Omit "--datastore" argument
end
Possibly related PRs
Suggested labels
Poem
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:
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/system/kamaji/charts/kamaji/README.md (1)
25-25: Markdownlint Notice: Format Bare URL if NeededThe URL
https://clastix.github.io/chartsis presented as a bare URL in the table, which may trigger markdownlint (MD034). Consider wrapping it in angle brackets or converting it to a markdown link if it aligns with your project's style guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
25-25: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/system/kamaji/charts/kamaji/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
packages/system/kamaji/charts/kamaji/Chart.yaml(2 hunks)packages/system/kamaji/charts/kamaji/README.md(3 hunks)packages/system/kamaji/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml(3 hunks)packages/system/kamaji/charts/kamaji/templates/controller.yaml(1 hunks)packages/system/kamaji/charts/kamaji/values.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/system/kamaji/charts/kamaji/values.yaml
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
packages/system/kamaji/charts/kamaji/README.md
25-25: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (14)
packages/system/kamaji/charts/kamaji/templates/controller.yaml (1)
36-38: Ensure Conditional Inclusion of--datastoreArgument Is Correctly ImplementedThe conditional block checks if
.Values.defaultDatastoreNameis not an empty string before adding the--datastoreargument. This change prevents potential deployment errors when the value is not provided, thereby addressing the regression.packages/system/kamaji/charts/kamaji/Chart.yaml (3)
2-2: App Version Downgrade tov0.0.0The
appVersionhas been updated tov0.0.0as required by the regression fix. Please confirm that this version downgrade aligns with your versioning strategy.
20-20: Chart Version Downgrade to0.0.0Changing the chart version to
0.0.0reflects the intended reset in versioning. Ensure that all dependent environments are ready for this rollback.
24-24: Dependency Version Constraint Update forkamaji-etcdThe dependency version constraint has been adjusted to
>=0.8.1, ensuring compatibility with updated requirements. This meets the new minimum version criteria for the dependency.packages/system/kamaji/charts/kamaji/README.md (3)
3-3: Updated Version Badges Reflect New VersioningThe version badges now display
0.0.0andv0.0.0, matching the changes in the Chart.yaml. This update enhances documentation accuracy and consistency.
25-25: Updated Dependency Version in Requirements TableThe requirements table now reflects the dependency version for
kamaji-etcdas>=0.8.1, aligning with the updated constraint in the Chart.yaml.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
25-25: Bare URL used
null(MD034, no-bare-urls)
73-73: Enhanced Description fordefaultDatastoreNameThe description now clearly states that if specified, all Kamaji instances with an unassigned DataStore will inherit this default value. This improved clarity helps users better understand the configuration behavior.
packages/system/kamaji/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml (7)
6415-6420: Enhanced documentation for the dataStore fieldThe updated description improves clarity by providing more detailed information about default values and migration capabilities for the dataStore field. This is a valuable improvement for users trying to understand how dataStore selection works.
6422-6432: Well-designed new dataStoreSchema field with proper immutability guaranteesThe introduction of the
dataStoreSchemafield provides important configuration flexibility while maintaining appropriate safeguards. The immutability enforcement through validation rules is particularly important as changing the schema after creation could lead to data access issues.
6554-6562: Good default and validation for the new clusterDomain fieldThe new
clusterDomainfield properly sets a sensible default of "cluster.local" and includes a pattern validation to ensure it contains at least one period. The immutability validation is critical since changing the cluster domain after creation would break communication between pods.
6563-6570: Improved clarity for dnsServiceIPs field descriptionThe enhanced description for the
dnsServiceIPsfield now clearly explains how the DNS Service IP is automatically computed when not specified, providing examples for both IPv4 and IPv6 addresses. This makes the behavior more predictable for users.
6571-6580: New loadBalancerClass field with proper constraintsThe addition of the
loadBalancerClassfield with immutability constraints is well-designed. The validation ensures it can only be used with LoadBalancer service types, preventing configuration errors.
6581-6591: Security-enhancing loadBalancerSourceRanges fieldThis new field allows restricting the IP ranges that can access the LoadBalancer service, which is an important security feature. The clear documentation includes a helpful example format, making it easier for users to understand how to use this feature.
6610-6621: Comprehensive validation rules strengthen configuration integrityThe added Kubernetes validation rules provide important guardrails to prevent misconfiguration. These rules help ensure:
- dataStore and dataStoreSchema cannot be unset once defined
- LoadBalancer-specific fields are only used with the appropriate service type
- LoadBalancerClass setting consistency is maintained at runtime
This significantly improves the robustness of the tenant control plane configuration.
This fix introduced Kamaji update #633
But helm chart didn't actually updated
This affected issue with creating new clusters.
Ref clastix/kamaji#623
Signed-off-by: Andrei Kvapil kvapss@gmail.com
Summary by CodeRabbit