helm: Allow multiple installations of the Tetragon Helm chart

[ashishkurmi]

fixes #1399

[kkourt]

Thanks!

@willfindlay or @mtardy could you please have a look at this? CC: @michi-covalent

[mtardy]

I think Helm recommended to use something like {{ template "fullname" . }} that was combining the release name and the chart name. But it would look weird to have that here and would result in very weird naming.

It was something very complicated like:

{{/*
Create a default fully qualified app name.
We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
If release name contains chart name it will be used as a full name.
*/}}
{{- define "tetragon-enterprise.fullname" -}}
{{- if .Values.fullnameOverride }}
{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- $name := default .Chart.Name .Values.nameOverride }}
{{- if contains $name .Release.Name }}
{{- .Release.Name | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
{{- end }}
{{- end }}
{{- end }}

Honestly, I feel like it's simpler to do it that way, we can just end up with invalid names because of Kubernetes resources naming restriction but that would be the user shooting shooting their own foot with a nice error message so I think it's fine.

If it's annoying for some reason (because we want people to be free with their release name) we can use something like that later:

{{/*
Expand the name of the chart.
*/}}
{{- define "tetragon-enterprise.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
{{- end }}

With a dedicated .Values.nameOverride that can be used for those advanced use cases.

[mtardy]

Hey @willfindlay do you want to take a look at this?

[willfindlay]

Yep this makes good sense to me

[mtardy]

I added the release-note/minor This PR introduces a minor user-visible change tag, we might need a sentence for the changelog for this PR.

Sorry @ashishkurmi we delayed your PR because of unavailability and unfortunately, you now need to rebase on main. Could you do that?

[ashishkurmi]

@mtardy no worries, I have rebased my pull request, could you please review and merge it if everything looks good? I have also requested code review from @willfindlay as the earlier sign off had become stale.

[mtardy]

@mtardy no worries, I have rebased my pull request, could you please review and merge it if everything looks good? I have also requested code review from @willfindlay as the earlier sign off had become stale.

so did you adapt in the rebase? because a new operator thingy was pushed in the helm chart right, or modified? If yes we can merge this :)

[ashishkurmi]

@mtardy yes, I updated my PR as part of the rebase. A cluster role name was renamed as part of 84f1a37, I have updated my PR to accomodate this change.

[mtardy]

@mtardy yes, I updated my PR as part of the rebase. A cluster role name was renamed as part of 84f1a37, I have updated my PR to accomodate this change.

Fantastic thanks!