core: revert increase storage deviceset name limit to 50

[subhamkrai]

Checklist:

• Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
• Reviewed the developer guide on Submitting a Pull Request
• Pending release notes updated with breaking and/or notable changes for the next minor release.
• Documentation has been updated, if necessary.
• Unit tests have been added, if necessary.
• Integration tests have been added, if necessary.

[BlaineEXE]

From my read of the code, this pvcID is used in the generateName field of a PersistentVolumeClaim object. If that's the case, isn't this check still valuable to users? We don't want to allow users to input names that are so long that Rook's install breaks.

[BlaineEXE]

After chat with Subham, I understand that the bigger overall issue that is that even if the deviceSetName is limited to 50 chars, the pvcTemplateName can still be up to 62/63 chars. It would be better to fix this by hashing some portion of the name if it exceeds 62 chars.

With just the 50-char limit, the case where the deviceSetName is 51 chars but the pvcTemplateName is only 3 chars should still be valid (less than 63 chars), but the commit being reverted would block that if users have this upgrade condition.

With deviceSetName limited to 50 chars, and pvcTemplateName to 63, this could be up to 113 chars long just with those 2. Then 2 dashes (-), and an index that in theory could be 4 chars long in a ridiculous cluster. That's 119 chars.

To really fix this issue, we have to reduce these 119 chars to 62. If necessary, we can stop using the PVC ID (name) as a static value and instead use labels to Get() the right PVC later using labels like pvcTemplateName=X, deviceSetName=Y, deviceSetIndex=Z.

Let's take this revert, and then make a follow up issue and PR to reduce the naming. A version of this function's hash strategy might be appropriate:

rook/pkg/operator/k8sutil/volume.go

Lines 35 to 74 in 80a96f8

	// PathToVolumeName converts a path to a valid volume name
	func PathToVolumeName(path string) string {
	// kubernetes volume names must match this regex: [a-z0-9]([-a-z0-9]*[a-z0-9])?
	sanitized := make([]rune, 0, len(path))
	for _, c := range path {
	switch {
	case '0' <= c && c <= '9':
	fallthrough
	case 'a' <= c && c <= 'z':
	sanitized = append(sanitized, c)
	continue
	case 'A' <= c && c <= 'Z':
	// convert upper to lower case
	sanitized = append(sanitized, c+('a'-'A'))
	default:
	// convert any non alphanum char to a hyphen
	sanitized = append(sanitized, '-')
	}
	}
	volumeName := string(sanitized)
	// trim any leading/trailing hyphens
	volumeName = strings.TrimPrefix(volumeName, "-")
	volumeName = strings.TrimSuffix(volumeName, "-")
	if len(volumeName) > validation.DNS1123LabelMaxLength {
	// keep an equal sample of the original name from both the beginning and from the end,
	// and add some characters from a hash of the full name to help prevent name collisions.
	// Make room for 3 hyphens in the middle (~ellipsis) and 1 hyphen to separate the hash.
	hashLength := 8
	sampleLength := int((validation.DNS1123LabelMaxLength - hashLength - 3 - 1) / 2)
	first := volumeName[0:sampleLength]
	last := volumeName[len(volumeName)-sampleLength:]
	hash := Hash(volumeName)
	volumeName = fmt.Sprintf("%s---%s-%s", first, last, hash[0:hashLength])
	}
	return volumeName
	}