kubeflow / notebooks

Kubeflow Notebooks lets you run web-based development environments on your Kubernetes cluster by running them inside Pods.
Apache License 2.0
18 stars 22 forks source link

Delete Jupyter Notebook volume along with notebook deletion #71

Open Jeffwan opened 5 years ago

Jeffwan commented 5 years ago

/kind feature

Why you need this feature: When user will create a Jupyter Notebook and create new persistent volume, PV doesn't have ownerReference of Jupyter Notebook, the time we delete notebook, PV is in isolated status.

Confirmed with community and it was by design and user may reuse storage some time later. I think GPU training notebook will have more case like this and user like to shutdown GPU notebook frequently to reduce cost.

But I notice another problem users feel it's hard to maintain isolated PV and users/operations are not sure if anyone will use it later and they can not delete notebook.

We need to way to delete associated PV based on user's choice.

Describe the solution you'd like:

  1. When user click delete button, populate a window with a check box to confirm with user if they're like to delete volume as well.

  2. By default, it could be unchecked.

Anything else you would like to add:

issue-label-bot[bot] commented 5 years ago

Issue-Label Bot is automatically applying the label improvement/enhancement to this issue, with a confidence of 0.88. Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

szinck1 commented 4 years ago

I would like this feature as well.

jl-massey commented 4 years ago

This feature is also helpful for use cases related to my users. The ability to select which volumes to delete that are attached to the notebook is valuable for maintaining the system.

Jeffwan commented 4 years ago

/reopen

k8s-ci-robot commented 4 years ago

@Jeffwan: Reopened this issue.

