Move all parameter defaults for images into the jupyterhub_config.py

[yuvipanda]

Right now, we have a bunch of parameter defaults that are in values.yaml, and a bunch that are in jupyterhub_config.py. This is quite confusing.

We should standardize on putting all the defaults in the jupyterhub_config.py. This lets us make the docker image a bit more self contained, so users who don't want to use the helm chart can still benefit from it. It would also allow us to possibly test the image without spinning up a full k8s cluster...

[consideRatio]

Action item: Identify what default values we have in values.yaml, that propegates to configmap, and then in turn ends up being consumed in images/hub/jupyterhub_config.py

[minrk]

I'm not quite sure what the best solution to this is. Having defaults in values.yaml is super useful for seeing the structure/type of parameters (way more useful for humans than the json schema). Putting the defaults only in the jupyterhub_config.py means users can't really see what they are (maybe this is good?).

From a technical perspective, it does make sense - it makes Hub images resilient to small variations in the configmap/chart version. But from that perspective, the defaults should really reside only in the classes themselves, and only defaults that deviate from the class defaults should be defined in jupyterhub_config.py.

[consideRatio]

Hmmm, there are JupyterHub's actual defaults, the defaults in values.yaml, propegated to configmap and into jupyterhub_config.py that can also have defaults.

I'd like:

• To avoid having two sets of defaults
• To make it easy for users to overview default values of the configuration that they can override, this will be within values.yaml

It is not very user friendly to inspect the values.yaml file in order to learn the default values of the Helm chart, but it is not too bad considering the complexity of the deployment in general.

---

All in all, I figure we should perhaps focus on setting the defaults in values.yaml, and only there, after all anyhow.

[yuvipanda]

I've spent more time thinking about this and seeing the approach that has worked in TLJH.

1. There should exist a 'z2jh' module (similar to tljh module).
2. This module has a 'configurer' that takes a YAML config, and applies that to JupyterHub / KubeSpawner config as needed (https://github.com/jupyterhub/the-littlest-jupyterhub/blob/master/tljh/configurer.py)
3. The defaults are in values.yaml. This is directly passed in to the configurer, with no go templating in the configmap. This eliminates all the passing around with the configmap we have.
4. Write unit tests for the configurer - like https://github.com/jupyterhub/the-littlest-jupyterhub/blob/master/tests/test_configurer.py

This lets us:

1. Have defaults in values.yaml
2. Unit test our config
3. Remove a ton of go templating from the big configmap
4. Allow easier deprecations in the future, since a python package is a much better place to do them than go templates.

How does this sound, @minrk @consideRatio?

[consideRatio]

@yuvipanda yes my initial feelings sais this is a good idea. Helm is coalescing a lot of value files, we are pretty much doing the same but instead of coalescing multiple value files we are doing it with their coalesced output ({{ .Values }}) and python code mostly setting traitlets with a clearly defined naming scheme.

Id like to get myself a better overview of configuration in light of these ideas. A lot of values in values.yaml is mapped directly to a traitlet set in jupyterhub_config.py, some are mapped to that AND to chart templates (other than the passthrough configmap), and others are mapped only to the chart templates. I think such overview could help is conclude best practices for the development of the helm chart.

I have recently deployed a self-hosted GitLab instance on Kubernetes using their fresh Helm chart that reached 1.0 a week ago. After looking at that, I'm a lot less afraid of the idea of maintaining a subcharts such as cert-manager. They do it real nice and have pieced together multiple smaller charts to make a nice cohesive whole.

[consideRatio]

I think we should refresh this repo with Lua scripted helm templates when Helm 3 is available, and it seems like that time would be a good time to solve this issue as well.

Right now, within Helm templates, we are working with text, and that causes a lot of challenges. The only non-text (non-string) stuff we handle are things access through .Values. The only things typed as something else are .Values directly accessed. On them we can use various nice functions from sprig such as merge on dictionaries etc. With Lua, we will be able to treat everything as dictionaries and use merge on all boilerplate stuff we make!

I have felt very frustrated with Helm 2 for many reasons. Mainly it is because shaky implementation of their hooks, the text based templating, and hard-to-debug errors during upgrades. With Helm 3 though, without the complexity of having tiller in the cluster hopefully making things easier to test and more robust, with Lua scripts for templating and with managed hooks: I'm very hopeful.

[minrk]

With #941, we can get rid of all of the templating of the configmap, and pass values through. Then the implementation can be simple, intelligible, and testable in Python.

[manics]

Sounds like this was closed by https://github.com/jupyterhub/zero-to-jupyterhub-k8s/pull/941