-
Notifications
You must be signed in to change notification settings - Fork 100
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
fix(controller): prevent volume deletion if snapshot exists #350
base: develop
Are you sure you want to change the base?
Conversation
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@akhilerm csi-sanity is failing. we need to have waitforSnapDestroy function similar to volume destroy zfs-localpv/pkg/driver/controller.go Line 169 in b32da5a
|
@pawanpraka1 But parameters are not present in the |
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
@pawanpraka1 Updated with the changes for |
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
Signed-off-by: Akhil Mohan <akhil.mohan@mayadata.io>
May I suggest there to be an exception to the constraint that a volume can't be deleted if there is a snapshot created for that volume. Let me explain on the ZFS level... Suppose we have a ZFS volume vol1 an create 3 snapshots for it: vol1@snap1, vol1@snap2, vol1@snap3. vol1 <- vol1@snap1 ZFS can turn these dependencise upside-down with "zfs promote vol2" command (https://openzfs.github.io/openzfs-docs/man/master/8/zfs-promote.8.html) and create this: vol2 <- vol2@snap1 Now after that promotion of vol2, vol1 may be deleted as nothing is dependent on it. So what I suggest is the following exception to the constraint you are adding with this pull request:
Perhaps even a better alternative would be to have a special CRD that would allow execution of "zfs promote" action. Or maybe an attribute on a PVC that would trigger the "zfs promote" action... In that case this pull request can do one thing only. |
Why is this PR required? What issue does it fix?:
What this PR does?:
Does this PR require any upgrade changes?:
No
If the changes in this PR are manually verified, list down the scenarios covered::
csi-zfspv
zfs-snap
from the above pvczfs-snap-clone
from the above snapshotzfs-vol-clone
from the pvccsi-zfspv
csi-zfspv
. Deletion fails with the errorzfs-snap
. The deletion fails with the error in controllerzfs-snap-clone
and the snapshot will be deleted.csi-zfspv
will be failing with errorsince a clone also exists for this volume
8. Delete the clone
zfs-vol-clone
and the volume gets deletedAny additional information for your reviewer? :
NOTE
Currently to find the clones of a snapshot, all volumes are listed and then searched for a matching snapname. This process can be made easier if a label is added to the clone volumes created from a snapshot
Checklist:
<type>(<scope>): <subject>