search

yuvipanda / jupyterhub-ssh

SSH Access to JupyterHubs
BSD 3-Clause "New" or "Revised" License
93 stars 29 forks source link

SFTP enabled by default, a pvc claimName is required #24

Open consideRatio opened 3 years ago

consideRatio commented 3 years ago

The SFTP part of the Helm chart is enabled by default.

https://github.com/yuvipanda/jupyterhub-ssh/blob/69a5bb08e199295748d2dc277d66a7e9001b0b76/helm-chart/jupyterhub-ssh/values.yaml#L50-L57

Also, it requires a PVC claimName to be explicitly set by default, otherwise, a user will experience the following error during helm install when the rendered Helm templates fail validation with the k8s api-server because of a empty string as a PVC claimName.

Error: unable to build kubernetes objects from release manifest: error validating "": error validating data: ValidationError(Deployment.spec.template.spec.volumes[0].persistentVolumeClaim): missing required field "claimName" in io.k8s.api.core.v1.PersistentVolumeClaimVolumeSource

Suggestions

  1. We disable SFTP by default.
  2. If SFTP require a PVC, we should probably make it a StatefulSet. A lot of Helm charts transitioned from Deployment resources to StatefulSet's a year back or so btw. By doing this, we become a bit more transparent with intentions etc and we avoid issues like being forced to manually set a Deployment's update strategy to Recreate etc - this is required as only one pod can mount to the typical PVC unless it has ReadWriteMany which is an exception to what is the most common.
yuvipanda commented 3 years ago

We should definitely disable SFTP by default!

The PVC is usually the same PVC used by JupyterHub - so it needs to be ReadWriteMany. As such, I think Deployment is more appropriate. What do you think?

This definitely needs some, any documentation :'(

glsdown commented 3 years ago

Hi @yuvipanda and @consideRatio

Just wondering if either of you found a solution to the issue with the error message:

Error: unable to build kubernetes objects from release manifest: error validating "": error validating data: ValidationError(Deployment.spec.template.spec.volumes[0].persistentVolumeClaim): missing required field "claimName" in io.k8s.api.core.v1.PersistentVolumeClaimVolumeSource

I'm really keen to be able to use the tool, but I know very little about K8s so struggling to work around the issues I'm having with this.

Thanks!

consideRatio commented 3 years ago

I know the feeling @glsdown, when I started learning about k8s I was a high school teacher trying to make JupyterHub available to my students :)

@glsdown using helm you can configure values that influence how templates of k8s manifests are rendered. In this case, you should configure...

# my-config.yaml, passed to "helm upgrade" as "--values my-config.yaml"
sftp:
  enabled: false

Note that if you want to use the sftp functionality that is enabled by default but requires configuration of sftp.pvc.name, it becomes more complicated and I won't try address that in this answer.

glsdown commented 3 years ago

Thanks @consideRatio

I’ve jumped in at the deep end rather with all this, so very grateful for you taking the time to respond.