[monitoring] add vpa for users k8s clusters #806
Conversation
WalkthroughThe changes update the Kubernetes Helm chart version from 0.17.1 to 0.18.0 and adjust HelmRelease dependencies. The monitoring agents configuration now removes a dependency on a legacy component and adds one for vertical pod autoscaler CRDs. Additionally, new HelmRelease configurations have been introduced for both the vertical pod autoscaler and its CRDs. A new section to override vertical pod autoscaler values has been added to the values file, and the versions map has been updated to reflect these changes. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Chart
participant MA as Monitoring Agents
participant VCR as VPA CRDs
participant VPA as Vertical Pod Autoscaler
U->>C: Initiate deployment
C->>MA: Deploy monitoring agents
MA->>VCR: Trigger VPA CRDs dependency
VCR->>VPA: Provide CRDs for deployment
VPA->>C: Complete VPA deployment
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
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 (
|
Signed-off-by: kklinch0 <kklinch0@gmail.com>
eab6c1b to
da81009
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/apps/kubernetes/values.yaml (1)
73-78: Consider adding anenabledparameter for consistency.All other addons have an
enabledparameter, but this one is missing it. If the VerticalPodAutoscaler's enablement is controlled by monitoringAgents (as suggested by related changes), consider adding a comment to make this dependency clear, or add a separateenabledflag for consistency with other addons.verticalPodAutoscaler: + ## @param addons.verticalPodAutoscaler.enabled Enable Vertical Pod Autoscaler + ## + enabled: false ## @param addons.verticalPodAutoscaler.valuesOverride Custom values to override ## valuesOverride: {}packages/apps/kubernetes/templates/helmreleases/vertical-pod-autoscaler-crds.yaml (2)
1-1: Helm Templating and YAML Lint Configuration
The use of Helm templating on line 1 (i.e.{{- if .Values.addons.monitoringAgents.enabled }}) is expected. However, YAML linters may flag this pattern as a syntax error. Please ensure your linter is configured to ignore or properly parse Helm directives.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
34-41: Conditional Dependency Block Formatting
The dependency block (lines 34–41) first conditionally adds a dependency on the release itself (lines 35–38) and then unconditionally includes the dependency on{{ .Release.Name }}-cilium(lines 39–40). The formatting appears correct for Helm templating, but please double-check that the YAML indentation renders as expected once templating is processed.packages/apps/kubernetes/templates/helmreleases/vertical-pod-autoscaler.yaml (1)
62-69: Dependency Configuration
In thedependsOnsection (lines 62–69), a dependency is conditionally added for the current release (lines 63–66) and then unconditionally for{{ .Release.Name }}-monitoring-agents(lines 67–68). This dependency ordering aligns with the updated deployment requirements. Verify that this meets your release orchestration strategy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/apps/kubernetes/Chart.yaml(1 hunks)packages/apps/kubernetes/templates/helmreleases/monitoring-agents.yaml(1 hunks)packages/apps/kubernetes/templates/helmreleases/vertical-pod-autoscaler-crds.yaml(1 hunks)packages/apps/kubernetes/templates/helmreleases/vertical-pod-autoscaler.yaml(1 hunks)packages/apps/kubernetes/values.yaml(1 hunks)packages/apps/versions_map(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/apps/kubernetes/templates/helmreleases/vertical-pod-autoscaler-crds.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/apps/kubernetes/templates/helmreleases/vertical-pod-autoscaler.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (14)
packages/apps/kubernetes/Chart.yaml (1)
19-19: Version increment looks appropriate.The increment from 0.17.1 to 0.18.0 correctly follows semantic versioning for the addition of new functionality (Vertical Pod Autoscaler).
packages/apps/versions_map (1)
60-61: Version mapping updated correctly.The change properly pins the previous version (0.17.1) to a specific commit and adds the new version (0.18.0) as HEAD, following the established pattern in this file.
packages/apps/kubernetes/templates/helmreleases/monitoring-agents.yaml (1)
43-44:✅ Verification successful
Dependency update for VPA integration.
The addition of a dependency on the vertical-pod-autoscaler-crds is appropriate for the monitoring agents, as it ensures that the VPA CRDs are installed before the monitoring agents are deployed.
🏁 Script executed:
#!/bin/bash # Description: Verify the existence of vertical-pod-autoscaler-crds HelmRelease template # Look for the vertical-pod-autoscaler-crds.yaml file in the templates directory fd --type f "vertical-pod-autoscaler-crds.yaml" packages/apps/kubernetes/templates/helmreleases/ # Check for the vertical-pod-autoscaler.yaml file as well fd --type f "vertical-pod-autoscaler.yaml" packages/apps/kubernetes/templates/helmreleases/Length of output: 347
Verified VPA Integration Dependency
The verification confirms that the file
packages/apps/kubernetes/templates/helmreleases/vertical-pod-autoscaler-crds.yamlexists (along withvertical-pod-autoscaler.yaml), ensuring that the VPA CRDs will be installed before the monitoring agents are deployed. The dependency update in themonitoring-agents.yamlfile (lines 43–44) is therefore correct.packages/apps/kubernetes/templates/helmreleases/vertical-pod-autoscaler-crds.yaml (4)
2-10: Metadata and Basic Specification
The metadata (lines 4–8) and top-level specification (lines 2–10) are correctly defined. The chart reference and repository details follow the project conventions.
11-20: Chart Source and KubeConfig Settings
The chart details (lines 12–19) along with the kubeConfig secret reference (lines 20–23) are correctly set. This configuration aligns with the standard HelmRelease deployment patterns in this repository.
24-32: Namespace and Remediation Configuration
The target and storage namespaces (lines 24–25) and the install/upgrade sections (lines 26–32) are configured to automatically create namespaces with unlimited retry remediation (retries: -1). Verify that using unlimited retries is an intentional design decision for critical deployments.
41-42: Closing the Conditional Block
The conditional block started on line 1 is correctly closed with{{- end }}. Ensure that the entire content is correctly encapsulated by this block as intended.packages/apps/kubernetes/templates/helmreleases/vertical-pod-autoscaler.yaml (7)
1-3: Variable Setup and Conditional Checks
The template begins by looking up the namespace ($myNS) and extracting the "namespace.cozystack.io/monitoring" annotation into$targetTenant. Consider using Helm’sdefaultfunction to provide a fallback value in case the annotation is missing, which can help avoid potential nil errors.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
4-10: Metadata and Label Consistency
The metadata block (lines 6–10) is well formed. One note: while the repository label usescozystack.io/repository, the target cluster label is set undercoztstack.io/target-cluster-name. Please verify that this slight domain variation is intentional and not a typographical error.
11-27: Chart Details and KubeConfig
The chart specification (lines 11–21) and the kubeConfig setup (lines 22–25) are correctly defined and mirror the patterns used in similar configurations.
28-34: Installation and Upgrade Settings
The install and upgrade sections (lines 28–34) include namespace creation and unlimited retry settings (retries: -1). Confirm that these settings match your operational requirements, as unlimited retries might complicate troubleshooting in failure scenarios.
35-55: Vertical Pod Autoscaler Values Configuration
The nested values block (lines 35–55) provides detailed configuration for the vertical pod autoscaler recommender, including extra arguments and resource constraints. Ensure that the metric query inmetric-for-pod-labels(line 45), which embeds Helm template variables, is correctly formed and returns the expected Prometheus data.
56-61: Conditional Values Override Block
The conditional block (lines 56–61) injects override values sourced from a secret when.Values.addons.verticalPodAutoscaler.valuesOverrideis enabled. Please confirm that the referenced secret exists and that the keyvaluescontains the expected configuration data.
69-70: End of Conditional Block
The final closing ({{- end }}on line 69) ensures that the configuration is only applied when.Values.addons.monitoringAgents.enabledis true. This is correctly implemented.
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Chores** - Updated the application version to 0.18.0 with refined version tracking for improved deployment clarity. - **New Features** - Enhanced the monitoring agents integration with updated dependency management. - Introduced new deployment configurations for the vertical pod autoscaler and its custom resource definitions, offering customizable override options and improved reconciliation strategies. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary by CodeRabbit
Chores
New Features