Extend helm chart to improve controller availability

chart/templates/_helpers.tpl

@@ -345,3 +345,17 @@ clientCache numLocks
 {{- end -}}
 {{- end -}}
 {{- end -}}
+
+{{/*
+topologySpreadConstraints appends the "vso.chart.selectorLabels" to .Values.controller.topologySpreadConstraints if no labelSelector was specified
+*/}}
+{{- define "vso.topologySpreadConstraints" -}}
+{{- $defaultLabelSelector := dict "labelSelector" (dict "matchLabels" (include "vso.chart.selectorLabels" . | fromYaml)) -}}
+{{- range $topologySpreadConstraint := .Values.controller.topologySpreadConstraints -}}
+{{- if hasKey $topologySpreadConstraint "labelSelector" -}}
+{{- $topologySpreadConstraint | list | toYaml -}}
+{{- else -}}
+{{- merge $topologySpreadConstraint $defaultLabelSelector | list | toYaml -}}
+{{- end -}}
+{{- end -}}
+{{- end -}}

chart/templates/deployment.yaml

@@ -141,6 +141,9 @@ spec:
         volumeMounts:
         - mountPath: /var/run/podinfo
           name: podinfo
+      {{- if .Values.controller.priorityClassName }}
+      priorityClassName: {{ .Values.controller.priorityClassName }}
+      {{- end }}
       securityContext:
         {{- toYaml .Values.controller.podSecurityContext | nindent 8 }}
       serviceAccountName: {{ include "vso.chart.fullname" . }}-controller-manager
@@ -161,6 +164,10 @@ spec:
       affinity:
       {{- toYaml .Values.controller.affinity | nindent 8 }}
       {{- end }}
+      {{- if .Values.controller.topologySpreadConstraints }}
+      topologySpreadConstraints:
+      {{- include "vso.topologySpreadConstraints" . | nindent 8 }}
+      {{- end }}
       volumes:
       - downwardAPI:
           items:
@@ -220,6 +227,9 @@ spec:
         securityContext:
         {{- toYaml .| nindent 10 }}
         {{- end}}
+      {{- if .Values.controller.priorityClassName }}
+      priorityClassName: {{ .Values.controller.priorityClassName }}
+      {{- end }}
       restartPolicy: Never
       {{- with .Values.controller.nodeSelector }}
       nodeSelector:

chart/templates/hook-upgrade-crds.yaml

@@ -106,6 +106,9 @@ spec:
         securityContext:
         {{- toYaml .| nindent 10 }}
         {{- end}}
+      {{- if .Values.controller.priorityClassName }}
+      priorityClassName: {{ .Values.controller.priorityClassName }}
+      {{- end }}
       restartPolicy: Never
       {{- with .Values.controller.nodeSelector }}
       nodeSelector:

chart/templates/poddisruptionbudget.yaml

@@ -0,0 +1,46 @@
+{{/*
+# SPDX-License-Identifier: BUSL-1.1
+*/}}
+
+{{- if and (gt (int .Values.controller.replicas) 1) .Values.controller.podDisruptionBudget.enabled }}
+apiVersion: policy/v1
+kind: PodDisruptionBudget
+metadata:
+  name: {{ include "vso.chart.fullname" . }}
+  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 }}
+
+  {{/* If maxUnavailable is set, use it */}}
+  {{- if not (empty $maxUnavailable) }}
+  maxUnavailable: 
+    {{- if contains "%" $maxUnavailable }}
+    "{{ $maxUnavailable }}"
+    {{- else }}
+    {{ $maxUnavailable }}
+    {{- end }}
+  {{- end }}
+
+  {{/* If minAvailable is set, use it */}}
+  {{- if not (empty $minAvailable) }}
+  minAvailable: 
+    {{- if contains "%" $minAvailable }}
+    "{{ $minAvailable }}"
+    {{- else }}
+    {{ $minAvailable }}
+    {{- end }}
+  {{- end }}
+
+  selector:
+    matchLabels:
+      {{- include "vso.chart.selectorLabels" . | nindent 6 }}
+{{- end }}

chart/values.yaml

@@ -8,6 +8,10 @@ controller:
   # @type: integer
   replicas: 1
 
+  # Set the priority class for the operator.
+  # @type: string
+  priorityClassName: ""
+
   # Configure update strategy for multi-replica deployments.
   # Kubernetes supports types Recreate, and RollingUpdate
   # ref: https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy
