search

jupyterhub / kubespawner

Kubernetes spawner for JupyterHub
https://jupyterhub-kubespawner.readthedocs.io
BSD 3-Clause "New" or "Revised" License
549 stars 305 forks source link

Question about logic when deleting a PVC #543

Open consideRatio opened 3 years ago

consideRatio commented 3 years ago

This was originally asked by @mriedem here: https://github.com/jupyterhub/kubespawner/pull/475#pullrequestreview-800437128.

I've got a question about this and the warning a couple of lines above. In our deployment we're currently using kubespawner 0.15 and jupyterhub 1.3. I'm looking to upgrade to z2jh 1.2.0, jupyterhub 1.5.0 and kubespawner 1.1.2. We use a single PVC that is backed by cloud object storage so all of the singleuser-server pods share the same PVC. We leave the default of storage_pvc_ensure=False which from looking at the code means kubespawner won't actually create a PVC named self.pvc_name in _start. So shouldn't delete_forever also be checking storage_pvc_ensure? Maybe not if it's assumed that someone could pre-create the PVC using the same pvc_name_template, and in that case we just try to delete the PVC and if it doesn't exist we get a 404 and log the warning.

I guess in our case the point is I don't want to see the warnings when users get culled (which happens after 5 days of inactivity for us) so to avoid that we just need to set delete_pvc=False.

I think @mriedem refers to the warning on line 2686 in this code section.

https://github.com/jupyterhub/kubespawner/blob/fdfa2ad8fb756d52175a0f1515c7c66addc50a55/kubespawner/spawner.py#L2664-L2693

mriedem commented 3 years ago

I think @mriedem refers to the warning on line 2686 in this code section.

Yup 👍

mriedem commented 3 years ago

Confirmed that the warning happens if delete_pvc is not set to False:

Nov 11 19:26:35 hub-6bc49f5445-8ll7b hub WARNING WARNING 2021-11-12T01:26:35.835Z [JupyterHub spawner:2686] No pvc claim-610d58aa08a527753092ff9d to delete. Assuming already deleted.

That was after upgrading to kubespawner 1.1.2 but before making the change in our helm chart to set delete_pvc=False.