search

container-storage-interface / spec

Container Storage Interface (CSI) Specification.
Apache License 2.0
1.34k stars 373 forks source link

Undefined behavior for DeleteVolume with snapshots #346

Closed Akrog closed 5 years ago

Akrog commented 5 years ago

I don't see the spec defining the behavior of a DeleteVolume call when the volume being deleted has snapshots.

In most backends deleting a volume with snapshots is not directly possible, one must delete the snapshots first. There are also backends that cascade delete the snapshots (after confirmation) when deleting such volume.

I see 3 possible behaviors, and we should be explicit about which one we expect in the spec:

  1. Fail the delete operation
  2. Cascade delete the snapshots
  3. "Not my problem": The CSI plugin will have to mark the volume as deleted and not display it when listing or allow its use as a volume source or the source of a snapshot, and once all its existing snapshots have been deleted the volume itself should be deleted.

From a storage driver perspective the first option is the most reasonable, and from a CO's perspective the third option is the most convenient, as it decouples volumes and snapshots, which makes sense for cases where snapshots are really off-premise backups that are uploaded as a post-processing operation and the cut snapshot is deleted after the post-processing.

mattsmithdatera commented 5 years ago

My vote is for consistency between plugins in this behavior. Lets have both 1 & 2 by specifying a boolean parameter in DeleteVolumeRequest called purgeSnapshots.

If purgeSnapshots is False, then the delete operation should fail if there are snapshots currently associated with the volume. If purgeSnapshots is True, then volume deletion and snapshot deletion should occur.

As part of DeleteVolumeResponse if purgeSnapshots is True we should have a parameter that allows for returning the snapshotIDs of all snapshots purged by the deletion of the volume for CO book-keeping.

Akrog commented 5 years ago

I like that idea, although I'm not sure if it'd fit all cases, as there was much discussion around remotely storing the data during the review of the snapshot feature.

What is the expected behavior of v1.0? Because if csi-sanity's behavior is a good reference, and I hope it is because I've used it for my CSI plugin, then it's applying option #3.