search

SAP / sap-btp-service-operator

SAP BTP service operator enables developers to connect Kubernetes clusters to SAP BTP accounts and to consume SAP BTP services within the clusters by using Kubernetes native tools.
Apache License 2.0
125 stars 49 forks source link

bug in helm chart + add support for pod affinity #459

Closed yyardeng closed 2 weeks ago

yyardeng commented 3 weeks ago

version 0.6.5 of the operator helm chart has a bug in the deployment template where the variable checked in the if condition for the node selector and tolerations is not the value that is used in the template:

      {{- if .Values.manager.nodeSelector }}
      nodeSelector: {{ toYaml .Values.deployment.nodeSelector | nindent 8 }}
      {{- end }}
      {{- if .Values.manager.tolerations }}
      tolerations: {{ toYaml .Values.deployment.tolerations | nindent 8 }}
      {{- end }}

which means the values have to be set twice in the values file for the configuration to take effect.

in addition, it would be great if support for configuring affinity in the deployment could be added. perhaps something like:

      {{- with .Values.manager.affinity }}
      affinity:
        {{- toYaml . | nindent 8 }}
      {{- end }}

adding support for topologySpreadConstraints instead or in addition to affinity would be helpful as well.

I065450 commented 3 weeks ago

Hi @yyardeng

Please review PR: https://github.com/SAP/sap-btp-service-operator/pull/460 if it meets your request.

Regards, Naama

yyardeng commented 3 weeks ago

@I065450 yes, thank you - it would be great to get support for topology spread constraints as well since that feature is better for high availability purposes (GA since 1.19, in case this is relevant). it would be something like:

      {{- with .Values.manager.topologySpreadConstraints }}
      topologySpreadConstraints:
        {{- toYaml . | nindent 8 }}
      {{- end }}
I065450 commented 2 weeks ago

fixed in PR: https://github.com/SAP/sap-btp-service-operator/pull/460