Enhance Helm Chart with Consistent Naming and Labeling Conventions #47
Conversation
| @@ -36,7 +36,6 @@ Create chart name and version as used by the chart label. | |||
| Common labels | |||
| */}} | |||
| {{- define "k8s-event-logger.labels" -}} | |||
| app.kubernetes.io/name: {{ include "k8s-event-logger.name" . }} | |||
There was a problem hiding this comment.
Why isn't app.kubernetes.io/name just part of this definition? Is there anywhere it's used separately?
There was a problem hiding this comment.
it very well could be - seems most charts I've been working with leave the name out of the template for "reasons" unknown to me, for this purpose I originally had it as part of the label helper
There was a problem hiding this comment.
Looks like a default from helm (helm create ) does this:
Expand the name of the chart.
*/}}
{{- define "k8s-event-logger.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
{{- end }}
{{/*
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 "k8s-event-logger.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 }}
{{/*
Create chart name and version as used by the chart label.
*/}}
{{- define "k8s-event-logger.chart" -}}
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }}
{{- end }}
{{/*
Common labels
*/}}
{{- define "k8s-event-logger.labels" -}}
helm.sh/chart: {{ include "k8s-event-logger.chart" . }}
{{ include "k8s-event-logger.selectorLabels" . }}
{{- if .Chart.AppVersion }}
app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
{{- end }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- end }}
{{/*
Selector labels
*/}}
{{- define "k8s-event-logger.selectorLabels" -}}
app.kubernetes.io/name: {{ include "k8s-event-logger.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
{{- end }}
{{/*
Create the name of the service account to use
*/}}
{{- define "k8s-event-logger.serviceAccountName" -}}
{{- if .Values.serviceAccount.create }}
{{- default (include "k8s-event-logger.fullname" .) .Values.serviceAccount.name }}
{{- else }}
{{- default "default" .Values.serviceAccount.name }}
{{- end }}
{{- end }}```
There was a problem hiding this comment.
So a difference of .labels and .selectorLabels, it seems, but the .lables includes all the selectorLabels and then just adds the chart metadata.
There was a problem hiding this comment.
And then the deployment and or services use the selectorLables as part of their "selector" key in the spec, but the "labels" are used int he "metadata" section.
There was a problem hiding this comment.
Anywho.... I have no strong opinions, it just caught my eye.
There was a problem hiding this comment.
I like it - we use the chart at work, in our case we need to modify the label pre-deployment so our overlay chart we put our own help for app.labels but we're not able to do that with this chart so here we are. I've forked it accordingly and made the change, thought I'd share - cheers
This should produce the same output as the current method. Only diff is the version: label
diff original.yaml new.yaml
22a23
> app.kubernetes.io/version: "2.2"
37a39
> app.kubernetes.io/version: "2.2"
This pull request introduces several improvements to the Helm chart to ensure consistent naming and labeling across all Kubernetes resources. The changes include the addition of helper templates and the application of these templates in various resource definitions.