Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable PVCs for per-user persistent storage #22

Closed
wants to merge 2 commits into from
Closed

Enable PVCs for per-user persistent storage #22

wants to merge 2 commits into from

Conversation

yuvipanda
Copy link
Contributor

Remember that tearing down a cluster doesn't clean up the PVCs
provisioned, and you have to actually delete the PVCs before
tearing down the cluster manually.

This relies on a default storage provisioner being present
on the k8s cluster. On GKE, this gives you 10Gi of stnadard
non-ssd storage.

Remember that tearing down a cluster doesn't clean up the PVCs
provisioned, and you have to actually delete the PVCs before
tearing down the cluster manually.

This relies on a default storage provisioner being present
on the k8s cluster. On GKE, this gives you 10Gi of stnadard
non-ssd storage.
@jlewi
Copy link
Contributor

jlewi commented Dec 8, 2017

I tried this and the PVC doesn't appear to get attached to the pod for the notebook.
Am I missing something?

I tried looking at https://github.com/jupyterhub/kubespawner/blob/master/kubespawner/spawner.py and I see where the claim gets created but I don't see where its attached to the pod.

@yuvipanda
Copy link
Contributor Author

Ah, right. we've to set the volumes too explicitly, see https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/master/images/hub/jupyterhub_config.py#L79 for the extra bits needed. Specifically c.KubeSpawner.volumes and c.KubeSpawner.volume_mounts

@yuvipanda
Copy link
Contributor Author

I updated it!

@jlewi
Copy link
Contributor

jlewi commented Dec 8, 2017

This works. Thanks for the quick fix.

@foxish
Copy link
Contributor

foxish commented Dec 8, 2017

We should add instructions for it to continue to work on Minikube before merging. (or verify if it already does work)

@yuvipanda
Copy link
Contributor Author

This should work on minikube, it ships with a default hostPath-based provisioner.

@foxish
Copy link
Contributor

foxish commented Dec 8, 2017

Ah, sounds good. As far as I know, that's an addon, but I guess it's enabled by default. We should have a line in the docs that says that we depend on the minikube addon for default-storageclass and how one can check it's state. Rest LGTM!

@jlewi
Copy link
Contributor

jlewi commented Dec 8, 2017

Now my notebook pods seems to be hitting some issues when it tries to startup

usermod: no changes
Execute the command as jovyan
[W 2017-12-08 22:49:42.057 SingleUserNotebookApp configurable:168] Config option `open_browser` not recognized by `SingleUserNotebookApp`.  Did you mean `browser`?
Traceback (most recent call last):
  File "/opt/conda/lib/python3.6/site-packages/traitlets/traitlets.py", line 528, in get
    value = obj._trait_values[self.name]
KeyError: 'runtime_dir'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/conda/bin/jupyterhub-singleuser", line 6, in <module>
    main()
  File "/opt/conda/lib/python3.6/site-packages/jupyterhub/singleuser.py", line 455, in main
    return SingleUserNotebookApp.launch_instance(argv)
  File "/opt/conda/lib/python3.6/site-packages/jupyter_core/application.py", line 266, in launch_instance
    return super(JupyterApp, cls).launch_instance(argv=argv, **kwargs)
  File "/opt/conda/lib/python3.6/site-packages/traitlets/config/application.py", line 657, in launch_instance
    app.initialize(argv)
  File "<decorator-gen-7>", line 2, in initialize
  File "/opt/conda/lib/python3.6/site-packages/traitlets/config/application.py", line 87, in catch_config_error
    return method(app, *args, **kwargs)
  File "/opt/conda/lib/python3.6/site-packages/notebook/notebookapp.py", line 1294, in initialize
    self.init_configurables()
  File "/opt/conda/lib/python3.6/site-packages/notebook/notebookapp.py", line 1033, in init_configurables
    connection_dir=self.runtime_dir,
  File "/opt/conda/lib/python3.6/site-packages/traitlets/traitlets.py", line 556, in __get__
    return self.get(obj, cls)
  File "/opt/conda/lib/python3.6/site-packages/traitlets/traitlets.py", line 535, in get
    value = self._validate(obj, dynamic_default())
  File "/opt/conda/lib/python3.6/site-packages/jupyter_core/application.py", line 99, in _runtime_dir_default
    ensure_dir_exists(rd, mode=0o700)
  File "/opt/conda/lib/python3.6/site-packages/jupyter_core/utils/__init__.py", line 13, in ensure_dir_exists
    os.makedirs(path, mode=mode)
  File "/opt/conda/lib/python3.6/os.py", line 210, in makedirs
    makedirs(head, mode, exist_ok)
  File "/opt/conda/lib/python3.6/os.py", line 210, in makedirs
    makedirs(head, mode, exist_ok)
  File "/opt/conda/lib/python3.6/os.py", line 210, in makedirs
    makedirs(head, mode, exist_ok)
  File "/opt/conda/lib/python3.6/os.py", line 220, in makedirs
    mkdir(name, mode)
PermissionError: [Errno 13] Permission denied: '/home/jovyan/.local'

I'm going to dry deleting the PV, PVC

@yuvipanda
Copy link
Contributor Author

Where are you running this on? It looks like the user can't write to the provisioned pv. I've forgotten how that works on GKE...

@jlewi
Copy link
Contributor

