Skip to content

CreateVolumeRequest: clarify additional rules, if any, for volume names #4

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
jdef opened this issue May 23, 2017 · 4 comments
Closed

Comments

@jdef
Copy link
Member

jdef commented May 23, 2017

The volume name in this request is a CO-generated string. Such names are likely to be incompatible with a variety of backend storage implementations (filesystem labels, partition labels, disk labels, logical volume tags all have different requirements/restrictions).

Part of a possible solution to the problem is for plugins to hash the volume name field. Depending on the hash used it may be difficult to for a storage admin to later determine the identity of a volume without help from the CO (consider that the CO had a brain meltdown and some poor admin is trying to pick up the pieces).

Another part of a possible solution is to impose restrictions on length, case, character set, etc. such that hash functions implemented by plugins may attempt to preserve as much of the original CO-generated name as possible (as far as it complies with their internal naming scheme and uniqueness constraints).

Or .. there's probably other ways to slice this problem too. Ideas welcome.

@Jgoldick
Copy link

I'd suggest that the Plugin take responsibility for mapping or rehashing illegal names into compliant ones, this has worked reasonably well in Cinder/OpenStack.

One gotcha to keep in mind is if the CO doesn't use a hash function but instead uses some form of human-readable names. Got seriously burned in the past when multiple admins wanted to use the same Volume name so they used case-permuted versions of the same UTF8 string, as in mysqlvol1 and MySQLVol1. While it's incredibly unlikely that two hashed names will be case-permuted versions of each other it's a total disaster if that happens in practice.

@gpaul
Copy link

gpaul commented Jul 11, 2017

The spec provides for a unique VolumeID per volume, returned in the CreateVolumeResponse. The name field in the CreateVolumeRequest is described as a "Suggested name". This leaves the plugin the option of modifying the given name in whatever way is convenient.

I see three options for a plugin that supports volume naming using a limited alphabet:

  1. Clean the volume name and use that as the Volume ID. As the source alphabet is larger than the target alphabet, users will get "Volume already exists" errors when they might not expect it, due to case-insensitivity, etc.

  2. Clean the volume name and append a N-bit hash of the original string. This allows users to specify arbitrary volume names but adds the risk of names colliding. Should the volume name+hash collide the user will get a "Volume already exists" error but will find that surprising.

  3. Given that any volume label limitation arises with the storage backend to begin with, attempt to create the volume with the given name and pass any resulting error to the user.

I tend towards (3.) as it simplifies the CSI plugin and makes them more future-proof. I imagine a scenario where AWS lifts some volume naming restriction and now everyone who wants to make use of that have to update their EBS CSI Volume Plugin to do so.

@gpaul
Copy link

gpaul commented Jul 11, 2017

The storage backend is the source of truth - we should use it as such.

saad-ali added a commit to saad-ali/spec that referenced this issue May 30, 2018
@saad-ali
Copy link
Member

We decided that we will let the plugin handle this case. We will tighten up name field requirements in #267

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants