Skip to content

Undefined behavior for DeleteVolume with snapshots #346

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

Closed
Akrog opened this issue Jan 25, 2019 · 2 comments · Fixed by #347
Closed

Undefined behavior for DeleteVolume with snapshots #346

Akrog opened this issue Jan 25, 2019 · 2 comments · Fixed by #347
Milestone

Comments

@Akrog
Copy link
Contributor

Akrog commented Jan 25, 2019

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.

Akrog added a commit to Akrog/spec that referenced this issue Jan 25, 2019
Spec is not clear on what should happen when on a DeleteVolume call when
the volume has snapshots.

This patch clarifies the situation by explicitly mentioning that the
operation should complete and the snapshots should still be operational.

Closes: container-storage-interface#346
Akrog added a commit to Akrog/ember-csi that referenced this issue Jan 25, 2019
This patch adds a soft-delete mechanism for volumes so they can be
"deleted" even when they have snapshots.

The expected behavior of deleting a volume with snapshots is not well
defined in the CSI spec, as stated in issue
container-storage-interface/spec#346 but until
this is clarified we will assume that PR #347 will be accepted and we
must behave like this.

This is also how the csi-sanity test suite seems to expect us to behave.

Close: embercsi#70
@mattsmithdatera
Copy link

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
Copy link
Contributor Author

Akrog commented Jan 26, 2019

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.

Akrog added a commit to Akrog/spec that referenced this issue Feb 27, 2019
Spec is not clear on what should happen when on a DeleteVolume call when
the volume has snapshots.

This patch clarifies the situation by explicitly mentioning that the
operation should complete and the snapshots should still be operational.

Closes: container-storage-interface#346
Akrog added a commit to Akrog/spec that referenced this issue Mar 4, 2019
Spec is not clear on what should happen when on a DeleteVolume call when
the volume has snapshots.

This patch clarifies the situation by explicitly mentioning that the
operation should complete and the snapshots should still be operational.

Closes: container-storage-interface#346
@saad-ali saad-ali added this to the v1.2 milestone Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants