Many different Helm chart updates

yuvipanda / jupyterhub-ssh

SSH Access to JupyterHubs

BSD 3-Clause "New" or "Revised" License

93 stars 29 forks source link

Many different Helm chart updates #19

Closed consideRatio closed 3 years ago

consideRatio commented 3 years ago

PR addressing ideas in #17, closing #17.

@yuvipanda I updated issue #17 to describe everything added in this PR, and I feel at this point that I have done what I felt was reasonable fixes to do right away without pre-optimizing something. It is now ready for review, where each commit may be easier to overview now that there are plenty of changes in a single PR. Each commit is probably quite self-contained.

In addition to whats listed in #17

I found the sftp folder outside the helm-chart folder in the end, and updated port to be exposed from 22 -> 2222 aligned with the sshd_config.
I mounted hubUrl as an environment variable and removed YAML dependencies from the sftp image and stopped mounting anything else.

Suggested but not part of PR

Relocate jupyterhub-sftp next to helm-chart/images.
We create a lint-and-validate-config.yaml to serve the function of lint-and-validate-config.yaml
We setup a github action someplace to do lint and validation of a Helm chart, and reuse it for z2jh/binderhub/etc, but to start with just something that does helm template or so.

yuvipanda commented 3 years ago

So currently, if I deploy this, the pod names are workshop-prod-jupyterhub-ssh-sftp-server-6f95fbcb8d-wq9df and workshop-prod-jupyterhub-ssh-ssh-server-58f9d5bb57-dklr6. I think that's too redundant... I'll fiddle around to see what I can get the defaults to be!

yuvipanda commented 3 years ago

fwiw, I think the kinda nams I'd like are jupyterhub-ssh and jupyterhub-sftp...

yuvipanda commented 3 years ago

More importantly for me, the service names are workshop-prod-jupyterhub-ssh-sftp-server and workshop-prod-jupyterhub-ssh-ssh-server, and I don't think those should be the default.

yuvipanda commented 3 years ago

I fiddled with this for a while, but couldn't get it to work :( I think my preference is for the default to produce deployments and services named jupyterhub-ssh and jupyterhub-sftp, while allowing admins to add release name as prefix. Do you think we can either take that part out of this PR, or try and fix it?

Thank you so much for working on this, @consideRatio! I really, really appreciate the thoroughness and thoughtfulness you put into this.

consideRatio commented 3 years ago

Example 1

Here fullnameOverride isn't set. Then, the default behavior of a Helm chart with would be to name the secret resource which is scoped as common to both the k8s services and deployments as ...

workshop-prod-jupyterhub-ssh

But, since we have multiple deploy/service/netpol resources, I have appended "ssh-server" and "sftp-server" to them, which can be found in _helpers.tpl, so the name of these resources becomes...

workshop-prod-jupyterhub-ssh-ssh-server workshop-prod-jupyterhub-ssh-sftp-server

Example 2

Assume fullnameOverride is set to yuvi, then the secret will be named

yuvi

And the two sets of deploy/service/netpol resources will be named

yuvi-ssh-server yuvi-sftp-server

Suggestion

I suggest to stick with this configuration option system, but you can absolutely make fullnameOverride something by default and change the suffix to separate the two sets of components from ssh-server to just ssh for example.

consideRatio commented 3 years ago

@yuvipanda I've set fullnameOverride: jupyterhub by default now, which makes services be named jupyterhub-ssh and jupyterhub-sftp.

I suggest we merge at this point, thank you for helping me debug things remotely!

yuvipanda commented 3 years ago

AMAZING, @consideRatio. I have been far away from actually working on helm charts for a while, so I deeply deeply appreciate all your work here. <3