Deleting named servers should delete the corresponding PVC

[PMende]

Current state

In the current implementation of z2jh on k8s, when a named server is started, a PVC is created, which is what we want to happen. When a user stops their server, this PVC persists, which we also want to happen. Currently, when a user deletes their named server, the corresponding PVC is not deleted or released.

Persisting the PVC after the deletion of the servers leaves stranded claims that have to be manually cleaned up. Additionally, if a user creates an identically-named server sometime later, their seemingly newly created server will be attached to this stranded claim since the k8s resource names will point at each other. Thus a "newly" created server will not be a clean environment.

Proposed change

The PVC for a named server should be deleted when the named server is deleted.

[consideRatio]

Steps towards resolving this issue

• Investigate to understand and clarify the goal technically
  • 1. A user clicks a red Delete button on JupyterHub, which isn't by itself aware of k8s. What happens then?
  • 2. ...

Related

• Named servers docs
• Spawner docs

My understanding

I think we need to expose an API in the Spawner base class that JupyterHub can use when Delete is pressed. Then, the spawner can be allowed to make cleanup actions such as removing a PVC it has created for the named server, or the user if we delete a user.

I'm moving this issue to KubeSpawner which is responsible for cleaning up the PVC, and then I'm creating a new issue in JupyterHub.

[welcome]

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.

You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[manics]

Can we move this to jupyterhub-idle-culler instead? There's discussion of expanding its role:

• Adding the option to delete persisted user data as well. jupyterhub-idle-culler#8
• How configurable should the culler be? jupyterhub-idle-culler#9

Potentially it could also clean up "orphaned" volumes and other resources. As a standalone service this should be faster to develop than a new Spawner/JupyterHub API.

If we later decided to make it a core part then then the work done in jupyterhub-idle-culler would provide useful input into the core design.

[consideRatio]

@manics hmmm, then the Culler would need to become k8s aware it sounds like to me. If the jupyterhub-idle-culler currently is generic to JupyterHub, I think this issue should stay in KubeSpawner.

Thank you for connecting these related issues together @manics! The question in my mind is not to flesh out a viable implementation path to resolving the issues. With the path I suggest, this belongs in KubeSpawner, but if we would embed k8s logic in jupyterhub-idle-culler, then it would belong there I think.

Hmmm... I think what's requested in the culler, is cleanup of inactive resources, while this is about cleanup of actively deleted user/named-servers. I find them to be related, because the cleanup logic may be the same.

[manics]

The idea of those linked issues is to look at ways of extending the culler, for instance with plugins. It would then be fine to have k8s specific methods in a plugin.

[nsshah1288]

Hi @consideRatio @manics
I am also looking into this issue.

I see in spawner.py this code:

kubespawner/kubespawner/spawner.py

Line 1970 in 40fa25a

self.api.delete_namespaced_pod,

, which makes an API request to delete the pod.

If I wanted to implement this, would I need to add a new method in spawner.py that calls the DELETE persistent volume API request? And then how to call this method from JupyterHub?

Am new to this code so any advice would be great :) Thanks!

[nsshah1288]

Hi @manics @consideRatio @minrk
I have gotten this to work, that when you delete a user from the admin console, it deletes the corresponding PVC and user's storage.
I have only so far tested locally using sqlite-pvc, but I am going to test as well with EFS in a few weeks.

If there is interest in this change, I would be happy to submit a PR, just let me know. Thanks

[octavd]

Hello,

It would be amazing to have that feature implemented in kubespawner. Is there any chance this will be implemented?
@nsshah1288 could you please detalil how you actually implemented this?

[nsshah1288]

hi @octavd ! Check out these two PRs:
jupyterhub/jupyterhub#3337
https://github.com/jupyterhub/kubespawner/pull/475/files

and this thread: jupyterhub/jupyterhub#3197

Unfortunately, I was not able to figure out a test case for kubespawner PR so that one is not approved yet, but the delete_forever method in spawner.py worked for me to delete the PVC.

[octavd]

Hey @nsshah1288 , so i was able to run your modified jupyterhub + kubespawner in an openshift 4 env.
Using the hub admin after clicking on remove user it removed also his PVC. Tried also with the JupyterHub api and it's ok.

Is there any way to speed up this feature to be added in the official branch? It would be very nice that these features available.

[nsshah1288]

Hi @octavd , great that it works for you!
I will try and see if I can figure out the test case, also if you have any idea on how to write the test be my guest :)

[nsshah1288]

Hi @manics @consideRatio could you please review the PR here: #475 ?
I recently fixed the test case. Thanks!