jlewi commented Dec 8, 2017

I'm running on GKE. I'm guessing the directory is owned by root so the fact that the container is running as user joyuan is a problem.

@jlewi
Copy link
Contributor

jlewi commented Dec 8, 2017

Yup the root directory of the PD is owned by root. I was able to modify the config so I mount the pd in a subdirectory of my home directory. I could then kubectl exec into the container to change the permissions to make it world readable.

I'm not sure how we would automate this.

@foxish
Copy link
Contributor

foxish commented Dec 8, 2017

Can we just run the jupyter containers in this configuration as Unix root? Every user gets a dedicated pod+pd anyway. The per-user model may make more sense in a shared NFS type of setup perhaps.

@jlewi
Copy link
Contributor

jlewi commented Dec 8, 2017

That's one option. It looks the juptyer container runs the script start-singleuser.sh as root and then does an su to joyuan. So presumably the startup script could create needed directories and set permissions before issuing the su.

@foxish
Copy link
Contributor

foxish commented Dec 8, 2017

That sounds like a good idea. To either do it in the startup script, or an init container, in the interest of disallowing users from changing certain aspects of their pod. We should ensure that this is all truly portable as well.

@yuvipanda
Copy link
Contributor Author

yuvipanda commented Dec 9, 2017 via email

@vishh
Copy link
Contributor

vishh commented Dec 9, 2017

Yeah, the trick is to use a filesystem group in the pod spec which will be a gid that will be applied to the filesystem (sticky gid) and also get set on the container process group. Here is an example. It's unfortunate that the default behavior in GKE does not support this. Container images are not easily introspectable.

@vishh
Copy link
Contributor

vishh commented Dec 9, 2017

Do we have to update kubespawner to set fsGroup or is it already supported?

@yuvipanda
Copy link
Contributor Author

Yep, we can set it with c.KubeSpawner.singleuser_fs_gid. What's the gid on the default image? 1000? I agree it's sad that it can't be easily introspected and has to be set explicitly.

@UnionGlobal
Copy link

◦	 FGBJQ

"}},705da7d71921eb85673912784f0d2c1b51],["CurrentCommunityInitialData",[],{},490],["ResourceWatcher",[],{"module":null},1404],["MPageletUtilities",[],{},358],["MJSEnvironment",[],{"IS_APPLE_WEBKIT_IOS":true,"IS_TABLET":false,"IS_ANDROID":false,"IS_CHROME":false,"IS_FIREFOX":false,"IS_WINDOWS_PHONE":false,"OS_VERSION":6.1,"PIXEL_RATIO":2,"BROWSER_NAME":"Mobile Safari"},46],["ZeroCategoryHeader",[],{},1127],["ZeroRewriteRules",[],{},1478],["UserAgentData",[],{"browserArchitecture":"32","browserFullVersion":
20799920_1500726386641166_711419247657835507_n.jpg
AeahgoYEfVcoFi1E

@jlewi
Copy link
Contributor

jlewi commented Dec 15, 2017

I suggest closing this PR in favor of #36. #36 introduces ksonnet configs and includes the ability to attach extra volumes in the JupyterConfig. If we need to add additional options like fsGroupId we can do so in our ksonnet configs.

@yuvipanda
Copy link
Contributor Author

The ksonnet stuff is great! Going to close this now :)

@yuvipanda yuvipanda closed this Dec 16, 2017
@yuvipanda yuvipanda deleted the user-storage branch December 16, 2017 21:03
@inc0 inc0 mentioned this pull request Feb 21, 2018
@akshaysaxena16
Copy link

Hi,

I have followed these steps https://github.com/kubeflow/kubeflow/blob/master/user_guide.md to deploy kubeflow.
I'm able to see Jupyter notebook interface after logging into the jupyter hub.
now to add persistent volume if I follow the steps mentioned here https://github.com/kubeflow/kubeflow/pull/22/files/48ffe99d30eb31abc4d54c4553483b6c5b2034bc and make these changes in jupyter_spawner.py file.
tf-hub goes into CrashLoopBackOff error saying
IndentationError: unexpected indent

Can you suggest some solution to this to add persistent volume ?

@jlewi
Copy link
Contributor

jlewi commented Feb 22, 2018

@akshaydtada Can you open a new issue please? @inc0 made some recent changes. I suspect an indent error in kube_spawner.py

jlewi pushed a commit that referenced this pull request Feb 23, 2018
While we were adding pvc for every jupyter instance, we didn't mount it
anywhere.
Let's mount it to work dir, as I assume this is dir where users will
likely put their notebooks. This will ensure that work will be retained
even if pod dies.

Fix #19
Fix #22
@jlewi jlewi mentioned this pull request Mar 1, 2018
jlewi added a commit to jlewi/kubeflow that referenced this pull request May 18, 2018
… node pool.

* Fix kubeflow#22; test flakes by making the GKE resource names depend on the deployment
name so multiple deploymnets won't reference the same resources.
yanniszark pushed a commit to arrikto/kubeflow that referenced this pull request Feb 15, 2021
Signed-off-by: YujiOshima <yuji.oshima0x3fd@gmail.com>
VaishnaviHire pushed a commit to VaishnaviHire/kubeflow that referenced this pull request Jul 1, 2022
[pull] master from kubeflow:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants