kubernetes-sigs / sig-storage-local-static-provisioner

Static provisioner of local volumes
Apache License 2.0
1.06k stars 329 forks source link

Delete local PV If directory not found #6

Open msau42 opened 5 years ago

msau42 commented 5 years ago

Migrating issue from: https://github.com/kubernetes-incubator/external-storage/issues/1052

/help

k8s-ci-robot commented 5 years ago

@msau42: This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/kubernetes-sigs/sig-storage-local-static-provisioner/issues/6): >Migrating issue from: https://github.com/kubernetes-incubator/external-storage/issues/1052 > >/help 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.
msau42 commented 5 years ago

/kind bug

cofyc commented 5 years ago

Thinking more about this (previous thoughts). It's hard to see delete local PV if directory not found is a good idea.

Possible scenarios when the volume is not found on deletion:

If we deleted PVs in these cases in provisioner, users are in charge of cleaning up volumes themselves which is a burden, and if users forgot to clean, data in volumes may be leaked to other programs in the future.

Instead of letting users clean up volumes, I think the best practice is to delete PV objects if users know volumes of these PVs are already cleaned or deleted and they want to clean these stale PV objects. And it's totally ok to not delete them if they are not sure. When a node gets recreated, users can delete all local PVs on it which is required too when a node is deleted.

(Updated: You can set owner references on PV objects, then they can be deleted when node objects are deleted)

Conversely, I think we should check mount point existence on deletion because if no mount point exists on the directory, we actually didn't clean local volume and should not delete PV objects. With this change, volumes are not able to be deleted if they are not mounted on reboot. When disks are mounted again, data can be cleaned and PV objects will be recycled by provisioner.

cofyc commented 5 years ago

/assign

fejta-bot commented 5 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot commented 5 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle rotten

cofyc commented 5 years ago

/lifecycle frozen