From 9317d853c4b7820de04033e5547adc2202f62f6b Mon Sep 17 00:00:00 2001
From: John Lahr <john@johnlahr.me>
Date: Sat, 4 Oct 2025 02:50:55 -0500
Subject: [PATCH 1/3] PDB Availability Bugfix

- Fixed PDB rendering both an explicit minAvailable and maxUnavailable at once
---
 chart/templates/poddisruptionbudget.yaml | 51 +++++++++++++-----------
 1 file changed, 27 insertions(+), 24 deletions(-)

diff --git a/chart/templates/poddisruptionbudget.yaml b/chart/templates/poddisruptionbudget.yaml
index 582c1c8ea..0696555a8 100644
--- a/chart/templates/poddisruptionbudget.yaml
+++ b/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 }}

From eae6f355abdf7d779c81397aa53c3c9a38aeb68d Mon Sep 17 00:00:00 2001
From: John Lahr <john@johnlahr.me>
Date: Mon, 24 Nov 2025 20:35:43 -0600
Subject: [PATCH 2/3] added tests

---
 test/unit/pdb.bat | 143 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 143 insertions(+)
 create mode 100644 test/unit/pdb.bat

diff --git a/test/unit/pdb.bat b/test/unit/pdb.bat
new file mode 100644
index 000000000..14fbc52bf
--- /dev/null
+++ b/test/unit/pdb.bat
@@ -0,0 +1,143 @@
+#!/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
+}
+
+@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" ]
+}
+
+@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: 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"
+}
+
+@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" ]
+}

From 34d4ad862952b0d2ddb8faef53ac25a3a369f9b7 Mon Sep 17 00:00:00 2001
From: John Lahr <john@johnlahr.me>
Date: Mon, 24 Nov 2025 21:08:55 -0600
Subject: [PATCH 3/3] updated tests

---
 test/unit/{pdb.bat => pdb.bats} | 84 +++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)
 rename test/unit/{pdb.bat => pdb.bats} (60%)
 mode change 100644 => 100755

diff --git a/test/unit/pdb.bat b/test/unit/pdb.bats
old mode 100644
new mode 100755
similarity index 60%
rename from test/unit/pdb.bat
rename to test/unit/pdb.bats
index 14fbc52bf..5de6e488a
--- a/test/unit/pdb.bat
+++ b/test/unit/pdb.bats
@@ -17,6 +17,9 @@ pdb_yaml() {
     | tee /dev/stderr
 }
 
+#--------------------------------------------------------------------
+# defaults & single-key behavior
+
 @test "controller/PodDisruptionBudget: defaults to minAvailable 1 when no constraints are set" {
   cd `chart_dir`
 
@@ -70,6 +73,9 @@ pdb_yaml() {
   [ "${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`
 
@@ -106,6 +112,27 @@ pdb_yaml() {
   [ "${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`
 
@@ -124,6 +151,9 @@ pdb_yaml() {
   echo "$output" | tee /dev/stderr | grep "maxUnavailable and minAvailable"
 }
 
+#--------------------------------------------------------------------
+# percentage handling
+
 @test "controller/PodDisruptionBudget: supports percentage values for minAvailable" {
   cd `chart_dir`
 
@@ -141,3 +171,57 @@ pdb_yaml() {
   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" ]
+}