Repository navigation
[platform] Autoscale the autoscaler - #1198
Conversation
The Vertical Pod Autoscaler is a component with resource requirements highly dependent on the environment it is running in, hence it also needs to be autoscaled to reduce the number of configuration parameters that platform admins need to manage. This patch introduces an ancillary autoscaler that watches only the primary autoscaler's namespace and adjusts its resource requests and limits, since the autoscaler cannot autoscale itself. In turn, the primary autoscaler can autoscale the ancillary autoscaler. [platform] Implement autoscaling for the Vertical Pod Autoscaler itself. Signed-off-by: Timofei Larkin <[email protected]>
WalkthroughA new Helm template file was added to conditionally deploy a dedicated namespace, HelmRelease, and two VerticalPodAutoscaler resources for managing the VPA itself. The configuration introduces a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant Kubernetes
participant Flux
participant VPA
User->>Helm: Set vpaForVPA=true
Helm->>Kubernetes: Deploy namespace cozy-vpa-for-vpa
Helm->>Flux: Create HelmRelease vpa-for-vpa
Flux->>Kubernetes: Deploy cozy-vertical-pod-autoscaler in cozy-vpa-for-vpa
Helm->>Kubernetes: Create VPA in release namespace (targets VPA deployment)
Helm->>Kubernetes: Create VPA in cozy-vpa-for-vpa (targets recommender)
Kubernetes->>VPA: Apply update policies via recommender
Possibly related PRs
Suggested reviewers
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello Timofei Larkin (@lllamnyp), I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request implements a self-autoscaling mechanism for the Vertical Pod Autoscaler (VPA) itself. It achieves this by introducing a dedicated, ancillary VPA instance that monitors and adjusts the resource requirements of the primary VPA. This design addresses the challenge of the VPA being unable to autoscale its own components, leading to more efficient resource utilization and reduced manual configuration for platform administrators.
Highlights
- Ancillary VPA Deployment: Introduced a new Helm template (
vpa-for-vpa.yaml) to deploy an ancillary Vertical Pod Autoscaler (VPA) instance in its own dedicated namespace (cozy-vpa-for-vpa). This new VPA is specifically configured to monitor and adjust the resource requests and limits of the primary VPA's recommender component. - Self-Management Loop: Established a self-management loop where the ancillary VPA autoscales the primary VPA, and conversely, the primary VPA is configured to autoscale the ancillary VPA. This ensures both components are dynamically resourced.
- Dynamic Resource Allocation: Removed the hardcoded CPU and memory resource requests and limits for the primary VPA's recommender component in
values.yaml. This change allows the newly introduced ancillary VPA to dynamically manage these resources, optimizing the primary VPA's performance and resource consumption. - Feature Flag: Enabled the
vpaForVPAfeature flag invalues.yamlto activate the deployment of the ancillary VPA and its associated configurations.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | Gemini (@gemini-code-assist) | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a clever mechanism to allow the Vertical Pod Autoscaler to autoscale itself by deploying an ancillary VPA instance. The primary VPA autoscales the ancillary one, and the ancillary VPA autoscales the primary one.
The implementation using a HelmRelease to deploy the ancillary VPA is sound. The configuration for both VPA instances and their corresponding VerticalPodAutoscaler resources appears correct to achieve the intended cross-autoscaling.
I have one suggestion to improve the stability of this setup by locking the chart version for the ancillary VPA to match the primary one. This will prevent unexpected upgrades and ensure both instances run the same code.
| kind: HelmRepository | ||
| name: cozystack-system | ||
| namespace: cozy-system | ||
| version: '>= 0.0.0-0' |
There was a problem hiding this comment.
Using a loose version constraint like >= 0.0.0-0 for the ancillary VPA's Helm chart can introduce instability. This will cause Flux to pull the latest available version from the Helm repository, which might be an unstable pre-release or a version with breaking changes, and could differ from the primary VPA's version.
To ensure stability and consistency, it's recommended to lock the chart version. A good practice is to use the same version as the parent chart, ensuring both VPA instances are synchronized.
version: '{{ .Chart.Version }}'
![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
🧹 Nitpick comments (2)
packages/system/vertical-pod-autoscaler/templates/vpa-for-vpa.yaml (2)
42-43: Inconsistent flag value may recurse indefinitelyInside the nested HelmRelease you set:
values: vpaForVPA: falseThis prevents the sub-chart from creating yet another “autoscaler of the autoscaler”, which is correct, but the intent is not obvious from the first glance. A short comment would avoid future accidents where someone flips this back to
true.
1-3: yamllint false-positive: silence the linter on Helm templatesCI is red due to YAMLlint complaining about the leading
{{- if … }}.
Add a directive to disable linting for this template file:+{{/* yamllint disable-file */}} {{- if .Values.vpaForVPA }}This keeps the pipeline green without relaxing linting globally.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/system/vertical-pod-autoscaler/templates/vpa-for-vpa.yaml(1 hunks)packages/system/vertical-pod-autoscaler/values.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
packages/system/vertical-pod-autoscaler/templates/vpa-for-vpa.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
| @@ -1,3 +1,5 @@ | |||
| vpaForVPA: true | |||
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Flag defaults to true: be explicit about rollout impact
Introducing vpaForVPA: true enables the ancillary autoscaler by default. This flips behaviour for every cluster that consumes this chart and may create extra namespaces / controllers unexpectedly.
Consider one of:
-vpaForVPA: true
+vpaForVPA: false # opt-in – set to true per-cluster once validatedand upgrade clusters incrementally, or at a minimum document the behavioural change clearly in the chart’s CHANGELOG.
#!/usr/bin/env bash
set -eux
echo "=== packages/system/vertical-pod-autoscaler/values.yaml ==="
sed -n '1,50p' packages/system/vertical-pod-autoscaler/values.yaml || true
echo "=== Search for vpaForVPA usage in templates and other files ==="
rg "vpaForVPA" -n -C3 || true
Be explicit about rollout impact: change default vpaForVPA to false
Introducing a new flag that defaults to true will immediately enable the ancillary autoscaler in every cluster that upgrades this chart—potentially creating extra namespaces/controllers without warning. To mitigate unexpected behavior:
- In
packages/system/vertical-pod-autoscaler/values.yaml, update:- vpaForVPA: true + vpaForVPA: false # opt-in feature; set to true per-cluster after validation
- Document this change clearly in the chart’s
CHANGELOG.mdunder breaking changes:Changed default for
vpaForVPAfromtruetofalse. Consumers must explicitly opt in (--set vpaForVPA=true) to enable the ancillary autoscaler.
Roll out the new default incrementally—verify on staging clusters before wider deployment.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| vpaForVPA: true | |
| # packages/system/vertical-pod-autoscaler/values.yaml | |
| vpaForVPA: false # opt-in feature; set to true per-cluster after validation |
🤖 Prompt for AI Agents
In packages/system/vertical-pod-autoscaler/values.yaml at line 1, change the
default value of vpaForVPA from true to false to prevent automatic enabling of
the ancillary autoscaler on upgrade. Additionally, update the chart's
CHANGELOG.md to document this breaking change, specifying that users must
explicitly set vpaForVPA to true to enable the feature. This ensures safer
rollout and clear communication of the change.
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| name: {{ dig "vertical-pod-autoscaler" "nameOverride" "" .Values.AsMap | default "vertical-pod-autoscaler" | trunc 63 | trimSuffix "-" }}-recommender |
There was a problem hiding this comment.
Template fails: .Values.AsMap is not a Helm object
dig should receive .Values, not .Values.AsMap (which does not exist and will raise “unable to cast to map”).
- name: {{ dig "vertical-pod-autoscaler" "nameOverride" "" .Values.AsMap | default "vertical-pod-autoscaler" | trunc 63 | trimSuffix "-" }}-recommender
+ name: {{ dig "vertical-pod-autoscaler" "nameOverride" "" .Values | default "vertical-pod-autoscaler" | trunc 63 | trimSuffix "-" }}-recommender📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| apiVersion: apps/v1 | |
| kind: Deployment | |
| name: {{ dig "vertical-pod-autoscaler" "nameOverride" "" .Values.AsMap | default "vertical-pod-autoscaler" | trunc 63 | trimSuffix "-" }}-recommender | |
| apiVersion: apps/v1 | |
| kind: Deployment | |
| name: {{ dig "vertical-pod-autoscaler" "nameOverride" "" .Values | default "vertical-pod-autoscaler" | trunc 63 | trimSuffix "-" }}-recommender |
🤖 Prompt for AI Agents
In packages/system/vertical-pod-autoscaler/templates/vpa-for-vpa.yaml around
lines 68 to 70, the template uses `.Values.AsMap` which does not exist and
causes a casting error. Replace `.Values.AsMap` with `.Values` in the `dig`
function call to correctly reference the Helm values object and avoid the
template failure.
What this PR does
The Vertical Pod Autoscaler is a component with resource requirements highly dependent on the environment it is running in, hence it also needs to be autoscaled to reduce the number of configuration parameters that platform admins need to manage. This patch introduces an ancillary autoscaler that watches only the primary autoscaler's namespace and adjusts its resource requests and limits, since the autoscaler cannot autoscale itself. In turn, the primary autoscaler can autoscale the ancillary autoscaler.
Release note
Summary by CodeRabbit
New Features
Configuration