In response to [this](https://github.com/kubeflow/notebooks/issues/71): >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
Jeffwan commented 4 years ago

As we have more users want this feature, I just reopen the issue for tracking purpose

karlschriek commented 4 years ago

Definitely need this

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Jeffwan commented 4 years ago

/reopen

google-oss-prow[bot] commented 3 weeks ago

@Jeffwan: The label(s) kind/feature cannot be applied, because the repository doesn't have them.

In response to [this](https://github.com/kubeflow/notebooks/issues/71): >/kind feature > >**Why you need this feature:** >When user will create a Jupyter Notebook and create new persistent volume, PV doesn't have ownerReference of Jupyter Notebook, the time we delete notebook, PV is in isolated status. > >Confirmed with community and it was by design and user may reuse storage some time later. >I think GPU training notebook will have more case like this and user like to shutdown GPU notebook frequently to reduce cost. > >But I notice another problem users feel it's hard to maintain isolated PV and users/operations are not sure if anyone will use it later and they can not delete notebook. > >We need to way to delete associated PV based on user's choice. > >**Describe the solution you'd like:** >1. When user click delete button, populate a window with a check box to confirm with user if they're like to delete volume as well. > >2. By default, it could be unchecked. > > >**Anything else you would like to add:** Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
k8s-ci-robot commented 4 years ago

@Jeffwan: Reopened this issue.

In response to [this](https://github.com/kubeflow/notebooks/issues/71): >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
mmuppidi commented 4 years ago

This is a feature we require as well. This will be key for use cases where data scientists need to handle sensitive data.

goloxxly commented 4 years ago

We'd also need this function.

davidspek commented 4 years ago

I also see this as an essential function for multi-user environments.

kimwnasptd commented 4 years ago

Since a lot of users have expressed their interest in this I think we should bump its priority.

From a UX side I think the UI should have a checkbox on each volume with Delete with Notebook [ or something similar], which will also be checked by default. Since the user might not want to delete some of the volumes we should allow them to fine grain which ones should be deleted.

Under the hood when the backend creates a PVC it could check if the user requested the volume to be deleted alongside the Notebook. If yes, then the backend will set an ownerReference to the PVC to be owned from the Notebook. This way when the Notebook get's deleted k8s will also garbage collect the PVCs the user requested to be deleted alongside the Notebook.

amostech commented 3 years ago

This is so important for us as well. Any word on whether it has been implemented? We're currently on Kubeflow 1.1.0. For now I'm running the following script manually, the users are kinda mad that they have to run manual solution but it was the only way out, can you guys think of a better idea?

So everytime they delete a jupyter-notebook we have to run:

export POD_NAME=test3
kubectl delete pvc -n admin `kubectl get pvc -n admin | grep ${POD_NAME} | awk '{print $1}'`
kubectl delete pv -n admin `kubectl get pv | grep ${POD_NAME} | awk '{print $1}'`

Note: Replace POD_NAME with the appropriate jupyter-notebook name.

Because if we don't delete the Persistent Volumes and Persistent Volume Claims a user is not allowed to create Jupyter Notebooks anymore. Newly created notebook pods will fail with the following error:

Traceback (most recent call last):
  File "/usr/local/bin/jupyter-notebook", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.6/dist-packages/jupyter_core/application.py", line 268, in launch_instance
    return super(JupyterApp, cls).launch_instance(argv=argv, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/traitlets/config/application.py", line 663, in launch_instance
    app.initialize(argv)
  File "</usr/local/lib/python3.6/dist-packages/decorator.py:decorator-gen-7>", line 2, in initialize
  File "/usr/local/lib/python3.6/dist-packages/traitlets/config/application.py", line 87, in catch_config_error
    return method(app, *args, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/notebook/notebookapp.py", line 1717, in initialize
    self.init_configurables()
  File "/usr/local/lib/python3.6/dist-packages/notebook/notebookapp.py", line 1372, in init_configurables
    connection_dir=self.runtime_dir,
  File "/usr/local/lib/python3.6/dist-packages/traitlets/traitlets.py", line 556, in __get__
    return self.get(obj, cls)
  File "/usr/local/lib/python3.6/dist-packages/traitlets/traitlets.py", line 535, in get
    value = self._validate(obj, dynamic_default())
  File "/usr/local/lib/python3.6/dist-packages/jupyter_core/application.py", line 99, in _runtime_dir_default
    ensure_dir_exists(rd, mode=0o700)
  File "/usr/local/lib/python3.6/dist-packages/jupyter_core/utils/__init__.py", line 13, in ensure_dir_exists
    os.makedirs(path, mode=mode)
  File "/usr/lib/python3.6/os.py", line 210, in makedirs
    makedirs(head, mode, exist_ok)
  File "/usr/lib/python3.6/os.py", line 210, in makedirs
    makedirs(head, mode, exist_ok)
  File "/usr/lib/python3.6/os.py", line 210, in makedirs
    makedirs(head, mode, exist_ok)
  File "/usr/lib/python3.6/os.py", line 220, in makedirs
    mkdir(name, mode)
**PermissionError: [Errno 13] Permission denied: '/home/jovyan/.local'**
davidspek commented 3 years ago

This issue and the linked blog post might also be of interest.

https://github.com/kubeflow/kubeflow/issues/4758

https://journal.arrikto.com/democratizing-the-use-of-pvcs-with-the-introduction-of-a-volume-manager-ui-a5bf616570d3

davidspek commented 3 years ago

Closing this issue as the Volumes Web App that has now been merged should solve this problem. /close

google-oss-robot commented 3 years ago

@DavidSpek: Closing this issue.

In response to [this](https://github.com/kubeflow/notebooks/issues/71): >Closing this issue as the Volumes Web App that has now been merged should solve this problem. >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
kimwnasptd commented 3 years ago

Lets keep this issue open. Allowing the users to more easily manage any dangling PVCs, via the Volumes Manager UI, is definitely going to help but I believe we should also extend the UX of the Notebooks launcher web app.

This is one of the features I would like to tackle for 1.4 and would like to hear some more feedback on some proposals, so I'll reopen the issue

/reopen /lifecycle frozen

google-oss-robot commented 3 years ago

@kimwnasptd: Reopened this issue.

In response to [this](https://github.com/kubeflow/notebooks/issues/71): >Lets keep this issue open. Allowing the users to more easily manage any dangling PVCs, via the Volumes Manager UI, is definitely going to help but I believe we should also extend the UX of the Notebooks launcher web app. > >This is one of the features I would like to tackle for 1.4 and would like to hear some more feedback on some proposals, so I'll reopen the issue > >/reopen >/lifecycle frozen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
davidspek commented 3 years ago

@kimwnasptd Sounds good. Related to this is to allow users to edit notebooks once they are created in the UI, allowing users to resize PVCs and allowing users to set the source for a PVC (so it can clone from an existing PVC or use a snapshot as a source).

Regarding something as specific as deleting a PVC though, that is what the Volumes Web App is specifically created for. This, as well as PVC resizing and setting the source should probably be done in the VWA, but the JWA should contain links to the VWA (or some other kind of integration) to make it easy to go from one to the other. Otherwise, the JWA and VWA might as well be the same application.

alexlatchford commented 3 years ago

For our use case at Zillow we use an AWS EFS volume mount, done via a PodDefault to auto-attach storage to notebooks so ideally also having a way to disable provisioning/mounting any PVCs by default would also be awesome or at least being able to set this new deletion flag at a cluster level so it's not down to users whether they turn this off or not (a lot of our users don't really know much about how to use Kubeflow so trying to make the defaults sane is a strong use case for us!).

juliusvonkohout commented 3 weeks ago

/transfer notebooks