Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new standard labels and allow custom labels #45

Merged
merged 1 commit into from
Sep 28, 2023
Merged

Add new standard labels and allow custom labels #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions charts/athens-proxy/ci/basic-values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ ingress:
paths:
- path: /
pathType: ImplementationSpecific
jaeger:
enabled: true
extraLabels:
athensIs: "awesome"
DrPsychick marked this conversation as resolved.
Show resolved Hide resolved
34 changes: 34 additions & 0 deletions charts/athens-proxy/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,40 @@ We truncate at 63 chars because some Kubernetes name fields are limited to this
{{- end -}}
{{- end -}}

{{/*
Common labels used by all resources in their meta fields.
Includes existing labels for passivity, new standard labels, and user-defined extra labels.
https://helm.sh/docs/chart_best_practices/labels/
https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/#labels
*/}}
{{- define "athens.metaLabels" -}}
{{- /* Allow an app suffix name to be passed in to append to the fullname */}}
{{- $defaultAppName := include "fullname" . }}
{{- $appName := printf "%s%s" $defaultAppName (default "" .appSuffix) }}
{{- /* Existing Legacy labels for passivity */}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added these comments with the intention to document why they were here, however this top one does create an extra empty line the in the helm template

apiVersion: apps/v1
kind: Deployment
metadata:
  name: release-name-athens-proxy-jaeger
  labels:
    
    app: release-name-athens-proxy-jaeger
    chart: athens-proxy-0.7.0
    release: "release-name"
    heritage: "Helm"
    app.kubernetes.io/name: release-name-athens-proxy-jaeger
    helm.sh/chart: athens-proxy-0.7.0
    app.kubernetes.io/managed-by: "Helm"
    app.kubernetes.io/instance: "release-name"
    app.kubernetes.io/version: "v0.12.0"
    athensIs: awesome
spec:

This output seems generally accepted by the helm docs https://helm.sh/docs/chart_best_practices/templates/#whitespace-in-generated-templates but i'm happy to remove these comments if the tradeoff isn't worth it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as the indentation is correct, it's fine. I've seen many helm charts with some empty lines. The comment should generally not be necessary when the code is readable, but it may help newcomers and does not hurt.

app: {{ $appName }}
chart: {{ template "athens.chart" . }}
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- /* New Standard labels */}}
app.kubernetes.io/name: {{ $appName }}
helm.sh/chart: {{ template "athens.chart" . }}
app.kubernetes.io/managed-by: "{{ .Release.Service }}"
app.kubernetes.io/instance: "{{ .Release.Name }}"
app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
{{- /* Include user defined labels */}}
{{- if .Values.extraLabels }}
{{ toYaml .Values.extraLabels }}
{{- end -}}
{{- end }}

{{/*
A common helper for creating the full chart name and version as used by the chart label.
*/}}
{{- define "athens.chart" -}}
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" -}}
{{- end -}}

