refactor: bump operator version to v0.1.3-alpha.3 #192
Conversation
WalkthroughThe pull request introduces several updates to the 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 (
|
zyy17
force-pushed
the
chore/upgrade-operator
branch
from
0e8d84f to
80151af
Compare
zyy17
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
charts/greptimedb-operator/templates/deployment.yaml (1)
64-64: Minor: Add space before closing braces.There's a minor formatting inconsistency in the end tag. Consider adding a space before the closing braces for consistency.
- {{- end}} + {{- end }}charts/greptimedb-operator/templates/clusterrole.yaml (1)
164-172: Consider adding a comment explaining the metrics.k8s.io permissions.Adding a brief comment above this section would help other developers understand when and why these permissions are needed.
+ # Required for collecting pod metrics via the metrics.k8s.io API when pod metrics collection is enabled {{- if and .Values.apiServer.enabled .Values.apiServer.podMetrics.enabled }} - apiGroups: - metrics.k8s.iocharts/greptimedb-operator/templates/crds/crd-greptimedbstandalone.yaml (1)
2926-2929: LGTM! The logging filters property is well-structured.The addition of the
filtersarray property to the logging configuration follows Kubernetes CRD best practices and provides a clean way to configure logging filters.Consider documenting the following in the chart's README or values file:
- Available filter options
- Filter format/syntax
- Examples of common filter configurations
charts/greptimedb-operator/templates/crds/crd-greptimedbcluster.yaml (1)
2913-2916: LGTM! The filters schema is consistently implemented across all components.The new
filtersproperty has been added with consistent schema definition across all logging configurations (datanode, flownode, frontend, meta, and standalone monitoring components).Consider adding:
- OpenAPI schema description field to document the purpose and expected values of filters
- Pattern validation if there are specific format requirements for the filter strings
Example enhancement:
filters: items: type: string + description: "Logging filters to control which log entries are processed" + pattern: "^[a-zA-Z0-9_.-]+$" # Add if there's a specific format requirement type: array + description: "List of logging filters"Also applies to: 5827-5830, 8724-8727, 11654-11657, 17489-17492
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
charts/greptimedb-operator/Chart.yaml(1 hunks)charts/greptimedb-operator/README.md(3 hunks)charts/greptimedb-operator/templates/clusterrole.yaml(1 hunks)charts/greptimedb-operator/templates/crds/crd-greptimedbcluster.yaml(6 hunks)charts/greptimedb-operator/templates/crds/crd-greptimedbstandalone.yaml(1 hunks)charts/greptimedb-operator/templates/deployment.yaml(1 hunks)charts/greptimedb-operator/values.yaml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- charts/greptimedb-operator/Chart.yaml
🔇 Additional comments (7)
charts/greptimedb-operator/values.yaml (2)
11-11: LGTM: Version bump is consistent.
The image tag update aligns with the PR objectives and is consistently applied across the chart files.
73-77: LGTM: Well-structured feature addition with secure defaults.
The new podMetrics configuration is properly documented and follows security best practices by being disabled by default.
Let's verify the related configuration changes mentioned in the summary:
✅ Verification successful
✅ Feature integration verified across chart components
The verification confirms proper integration of the podMetrics feature:
- Command-line flag
--enable-pod-metricsis correctly added in deployment.yaml - Documentation is properly updated in README.md with matching configuration structure and defaults
- However, we should verify the ClusterRole permissions as they weren't found in the initial search
✅ Complete feature integration verified with proper RBAC configuration
The podMetrics feature is correctly integrated across all components:
- Command-line flag
--enable-pod-metricsis present in deployment.yaml - RBAC permissions are properly configured with conditional rules for metrics.k8s.io API group
- Documentation in README.md accurately reflects the configuration options
- Security is well-implemented:
- Feature is disabled by default
- Required RBAC permissions are only added when both
apiServer.enabledandapiServer.podMetrics.enabledare true
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the integration of podMetrics feature across chart files
# Test 1: Check if deployment.yaml correctly uses this flag
echo "Checking deployment.yaml for pod metrics argument..."
rg --type yaml "enable-pod-metrics" charts/greptimedb-operator/templates/
# Test 2: Verify RBAC rules in ClusterRole
echo "Checking ClusterRole for pod metrics permissions..."
rg --type yaml "pods/metrics" charts/greptimedb-operator/templates/
# Test 3: Confirm documentation in README
echo "Checking README documentation..."
rg "podMetrics" charts/greptimedb-operator/README.md
Length of output: 948
Script:
#!/bin/bash
# Let's check the complete RBAC rules to verify metrics-server permissions
echo "Checking complete RBAC configuration..."
rg -A 5 "rules:" charts/greptimedb-operator/templates/clusterrole.yaml
Length of output: 270
Script:
#!/bin/bash
# Let's get the complete ClusterRole content to verify all permissions
echo "Checking full ClusterRole content..."
cat charts/greptimedb-operator/templates/clusterrole.yaml
# Also check if there's a separate ClusterRole for metrics
echo -e "\nChecking for other RBAC files..."
fd clusterrole charts/greptimedb-operator/templates/
Length of output: 3171
charts/greptimedb-operator/templates/deployment.yaml (1)
61-64: LGTM! Verify values documentation.
The conditional pod metrics flag implementation looks good. The feature is properly gated behind both the API server and pod metrics configuration.
Let's verify the documentation of this new flag:
charts/greptimedb-operator/README.md (2)
5-5: LGTM: Version updates are consistent.
The version updates align with the PR objective of bumping the operator version to v0.1.3-alpha.3.
118-118: LGTM: Image tag update is consistent.
The image tag update to v0.1.3-alpha.3 aligns with the appVersion and PR objective.
charts/greptimedb-operator/templates/crds/crd-greptimedbcluster.yaml (2)
Line range hint 1-7: LGTM! The conditional CRD installation is properly implemented.
The CRD installation is correctly controlled by:
.Values.crds.installfor optional installation.Values.crds.keepannotation to protect CRD from deletion
Line range hint 23-82: LGTM! The CRD versioning and status configuration follows best practices.
The implementation includes:
- Proper versioning with v1alpha1
- Enabled status subresource
- Well-defined printer columns for kubectl output
Summary by CodeRabbit
Release Notes
New Features
datanode,flownode,frontend, andmetacomponents in theGreptimeDBClusterandGreptimeDBStandaloneconfigurations.Version Updates
0.1.3-alpha.3and chart version to0.2.10.Configuration Changes
podMetricssection added to theapiServerconfiguration, defaulting to disabled.