Add DeleteSnapshot handler for VolumeSnapshot support #33
Conversation
Signed-off-by: hlts2 <[email protected]>
Signed-off-by: hlts2 <[email protected]>
Signed-off-by: hlts2 <[email protected]>
Signed-off-by: hlts2 <[email protected]>
Signed-off-by: hlts2 <[email protected]>
pkg/driver/controller_server.go
Outdated
| @@ -590,7 +591,32 @@ func (d *Driver) CreateSnapshot(context.Context, *csi.CreateSnapshotRequest) (*c | |||
| } | |||
|
|
|||
| // DeleteSnapshot is part of implementing Snapshot & Restore functionality, but we don't support that | |||
There was a problem hiding this comment.
Should "but we don't support that" be removed from the comment now?
There was a problem hiding this comment.
Thank you for your comment 🙏
I agree; this should be revised! However, removing it entirely might suggest that this feature is already supported, so I believe it would be better to adjust the phrasing.
How about the following comment? 🤔
DeleteSnapshot is part of implementing Snapshot & Restore functionality, and it will be supported in the future.
There was a problem hiding this comment.
Looks good, but will have to be updated later.
something like, "// DeleteSnapshot is implemented to support Snapshot & Restore functionality, allowing snapshots to be deleted as needed." can work too. As this code will be merged only when all pieces are in place I guess. what do you say?
There was a problem hiding this comment.
Yes, it looks good to me! Thank you 🙏
As this code will be merged only when all pieces are in place I guess. what do you say?
I was talking with Ryota about this point. Since this implementation does not particularly affect the existing behavior, we can merge this PR!
…error and update comment Signed-off-by: hlts2 <[email protected]>
WHAT
I have implemented DeleteSnapshot handler.
WHY
We plan to add support for VolumeSnapshot in the civo-csi driver.
To achieve this, we need to implement the following handlers.
In this PR, I have implemented the DeleteSnapshot handler.
FYI