Open cccs-nik opened 3 months ago
I think this is reasonable. I'd like to give a bit of thought about the new property though. For instance, if we want to let people choose between multiple implementations in future (e.g. chp, traefik, none) then instead of enabled
it might be better to have a string property.
why not provide traefik as an optional chart dependency?
# Chart.yaml (partial contents)
name: jupyterhub
dependencies:
- name: traefik
condition: traefik.enabled
repository: #....
version: "2.x.x"
That would require us to create a new Helm chart just for https://github.com/jupyterhub/traefik-proxy , and we'd still need some configuration parameters in Z2JH to use Traefik-proxy instead of Configurable-HTTP-Proxy.
https://github.com/traefik/traefik-helm-chart comes with CRDs so you can create IngressRoute
objects to configure ingress rules.
Proposed change
I would like the ability to disable the current proxy solution. The reason being that I implemented our own Traefik + Redis solution using the currently unreleased 2.0.0 Traefik Proxy because #1162 was closed for the time being. In the clusters I manage, we're seeing the same memory leaks described in Memory leak in proxy? and had to switch away from CHP as a result.
I understand that disabling CHP in the current chart leads to an incomplete deployment but because #1162 isn't ready yet, it would be nice to be able to disable CHP anyway to allow custom solutions. I currently extract the files from the .tgz chart, nuke the proxy folder under templates, repackage using helm and then place random values into the PROXY_API_SERVICE_PORT and PROXY_PUBLIC_SERVICE_PORT env vars for the hub pod to disable the proxy properly.
Alternative options
Replace CHP with Traefik would be an alternative option but I understand that this is a lot of work and not ready yet. Allowing custom proxy solutions could be a nice stopgap.
Who would use this feature?
People suffering from issues with CHP that want to implement their own proxy solution.
(Optional): Suggest a solution
Probably just adding some {{- if .Values.proxy.enabled }} in a few places.