gadsme / charts

Gadsme Helm chart repository
Apache License 2.0
44 stars 25 forks source link

cubestore router.service template has bad if condition for tolerations section #20

Closed pdaBlueVoyant closed 1 year ago

pdaBlueVoyant commented 1 year ago

Chart version 0.8.0 (cubestore)

I think this is a simple typo rather than a desired behavior. We are trying to add "tolerations" to both the router and workers. The workers helm chart correctly adds our tolerations YAML. The router chart ignores the value (same value via copy/paste). I think the problem is here at line 145 in templates/router.statefulset.yaml where the if .Values.router.spreadConstraints is misplaced. We would like to continue using this helm chart you have created (thank you very much) but have created a local copy for now to test/debug further. Once this change is working, I can work to package up a MR/change if you would like and approve of the fix.

This block 145 {{- if .Values.router.spreadConstraints }} 146 {{- if .Values.router.tolerations }} 147 tolerations: 148 {{- toYaml .Values.router.tolerations | nindent 8 }} 149 {{- end }} 150 {{- if .Values.router.nodeSelector }} 151 nodeSelector: 152 {{- toYaml .Values.router.nodeSelector | nindent 8 }} 153 {{- end }} 154 topologySpreadConstraints: 155 {{- toYaml .Values.router.spreadConstraints | nindent 8 }} 156 {{- end }} should really be {{- if .Values.router.tolerations }} tolerations: {{- toYaml .Values.router.tolerations | nindent 8 }} {{- end }} {{- if .Values.router.nodeSelector }} nodeSelector: {{- toYaml .Values.router.nodeSelector | nindent 8 }} {{- end }} {{- if .Values.router.spreadConstraints }} topologySpreadConstraints: {{- toYaml .Values.router.spreadConstraints | nindent 8 }} {{- end }}

lvauvillier commented 1 year ago

Good catch @pdaBlueVoyant! You are right this is a copy/paste mistake.

Now fixed