pegasystems / pega-helm-charts

Orchestrate a Pega Platform™ deployment by using Docker, Kubernetes, and Helm to take advantage of Pega Platform Cloud Choice flexibility.
https://community.pega.com/knowledgebase/articles/cloud-choice
Apache License 2.0
126 stars 201 forks source link

Helm 3 - Deployment strategy with Stream tier #37

Closed Emilio-Pega closed 4 years ago

Emilio-Pega commented 4 years ago
tier:
  - name: custom
    nodeType: Stream
    volumeClaimTemplate:
        resources:
          requests:
            storage: 5Gi
    deploymentStrategy:
      rollingUpdate:
        maxSurge: 25%
        maxUnavailable: 25%
      type: RollingUpdate

This fails because a Stream nodeType uses a StatefulSet under the covers which does not support a strategy element as defined here: https://github.com/pegasystems/pega-helm-charts/blob/754066f10ff423c218c1162265fb295f1eb0fed8/charts/pega/templates/_pega-deployment.tpl#L20

Helm 3 will barf with this setup.

There is no obvious documentation for this, nor is their any protection in the helm charts themselves.

Request that the addition of strategy be fixed to work with StatefulSets, by automatically ignoring -or- by doing the correct equivalent for StatefulSets (if one exists)

dcasavant commented 4 years ago

Looking at the difference between Deployment and StatefulSet update strategies, it looks like there are a couple primary differences.

Given the difference in configuration, the gaps seem to be:

  1. You cannot specify a StatefulSet updateStrategy.rollingUpdate.partition.

  2. You cannot specify a StatefulSet to have an updateStrategy.type of OnDelete.

Did I miss any gaps here? @Emilio-Pega, do you have use cases that require support for 1 or 2 above?

If yes, we can update the charts to include the appropriate subfields and correctly use updateStrategy instead of strategy when creating a StatefulSet.

If not, the doc can be updated to reflect that deploymentStrategy in values.yaml does not apply to StatefulSets (and the code block can be omitted or cause failure during helm install).

Emilio-Pega commented 4 years ago

@dcasavant I'm not sure if there is a specific use case for setting the update values for the staefuleset, my primary complaint is that based on the current values.yaml, documentation, etc its was very easy for me to mess this up and accidentally add the deplotmentStrategy to a stream node. Basically the assumption (without knowing the underlying implementation) is that a tier is a tier, so from an API perspective (api = values.yaml) I would expect this to "work" in some manner, either by ignoring it or doing the correct setup for statefulset

I would suspect the use case for specifying an update strategy would be when you are building up custom tiers, like maybe nodeType: WebUser, Stream where you would want some kind of strategy.

If that is not allowed (i.e Stream must always be isolated) that should be made clear

dcasavant commented 4 years ago

Merged