Labels and Selectors - a common practice within the templates

[consideRatio]

Related issue: #615

---

I was about to make set labels and selectors for a StatefulSet I tried out, and wanted to mimic what had been done for other k8s object templates, but did not find a clear pattern.

So, I'd like to figure out a practice to follow within the guide.

• What labels do we declare?
• What are important labels within a selector?

metadata:
  name: proxy
  labels:
    chart: {{ .Chart.Name }}-{{ .Chart.Version }}
    component: proxy
    heritage: {{ .Release.Service }}
    release: {{ .Release.Name }}
spec:
  replicas: 1
  selector:
    matchLabels:
      component: proxy
      release: {{ .Release.Name }}
      heritage: {{ .Release.Service }}

I'd like to discuss the following labels:

• chart, app, component, heritage, release

My take

• I recommend we don't use the chart label, or is there a benefit to including that?
• I recommend we select on few labels in a minimalistic manner, perhaps only component or component and release is needed? What can go wrong by including / not including various labels?

Perhaps something like below?

The labels we declare on controllers

• chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
• app: jupyterhub
• component: hub
• release: {{ .Release.Name }}
• heritage: {{ .Release.Service }}

The important labels within a selector and a controllers pod template

• chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
• app: jupyterhub
• component: hub
• release: {{ .Release.Name }}
• heritage: {{ .Release.Service }}

[yuvipanda]

https://github.com/kubernetes/helm/blob/master/docs/chart_best_practices/labels.md is where I took my cues from, and I think we should stick to it. Note that I didn't see that page until after a few releases, so it isn't consistently applied.

I'm happy for us to standardize this! I think we should include all the 5 ones that helm recommends in all our components. There's nothing wrong with changing them, but I think sticking to the standards helps other tooling / people who are used to helm charts to identify / write additional scripts that deal with z2jh.

[consideRatio]

Ah! What about label selection in a selector:? It would be overkill to select based on every label. What about going for component + release?

[yuvipanda]

@consideRatio good question! I wonder what the charts in github.com/kubernetes/charts do. We should probably follow whatever it is they do...

[gsemet]

Hello. I like the fact that every names are prepended with the release name, so that two helm deployment can be done on the same namespace without conflict. Do you think it can be feasible for Zero-to-Jupyterhub?
In _helpers.tpl we would have:

{{/*
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).
*/}}
{{- define "jupyterhub.fullname" -}}
{{- printf "%s" .Release.Name | trunc 63 | trimSuffix "-" -}}
{{- end -}}

And all name metadata are prepended with this:

• Configmap:

apiVersion: v1
kind: ConfigMap
metadata:
  name: "{{ template "jupyterhub.fullname" . }}-hub-config"
data:

• Deployment:

apiVersion: extensions/v1beta1
kind: Deployment
metadata:
  name: {{ template "jupyterhub.fullname" . }}-hub

• PVC:

kind: PersistentVolumeClaim
apiVersion: v1
metadata:
  name: {{ template "jupyterhub.fullname" . }}-hub-db-dir

And so on.

This is not really documented in the Helm Best Practice, but we see more and more in the official charts (ex: stable/influxdb)

[minrk]

@gsemet while it's not impossible for two hubs in one namespace to work, I think it's probably good practice to always deploy only one jupyterhub per namespace. Namespaces are a very low-cost entity, and it makes it very easy to be sure what belongs together.

There's one case where we have app: kube-lego which has a comment stating that this is needed for kube-lego to do its magic. Maybe there's another way? I do think we should have app: jupyterhub on all of these, if we can.

[gsemet]

I understand, however I do not have this option on our environment (baremetal with team that is ramping up on this subject) :(
For automatic test, it may be useful to use several deployment even in the same namespace. Helm charts usually add the "release name" in front of each element (pod, configmap, claim,...) to ease the readability, I find it a good practice.

[consideRatio]

@minrk the app: kube-lego situation is will go away by #592 !

@gsemet thanks for insights into this! For that reason it makes sense to me to add a release to the name. I will probably clutter output from kubectl get pod etc and make the need for a kubectl autocomplete even bigger though.

I conclude that I don't understand enough about namespaces to be confident enough on a stance for or against including release in the kubernetes objects' names or not.

Related

What is a release? The helm documentation.

Update 1

I realize that if we are using labels like this, and selectors like that below, we allow for multiple releases, but not really because there is a name conflict anyhow without @gsemet's suggestion. So I figure we should not include the release name in the selection if we do not add the release name as part of the name as well.

The labels we declare

• chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
• app: jupyterhub
• component: hub
• release: {{ .Release.Name }}
• heritage: {{ .Release.Service }}

The important labels within a selector

• chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}
• app: jupyterhub
• component: hub
• release: {{ .Release.Name }}
• heritage: {{ .Release.Service }}

Ping @yuvipanda - got further input?

I'm hoping to get actionable enough in order to make a PR making the k8s objects' labels and label selectors systematic throughout z2jh.

Update 2

Regarding chart names and replacing of + with _.

• https://docs.helm.sh/chart_best_practices/#version-numbers

[consideRatio]

I'll make a PR focusing on the labels and selectors

Included in PR

I'll let the name field (not a label) be as it is
I'll make all k8s object get labels of chart, app, component, release, and heritage
I'll make all k8s objects, such as services, use simple label selectors that only selects based on the component label.

Potential future PR

If we decide on updating the name to include the helm release name, we should also select based on the release label.

[minrk]

@consideRatio that sounds great!

[consideRatio]

@minrk got any ideas on the chart label? ingress.yaml is deviating from the others for some reason I fail to find, but I recall reading something about this, perhaps it was in the gitter chat.

zero-to-jupyterhub-k8s/jupyterhub/templates/ingress.yaml

Line 8 in be3d3b7

chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}

Should I make all chart labels replace +, only the ingress.yaml one, or none?

[gsemet]

sounds great!

I must admit I do not have the full experience of helm yet to tell if it is better to use 1 namespace per app vs several app per namespace. There are a tons of internal talks in our team here.
We see that the official documentation states:

It is not necessary to use multiple namespaces just to separate slightly different resources, such as different versions of the same software: use labels to distinguish resources within the same namespace.

Also, when I look at the tons of official helm charts that provide this release prefix (throught a template), to app, selectors, pod names, etc, I simply want to the same.

So, I just would like to recommend to use _helpers.tpl trick to automatize this template that could render the code highly difficult to maintain (see this example for instance)

[yuvipanda]

@gsemet The bigger problem with multiple hubs in a namespace is that kubespawner doesn't support that atm - it assumes it 'owns' the namespace. We need to fix that first IMO.

Changing the names might also have backwards compatibility issues, primarily around renaming the hub PVC. So we should do that carefully and IMO make a point release specifically for it.

[yuvipanda]

@consideRatio I think that .replace exists simply from copy pasting from kubernetes/charts :) I think including it is a good option!

[gsemet]

I know that helm support hooks that may be used (I guess) to handle smooth transition, maybe it can do the pod rename... never tried it myself sadly

[consideRatio]

@yuvipanda according to helm's best practices one should use the replace + with _. Still not confident why. I also found about 1/4 of all the kubernetes/charts to do that. I'll make sure to follow the Helm best practices.

@gsemet regarding the name field, it seems almost all charts in kubernetes/charts is naming their objects similarly to how you suggest! I'm creating an issue for further discussion about implementing helm-release dependent naming.

[gsemet]

i guess restriction related to chart templating has something to do with DNS, when one use systems such as calico or other advanced networking environment.

[consideRatio]

Closed by #625