feat: add pdb for sbx/dev/test envs + logic (#109)

* feat: add pdb for sbx/dev/test envs + logic * fix: pdb on replicas 1 --------- Co-authored-by: Alexander Brevig <[email protected]>
entur · Jun 29, 2023 · b4b246a · b4b246a
1 parent 320b80a
commit b4b246a
Show file tree

Hide file tree

Showing 4 changed files with 110 additions and 44 deletions.
diff --git a/charts/common/README.md b/charts/common/README.md
@@ -27,7 +27,7 @@ A Helm chart for Entur's Kubernetes workloads
 | container.cpuLimit | float | `5 x cpu` | Set CPU limit without any unit. 100m is 0.1 |
 | container.env | list | `[]` | Specify `env` entries for your container |
 | container.envFrom | list | `[]` | Attach secrets and configmaps to your `env` |
-| container.forceReplicas | int | `nil` | Force replicas disables autoscaling, if set to 1 it will use Recreate strategy |
+| container.forceReplicas | int | `nil` | Force replicas disables autoscaling and PDB, if set to 1 it will use Recreate strategy |
 | container.labels | object | `{}` | Add labels to your pods |
 | container.lifecycle | object | `{}` | Set pod lifecycle handlers |
 | container.maxReplicas | int | `nil` | Set the maxReplicas for your HPA |
@@ -55,7 +55,7 @@ A Helm chart for Entur's Kubernetes workloads
 | container.prometheus.enabled | bool | `false` | Enable or disable Prometheus |
 | container.prometheus.path | string | /actuator/prometheus | Set the path for scraping metrics |
 | container.prometheus.port | int | service.internalPort | Set the port for prometheus scraping |
-| container.replicas | int | 1 | Set the target replica count |
+| container.replicas | int | 1 | Set the target replica count, if equal to 1 the PDB minAvailable will be set to 100% |
 | container.terminationGracePeriodSeconds | int | `nil` | Override pod terminationGracePeriodSeconds (default 30s). |
 | container.uid | int | 1000 | Set the uid that your user runs with |
 | container.volumeMounts | list | `[]` | Configure volume mounts, accepts kubernetes syntax |
@@ -73,7 +73,7 @@ A Helm chart for Entur's Kubernetes workloads
 | cron.terminationGracePeriodSeconds | int | false | Override pod terminationGracePeriodSeconds (default 30s). |
 | cron.volumes | list | `[]` | Configure volume, accepts kubernetes syntax |
 | deployment.enabled | bool | `true` | Enable or disable the deployment |
-| deployment.forceReplicas | int | `nil` | Force replicas disables autoscaling, if set to 1 it will use Recreate strategy |
+| deployment.forceReplicas | int | `nil` | Force replicas disables autoscaling and PDB, if set to 1 it will use Recreate strategy |
 | deployment.labels | object | `{}` | Add labels to your pods |
 | deployment.maxReplicas | string | 10 | Set the max replica count |
 | deployment.maxSurge | string | 25% | Limit max surge for rolling updates (default 25%). Not in use when using forceReplicas. |
@@ -93,7 +93,7 @@ A Helm chart for Entur's Kubernetes workloads
 | ingress.trafficType | string | `nil` | Set the traffic type, typically `api` or `public` |
 | ingresses | list | `[]` | Specify a list of `ingress` specs |
 | labels | object | `{ app shortname team common:version environment }` | Specify additional labels for every resource |
-| pdb.minAvailable | string | 50% | Set minimum available % |
+| pdb.minAvailable | string | 50% | Set minimum available %, this overrides pdb setting minAvailable in deployment/container |
 | postgres.connectionConfig | string | `nil` | Override name for connection configmap. This must at least contain `INSTANCES`. |
 | postgres.cpu | float | 0.05 | Configure cpu request for proxy |
 | postgres.cpuLimit | float | `nil` | Configure optional cpu limit for proxy |

diff --git a/charts/common/templates/pdb.yaml b/charts/common/templates/pdb.yaml
@@ -1,30 +1,30 @@
 {{- /* Rules */}}
-{{- $env := .Values.env | required ".Values.common.env is required." -}}
 {{- $releaseName := include "name" . -}}
-{{- $minAvailable := .Values.deployment.minAvailable | default .Values.container.minAvailable }}
-{{- $forceReplicas := .Values.deployment.forceReplicas | default .Values.container.forceReplicas }}
-{{- $replicas := .Values.deployment.replicas | default .Values.container.replicas }}
-
-{{- if (and (not (eq (int $forceReplicas) 1)) (or (eq "prd" .Values.env) $minAvailable) )}}
-  {{- if (and (ne "prd" .Values.env) (eq 1 (int $replicas))) }}
-    {{ $checkReplicas := .Values.error | required ".Values.common.container.replicas must be greater than 1 when using minAvailable" }}
-  {{- end }}
-
+{{- $releaseNamespace := .Release.Namespace -}}
+{{- $forceReplicas := .Values.deployment.forceReplicas | default .Values.container.forceReplicas -}}
+{{- $minAvailable := .Values.deployment.minAvailable | default .Values.container.minAvailable -}}
+{{- $minAvailablePDB := .Values.pdb.minAvailable -}}
+{{- $replicas := .Values.deployment.replicas | default .Values.container.replicas -}}
 {{- /* YAML Spec */}}
 apiVersion: policy/v1
 kind: PodDisruptionBudget
 metadata:
   name: {{ $releaseName }}
-  namespace: {{ .Release.Namespace }}
+  namespace: {{ $releaseNamespace }}
   labels:
     {{- include "labels" . | indent 4 }}
 spec:
-  {{- if ((.Values.pdb).minAvailable) }}
-  minAvailable: {{ .Values.pdb.minAvailable }}
+  {{- if (or (eq (int $replicas) 1) (eq (int $forceReplicas) 1)) }}
+  {{- /* We set PDB even if forceReplicas or replicas = 1 */}}
+  {{- /* This is because helm is not able to delete unknown-previous config. */}}
+  {{- /* In this case we set the minAvailable to 0% so it behaves the same way as a PDB does not exist. */}}
+  minAvailable: 0%
+  {{- else if ($minAvailablePDB) }}
+  {{- /* PDB.minAvailable takes precedence over deployment/container.minAvailable */}}
+  minAvailable: {{ $minAvailablePDB }}
   {{- else }}
   minAvailable: {{ $minAvailable | default "50%" }}
   {{- end }}
   selector:
     matchLabels:
       app: {{ $releaseName }}
-{{- end }}
diff --git a/charts/common/tests/pdb_test.yaml b/charts/common/tests/pdb_test.yaml
@@ -7,6 +7,7 @@ values: &values
     trafficType: public
   container:
     image: img
+    replicas: 2
 
 suite: test pdb
 templates:
@@ -17,100 +18,154 @@ tests:
       <<: *values
       env: dev
       container:
-        image: img
-        replicas: 2
         minAvailable: 50%
     asserts:
       - isNotEmpty:
           template: pdb.yaml
           path: metadata.labels
-  - it: must default for prd
+  - it: must default for prd with 2 replicas or more
     set:
       <<: *values
       env: prd
     asserts:
       - equal:
           path: spec.minAvailable
           value: "50%"
+  - it: must default for tst
+    set:
+      <<: *values
+      env: tst
+    asserts:
+      - equal:
+          path: spec.minAvailable
+          value: "50%"
+  - it: must default for dev
+    set:
+      <<: *values
+      env: dev
+    asserts:
+      - equal:
+          path: spec.minAvailable
+          value: "50%"
   - it: use minAvailable from container if not set on pdb
     set:
       <<: *values
       env: prd
       container:
-        minAvailable: 25%
+        replicas: 2
+        minAvailable: 27%
     asserts:
       - equal:
           path: spec.minAvailable
-          value: "25%"
+          value: "27%"
   - it: use minAvailable from deployment if not set on pdb or container
     set:
       <<: *values
       env: prd
       deployment:
-        minAvailable: 25%
-      container: {}
+        replicas: 2
+        minAvailable: 26%
       containers:
         - image: app
     asserts:
       - equal:
           path: spec.minAvailable
-          value: "25%"
-  - it: check for replicas on deployment before container
+          value: "26%"
+  - it: check for minAvailable on deployment before container
     set:
       <<: *values
       env: tst
       deployment:
-        minAvailable: 25%
         replicas: 2
+        minAvailable: 30%
       container:
-        replicas: 1
+        replicas: 2
+        minAvailable: 50%
       containers:
         - image: app
     asserts:
       - equal:
           path: spec.minAvailable
-          value: "25%"
+          value: "30%"
   - it: use minAvailable from pdb if not set on pdb or container
     set:
       <<: *values
       env: prd
       pdb:
         minAvailable: 25%
-      container: {}
+      container:
+        replicas: 2
       containers:
         - image: app
     asserts:
       - equal:
           path: spec.minAvailable
           value: "25%"
-  - it: must not use pdb if forceReplicas is set to 1
+  - it: if container Replicas is set to 1, minAvailable must be 0%
+    set:
+      <<: *values
+      env: prd
+      container:
+        image: some
+        replicas: 1
+    asserts:
+      - equal:
+          path: spec.minAvailable
+          value: "0%"
+  - it: if deployment Replicas is set to 1, minAvailable must be 0%
+    set:
+      <<: *values
+      env: prd
+      deployment:
+        replicas: 1
+      container:
+        image: some
+    asserts:
+      - equal:
+          path: spec.minAvailable
+          value: "0%"
+  - it: if container forceReplicas is set to 1, minAvailable must be 0%
     set:
       <<: *values
       env: prd
       container:
         image: some
         forceReplicas: 1
     asserts:
-      - hasDocuments:
-          count: 0
-  - it: must not use pdb if forceReplicas is set to 1 on deployment
+      - equal:
+          path: spec.minAvailable
+          value: "0%"
+  - it: if deployment forceReplicas is set to 1, minAvailable must be 0%
     set:
       <<: *values
       env: prd
       deployment:
         forceReplicas: 1
       containers:
         - image: some
+    asserts:
+      - equal:
+          path: spec.minAvailable
+          value: "0%"
+  - it: must use pdb if forceReplicas is set to more than 1
+    set:
+      <<: *values
+      env: prd
+      container:
+        image: some
+        forceReplicas: 2
+        replicas: 2
     asserts:
       - hasDocuments:
-          count: 0
+          count: 1
   - it: must use pdb if forceReplicas is set to more than 1
     set:
       <<: *values
       env: prd
       container:
         image: some
         forceReplicas: 2
+        replicas: 2
     asserts:
       - hasDocuments:
           count: 1
@@ -125,6 +180,17 @@ tests:
     asserts:
       - hasDocuments:
           count: 1
+  - it: must use pdb if forceReplicas is set to 1 on deployment
+    set:
+      <<: *values
+      env: prd
+      deployment:
+        forceReplicas: 1
+      containers:
+        - image: some
+    asserts:
+      - hasDocuments:
+          count: 1
   - it: can override release name
     set:
       <<: *values

diff --git a/charts/common/values.yaml b/charts/common/values.yaml
@@ -45,11 +45,11 @@ deployment:
   #prometheus: same as container.prometheus stanza
   # -- Set the target replica count
   # @default -- container.replicas
-  replicas: 
+  replicas:
   # -- Set the max replica count
   # @default -- 10
-  maxReplicas: 
-  # -- (int) Force replicas disables autoscaling, if set to 1 it will use Recreate strategy
+  maxReplicas:
+  # -- (int) Force replicas disables autoscaling and PDB, if set to 1 it will use Recreate strategy
   forceReplicas:
   # -- (int) Override pod terminationGracePeriodSeconds (default 30s).
   terminationGracePeriodSeconds:
@@ -104,7 +104,7 @@ hpa:
     #behaviour: ...
 
 pdb:
-  # -- (string) Set minimum available %
+  # -- (string) Set minimum available %, this overrides pdb setting minAvailable in deployment/container
   # @default -- 50%
   minAvailable:
 
@@ -148,10 +148,10 @@ container:
   # -- Set the uid that your user runs with
   # @default -- 1000
   uid: 1000
-  # -- Set the target replica count
+  # -- Set the target replica count, if equal to 1 the PDB minAvailable will be set to 100%
   # @default -- 1
   replicas: 1
-  # -- (int) Force replicas disables autoscaling, if set to 1 it will use Recreate strategy
+  # -- (int) Force replicas disables autoscaling and PDB, if set to 1 it will use Recreate strategy
   forceReplicas:
   # -- (string) Set the minimal available replicas, used by PDB
   # @default -- 50%
@@ -176,7 +176,7 @@ container:
     path: "/actuator/prometheus"
     # -- (int) Set the port for prometheus scraping
     # @default -- service.internalPort
-    port: 
+    port:
   probes:
     # -- Enable or disable probes
     enabled: true
@@ -257,7 +257,7 @@ postgres:
   # @default -- 16
   memoryLimit: 16
   # -- Override name for connection configmap. This must at least contain `INSTANCES`.
-  connectionConfig: 
+  connectionConfig:
   # -- Override name for credentials secret. This must at least contain `PGUSER` and `PGPASSWORD`.
   credentialsSecret: