rstudio / helm

Helm Resources for RStudio Products
MIT License
35 stars 28 forks source link

Inconsistent behavior across charts #424

Closed Almenon closed 5 months ago

Almenon commented 1 year ago

The posit connect chart specifies the following:

    {{- if eq .Values.strategy.type "RollingUpdate" }}
    rollingUpdate:
      maxUnavailable: {{ .Values.strategy.rollingUpdate.maxUnavailable }}
      maxSurge: {{ .Values.strategy.rollingUpdate.maxSurge }}
    {{- end }}

The PM chart just has:

  {{- with .Values.strategy }}
  strategy:
    {{- toYaml . | nindent 4 }}
  {{- end }}

It would be nice if the templating was consistent, as this tripped me up when working with the charts. I thought just specifying strategy.type: RollingUpdate was enough because it worked for the connect chart, but in the PM chart you also need to specify rollingUpdate: null under strategy.

colearendt commented 1 year ago

Thanks for reporting @Almenon ! To be clear: do you think that the Connect chart's approach is preferable?

Almenon commented 1 year ago

I don't have much of a preference, but if you want me to pick one as the 'best' I'll pick the PM chart as it is more flexible. But up to you :)

Actually, yes, connect is preferable