Skip to content

feat(chart): VPA Updater PodDisruptionBudget #8718

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 20, 2025
+37 −2
Merged

feat(chart): VPA Updater PodDisruptionBudget #8718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion vertical-pod-autoscaler/charts/vertical-pod-autoscaler/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.5.0
version: 0.6.0

# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
Expand Down
5 changes: 4 additions & 1 deletion vertical-pod-autoscaler/charts/vertical-pod-autoscaler/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ WARNING: This chart is currently under development and is not ready for producti

Automatically adjust resources for your workloads

![Version: 0.5.0](https://img.shields.io/badge/Version-0.5.0-informational?style=flat-square)
![Version: 0.6.0](https://img.shields.io/badge/Version-0.6.0-informational?style=flat-square)
![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square)
![AppVersion: 1.5.1](https://img.shields.io/badge/AppVersion-1.5.1-informational?style=flat-square)

Expand Down Expand Up @@ -107,6 +107,9 @@ The Vertical Pod Autoscaler (VPA) automatically adjusts the CPU and memory resou
| updater.leaderElection.resourceNamespace | string | `""` | |
| updater.leaderElection.retryPeriod | string | `"2s"` | |
| updater.podAnnotations | object | `{}` | |
| updater.podDisruptionBudget.enabled | bool | `true` | |
| updater.podDisruptionBudget.maxUnavailable | int or string | `nil` | Maximum number/percentage of pods that can be unavailable after the eviction. IMPORTANT: You can specify either 'minAvailable' or 'maxUnavailable', but not both. |
| updater.podDisruptionBudget.minAvailable | int or string | `1` | Minimum number/percentage of pods that must be available after the eviction. IMPORTANT: You can specify either 'minAvailable' or 'maxUnavailable', but not both. |
| updater.podLabels | object | `{}` | |
| updater.replicas | int | `2` | |
| updater.serviceAccount.annotations | object | `{}` | |
Expand Down
22 changes: 22 additions & 0 deletions vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/updater-pdb.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{{- if and .Values.updater.enabled .Values.updater.podDisruptionBudget.enabled -}}
{{- if and .Values.updater.podDisruptionBudget.minAvailable .Values.updater.podDisruptionBudget.maxUnavailable }}
{{- fail "Only one of updater.podDisruptionBudget.minAvailable or updater.podDisruptionBudget.maxUnavailable should be set." }}
{{- end }}
apiVersion: policy/v1
kind: PodDisruptionBudget
metadata:
name: {{ include "vertical-pod-autoscaler.updater.fullname" . }}
namespace: {{ .Release.Namespace }}
labels:
{{- include "vertical-pod-autoscaler.updater.labels" . | nindent 4 }}
spec:
selector:
matchLabels:
{{- include "vertical-pod-autoscaler.updater.selectorLabels" . | nindent 6 }}
{{- if .Values.updater.podDisruptionBudget.minAvailable }}
Copy link
Contributor

@jackfrancis jackfrancis Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the Kubernetes documentation:

You can specify only one of maxUnavailable and minAvailable in a single PodDisruptionBudget. maxUnavailable can only be used to control the eviction of pods that all have the same associated controller managing them. In the examples below, "desired replicas" is the scale of the controller managing the pods being selected by the PodDisruptionBudget.

See how we handle this in the Cluster Autoscaler chart:

https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/charts/cluster-autoscaler/templates/pdb.yaml

https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/charts/cluster-autoscaler/values.yaml#L334-L337

Essentially, we want to ensure we only default to one or the other (you've already done the equivalent of that by defaulting maxUnavailable to empty. But also IMO the checks in the template foo as well are useful:

  • Fail processing if both values are set
  • Only inject a min or max value if it is the only one set

Copy link
Contributor Author

@phuhung273 phuhung273 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look Jack. I can see that validation in

autoscaler/vertical-pod-autoscaler/charts/vertical-pod-autoscaler/templates/admission-controller-pdb.yaml

Lines 1 to 4 in f7fc231

{{- if and .Values.admissionController.enabled .Values.admissionController.podDisruptionBudget.enabled -}}
{{- if and .Values.admissionController.podDisruptionBudget.minAvailable .Values.admissionController.podDisruptionBudget.maxUnavailable }}
{{- fail "Only one of admissionController.podDisruptionBudget.minAvailable or admissionController.podDisruptionBudget.maxUnavailable should be set." }}
{{- end }}

But I decided not to do the same since k8s already has builtin validation

Error: 1 error occurred:
        * PodDisruptionBudget.policy "vpa-vertical-pod-autoscaler-updater" is invalid: spec: Invalid value: {"MinAvailable":1,"Selector":{"matchLabels":{"app.kubernetes.io/component":"updater","app.kubernetes.io/instance":"vpa","app.kubernetes.io/name":"vertical-pod-autoscaler"}},"MaxUnavailable":1,"UnhealthyPodEvictionPolicy":null}: minAvailable and maxUnavailable cannot be both set

Can you have a try and let me know if we need our own validation.

Copy link
Member

@adrianmoisey adrianmoisey Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to validate in Helm. The reason is someone may test locally, before applying to the cluster. Failing validation on an apply may be too late

Copy link
Contributor Author

@phuhung273 phuhung273 Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thanks for the feedback. I've updated to include Helm validation.

minAvailable: {{ .Values.updater.podDisruptionBudget.minAvailable }}
{{- end }}
{{- if .Values.updater.podDisruptionBudget.maxUnavailable }}
maxUnavailable: {{ .Values.updater.podDisruptionBudget.maxUnavailable }}
{{- end }}
{{- end -}}
10 changes: 10 additions & 0 deletions vertical-pod-autoscaler/charts/vertical-pod-autoscaler/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,3 +227,13 @@ updater:
renewDeadline: 10s
# Duration the clients should wait between attempting acquisition and renewal of a leadership.
retryPeriod: 2s

# PodDisruptionBudget for the Updater.
podDisruptionBudget:
enabled: true
# -- (int or string) Minimum number/percentage of pods that must be available after the eviction.
# IMPORTANT: You can specify either 'minAvailable' or 'maxUnavailable', but not both.
minAvailable: 1
# -- (int or string) Maximum number/percentage of pods that can be unavailable after the eviction.
# IMPORTANT: You can specify either 'minAvailable' or 'maxUnavailable', but not both.
maxUnavailable: