From cf76a2d2d5761015156a735c9432cc24bfb0fa3d Mon Sep 17 00:00:00 2001
From: Rebecca Zanzig <rzazzl@hotmail.com>
Date: Fri, 9 Nov 2018 16:19:50 -0800
Subject: [PATCH] Provide a valid maxUnavailable value when using a single
 replica

This adds a condition to set `maxUnavailable` to 0 when replicas=1.
Previously, the logic returned an invalid value of `-1`.

The larger issue of not being able to set something explicitly to 0
remains, as this is a limitation of the way helm templating works.
For more information, see:

Helm documentation that a numerical 0 is treated identically to null:
https://docs.helm.sh/chart_template_guide/#if-else

Github issue for this:
https://github.com/helm/helm/issues/3164
---
 templates/_helpers.tpl                 |  5 ++++-
 test/unit/server-disruptionbudget.bats | 28 +++++++++++++++++++++-----
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/templates/_helpers.tpl b/templates/_helpers.tpl
index ec9e77359b55..6ba475d31dc4 100644
--- a/templates/_helpers.tpl
+++ b/templates/_helpers.tpl
@@ -34,9 +34,12 @@ Expand the name of the chart.
 {{/*
 Compute the maximum number of unavailable replicas for the PodDisruptionBudget.
 This defaults to (n/2)-1 where n is the number of members of the server cluster.
+Add a special case for replicas=1, where it should default to 0 as well.
 */}}
 {{- define "consul.pdb.maxUnavailable" -}}
-{{- if .Values.server.disruptionBudget.maxUnavailable -}}
+{{- if eq (int .Values.server.replicas) 1 -}}
+{{ 0 }}
+{{- else if .Values.server.disruptionBudget.maxUnavailable -}}
 {{ .Values.server.disruptionBudget.maxUnavailable -}}
 {{- else -}}
 {{- ceil (sub (div (int .Values.server.replicas) 2) 1) -}}
diff --git a/test/unit/server-disruptionbudget.bats b/test/unit/server-disruptionbudget.bats
index c2f2e988f012..28e8d0d518ac 100755
--- a/test/unit/server-disruptionbudget.bats
+++ b/test/unit/server-disruptionbudget.bats
@@ -11,7 +11,7 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
-@test "server/DisruptionBudget: enable with global.enabled false" {
+@test "server/DisruptionBudget: enabled with global.enabled=false" {
   cd `chart_dir`
   local actual=$(helm template \
       -x templates/server-disruptionbudget.yaml  \
@@ -22,7 +22,7 @@ load _helpers
   [ "${actual}" = "true" ]
 }
 
-@test "server/DisruptionBudget: disable with server.enabled" {
+@test "server/DisruptionBudget: disabled with server.enabled=false" {
   cd `chart_dir`
   local actual=$(helm template \
       -x templates/server-disruptionbudget.yaml  \
@@ -32,7 +32,7 @@ load _helpers
   [ "${actual}" = "false" ]
 }
 
-@test "server/DisruptionBudget: disable with server.disruptionBudget.enabled" {
+@test "server/DisruptionBudget: disabled with server.disruptionBudget.enabled=false" {
   cd `chart_dir`
   local actual=$(helm template \
       -x templates/server-disruptionbudget.yaml  \
@@ -42,7 +42,7 @@ load _helpers
   [ "${actual}" = "false" ]
 }
 
-@test "server/DisruptionBudget: disable with global.enabled" {
+@test "server/DisruptionBudget: disabled with global.enabled=false" {
   cd `chart_dir`
   local actual=$(helm template \
       -x templates/server-disruptionbudget.yaml  \
@@ -52,7 +52,10 @@ load _helpers
   [ "${actual}" = "false" ]
 }
 
-@test "server/DisruptionBudget: correct maxUnavailable with n=3" {
+#--------------------------------------------------------------------
+# maxUnavailable
+
+@test "server/DisruptionBudget: correct maxUnavailable with replicas=3" {
   cd `chart_dir`
   local actual=$(helm template \
       -x templates/server-disruptionbudget.yaml  \
@@ -61,3 +64,18 @@ load _helpers
       yq '.spec.maxUnavailable' | tee /dev/stderr)
   [ "${actual}" = "0" ]
 }
+
+@test "server/DisruptionBudget: correct maxUnavailable with replicas=1" {
+  cd `chart_dir`
+  local actual=$(helm template \
+      -x templates/server-disruptionbudget.yaml  \
+      --set 'server.replicas=1' \
+      . | tee /dev/stderr |
+      yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "0" ]
+}
+
+# Note: It is not possible to test anything but the default behavior of the
+# maxUnavailable definition in the _helpers.tpl with the current test setup
+# because the `--set` flag overrides values AFTER they're been run through
+# the helper functions.