add grafana size configure #536
Conversation
WalkthroughThis pull request introduces updates to the monitoring package configuration, focusing on enhancing flexibility and removing hardcoded defaults. The changes include updating the package version from 1.5.2 to 1.5.3, modifying the Telegram alert configuration by removing specific token and chat ID defaults, and adding a new configurable parameter for Grafana database persistent volume size. Changes
Possibly related PRs
Suggested labels
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/extra/monitoring/values.yaml (1)
47-52: LGTM! Consider adding storageClassName for consistencyThe Grafana DB configuration is well-structured and follows the existing patterns. However, for consistency with other storage configurations in this file (e.g., alerta, metricsStorages), consider adding a
storageClassNameparameter.grafana: db: size: 10Gi + storageClassName: ""packages/extra/monitoring/README.md (1)
15-15: Format the URL properly in MarkdownThe bare URL should be properly formatted as a Markdown link for better readability and to follow Markdown best practices.
-| `alerta.alerts.telegram.chatID` | specify multiple ID's separated by comma. Get yours in https://t.me/chatid_echo_bot | `""` | +| `alerta.alerts.telegram.chatID` | specify multiple ID's separated by comma. Get yours in [chatid_echo_bot](https://t.me/chatid_echo_bot) | `""` |🧰 Tools
🪛 Markdownlint (0.37.0)
15-15: null
Bare URL used(MD034, no-bare-urls)
packages/extra/monitoring/values.schema.json (1)
67-71: Consider adding size format validationThe
sizeproperty could benefit from pattern validation to ensure values follow Kubernetes resource quantity format (e.g., "10Gi", "1Ti").Add pattern validation like this:
"size": { "type": "string", "description": "Persistent Volume size for grafana database", + "pattern": "^[0-9]+(E|P|T|G|M|K|Ei|Pi|Ti|Gi|Mi|Ki)?$", "default": "10Gi" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/extra/monitoring/Chart.yaml(1 hunks)packages/extra/monitoring/README.md(1 hunks)packages/extra/monitoring/templates/grafana/db.yaml(1 hunks)packages/extra/monitoring/values.schema.json(1 hunks)packages/extra/monitoring/values.yaml(1 hunks)packages/extra/versions_map(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/extra/monitoring/Chart.yaml
🧰 Additional context used
🪛 Markdownlint (0.37.0)
packages/extra/monitoring/README.md
15-15: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (7)
packages/extra/versions_map (2)
19-19: LGTM! Good practice pinning version 1.5.2 to specific commit.
Pinning version 1.5.2 to commit 898374b5 instead of HEAD improves version stability and traceability.
20-20: Verify commit hash and consider pinning version 1.5.3.
Using HEAD for the latest version (1.5.3) is acceptable during development. However, once the changes are finalized, consider pinning it to the specific commit hash for better version control and traceability.
Let's verify the commit hash and changes:
packages/extra/monitoring/templates/grafana/db.yaml (1)
8-8: Verify storage class compatibility with replicated PostgreSQL
The configuration looks correct. However, since this is a replicated PostgreSQL cluster (instances: 2), ensure that the underlying storage class supports the required access modes for database replication.
✅ Verification successful
Storage class compatibility verified for replicated PostgreSQL
The configuration is safe. The PostgreSQL cluster is using CloudNativePG operator (postgresql.cnpg.io/v1) which handles replication internally using streaming replication, not shared storage. Each pod gets its own PVC, so standard storage classes with ReadWriteOnce access mode are fully compatible with the replicated setup.
The other replicated databases in the monitoring stack (VMCluster, VLogs) follow the same pattern - each replica gets its own independent storage volume.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check storage classes in the cluster for their access modes
kubectl get storageclass -o=jsonpath='{range .items[*]}{.metadata.name}{"\t"}{.provisioner}{"\t"}{.volumeBindingMode}{"\t"}{.allowVolumeExpansion}{"\n"}{end}'
Length of output: 258
Script:
#!/bin/bash
# Let's check the complete template file to understand storage configuration
cat packages/extra/monitoring/templates/grafana/db.yaml
Length of output: 293
Script:
#!/bin/bash
# Check if storage class is defined in values file
fd values.yaml --exec cat {}
Length of output: 79673
Script:
#!/bin/bash
# Look for any storage class definitions in other templates
rg -l "storageClassName" packages/extra/monitoring/templates/
Length of output: 226
Script:
#!/bin/bash
# Let's check the storage class definitions in the cluster configuration
cat packages/extra/monitoring/templates/vm/vmcluster.yaml packages/extra/monitoring/templates/vlogs/vlogs.yaml packages/extra/monitoring/templates/alerta/alerta-db.yaml
Length of output: 2139
Script:
#!/bin/bash
# Let's check if there's a default storage class defined in values
rg "storageClass:" packages/extra/monitoring/
Length of output: 129
Script:
#!/bin/bash
# Check if there are any storage class related configurations in the parent directories
fd -t f -e yaml -e yml . packages/extra/ -x grep -l "storageClass"
Length of output: 436
packages/extra/monitoring/README.md (1)
7-16: LGTM! Documentation is clear and well-formatted
The parameter documentation is comprehensive and follows a consistent format. The table structure is well-maintained with proper alignment.
🧰 Tools
🪛 Markdownlint (0.37.0)
15-15: null
Bare URL used
(MD034, no-bare-urls)
packages/extra/monitoring/values.schema.json (3)
48-48: Good security practice: Removed hardcoded Telegram credentials
Replacing hardcoded Telegram token and chatID with empty string defaults is a security improvement. This ensures sensitive credentials are provided by users during deployment rather than being exposed in the code.
Also applies to: 53-53
61-74: LGTM: Well-structured Grafana DB size configuration
The new configuration block follows the existing schema patterns and provides clear documentation.
70-70: Verify consistent size defaults across files
Let's ensure the default size of "10Gi" is consistent with other configuration files.
✅ Verification successful
Default size value "10Gi" is consistently defined
The verification shows that:
- The schema file correctly defines "10Gi" as the default value for both alerta storage and grafana database size
- The values.yaml file consistently uses "10Gi" across all storage configurations
- The templates properly reference these values through variables (e.g.,
.Values.grafana.db.size,.Values.alerta.storage) - The only deviation is in vmcluster.yaml which has a specific 2Gi storage value for a different component
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for size configuration in related files
echo "Checking size configuration in values.yaml and templates:"
rg -g '*.{yaml,yml}' '"?10Gi"?' packages/extra/monitoring/
Length of output: 454
Script:
#!/bin/bash
# Let's check the actual schema file and templates to ensure alignment
echo "Checking schema file:"
cat packages/extra/monitoring/values.schema.json | grep -A 5 -B 5 '"10Gi"'
echo -e "\nChecking templates:"
rg -g '*.{yaml,yml,tpl}' 'size:|storage:' packages/extra/monitoring/templates/
Length of output: 1775
Summary by CodeRabbit
New Features
Bug Fixes
alerta.alerts.telegram.tokenandalerta.alerts.telegram.chatIDto empty strings.Documentation
Chores