feat: Add additional labels to the kubernetes resources #218
Conversation
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
WalkthroughThe pull request introduces a comprehensive labeling enhancement across multiple Helm charts for GreptimeDB (cluster, operator, and standalone). The changes focus on adding a new Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
charts/greptimedb-operator/templates/_helpers.tpl (1)
54-59: LGTM! Good alignment with Kubernetes recommended labels.The added labels (
app.kubernetes.io/componentandapp.kubernetes.io/part-of) follow Kubernetes best practices for metadata management. The implementation ofadditionalLabelsprovides good flexibility for custom labels.Consider adding validation for
additionalLabelsto ensure they don't override the standard Kubernetes labels. Example:{{- with .Values.additionalLabels }} +{{- range $key, $value := . }} +{{- if not (hasPrefix "app.kubernetes.io/" $key) }} {{ toYaml . }} +{{- end }} +{{- end }} {{- end }}charts/greptimedb-operator/templates/serviceaccount.yaml (1)
Line range hint
1-1: Consider adding documentation for label usage and validation.The labeling system is well-implemented across all charts. To enhance maintainability and user experience, consider:
- Adding documentation in the chart's README about:
- Available label customization options
- Best practices for using additionalLabels
- Examples of common label configurations
- Adding validation for additionalLabels in values.yaml schema (if using JSON Schema validation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
charts/greptimedb-cluster/templates/_helpers.tpl(1 hunks)charts/greptimedb-cluster/templates/cluster.yaml(1 hunks)charts/greptimedb-cluster/templates/debug-depoyment.yaml(1 hunks)charts/greptimedb-cluster/templates/grafana-dashboards-configmap.yaml(1 hunks)charts/greptimedb-cluster/templates/secret.yaml(1 hunks)charts/greptimedb-cluster/templates/serviceaccount-datanode.yaml(1 hunks)charts/greptimedb-cluster/templates/serviceaccount-flownode.yaml(1 hunks)charts/greptimedb-cluster/templates/serviceaccount-frontend.yaml(1 hunks)charts/greptimedb-cluster/templates/serviceaccount-meta.yaml(1 hunks)charts/greptimedb-cluster/templates/users-auth-secret.yaml(1 hunks)charts/greptimedb-cluster/values.yaml(1 hunks)charts/greptimedb-operator/templates/_helpers.tpl(1 hunks)charts/greptimedb-operator/templates/clusterrole.yaml(1 hunks)charts/greptimedb-operator/templates/clusterrolebinding.yaml(1 hunks)charts/greptimedb-operator/templates/role.yaml(1 hunks)charts/greptimedb-operator/templates/rolebinding.yaml(1 hunks)charts/greptimedb-operator/templates/service.yaml(1 hunks)charts/greptimedb-operator/templates/serviceaccount.yaml(1 hunks)charts/greptimedb-operator/values.yaml(1 hunks)charts/greptimedb-standalone/templates/_helpers.tpl(1 hunks)charts/greptimedb-standalone/templates/podmonitor.yaml(1 hunks)charts/greptimedb-standalone/templates/secret.yaml(1 hunks)charts/greptimedb-standalone/templates/users-auth-secret.yaml(1 hunks)charts/greptimedb-standalone/values.yaml(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
charts/greptimedb-cluster/templates/cluster.yaml
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
charts/greptimedb-standalone/templates/podmonitor.yaml
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
🔇 Additional comments (23)
charts/greptimedb-cluster/templates/_helpers.tpl (1)
54-59: LGTM! Consistent implementation with operator template.The labels implementation maintains consistency with the operator chart while correctly scoping the values to the cluster component. Good job on maintaining a standardized approach across charts.
charts/greptimedb-standalone/templates/_helpers.tpl (1)
42-47: LGTM! Maintains consistency across all chart templates.The labeling implementation correctly follows the established pattern while using appropriate standalone-specific values. The integration with existing helpers is clean and well-maintained.
charts/greptimedb-operator/templates/serviceaccount.yaml (1)
7-8: LGTM! Clean implementation of labels in ServiceAccount.The labels are correctly implemented in the ServiceAccount metadata, with proper indentation and template inclusion.
charts/greptimedb-cluster/templates/serviceaccount-meta.yaml (1)
7-8: LGTM! Verify template existence.The label implementation follows Helm best practices and maintains proper indentation.
Let's verify the existence and content of the labels template:
✅ Verification successful
Template verification successful
The
greptimedb-cluster.labelstemplate and its dependencies are properly implemented, following Kubernetes labeling best practices and Helm conventions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence and content of the labels template rg -U "greptimedb-cluster\.labels" charts/greptimedb-cluster/templates/_helpers.tplLength of output: 128
Script:
#!/bin/bash # Get full template definition with context rg -U "greptimedb-cluster\.labels" charts/greptimedb-cluster/templates/_helpers.tpl -A 10Length of output: 530
Script:
#!/bin/bash # Check nested template definitions rg -U "greptimedb-cluster\.(chart|selectorLabels)" charts/greptimedb-cluster/templates/_helpers.tpl -A 5Length of output: 754
charts/greptimedb-cluster/templates/users-auth-secret.yaml (1)
7-8: LGTM! Consistent with labeling pattern.The label implementation is consistent with other resources and maintains proper conditional logic.
charts/greptimedb-cluster/templates/serviceaccount-datanode.yaml (1)
7-8: LGTM! Maintains consistency.The implementation maintains consistency with other ServiceAccount resources.
charts/greptimedb-cluster/templates/serviceaccount-frontend.yaml (1)
7-8: LGTM! Consistent implementation.The label implementation maintains consistency with other ServiceAccount resources.
charts/greptimedb-standalone/templates/users-auth-secret.yaml (1)
7-8: LGTM! Verify standalone template.The implementation maintains consistency with the cluster chart pattern while appropriately using standalone-specific templates.
Let's verify the standalone labels template:
✅ Verification successful
Labels template verification successful ✅
The
greptimedb-standalone.labelstemplate is properly implemented with all required Kubernetes metadata labels, selector labels, and follows best practices for Helm charts.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence and content of the standalone labels template rg -U "greptimedb-standalone\.labels" charts/greptimedb-standalone/templates/_helpers.tplLength of output: 137
Script:
#!/bin/bash # Get full template content with context rg -U "greptimedb-standalone\.labels" charts/greptimedb-standalone/templates/_helpers.tpl -A 10Length of output: 551
Script:
#!/bin/bash # Get selector labels template content rg -U "greptimedb-standalone\.selectorLabels" charts/greptimedb-standalone/templates/_helpers.tpl -A 5Length of output: 541
charts/greptimedb-cluster/templates/grafana-dashboards-configmap.yaml (1)
9-10: LGTM! Labels properly implemented for ConfigMapThe implementation follows Helm best practices with proper template inclusion and indentation.
charts/greptimedb-cluster/templates/serviceaccount-flownode.yaml (1)
7-8: LGTM! Labels properly implemented for ServiceAccountThe implementation correctly adds labels while maintaining existing conditional logic and annotations.
charts/greptimedb-operator/templates/service.yaml (1)
7-8: Verify consistency between labels and selectorWhile the label implementation looks good, ensure that the labels generated by the template are consistent with the selector labels used in the spec section, particularly the
app.kubernetes.io/namelabel.✅ Verification successful
Labels and selector are properly aligned
The labels implementation is consistent with the selector. The
app.kubernetes.io/namelabel used in the selector is included in the main labels through the selectorLabels template, and both use the same template function to generate the value.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check consistency between labels template and selector echo "Checking label consistency in service.yaml..." rg "greptimedb-operator\.labels" charts/greptimedb-operator/templates/_helpers.tpl -A 5 echo "Compare with selector labels:" rg "selector:" charts/greptimedb-operator/templates/service.yaml -A 2Length of output: 675
Script:
#!/bin/bash # Check selectorLabels template definition echo "Checking selectorLabels template:" rg "greptimedb-operator\.selectorLabels" charts/greptimedb-operator/templates/_helpers.tpl -A 2Length of output: 484
charts/greptimedb-operator/templates/clusterrolebinding.yaml (1)
6-7: LGTM! Labels properly implemented for ClusterRoleBindingThe implementation follows Helm best practices and maintains consistency with other RBAC resources.
charts/greptimedb-operator/templates/rolebinding.yaml (1)
7-8: LGTM! Labels properly implemented for RoleBindingThe implementation follows Helm best practices and maintains consistency with other RBAC resources.
charts/greptimedb-operator/templates/role.yaml (1)
7-8: LGTM! Labels properly added to Role metadataThe addition of labels using the common template function ensures consistent labeling across operator resources.
charts/greptimedb-standalone/templates/podmonitor.yaml (1)
8-11: LGTM! Good approach to label mergingThe implementation correctly combines both standard labels and monitoring-specific labels, maintaining flexibility while ensuring consistency.
🧰 Tools
🪛 yamllint (1.35.1)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
charts/greptimedb-cluster/templates/debug-depoyment.yaml (1)
9-9: LGTM! Labels properly added while preserving deployment selectorsThe implementation correctly adds common labels while maintaining the necessary app label for pod selection.
charts/greptimedb-cluster/templates/secret.yaml (1)
8-9: LGTM! Labels properly added to Secret metadataThe addition of labels follows the consistent pattern while maintaining the security-sensitive aspects of the Secret resource.
charts/greptimedb-standalone/templates/secret.yaml (1)
8-9: LGTM! Labels properly added to Secret metadataThe implementation maintains consistency with other resources while properly handling the Secret's sensitive nature.
charts/greptimedb-operator/values.yaml (1)
15-16: LGTM! Well-structured addition of additionalLabels configuration.The addition follows Helm chart best practices with proper documentation and initialization.
charts/greptimedb-operator/templates/clusterrole.yaml (1)
6-7: LGTM! Proper implementation of Kubernetes metadata labels.The addition of labels using the template helper ensures consistent labeling across resources.
charts/greptimedb-standalone/values.yaml (1)
19-20: LGTM! Consistent implementation of additionalLabels configuration.The addition maintains consistency with other chart components and follows the same documentation pattern.
charts/greptimedb-cluster/templates/cluster.yaml (1)
6-7: LGTM! Proper implementation of Kubernetes metadata labels.The addition of labels at the top level ensures proper propagation to all resources and maintains consistency with Kubernetes labeling practices.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
charts/greptimedb-cluster/values.yaml (1)
11-12: LGTM! Consistent implementation of additionalLabels configuration.The addition maintains consistency with other chart components and follows the established pattern for configuration and documentation.
|
Hi @nlamirault
Next, update the chart docs using the command |
Signed-off-by: Nicolas Lamirault <[email protected]>
Signed-off-by: Nicolas Lamirault <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
charts/greptimedb-standalone/README.md (1)
53-53: Enhance additionalLabels documentation with Kubernetes best practices.While the description is clear, it would be helpful to add references to Kubernetes label best practices and provide examples. Consider expanding the description to:
additional labels to add to all resources (see https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
charts/greptimedb-cluster/Chart.yaml(1 hunks)charts/greptimedb-cluster/README.md(2 hunks)charts/greptimedb-cluster/templates/cluster.yaml(1 hunks)charts/greptimedb-cluster/values.yaml(1 hunks)charts/greptimedb-operator/Chart.yaml(1 hunks)charts/greptimedb-operator/README.md(2 hunks)charts/greptimedb-operator/values.yaml(1 hunks)charts/greptimedb-standalone/Chart.yaml(1 hunks)charts/greptimedb-standalone/README.md(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- charts/greptimedb-operator/Chart.yaml
- charts/greptimedb-standalone/Chart.yaml
- charts/greptimedb-cluster/Chart.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- charts/greptimedb-operator/values.yaml
- charts/greptimedb-cluster/values.yaml
🧰 Additional context used
🪛 yamllint (1.35.1)
charts/greptimedb-cluster/templates/cluster.yaml
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (6)
charts/greptimedb-standalone/README.md (1)
5-5: LGTM! Version bump aligns with PR comments.The version update to 0.1.40 matches the version bump requested in the PR comments while maintaining the same AppVersion.
charts/greptimedb-operator/README.md (1)
103-103: LGTM! New configuration for additional labelsThe addition of
additionalLabelsconfiguration aligns with Kubernetes labeling best practices and the PR objectives.charts/greptimedb-cluster/README.md (2)
5-5: Version update looks goodThe version bump to 0.2.45 aligns with the PR comments.
83-83: LGTM! Consistent labeling configurationThe addition of
additionalLabelsconfiguration is consistent with the operator chart and follows Kubernetes best practices.charts/greptimedb-cluster/templates/cluster.yaml (2)
6-7: LGTM! Follows Kubernetes labeling best practices.The addition of labels using a centralized template helper ensures consistent labeling across resources, which aligns with Kubernetes best practices for resource management and organization.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
6-7: Verify the template helper implementation.Let's verify that the
greptimedb-cluster.labelstemplate helper is properly implemented and includes all recommended Kubernetes labels.✅ Verification successful
Template helper implementation is correct and follows best practices
The
greptimedb-cluster.labelshelper includes all recommended Kubernetes labels and is properly implemented with:
- Complete set of
app.kubernetes.io/*labels- Support for version and name overrides
- Extensibility via
additionalLabels- Consistent usage across all resources
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of the labels template helper # Expected: Find the template helper definition and verify its contents # Search for the template helper definition rg -A 10 "greptimedb-cluster.labels" "charts/greptimedb-cluster/templates/_helpers.tpl" # Search for any other usage of the template helper to ensure consistency rg "greptimedb-cluster.labels" "charts/greptimedb-cluster/templates/"Length of output: 1772
Script:
#!/bin/bash # Verify the selector labels helper implementation rg -A 5 "greptimedb-cluster.selectorLabels" "charts/greptimedb-cluster/templates/_helpers.tpl" # Also check if there's a name helper that might be used in selectors rg -A 5 "greptimedb-cluster.name" "charts/greptimedb-cluster/templates/_helpers.tpl"Length of output: 899
🧰 Tools
🪛 yamllint (1.35.1)
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
|
@nlamirault Please run |
daviderli614
changed the title
Summary by CodeRabbit
New Features
additionalLabelsconfiguration option in standalone, cluster, and operator charts.Improvements
Chores