Helm: Pod Disruption Budget Availability Bugfix

chart/templates/poddisruptionbudget.yaml

@@ -7,39 +7,42 @@ apiVersion: policy/v1
 kind: PodDisruptionBudget
 metadata:
   name: {{ include "vso.chart.fullname" . }}
+  namespace: {{ .Release.Namespace }}
   labels:
     app.kubernetes.io/component: controller-manager
     control-plane: controller-manager
     {{- include "vso.chart.labels" . | nindent 4 }}
-  namespace: {{ .Release.Namespace }}
 spec:
-  {{/* Throw an error if both maxUnavailable and minAvailable are set and non-zero */}}
-  {{- $maxUnavailable := toString .Values.controller.podDisruptionBudget.maxUnavailable | trim }}
-  {{- $minAvailable := toString .Values.controller.podDisruptionBudget.minAvailable | trim }}
-  {{- if and (not (empty $maxUnavailable)) (not (empty $minAvailable)) (ne $maxUnavailable "0") (ne $minAvailable "0") }}
-  {{- fail "You cannot set both maxUnavailable and minAvailable in the PodDisruptionBudget" }}
-  {{- end }}
+  {{- $rawMax := default "" .Values.controller.podDisruptionBudget.maxUnavailable -}}
+  {{- $rawMin := default "" .Values.controller.podDisruptionBudget.minAvailable -}}
+  {{- $max := toString $rawMax | trim -}}
+  {{- $min := toString $rawMin | trim -}}
 
-  {{/* If maxUnavailable is set, use it */}}
-  {{- if not (empty $maxUnavailable) }}
-  maxUnavailable: 
-    {{- if contains "%" $maxUnavailable }}
-    "{{ $maxUnavailable }}"
-    {{- else }}
-    {{ $maxUnavailable }}
-    {{- end }}
-  {{- end }}
+  {{- /* Presence & zero-ness checks */ -}}
+  {{- $maxProvided := ne $max "" -}}
+  {{- $minProvided := ne $min "" -}}
+  {{- $maxZero := or (eq $max "0") (eq $max "0%") -}}
+  {{- $minZero := or (eq $min "0") (eq $min "0%") -}}
+  {{- $maxNonZero := and $maxProvided (not $maxZero) -}}
+  {{- $minNonZero := and $minProvided (not $minZero) -}}
 
-  {{/* If minAvailable is set, use it */}}
-  {{- if not (empty $minAvailable) }}
-  minAvailable: 
-    {{- if contains "%" $minAvailable }}
-    "{{ $minAvailable }}"
-    {{- else }}
-    {{ $minAvailable }}
-    {{- end }}
+  {{- /* Hard stop only when both sides are non-zero (K8s forbids both keys at once) */ -}}
+  {{- if and $minNonZero $maxNonZero -}}
+  {{- fail "controller.podDisruptionBudget: you cannot set both maxUnavailable and minAvailable to non-zero values." -}}
   {{- end }}
 
+  {{- /* Emit exactly one key */ -}}
+  {{- if $minNonZero }}
+  minAvailable:{{ if contains "%" $min }} "{{ $min }}"{{ else }} {{ $min }}{{ end }}
+  {{- else if $maxNonZero }}
+  maxUnavailable:{{ if contains "%" $max }} "{{ $max }}"{{ else }} {{ $max }}{{ end }}
+  {{- else if and $minProvided (not $maxProvided) }}
+  minAvailable:{{ if contains "%" $min }} "{{ $min }}"{{ else }} {{ $min }}{{ end }}
+  {{- else if and $maxProvided (not $minProvided) }}
+  maxUnavailable:{{ if contains "%" $max }} "{{ $max }}"{{ else }} {{ $max }}{{ end }}
+  {{- else }}
+  minAvailable: 1
+  {{- end }}
   selector:
     matchLabels:
       {{- include "vso.chart.selectorLabels" . | nindent 6 }}

test/unit/pdb.bats

