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

Added labels following helm best practises #1783

Merged
merged 4 commits into from
Jun 17, 2020
Merged

Added labels following helm best practises #1783

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6155ae0
Added labels following helm best practises
fjagugar Jun 8, 2020
c22bd38
Applied PR feedback
fjagugar Jun 9, 2020
774affe
Applied PR feedback: set same app.kubernetes.io/name everywhere
fjagugar Jun 16, 2020
6862196
Remove duplicated label
Jun 17, 2020
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
21 changes: 18 additions & 3 deletions chart/kubeapps/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,25 @@ Create chart name and version as used by the chart label.
{{- end -}}

{{/*
Common labels
Common labels for additional kubeapps applications. Used on resources whose app name is different
from kubeapps
*/}}
{{- define "kubeapps.labels" -}}
app: {{ include "kubeapps.name" . }}
{{- define "kubeapps.extraAppLabels" -}}
chart: {{ include "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
helm.sh/chart: {{ template "kubeapps.chart" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also add the other two annotations that are recommended: app.kubernetes.io/name and app.kubernetes.io/managed-by?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I see that these labels are defined in the _helpers.tpl file too:

{{/*
Common labels
*/}}
{{- define "kubeapps.labels" -}}
app: {{ include "kubeapps.name" . }}
chart: {{ include "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
{{- end -}}

We can add the new labels there and simply use the helper everywhere (to avoid having to add the labels one by one).

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 see that not all the labels added in all the cases use the same parameters. It seem the app/app.kubernetes.io/name usually changes.

I've created a new template that adds all the labels that are common except those two ones and used it wherever those labels were the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤦 this is a bit of a mess (our fault). I guess app should have been always the same but we changed it in several places and now we cannot fix it without making it a breaking change.

From the definition in the docs: app.kubernetes.io/name: This should be the app name, reflecting the entire app. Usually {{ template "name" . }} is used for this. This is used by many Kubernetes manifests, and is not Helm-specific.. Can we at least have a common app.kubernetes.io/name? We can move that new label to the common group.

app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- end -}}

{{/*
Common labels
*/}}
{{- define "kubeapps.labels" -}}
app: {{ include "kubeapps.name" . }}
app.kubernetes.io/name: {{ include "kubeapps.name" . }}
{{ template "kubeapps.extraAppLabels" . }}
{{- end -}}

{{/*
Expand All @@ -47,6 +59,9 @@ Labels to use on deploy.spec.selector.matchLabels and svc.spec.selector
{{- define "kubeapps.matchLabels" -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this matchLabels is not used anywhere. Can we delete it?

app: {{ include "kubeapps.name" . }}
release: {{ .Release.Name }}
app.kubernetes.io/instance: {{ .Release.Name }}
Copy link
Contributor

Choose a reason for hiding this comment

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

here you are just adding one of the labels, is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, as this did not had the chart tag before. I'm only adding the app.kubernetes.io/instance which is the equivalent to the release one.

app.kubernetes.io/name: {{ include "kubeapps.name" . }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
{{- end -}}

{{/*
Expand Down
6 changes: 1 addition & 5 deletions chart/kubeapps/templates/apprepositories-secret.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,7 @@ metadata:
name: {{ template "kubeapps.apprepository-secret.name" . }}
annotations:
"helm.sh/hook": pre-install
labels:
app: {{ template "kubeapps.name" $ }}
chart: {{ template "kubeapps.chart" $ }}
release: {{ $.Release.Name }}
heritage: {{ $.Release.Service }}
labels:{{ include "kubeapps.labels" $ | nindent 4 }}
data:
{{- if .caCert }}
ca.crt: |-
Expand Down
6 changes: 2 additions & 4 deletions chart/kubeapps/templates/apprepositories.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ metadata:
name: {{ .name }}
annotations:
"helm.sh/hook": post-install
labels:
labels:{{ include "kubeapps.extraAppLabels" $ | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" $ }}
chart: {{ template "kubeapps.chart" $ }}
release: {{ $.Release.Name }}
heritage: {{ $.Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" $ }}
spec:
type: helm
url: {{ .url }}
Expand Down
8 changes: 3 additions & 5 deletions chart/kubeapps/templates/apprepository-crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ metadata:
name: apprepositories.kubeapps.com
annotations:
"helm.sh/hook": crd-install
labels:
app: {{ template "kubeapps.apprepository.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
labels:{{ include "kubeapps.extraAppLabels" $ | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" $ }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" $ }}
spec:
group: kubeapps.com
scope: Namespaced
Expand Down
8 changes: 4 additions & 4 deletions chart/kubeapps/templates/apprepository-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ template "kubeapps.apprepository.fullname" . }}
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" . }}
spec:
replicas: {{ .Values.apprepository.replicaCount }}
selector:
Expand All @@ -18,6 +16,8 @@ spec:
labels:
app: {{ template "kubeapps.apprepository.fullname" . }}
release: {{ .Release.Name }}
app.kubernetes.io/instance: {{ .Release.Name }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" . }}
spec:
serviceAccountName: {{ template "kubeapps.apprepository.fullname" . }}
{{- include "kubeapps.imagePullSecrets" . | indent 6 }}
Expand Down
12 changes: 4 additions & 8 deletions chart/kubeapps/templates/apprepository-jobs-cleanup-rbac.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ metadata:
helm.sh/hook: post-delete
helm.sh/hook-delete-policy: hook-succeeded
helm.sh/hook-weight: "-10"
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository-jobs-cleanup.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository-jobs-cleanup.fullname" . }}
rules:
- apiGroups:
- kubeapps.com
Expand All @@ -36,11 +34,9 @@ metadata:
helm.sh/hook: post-delete
helm.sh/hook-delete-policy: hook-succeeded
helm.sh/hook-weight: "-10"
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository-jobs-cleanup.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository-jobs-cleanup.fullname" . }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
Expand Down
6 changes: 2 additions & 4 deletions chart/kubeapps/templates/apprepository-jobs-cleanup-serviceaccount.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ metadata:
helm.sh/hook: post-delete
helm.sh/hook-delete-policy: hook-succeeded
helm.sh/hook-weight: "-10"
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository-jobs-cleanup.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository-jobs-cleanup.fullname" . }}
8 changes: 4 additions & 4 deletions chart/kubeapps/templates/apprepository-jobs-cleanup.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ metadata:
annotations:
helm.sh/hook: post-delete
helm.sh/hook-delete-policy: hook-succeeded
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository-jobs-cleanup.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository-jobs-cleanup.fullname" . }}
spec:
template:
metadata:
labels:
app: {{ template "kubeapps.apprepository-jobs-cleanup.fullname" . }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository-jobs-cleanup.fullname" . }}
release: {{ .Release.Name }}
app.kubernetes.io/instance: {{ .Release.Name }}
spec:
{{- include "kubeapps.imagePullSecrets" . | indent 6 }}
{{- if .Values.hooks.affinity }}
Expand Down
12 changes: 4 additions & 8 deletions chart/kubeapps/templates/apprepository-jobs-postupgrade-rbac.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: {{ template "kubeapps.apprepository-job-postupgrade.fullname" . }}
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" . }}
rules:
- apiGroups:
- ""
Expand All @@ -24,11 +22,9 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: {{ template "kubeapps.apprepository-job-postupgrade.fullname" . }}
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" . }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
Expand Down
8 changes: 4 additions & 4 deletions chart/kubeapps/templates/apprepository-jobs-postupgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ metadata:
helm.sh/hook: post-upgrade
helm.sh/hook-weight: "0"
helm.sh/hook-delete-policy: hook-succeeded
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository-job-postupgrade.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository-job-postupgrade.fullname" . }}
spec:
template:
metadata:
labels:
app: {{ template "kubeapps.apprepository-job-postupgrade.fullname" . }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository-job-postupgrade.fullname" . }}
release: {{ .Release.Name }}
app.kubernetes.io/instance: {{ .Release.Name }}
spec:
{{- include "kubeapps.imagePullSecrets" . | indent 6 }}
{{- if .Values.hooks.affinity }}
Expand Down
30 changes: 10 additions & 20 deletions chart/kubeapps/templates/apprepository-rbac.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: {{ template "kubeapps.apprepository.fullname" . }}
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" . }}
rules:
- apiGroups:
- ""
Expand Down Expand Up @@ -47,11 +45,9 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: {{ template "kubeapps.apprepository.fullname" . }}
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" . }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
Expand Down Expand Up @@ -101,11 +97,9 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: "kubeapps:{{ .Release.Namespace }}:apprepositories-read"
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" . }}
rules:
- apiGroups:
- kubeapps.com
Expand All @@ -121,11 +115,9 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: "kubeapps:controller:{{ .Release.Namespace }}:apprepositories-read"
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" . }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
Expand All @@ -139,11 +131,9 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: "kubeapps:{{ .Release.Namespace }}:apprepositories-write"
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" . }}
rules:
- apiGroups:
- kubeapps.com
Expand Down
12 changes: 4 additions & 8 deletions chart/kubeapps/templates/apprepository-serviceaccount.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,14 @@ apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ template "kubeapps.apprepository.fullname" . }}
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" . }}
---
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ template "kubeapps.apprepository-job-postupgrade.fullname" . }}
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.apprepository.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.apprepository.fullname" . }}
8 changes: 4 additions & 4 deletions chart/kubeapps/templates/assetsvc-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ template "kubeapps.assetsvc.fullname" . }}
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.assetsvc.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.assetsvc.fullname" . }}
spec:
replicas: {{ .Values.assetsvc.replicaCount }}
selector:
Expand All @@ -17,7 +15,9 @@ spec:
metadata:
labels:
app: {{ template "kubeapps.assetsvc.fullname" . }}
app.kubernetes.io/name: {{ template "kubeapps.assetsvc.fullname" . }}
release: {{ .Release.Name }}
app.kubernetes.io/instance: {{ .Release.Name }}
spec:
{{- include "kubeapps.imagePullSecrets" . | indent 6 }}
{{- if .Values.assetsvc.affinity }}
Expand Down
6 changes: 2 additions & 4 deletions chart/kubeapps/templates/assetsvc-service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ apiVersion: v1
kind: Service
metadata:
name: {{ template "kubeapps.assetsvc.fullname" . }}
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.name" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.name" . }}
spec:
type: ClusterIP
ports:
Expand Down
6 changes: 2 additions & 4 deletions chart/kubeapps/templates/dashboard-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ apiVersion: v1
kind: ConfigMap
metadata:
name: {{ template "kubeapps.dashboard-config.fullname" . }}
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.dashboard-config.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.dashboard-config.fullname" . }}
data:
vhost.conf: |-
server {
Expand Down
10 changes: 6 additions & 4 deletions chart/kubeapps/templates/dashboard-deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ template "kubeapps.dashboard.fullname" . }}
labels:
labels:{{ include "kubeapps.extraAppLabels" . | nindent 4 }}
app: {{ template "kubeapps.dashboard.fullname" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
app.kubernetes.io/name: {{ template "kubeapps.dashboard.fullname" . }}
spec:
replicas: {{ .Values.dashboard.replicaCount }}
selector:
Expand All @@ -19,7 +17,11 @@ spec:
checksum/config: {{ include (print $.Template.BasePath "/dashboard-config.yaml") . | sha256sum }}
labels:
app: {{ template "kubeapps.dashboard.fullname" . }}
app.kubernetes.io/name: {{ template "kubeapps.dashboard.fullname" . }}
release: {{ .Release.Name }}
app.kubernetes.io/instance: {{ .Release.Name }}
chart: {{ template "kubeapps.chart" . }}
helm.sh/chart: {{ template "kubeapps.chart" . }}
spec:
{{- include "kubeapps.imagePullSecrets" . | indent 6 }}
{{- if .Values.dashboard.affinity }}
Expand Down
6 changes: 1 addition & 5 deletions chart/kubeapps/templates/dashboard-service.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@ apiVersion: v1
kind: Service
metadata:
name: {{ template "kubeapps.dashboard.fullname" . }}
labels:
app: {{ template "kubeapps.name" . }}
chart: {{ template "kubeapps.chart" . }}
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
labels:{{ include "kubeapps.labels" . | nindent 4 }}
spec:
type: ClusterIP
ports:
Expand Down
Loading