pingidentity / helm-charts

Apache License 2.0
22 stars 31 forks source link

Don't Set Deployment Replicas When Autoscaling #276

Closed StokesM closed 7 months ago

StokesM commented 9 months ago

The deployment template always sets .spec.replicas which the charts values defaults to 1:

spec:
  replicas: {{ $v.container.replicaCount }}
  selector:
    matchLabels: {{ include "pinglib.selector.labels" . | nindent 6 }}

Replicas should not be set on a deployment when a HPA is managing this field. If replicas is provided, during helm upgrade time the underlying Replica Set is scaled to this value during the deployment breaking any expected deployment upgrade strategy.

For example, if:

During a helm upgrade that triggers a deployment upgrade the specified strategy will not be followed, the number of replicas will be immediately dropped to 1 (the default for replicaCount) instead of starting 1 surge container and swapping pods out 1 by 1.

The code above in _workload.tpl needs to read:

spec:
  {{- if not $v.clustering.autoscaling.enabled }}
  replicas: {{ $v.container.replicaCount }}
  {{- end }}
  selector:
    matchLabels: {{ include "pinglib.selector.labels" . | nindent 6 }}

For this to work properly.

For reference on the fact that replicas should not be set when it is being managed elsewhere (HPA) see: https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#replicas

henryrecker-pingidentity commented 9 months ago

This will be included in the next release.