Zero-to-jupyterhub-k8s: Multiple Helm installations in the same namespace - avoiding name collisions by including Helm release name?

Created on 31 Mar 2018  路  10Comments  路  Source: jupyterhub/zero-to-jupyterhub-k8s

Related issues: #538


@gsemet wrote in issue #538

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?

It seems like a very common practice within the publicly available kubernetes/charts to do this. I suggest we do this an optional behaviour based on a value disabled by default.

Pros

  • Allows multiple charts to be published in the same namespace
  • Aligns with common helm chart practices

Cons

  • Added clutter when working with one release only within a namespace, for example when writing kubectl get pods. Everybody better utilize the kubectl autocomplete possibility.

Suggestion

We add an option to the value file to add the helm release name to the object names, that way it would be a win win I think.

Code example

Research

Name field

The name field is often simply hub or proxy for multiple objects within our project, they can be the same as long as we don't have the same name for multiple object types.

Deviations:

  • hub

    • secret.yaml (name: hub-secret)

    • pvc.yaml (name: hub-db-dir)

    • configmap.yaml (name: hub-config)

    • netpol-singleuser.yaml (name: singleuser-network-policy)

    • netpol-hub.yaml (name: hub-network-policy)

  • image-puller

    • daemonset.yaml (hook-image-puller-...)

    • daemonset.yaml (continuous-image-puller-...)

    • job.yaml (hook-image-awaiter-...)

  • proxy

    • netpol-proxy.yaml (proxy-network-policy)

    • secret.yaml (manual-tls-proxy-...)

    • service.yaml (proxy-api)

    • service.yaml (proxy-public)

    • autohttps

    • deployment.yaml (autohttps)

    • ingress-internal (jupyterhub-internal)

    • nginx-configmap.yaml (nginx-proxy-config)

    • rbac.yaml (multiple different)

    • service.yaml (proxy-http)

  • dynamically-created-pods

    • singleuser pods ?

App label

It seems like a quite common practice for the app label, but I really do not understand why. We could combine the app label with the release label to filter objects if needed. The name field on the other hand must be unique for that object type (Deployment, ServiceAccount, etc.) within a namespace to avoid clashes.

63 char truncation

The name field must not be more than 63 characters in order for the DNS to function properly. So templates for the name field often truncate to 63 characters. For this to be a useful safeguard, we must not append additional naming to the template output as many actually do.

Controller created pod's names

Assuming we truncate the name field to 63 letters, what happens if a controller (daemonset, deployment, replica set etc.) is adding some unique portion to a name as well?

Adding Chart and release name vs. Release name only

Some charts seem to include the helm chart name in the name as well. I don't understand why this would be fruitful though, we are labeling the objects with the chart anyhow. Perhaps it is to avoid multiple charts published in the same namespace, with the same release name, to have a name collision?

# If we ensured that "continuous-image-puller" only had 63 letters...
# we might fail the name length requirement anyhow if "-xxxxx" is added later
continuous-image-puller-rzsv7
  • [ ] Read up on how controller name their pods, is it augmentation or will they replace the later parts if needed to meet the 63 character criteria?
  • [ ] Consider lowering truncation to 63-6=57 characters.

RoleBindings references names not labels

RBAC role bindings reference roles based on their name, this is not very relevant but I was thinking "we never reference objects by their name in the yaml files", and that was apparently false.

Implementation

TODO

  • [ ] Decide on including helm release name in all the names, do it optionally disabled or enabled by default, or not at all.

    • I suggest we consider allowing the optional option and having it disabled by default.

  • [ ] Figure out what the consequences could be by going from the optionally false value to the optionally true value.
  • [ ] Figure out how we name our objects, we commonly name them "hub" if they are in the hub templates folder, but we have some deviations from this. Should we make the deviation naming default, or the other way around, or simply ignore this detail?

    • I suggest we consider naming every object within templates/hub to hub etc. unless we have a name clash for two objects of the same type, then we add something specific after.

