Allow additional annotations for standby and active services via config #896
Conversation
|
|
|
This is a must have feature for Vault HELM chart to set difference annotations for individual services, how long we can see this appoved? |
tekicat
force-pushed
the
allow_config_for_standby_and_active_services
branch
from
e6527d0
to
663a24e
Compare
There was a problem hiding this comment.
Hi @tekicat -
Thanks for submitting this, I just got around to reviewing. I think the approach looks good, and is a nice+clean iteration of the original PR.
I left a few suggestions about missing tests, and some general cleanup, and after these get addressed I think we're good to go!
Let me know if you have any questions about any the feedback.
~Kyle
| {{/* | ||
| Sets extra vault server Service (active) annotations | ||
| */}} | ||
| {{- define "vault.service.active.annotations" -}} | ||
| {{- if .Values.server.service.active.annotations }} | ||
| {{- $tp := typeOf .Values.server.service.active.annotations }} | ||
| {{- if eq $tp "string" }} | ||
| {{- tpl .Values.server.service.active.annotations . | nindent 4 }} | ||
| {{- else }} | ||
| {{- toYaml .Values.server.service.active.annotations | nindent 4 }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- end -}} | ||
| {{/* | ||
| Sets extra vault server Service annotations | ||
| */}} | ||
| {{- define "vault.service.standby.annotations" -}} | ||
| {{- if .Values.server.service.standby.annotations }} | ||
| {{- $tp := typeOf .Values.server.service.standby.annotations }} | ||
| {{- if eq $tp "string" }} | ||
| {{- tpl .Values.server.service.standby.annotations . | nindent 4 }} | ||
| {{- else }} | ||
| {{- toYaml .Values.server.service.standby.annotations | nindent 4 }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- end -}} | ||
|
|
There was a problem hiding this comment.
I was going to suggest that we dry this up a bit and make it a single function where we just pass in .Values.server.service.active/standby as the root, but I ran into helm/helm#5979 trying to implement it myself, so I think it's okay as is.
Co-authored-by: Kyle Schochenmaier <[email protected]>
|
Thanks for the suggestions @kschoche , they looked good and I have applied them. :) |
Revival of #546
Currently there is no ability to annotate services for active and standby services, as they inherit their settings from the main server service configuration. This is a problem, when you need to attach different annotations to the different services. Instead of just allowing for setting of annotations, I opted to allow configuring the services for more flexibility.
Helps to close #674
Partially resolves #588
Thanks @jamesgoodhouse