bjw-s / helm-charts

A collection of Helm charts
https://bjw-s.github.io/helm-charts/
Apache License 2.0
522 stars 98 forks source link

Template version conflicts #329

Open LarryGF opened 3 weeks ago

LarryGF commented 3 weeks ago

Details

Describe the solution you'd like:

Let's assume a test scenario:

When deploying the umbrella chart Helm will load the templates for both child charts and when template names collide, it will use the template that was loaded last as mentioned here. Best case scenario: it will render a manifest that won't have the desired features, worst case scenario: it will prevent manifest generation entirely. In both cases it's a really tricky thing to troubleshoot and will cause unexpected behaviour.

This is an issue with how Helm handles templates and it's especially bothersome when using library charts. There's no way of isolating templates or assigning priority to which template gets loaded last, and from comments I've seen there are no plans of supporting this in the near future.

Anything else you would like to add:

It seems like the community approaches this problem in one of two ways:

The only issue I see with the second approach is that if templates are updated manually and independently, this will cause versions to slip out of date, and will be a chore to keep up.

Additional Information:

I think there's a nice way of solving this: Since the only thing needed to load the common library is to have a template including "bjw-s.common.loader.init" and "bjw-s.common.loader.generate" we can use "bjw-s.common.loader.all template, leave that template as unversioned to avoid forcing the users to update their chart's templates and suffixing the chart's version into the other templates` names. It would look something like this:

...

{{- include (printf "bjw-s.common.loader.init.%s" .Chart.Version) . }}

{{/ Render the templates /}} {{ include (printf "bjw-s.common.loader.generate.%s" .Chart.Version) . }}

...

{{- end -}}


```yaml
# templates/loader/_init.tpl
{{- define (printf "bjw-s.common.loader.init.%s" .Chart.Version) -}}
  {{- /* Merge the local chart values and the common chart defaults */ -}}
  {{- include (printf "bjw-s.common.values.init.%s" .Chart.Version)  . }}
{{- end -}}
# templates/loader/_generate.tpl
{{- define (printf "bjw-s.common.loader.generate.%s" .Chart.Version) -}}
  {{- /* Run global chart validations */ -}}
  {{- include (printf "bjw-s.common.lib.chart.validate.%s" .Chart.Version)  . -}}

  {{- /* Build the templates */ -}}
  {{- include (printf "bjw-s.common.render.pvcs.%s" .Chart.Version) . | nindent 0 -}}
  {{- include (printf "bjw-s.common.render.serviceAccount.%s" .Chart.Version) . | nindent 0 -}}
  {{- include (printf "bjw-s.common.render.controllers.%s" .Chart.Version) . | nindent 0 -}}

...

{{- end -}}

The downside of this is that the replacement will need to happen for all templates except for "bjw-s.common.loader.all", the upside is that it will only need to happen once.

If this is something that looks interesting to you I'll be more that happy to submit a PR with these changes

LarryGF commented 2 weeks ago

Actually, doing it like this wouldn't be feasible, since the define "name" must be strings and can't have () or $. So maybe a github action that replaces the version in the names? It would still work for the includes, those can be templated.