@@ -66,6 +70,36 @@ controller:
   #           - antarctica-west1
   affinity: {}
 
+  # TopologySpreadConstraints settings for vault-secrets-operator pod.
+  # The value is an array of PodSpec TopologySpreadConstraint maps.
+  # A labelSelector for the pods will be added automatically to the template in case it is not set.
+  # ref: https://kubernetes.io/docs/concepts/scheduling-eviction/topology-spread-constraints/
+  # Example:
+  # topologySpreadConstraints:
+  # - maxSkew: 1
+  #   topologyKey: zone
+  #   whenUnsatisfiable: DoNotSchedule
+  topologySpreadConstraints: []
+
+  # Configure the PodDisruptionBudget for the controller deployment.
+  podDisruptionBudget:
+
+    # toggles the deployment of the PodDisruptionBudget for the controller.
+    # @type: boolean
+    enabled: false
+
+    # Sets the maximum number of pods that can be unavailable during the eviction.
+    # This field cannot be set if minAvailable is set.
+    # Can be set as an integer (e.g. "2") or a percentage (e.g. "50%").
+    # @type: string
+    maxUnavailable: "0"
+
+    # Sets the number of pods that must be available during the eviction.
+    # This field cannot be set if maxUnavailable is set.
+    # Can be set as an integer (e.g. "2") or a percentage (e.g. "50%").
+    # @type: string
+    minAvailable: "0"
+
   rbac:
     # clusterRoleAggregation defines the roles included in the aggregated ClusterRole.
     clusterRoleAggregation:

test/unit/deployment.bats

@@ -1350,3 +1350,162 @@ load _helpers
   actual=$(echo "$object" | yq 'contains(["--kube-client-burst=2000"])' | tee /dev/stderr)
   [ "${actual}" = "true" ]
 }
