spec icon indicating copy to clipboard operation
spec copied to clipboard

Pass Topology Information for DeleteVolume

Open leakingtapan opened this issue 6 years ago • 5 comments

With latest CSI spec, topology information is passed into CreateVolume call, so that CSI driver can provision a volume by the given TopologyRequirement. The topology requirement could be region, zones, or racks. This enables CSI driver to provision volume in different regions. Eg. a CSI driver could provision volume is both us-west-2 and us-east-1. However, due to topology information is missing from DeleteVolume call, a CSI driver won't be able to delete a volume for different regions.

In order to keep API symmetric between CreateVolume and DeleteVolume, topology information should be passed into DeleteVolume call. So that a CSI driver controller service could manage volumes from different regions for clusters that have worker nodes in many different regions.

/cc @gnufied @msau42

leakingtapan avatar Jan 14 '20 08:01 leakingtapan

Generally, the volumedelete call cannot pass in information other than ID because it may not always available in the teardown case.

The way that we implemented it in the GCP PD driver is to encode zone/region information in the volume id.

msau42 avatar Jan 15 '20 00:01 msau42

The way that we implemented it in the GCP PD driver is to encode zone/region information in the volume id.

Yep. that's something could be achieved at driver side.

Generally, the volumedelete call cannot pass in information other than ID because it may not always available in the teardown case

Could you give some example of when the topology info is not available? One case I can think of is pre-provisioned volume that doesn't have nodeAffinity constraint. But in that case, driver could still fall back to some default region that the driver thinks appropriate.

And where do we have such doc about encoding region in volumeHandle from the CSI spec? This feels more of a workaround than best practice for me.

leakingtapan avatar Jan 15 '20 01:01 leakingtapan

Same problem with VolumeContext missing in DeleteVolume, this is really inconvenient and a lot of drivers end up with encoding all the information into volume name... which is of course ugly as hell

vitalif avatar Jul 16 '21 20:07 vitalif

Same problem with VolumeContext missing in DeleteVolume, this is really inconvenient and a lot of drivers end up with encoding all the information into volume name... which is of course ugly as hell

@vitalif agree on this, VolumeContext should also be in DeleteVolumeRequest, currently we have to encoding all info as a volumeID, now the volumeID is longer and longer, that's really ugly design. added a new feature request here: https://github.com/container-storage-interface/spec/issues/507

andyzhangx avatar Apr 18 '22 06:04 andyzhangx

https://github.com/container-storage-interface/spec/issues/507#issuecomment-1101947052

jdef avatar Apr 19 '22 03:04 jdef