Unique names for multiple helm installations in a single namespace

consideRatio commented 3 years ago

Update

To a large extent, this is addressed by #1923, see the checklist below.

It is a common practice to use the Helm release name as a prefix to the k8s resource names. A Helm release name is the name of a specific installation of a Helm chart, which can of course be installed multiple times in a Kubernetes cluster. Without a Helm release name prefixed to our resources, we can get name collisions that makes installations fail or misbehave after installation.

Cluster resources - solved

The JupyterHub helm chart has following its launch learned to ensure we can install two releases in the same k8s cluster but in different namespaces, that only required us to prefix a release name to the cluster wide resources like ClusterRole and ClusterRoleBinding.

Namespaced resources - an open issue

The JupyterHub helm chart still doesn't prefix namespaced resources, and this makes us unable to install the JupyterHub Helm chart twice in the same k8s namespace.

Goal discussion

I think it will be too complicated to support a transition where a Helm chart that is already installed gain a prefix during an upgrade. To understand this complexity, consider for example how we are to ensure Kubespawner's dynamically created PVCs are to be migrated without creating new PVs or even deleting PVs associated with the old PVCs.

What I do think is a plausible goal, is to allow a fresh installation to be configured to use a prefix. And, I have a clear suggestion on how to get this done.

Implementation specification

[x] We define a helper template in _helpers.tpl which we call to get a string back with a prefixed name depending on the configuration. We are mindful to mimic typical practices in Helm charts for the name of this template and its associated configuration.
[x] We systematically use the helper template to prefix our k8s resource names to ensure they are unique for each Helm release.
[x] We ensure the helper template doesn't add a prefix unless we explicitly configure it to do so, in order to ensure we don't break things for existing deployments.
[x] We update jupyterhub_config.py properly.
- There are references to resource names such as proxy-public and hub hardcoded here that should now also adjust given the resource prefix.
[ ] We document explicitly that changing the prefix related configuration after the initial install can have severe consequences, and strongly suggest to not do so.
[ ] We update KubeSpawner properly.
- When KubeSpawner monitors pods, it should only consider those with a label matching the specific Helm release.
- When KubeSpawner creates pods and PVCs, it should use another template to avoid name collisions for users with the same username but associated with different Helm releases of the JupyterHub Helm chart
- Implementation details about this are discussed in https://github.com/jupyterhub/kubespawner/pull/431#issuecomment-716012114.

Development advice

While developing this, I suggest using watch, helm template, and grep to identify the resources that we want to update, and while making changes to the templates observing that we successfully render and change the resource names we want to change.

Since this PR will change a massive amount of files and have a high risk of introducing something breaking by mistake, I'd like to ask that that you as PR author is considerate of the review work required to review the PR. You can help a reviewer by making a sensible number of cohesive commits and providing a clear overview of what you have changed, for example by describing the kind of changes that was needed and will be found.

PRs and Issues closed in favor of this issue

I am happy to contribute to this topic. I have a fully operational branch working based on the 0.9.0. I was focussing on that first due to timing constraints, but I am willing to help port this to master as well. It does not contain the helper template that you proposed yet, and I did some workarounds in the jupyterhub_config.py to avoid a kubespawner update (until a related kubespawner update gets merged https://github.com/jupyterhub/kubespawner/pull/431). https://github.com/kristofmartens/zero-to-jupyterhub-k8s/tree/feature/renaming_effort

consideRatio commented 3 years ago

Thank you @kristofmartens!

For this specific issue in this repo, I fear that the review process will be so tough and require a lot of local testing, that I'd prefer to do do most of the implementation work myself for quicker iterations. I have struggled a lot with the code in these Helm templates files, and I've created quite complicated logic (#964), so at this point I feel I'd like to just clean it up properly myself.

We will need something like https://github.com/jupyterhub/kubespawner/pull/431 as well indeed! Thank you for your work in this open source ecosystem @kristofmartens :heart:

kristofmartens commented 3 years ago

OK, thanks for taking this up. As I had to touch almost every helm template file and making sure this change would be backwards compatible is not trivial, I understand why you want to do this yourselves. Looking forward to the PR!

yan-v commented 1 year ago

Hi guys! i want to deploy jupyterhub in the same namespace, i see all issues here were resolved. Are there any instructions how to set prefix in help upgrade command or config.yaml file?

jupyterhub / zero-to-jupyterhub-k8s