+
+#--------------------------------------------------------------------
+# podDisruptionBudget
+
+@test "controller/PodDisruptionBudget: not created by default" {
+  cd `chart_dir`
+  local actual=$(helm template \
+    -s templates/poddisruptionbudget.yaml \
+    . | tee /dev/stderr |
+    yq 'select(.kind == "PodDisruptionBudget")' | tee /dev/stderr)
+  [ "${actual}" = "" ]
+}
+
+@test "controller/PodDisruptionBudget: created when replicas > 1 and enabled" {
+  cd `chart_dir`
+  local actual=$(helm template \
+    -s templates/poddisruptionbudget.yaml \
+    --set 'controller.replicas=3' \
+    --set 'controller.podDisruptionBudget.enabled=true' \
+    . | tee /dev/stderr |
+    yq 'select(.kind == "PodDisruptionBudget")' | tee /dev/stderr)
+  [ "${actual}" != "" ]
+}
+
+@test "controller/PodDisruptionBudget: maxUnavailable can be set to an integer" {
+  cd `chart_dir`
+  local actual=$(helm template \
+    -s templates/poddisruptionbudget.yaml \
+    --set 'controller.replicas=3' \
+    --set 'controller.podDisruptionBudget.enabled=true' \
+    --set 'controller.podDisruptionBudget.maxUnavailable=2' \
+    . | tee /dev/stderr |
+    yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "2" ]
+}
+
+@test "controller/PodDisruptionBudget: maxUnavailable can be set as percentage" {
+  cd `chart_dir`
+  local actual=$(helm template \
+    -s templates/poddisruptionbudget.yaml \
+    --set 'controller.replicas=3' \
+    --set 'controller.podDisruptionBudget.enabled=true' \
+    --set 'controller.podDisruptionBudget.maxUnavailable=50%' \
+    . | tee /dev/stderr |
+    yq '.spec.maxUnavailable' | tee /dev/stderr)
+  [ "${actual}" = "50%" ]
+}
+
+@test "controller/PodDisruptionBudget: minAvailable can be set to an integer" {
+  cd `chart_dir`
+  local actual=$(helm template \
+    -s templates/poddisruptionbudget.yaml \
+    --set 'controller.replicas=3' \
+    --set 'controller.podDisruptionBudget.enabled=true' \
+    --set 'controller.podDisruptionBudget.minAvailable=2' \
+    . | tee /dev/stderr |
+    yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "2" ]
+}
+
+@test "controller/PodDisruptionBudget: minAvailable can be set as a percentage" {
+  cd `chart_dir`
+  local actual=$(helm template \
+    -s templates/poddisruptionbudget.yaml \
+    --set 'controller.replicas=3' \
+    --set 'controller.podDisruptionBudget.enabled=true' \
+    --set 'controller.podDisruptionBudget.minAvailable=50%' \
+    . | tee /dev/stderr |
+    yq '.spec.minAvailable' | tee /dev/stderr)
+  [ "${actual}" = "50%" ]
+}
+
+@test "controller/PodDisruptionBudget: maxUnavailable and minAvailable cannot be set together" {
+  cd `chart_dir`
+  run helm template \
+    -s templates/poddisruptionbudget.yaml \
+    --set 'controller.replicas=3' \
+    --set 'controller.podDisruptionBudget.enabled=true' \
+    --set 'controller.podDisruptionBudget.maxUnavailable=2' \
+    --set 'controller.podDisruptionBudget.minAvailable=2' \
+    .
+  [ "$status" -eq 1 ]
+}
+
+#--------------------------------------------------------------------
+# priorityClassName
+
+@test "controller/Deployment: priorityClassName not set by default" {
+  cd `chart_dir`
+  local object
+  object=$(helm template \
+  -s templates/deployment.yaml  \
+  . | tee /dev/stderr |
+  yq 'select(.kind == "Deployment" and .metadata.labels."control-plane" == "controller-manager") | .spec.template.spec.priorityClassName' | tee /dev/stderr)
+
+  [ "${object}" = "null" ]
+}
+
+@test "controller/Deployment: priorityClassName can be set" {
+  cd `chart_dir`
+  local object
+  object=$(helm template \
+  -s templates/deployment.yaml  \
+  --set 'controller.priorityClassName=high-priority' \
+  . | tee /dev/stderr |
+  yq 'select(.kind == "Deployment" and .metadata.labels."control-plane" == "controller-manager") | .spec.template.spec.priorityClassName' | tee /dev/stderr)
+
+  [ "${object}" = "high-priority" ]
+}
+
+@test "controller/Deployment: priorityClassName applied to all Jobs" {
+  cd `chart_dir`
+  local objects
+  objects=$(helm template \
+  -s templates/deployment.yaml  \
+  --set 'controller.priorityClassName=high-priority' \
+  . | tee /dev/stderr |
+  yq 'select(.kind == "Job") | .spec.template.spec.priorityClassName' | tee /dev/stderr)
+
+  for object in $objects; do
+    [ "${object}" = "high-priority" ]
+  done
+}
+
+#--------------------------------------------------------------------
+# topologySpreadConstraints
+
+@test "controller/Deployment: topologySpreadConstraints not set by default" {
+  cd `chart_dir`
+  local object
+  object=$(helm template \
+    -s templates/deployment.yaml \
+    . | tee /dev/stderr |
+    yq '.spec.template.spec.topologySpreadConstraints | select(documentIndex == 1)' | tee /dev/stderr)
+
+  [ "${object}" = null ]
+}
+
+@test "controller/Deployment: single topologySpreadConstraint can be set" {
+  cd `chart_dir`
+  local object
+  object=$(helm template \
+    -s templates/deployment.yaml \
+    --set "controller.topologySpreadConstraints[0].maxSkew=1" \
+    --set "controller.topologySpreadConstraints[0].topologyKey=zone" \
+    --set "controller.topologySpreadConstraints[0].whenUnsatisfiable=DoNotSchedule" \
+    . | tee /dev/stderr |
+    yq '.spec.template.spec.topologySpreadConstraints | select(documentIndex == 1)' | tee /dev/stderr)
+
+  local actual
+  actual=$(echo "$object" | yq '. | length' | tee /dev/stderr)
+  [ "${actual}" = "1" ]
+  actual=$(echo "$object" | yq '.[0].maxSkew' | tee /dev/stderr)
+  [ "${actual}" = "1" ]
+  actual=$(echo "$object" | yq '.[0].topologyKey' | tee /dev/stderr)
+  [ "${actual}" = "zone" ]
+  actual=$(echo "$object" | yq '.[0].whenUnsatisfiable' | tee /dev/stderr)
+  [ "${actual}" = "DoNotSchedule" ]
+}