Preliminary implementation suggestion

We create a _helpers.tpl that contains a defined helper function accessible from all different template folders (hub, proxy, image-puller, ...), that outputs a name definition based on inputs when called. We call this helper function providing the template folder name plus something additional if we need to separate two objects of the same object type.

All 10 comments

Hi. If I can help tell me. We use Jupyterhub behind traefik on our baremetal cluster. Just ping me (email: gaetan at xeberon.net)

Thanks for writing this up, @consideRatio! Very detailed and thoughtful!

I'll admit I wish we had designed the chart and kubespawner to be useful with multiple deployments per namespace from the start. It would've been easier than retrofitting it in now. The primary reason I didn't do it was because of the clutter (and I'll admit, some amount of laziness - z2jh was distilled out of the berkeley deployment...).

We will need a change in kubespawner I believe (but I might be wrong!) to allow multiple installations in the same namespace. I think it currently counts all singleuser pods in the same namespace to be owned by it. We might have to introduce additional labels that kubespawner will watch for? We'll also have to add the release name to all the singleuser pods too, to prevent clashes.

I love the idea of this being optional! It satisfies the use case of folks like @gsemet without us running into big backwards compatibility issues. Can we put the release as a suffix rather than a prefix though? I think that makes kubectl autocomplete much nicer, but feel free to ignore this :)

I think sequence of events should be:

  1. Add support for this to kubespawner
  2. Make it optional (and document this) in z2jh
  3. Think of ways to do this transition, and document those.

How does this sound, @gsemet @consideRatio?

Thanks @yuvipanda for your encouragement and input!!

We might have to introduce additional labels that kubespawner will watch for?

Perhaps not! We already have the release: {{ .Release.Name }} label on everything (at least after #538), that should be enough right?

Can we put the release as a suffix rather than a prefix though?

Excellent you give an opinion about this, I was considering both variants but then I decided I figured it was too hard for me to be clairvoyant about this. Let's go with a suffix or whats appreciated at the time we implement this. I figure we will have a helper .tpl file setting the names, and it would be easy to switch if .Release.Name is added as a prefix or suffix later.

@yuvipanda it's great that you suggested sequence of actions! I'll spend some time and learn about the kubespawner project for now, hoping to get a good picture on whats required.

I have no idea why they prefer prefix over suffix. I like it so all pods/services under the same release begin by the same string (listing are easier?), but suffix would do the job
If working in the kubespawner is required I鈥檒l try to have a look, adding the release in the kube spawner should not be that hard so we can filter all singleuser pods.

And no pbl to make it optional (for the single user pod only?)

@gsemet oh the singleuser pods, hmmmm, oh they are dynamically created... interesting... Do we have access to the release name and such within them? Ah... Another learning objective related to kubespawner!

Kubespawner TODO

  • [ ] Ensure dynamically spawned pods have the same types of labels as well

Regarding the optional part, I'm was thinking we can make the prefix-/suffix-ing of the helm release name optional for all kubernetes objects, so if you only plan on having one release in the same namespace, you don't have to clutter the release name on the objects at all.

Do we have access to the release name and such within them
You will have to use the power of templating! env var/configmap etc.
I saw there was a recent change introducing a prefix to all singleuser pods, why not use it ?

I had this in mind while working through #625, but I've realized that implementing this would add a lot of clutter to the yaml files and become more error prone. I do not recommend we pursue this unless there is a bigger demand for it. The templates could do a lot of work, but there are some exceptions causing a lot of additional code bloat.

I'm interested in doing multiple deployments in a single namespace, so please count me as a +1 for interest in this feature. If you change your mind about the code bloat trade-off please let me know, I'm happy to take a crack at a PR or help however I can.

Closed in favor of the PR that just mentioned this issue above.

Was this page helpful?
0 / 5 - 0 ratings