{{/*
Determine the home directory of the current user.
*/}}
Expand Down
5 changes: 1 addition & 4 deletions charts/athens-proxy/templates/config-ssh-git-servers.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ kind: ConfigMap
metadata:
name: {{ template "fullname" . }}-ssh-git-servers
labels:
app: {{ template "fullname" . }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- include "athens.metaLabels" . | nindent 4 }}
data:
ssh_config: |
{{- range $server := .Values.sshGitServers }}
Expand Down
5 changes: 1 addition & 4 deletions charts/athens-proxy/templates/config-upstream.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ kind: ConfigMap
metadata:
name: {{ template "fullname" . }}-upstream
labels:
app: {{ template "fullname" . }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- include "athens.metaLabels" . | nindent 4 }}
data:
FilterForUpstreamProxy: |-
# FilterFile for fetching modules directly from upstream proxy
Expand Down
9 changes: 2 additions & 7 deletions charts/athens-proxy/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@ kind: Deployment
metadata:
name: {{ template "fullname" . }}
labels:
app: {{ template "fullname" . }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- include "athens.metaLabels" . | nindent 4 }}
spec:
replicas: {{ .Values.replicaCount }}
{{- if .Values.strategy }}
Expand All @@ -23,9 +20,7 @@ spec:
template:
metadata:
labels:
app: {{ template "fullname" . }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
{{- include "athens.metaLabels" . | nindent 8 }}
DrPsychick marked this conversation as resolved.
Show resolved Hide resolved
annotations:
checksum/upstream: {{ include (print $.Template.BasePath "/config-upstream.yaml") . | sha256sum }}
checksum/ssh-config: {{ include (print $.Template.BasePath "/config-ssh-git-servers.yaml") . | sha256sum }}
Expand Down
5 changes: 1 addition & 4 deletions charts/athens-proxy/templates/hpa.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ kind: HorizontalPodAutoscaler
metadata:
name: {{ template "fullname" . }}
labels:
app: {{ template "fullname" . }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- include "athens.metaLabels" . | nindent 4 }}
spec:
scaleTargetRef:
apiVersion: apps/v1
Expand Down
5 changes: 1 addition & 4 deletions charts/athens-proxy/templates/ingress.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ kind: Ingress
metadata:
name: {{ $fullName }}
labels:
app: {{ template "fullname" . }}
chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
{{- include "athens.metaLabels" . | nindent 4 }}
{{- with .Values.ingress.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
Expand Down
5 changes: 1 addition & 4 deletions charts/athens-proxy/templates/jaeger-deploy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ kind: Deployment
metadata:
name: {{ template "fullname" . }}-jaeger
labels:
app: {{ template "fullname" . }}-jaeger
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- include "athens.metaLabels" (dict "appSuffix" "-jaeger" "Values" .Values "Release" .Release "Chart" .Chart) | nindent 4 }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a bit gross since it has to be aware of the values being used by the underlying helper, but I wanted to find a way to re-use the helper for these couple of unique cases that have a different name specified currently.

FWIW, these differences seem a bit strange, I'd normally expect all of the resources created by the helm chart to have this same app name. However, I tried to keep things as passive as possible.

I did consider creating a separate helper function that was metaLabelsWithoutAppName that would be re-used by the new helper and could be used here and have the app and app.kubernetes.io/name be set here explicity. I favored this approach but am happy to change things up

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, not the nicest include I've seen, but it helps keeping the labels consistent and may not be worth to add an extra helper.

spec:
replicas: 1
selector:
Expand Down
5 changes: 1 addition & 4 deletions charts/athens-proxy/templates/jaeger-svc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ kind: Service
metadata:
name: {{ template "fullname" . }}-jaeger
labels:
app: {{ template "fullname" . }}-jaeger
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- include "athens.metaLabels" (dict "appSuffix" "-jaeger" "Values" .Values "Release" .Release "Chart" .Chart) | nindent 4 }}
spec:
type: {{ .Values.jaeger.type }}
ports:
Expand Down
2 changes: 2 additions & 0 deletions charts/athens-proxy/templates/secret-ssh-git-servers.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ kind: Secret
apiVersion: v1
metadata:
name: {{ template "fullname" . }}-ssh-git-servers
labels:
{{- include "athens.metaLabels" . | nindent 4 }}
type: Opaque
data:
{{- range $server := .Values.sshGitServers }}
Expand Down
2 changes: 2 additions & 0 deletions charts/athens-proxy/templates/secret.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ kind: Secret
apiVersion: v1
metadata:
name: {{ template "fullname" . }}-secret
labels:
{{- include "athens.metaLabels" . | nindent 4 }}
type: Opaque
data:
{{- if .Values.storage.mongo.url }}
Expand Down
5 changes: 1 addition & 4 deletions charts/athens-proxy/templates/service-account.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ kind: ServiceAccount
metadata:
name: {{ template "fullname" . }}
labels:
app: {{ template "fullname" . }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- include "athens.metaLabels" . | nindent 4 }}
{{- with .Values.serviceAccount.annotations }}
annotations:
{{ toYaml . | indent 4 }}
Expand Down
5 changes: 1 addition & 4 deletions charts/athens-proxy/templates/service-monitor.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
labels:
app: {{ template "fullname" . }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- include "athens.metaLabels" . | nindent 4 }}
prometheus: default
{{- with .Values.service.annotations }}
annotations:
Expand Down
5 changes: 1 addition & 4 deletions charts/athens-proxy/templates/service-scrape.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ apiVersion: operator.victoriametrics.com/v1beta1
kind: VMServiceScrape
metadata:
labels:
app: {{ template "fullname" . }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- include "athens.metaLabels" . | nindent 4 }}
prometheus: default
{{- with .Values.service.annotations }}
annotations:
Expand Down
5 changes: 1 addition & 4 deletions charts/athens-proxy/templates/service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ metadata:
{{ toYaml .Values.service.annotations | indent 4 }}
{{- end }}
labels:
app: {{ template "fullname" . }}
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- include "athens.metaLabels" . | nindent 4 }}
spec:
type: {{ .Values.service.type }}
ports:
Expand Down
5 changes: 1 addition & 4 deletions charts/athens-proxy/templates/storage-pvc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ apiVersion: v1
metadata:
name: {{ template "fullname" . }}-storage
labels:
app: {{ template "fullname" . }}-storage
chart: "{{ .Chart.Name }}-{{ .Chart.Version }}"
release: "{{ .Release.Name }}"
heritage: "{{ .Release.Service }}"
{{- include "athens.metaLabels" (dict "appSuffix" "-storage" "Values" .Values "Release" .Release "Chart" .Chart) | nindent 4 }}
spec:
accessModes:
- {{ .Values.storage.disk.persistence.accessMode | quote }}
Expand Down
3 changes: 3 additions & 0 deletions charts/athens-proxy/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ configEnvVars: {}
# Extra annotations to be added to the athens pods
annotations: {}

# Extra labels to be added to all resources
extraLabels: {}

# HTTP basic auth
basicAuth:
enabled: false
Expand Down