PVC.UID for volume name unique but hard for humans to differentiate

[saad-ali]

Issue raised by @cduchesne

With PR #66 the names of the newly provisioned volumes will be based on the UID of the Kubernetes PVC object. Although the UIDs are unique, the UID does not differ much, making it harder for humans to read and differentiate between them.

Example from @cduchesne:

[root@prometheus csi-scaleio]# for i in {01..10}; do curl -s http://127.0.0.1:8080/api/v1/namespaces/scaleio/persistentvolumeclaims/vol$i | jq '.metadata.uid'; done
"f151bfd8-3751-11e8-ba9f-0cc47ac6c23c"
"f16e31ee-3751-11e8-ba9f-0cc47ac6c23c"
"f18a35c0-3751-11e8-ba9f-0cc47ac6c23c"
"f1a737cc-3751-11e8-ba9f-0cc47ac6c23c"
"f1c6e464-3751-11e8-ba9f-0cc47ac6c23c"
"f1e6f657-3751-11e8-ba9f-0cc47ac6c23c"
"f2034a06-3751-11e8-ba9f-0cc47ac6c23c"
"f223b1e4-3751-11e8-ba9f-0cc47ac6c23c"
"f2422240-3751-11e8-ba9f-0cc47ac6c23c"
"f25d4bec-3751-11e8-ba9f-0cc47ac6c23c"

Since the primary issue here is readability and Kubernetes does the same thing (#66 (comment)), I would consider this issue low pri. It would be nice if we can come up with a better naming scheme that is deterministicly generated from PVC without collisions.

[chakri-nelluri]

+1 for naming based on PVC
For a storage admin, it is impossible to figure out the source of these volumes.

[saad-ali]

Was discussing this with @cduchesne some more. He suggested instead of pvc.uid we could do sha256(pvc.uid) then pull first x characters of the sha https://play.golang.org/p/me6YlbwHhRl

Thoughts?

[saad-ali]

One draw back of this is reverse association becomes more difficult.

[princerachit]

@cduchesne suggestion sounds good. Focusing on reverse association will result in long name which will cause readability issue. eg. pvc-[sha256(uid)_{x chars}]-uid is readable and reversely associated but too long.

[mcronce]

To solve the issue of reverse association as well as making it easy for storage admins to quickly figure out where a volume comes from and what it applies to, what are people's thoughts on (either always or with an option) naming the PV something human-readable, e.g. pvc-namespacename-pvcname? Including the PVC's namespace and name should guarantee uniqueness.

In case anybody's interested in flavor, my use case is to, with cephfs-backed storage, quickly locate state and configuration for my workloads.

@saad-ali I'm happy to make the changes required for this, but want to get input before just throwing a PR over the fence ;)

[princerachit]

@mcronce it is not guaranteed that pvc-namespacename-pvcname will give a unique name.
Consider these two pairs of namespace and name:

1. deploy-primary, vol
2. deploy, primary-vol

Both will result in pvc-deploy-primary-vol

[mcronce]

This is a good point. It makes sense to include the SHA256 (or a portion of it) of the UID, as you mentioned above, tacked on after the PVC namespace and name. In that case, using your examples above, you get pvc-deploy-primary-vol-12fe34dc56ba and pvc-deploy-primary-vol-09ab87cd76ef.

Or maybe that caveat means it makes more sense to put the behavior behind a feature flag, e.g. have --hash-pv-names, default it to true, and document that here's why you might want to set it to false, and here's the trade-off - you will get collisions under these circumstances.

My preference, personally, is to put it behind a flag, default to the current behavior, and include (a portion of) the hash of the UID when the functionality is enabled.

Another idea that would satisfy my use case would be to include the PVC namespace and name as some piece(s) of metadata in the Parameters passed to the CSI driver. This requires more cooperation from the CSI driver, though, than simply using the suggested name...which means code that ends up, in reality, probably being Kubernetes-specific. Functional, but not awesome.

[mcronce]

I have the base functionality I described implemented in mcronce@4d159a9 - I'd be interested in feedback

I also have a follow-up change in mcronce@ed71663 which, when enabled, makes the entire PV name unique but deterministic based on the PVC namespace and name. This behavior is nice because with a StorageClass's reclaimPolicy set to Retain, if a PVC is deleted and recreated it can be reattached to the same piece of storage. However, the provisioner doesn't seem to do that successfully at this time. While I would like it, I don't know whether or not that functionality is desired by the community as a whole :)

[bukzor]

Isn't there prior art for solving this, in the naming of Deployment pods? A human-readable prefix followed by a uniquifying nonce. So pvc "my-thing" would result in pv "my-thing-fviwm".

CC: @bulldozier

[jsafrane]

PV name should be "computable" from PVC, so multiple provisioners create the same PV name (and get conflict if there are multiple provisioners running accidentally). I am not sure "-fviwm" can be generated from PVC so PVCs with the same name in different namspaces would not conflict - there may be thousands of them and we don't want any conflict here. UID is assigned by the cluster and is guaranteed to be unique.

Deployments/ReplicaSets are easy, if they accidentally start one extra replica they can delete it easily without bad things happening. In PV/PVC case it's harder, we really don't want to loose any data.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten