spec icon indicating copy to clipboard operation
spec copied to clipboard

Volume name and volume ID must be different?

Open aibarbetta opened this issue 5 years ago • 4 comments

Hi, opening an issue for this since I can't find the answer in the other ones.

I implemented a CSI driver way back when the spec said something along the lines of "Volume name and volume ID MUST be different". My issue is that I use this driver on k8s and users get confused by the volumeHandle field of the PersistentVolume resource (specifically when doing static provisioning), usually they fill it with the volume name instead of the volume ID, causing CSI failures.

I don't really need to make the volume ID differ from the volume name, so I'm thinking of making them equal. But before doing that I would like to ask, is this not advised? Why?

fyi, the only place in the current spec that seems to talk about this is:

It is worth noting that the plugin-generated volume_id is a REQUIRED field for the DeleteVolume RPC, as opposed to the CO-generated volume name that is REQUIRED for the CreateVolume RPC: these fields MAY NOT contain the same value.

aibarbetta avatar Nov 06 '19 19:11 aibarbetta

Please see https://github.com/container-storage-interface/spec/issues/343

gman0 avatar Nov 06 '19 20:11 gman0

  • As far as I known, for Kubernetes CSI driver, volume ID is generated by storage system or CSI plugin. Volume name is generated by Kubernetes CSI sidecar. https://github.com/kubernetes-csi/external-provisioner/blob/0c56ab07a68cbe7b186d96f2afa8e00b5895066b/pkg/controller/controller.go#L455

  • Most Kubernetes CSI drivers obeys the two equations shown as below. volumeHandle in PV == CreateVolumeResponse.Volume.VolumeId in CSI == Volume Id in storage system the name of PV == CreateVolumeRequest.Name in CSI == Volume Name in storage system

wnxn avatar Nov 07 '19 09:11 wnxn

Thanks for the answers,

@wnxn that's exactly how my driver is working now except for the fact that my storage system does not have volume ids, it just has volume names. So my driver is obeying to:

  • volumeHandle in PV == CreateVolumeResponse.Volume.VolumeId in CSI
  • the name of PV == CreateVolumeRequest.Name in CSI == Volume Name in storage system And volume_id is only used within the CSI and in the volumeHandle of the PV

@gman0 from #343 I understand that having volume_id == volume_name is possible, although there are only clarifications for storage systems that allow setting a volume_id,

Some storage systems allow callers to specify an identifier by which to refer to the newly provisioned storage. If a storage system supports this, it can optionally use this name as the identifier for the new volume.

and no suggestion on how to set these attributes if your storage system does not allow setting a volume_id, or are there?

aibarbetta avatar Nov 07 '19 14:11 aibarbetta

If your storage system is only allowed to set volume name, you cloud set volume name to CSI suggested name and regard the volume name as primary key. https://github.com/container-storage-interface/spec/blob/master/spec.md#createvolume

message CreateVolumeRequest {
  // The suggested name for the storage space. This field is REQUIRED.
  // It serves two purposes:
  // 1) Idempotency - This name is generated by the CO to achieve
  //    idempotency.  The Plugin SHOULD ensure that multiple
  //    `CreateVolume` calls for the same name do not result in more
  //    than one piece of storage provisioned corresponding to that
  //    name. If a Plugin is unable to enforce idempotency, the CO's
  //    error recovery logic could result in multiple (unused) volumes
  //    being provisioned.
  //    In the case of error, the CO MUST handle the gRPC error codes
  //    per the recovery behavior defined in the "CreateVolume Errors"
  //    section below.
  //    The CO is responsible for cleaning up volumes it provisioned
  //    that it no longer needs. If the CO is uncertain whether a volume
  //    was provisioned or not when a `CreateVolume` call fails, the CO
  //    MAY call `CreateVolume` again, with the same name, to ensure the
  //    volume exists and to retrieve the volume's `volume_id` (unless
  //    otherwise prohibited by "CreateVolume Errors").
  // 2) Suggested name - Some storage systems allow callers to specify
  //    an identifier by which to refer to the newly provisioned
  //    storage. If a storage system supports this, it can optionally
  //    use this name as the identifier for the new volume.
  // Any Unicode string that conforms to the length limit is allowed
  // except those containing the following banned characters:
  // U+0000-U+0008, U+000B, U+000C, U+000E-U+001F, U+007F-U+009F.
  // (These are control characters other than commonly used whitespace.)
  string name = 1;

wnxn avatar Nov 07 '19 15:11 wnxn