@@ -0,0 +1,227 @@
+#!/usr/bin/env bats
+
+#
+# Copyright (c) HashiCorp, Inc.
+# SPDX-License-Identifier: BUSL-1.1
+#
+
+load _helpers
+
+pdb_yaml() {
+  helm template \
+    --set "controller.podDisruptionBudget.enabled=true" \
+    --set "controller.replicas=2" \
+    "$@" \
+    . | tee /dev/stderr |
+    yq 'select(.kind == "PodDisruptionBudget" and .metadata.labels."app.kubernetes.io/component" == "controller-manager")' \
+    | tee /dev/stderr
+}
+
+#--------------------------------------------------------------------
+# defaults & single-key behavior
+
+@test "controller/PodDisruptionBudget: defaults to minAvailable 1 when no constraints are set" {
+  cd `chart_dir`
+
+  local output
+  output=$(pdb_yaml)
+
+  local actual
+
+  # Should default to a safe minAvailable: 1
+  actual=$(echo "$output" | yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "1" ]
+
+  # And must not set maxUnavailable at all
+  actual=$(echo "$output" | yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "controller/PodDisruptionBudget: uses only maxUnavailable when set and minAvailable is unset" {
+  cd `chart_dir`
+
+  local output
+  output=$(pdb_yaml \
+    --set "controller.podDisruptionBudget.maxUnavailable=2")
+
+  local actual
+
+  # Should render only maxUnavailable: 2
+  actual=$(echo "$output" | yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "2" ]
+
+  # minAvailable must not be set
+  actual=$(echo "$output" | yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "controller/PodDisruptionBudget: uses only minAvailable when set and maxUnavailable is unset" {
+  cd `chart_dir`
+
+  local output
+  output=$(pdb_yaml \
+    --set "controller.podDisruptionBudget.minAvailable=2")
+
+  local actual
+
+  # Should render only minAvailable: 2
+  actual=$(echo "$output" | yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "2" ]
+
+  # maxUnavailable must not be set
+  actual=$(echo "$output" | yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+#--------------------------------------------------------------------
+# zero-handling when both keys are present
+
+@test "controller/PodDisruptionBudget: when both set and minAvailable is zero, render only non-zero maxUnavailable" {
+  cd `chart_dir`
+
+  local output
+  output=$(pdb_yaml \
+    --set "controller.podDisruptionBudget.maxUnavailable=3" \
+    --set "controller.podDisruptionBudget.minAvailable=0")
+
+  local actual
+
+  # Only maxUnavailable should be emitted
+  actual=$(echo "$output" | yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "3" ]
+
+  actual=$(echo "$output" | yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "controller/PodDisruptionBudget: when both set and maxUnavailable is zero, render only non-zero minAvailable" {
+  cd `chart_dir`
+
+  local output
+  output=$(pdb_yaml \
+    --set "controller.podDisruptionBudget.maxUnavailable=0" \
+    --set "controller.podDisruptionBudget.minAvailable=3")
+
+  local actual
+
+  # Only minAvailable should be emitted
+  actual=$(echo "$output" | yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "3" ]
+
+  actual=$(echo "$output" | yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "controller/PodDisruptionBudget: when both constraints explicitly zero, falls back to minAvailable 1" {
+  cd `chart_dir`
+
+  local output
+  output=$(pdb_yaml \
+    --set "controller.podDisruptionBudget.maxUnavailable=0" \
+    --set "controller.podDisruptionBudget.minAvailable=0")
+
+  local actual
+
+  # Both sides zero is treated as "no explicit constraints" -> safe default 1
+  actual=$(echo "$output" | yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "1" ]
+
+  actual=$(echo "$output" | yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+#--------------------------------------------------------------------
+# failure behaviour
+
+@test "controller/PodDisruptionBudget: fails when both maxUnavailable and minAvailable are non-zero" {
+  cd `chart_dir`
+
+  # Use Bats' `run` helper because we *expect* helm to fail here
+  run helm template \
+    --set "controller.podDisruptionBudget.enabled=true" \
+    --set "controller.replicas=2" \
+    --set "controller.podDisruptionBudget.maxUnavailable=1" \
+    --set "controller.podDisruptionBudget.minAvailable=1" \
+    .
+
+  # Helm should fail due to both constraints being non-zero
+  [ "$status" -ne 0 ]
+
+  # Error message should mention both maxUnavailable and minAvailable
+  echo "$output" | tee /dev/stderr | grep "maxUnavailable and minAvailable"
+}
+
+#--------------------------------------------------------------------
+# percentage handling
+
+@test "controller/PodDisruptionBudget: supports percentage values for minAvailable" {
+  cd `chart_dir`
+
+  local output
+  output=$(pdb_yaml \
+    --set "controller.podDisruptionBudget.minAvailable=34%")
+
+  local actual
+
+  # Template should preserve the percentage as a string value
+  actual=$(echo "$output" | yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "34%" ]
+
+  # And maxUnavailable should not be set in this case
+  actual=$(echo "$output" | yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "controller/PodDisruptionBudget: supports percentage values for maxUnavailable" {
+  cd `chart_dir`
+
+  local output
+  output=$(pdb_yaml \
+    --set "controller.podDisruptionBudget.maxUnavailable=34%")
+
+  local actual
+
+  # Template should preserve the percentage as a string value
+  actual=$(echo "$output" | yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "34%" ]
+
+  # And minAvailable should not be set in this case
+  actual=$(echo "$output" | yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "controller/PodDisruptionBudget: treats 0% minAvailable as zero when maxUnavailable is non-zero" {
+  cd `chart_dir`
+
+  local output
+  output=$(pdb_yaml \
+    --set "controller.podDisruptionBudget.maxUnavailable=3" \
+    --set "controller.podDisruptionBudget.minAvailable=0%")
+
+  local actual
+
+  # minAvailable=0% is treated as zero; only the non-zero maxUnavailable is emitted
+  actual=$(echo "$output" | yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "3" ]
+
+  actual=$(echo "$output" | yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}
+
+@test "controller/PodDisruptionBudget: treats 0% maxUnavailable as zero when minAvailable is non-zero" {
+  cd `chart_dir`
+
+  local output
+  output=$(pdb_yaml \
+    --set "controller.podDisruptionBudget.maxUnavailable=0%" \
+    --set "controller.podDisruptionBudget.minAvailable=3")
+
+  local actual
+
+  # maxUnavailable=0% is treated as zero; only the non-zero minAvailable is emitted
+  actual=$(echo "$output" | yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "3" ]
+
+  actual=$(echo "$output" | yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "null" ]
+}