Merge pull request rook#14134 from subhamkrai/add-validation-scds

osd: limit storageClassDeviceSets to 63 chars

deploy/charts/rook-ceph/templates/resources.yaml

@@ -3523,6 +3523,7 @@ spec:
                             type: boolean
                           name:
                             description: Name is a unique identifier for the set
+                            maxLength: 40
                             type: string
                           placement:
                             nullable: true

deploy/examples/crds.yaml

@@ -3521,6 +3521,7 @@ spec:
                             type: boolean
                           name:
                             description: Name is a unique identifier for the set
+                            maxLength: 40
                             type: string
                           placement:
                             nullable: true

pkg/apis/ceph.rook.io/v1/types.go

@@ -2937,6 +2937,7 @@ type PriorityClassNamesSpec map[KeyType]string
 // +nullable
 type StorageClassDeviceSet struct {
 	// Name is a unique identifier for the set
+	// +kubebuilder:validation:MaxLength=40
 	Name string `json:"name"`
 	// Count is the number of devices in this set
 	// +kubebuilder:validation:Minimum=1

pkg/operator/ceph/cluster/osd/deviceSet.go

@@ -205,16 +205,20 @@ func (c *Cluster) createDeviceSetPVC(existingPVCs map[string]*v1.PersistentVolum
 	// old labels and PVC ID for backward compatibility
 	pvcID := legacyDeviceSetPVCID(deviceSetName, setIndex)
 
+	var err error
 	// check for the existence of the pvc
 	existingPVC, ok := existingPVCs[pvcID]
 	if !ok {
 		// The old name of the PVC didn't exist, now try the new PVC name and label
-		pvcID = deviceSetPVCID(deviceSetName, pvcTemplate.GetName(), setIndex)
+		pvcID, err = deviceSetPVCID(deviceSetName, pvcTemplate.GetName(), setIndex)
+		if err != nil {
+			return nil, err
+		}
 		existingPVC = existingPVCs[pvcID]
 	}
 
 	pvc := makeDeviceSetPVC(deviceSetName, pvcID, setIndex, pvcTemplate, c.clusterInfo.Namespace, createValidImageVersionLabel(c.spec.CephVersion.Image), createValidImageVersionLabel(c.rookVersion))
-	err := c.clusterInfo.OwnerInfo.SetControllerReference(pvc)
+	err = c.clusterInfo.OwnerInfo.SetControllerReference(pvc)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to set owner reference to osd pvc %q", pvc.Name)
 	}
@@ -291,10 +295,14 @@ func legacyDeviceSetPVCID(deviceSetName string, setIndex int) string {
 
 // This is the new function that generates the labels
 // It includes the pvcTemplateName in it
-func deviceSetPVCID(deviceSetName, pvcTemplateName string, setIndex int) string {
+func deviceSetPVCID(deviceSetName, pvcTemplateName string, setIndex int) (string, error) {
 	cleanName := strings.Replace(pvcTemplateName, " ", "-", -1)
 	deviceSetName = strings.Replace(deviceSetName, ".", "-", -1)
-	return fmt.Sprintf("%s-%s-%d", deviceSetName, cleanName, setIndex)
+	pvcID := fmt.Sprintf("%s-%s-%d", deviceSetName, cleanName, setIndex)
+	if len(pvcID) > 62 {
+		return "", fmt.Errorf("the OSD PVC name requested is %q (length %d) is too long and must be less than 63 characters, shorten either the storageClassDeviceSet name or the storage class name", pvcID, len(pvcID))
+	}
+	return pvcID, nil
 }
 
 func createValidImageVersionLabel(image string) string {

pkg/operator/ceph/cluster/osd/deviceset_test.go

@@ -280,16 +280,20 @@ func TestPrepareDeviceSetsWithCrushParams(t *testing.T) {
 }
 
 func TestPVCName(t *testing.T) {
-	id := deviceSetPVCID("mydeviceset", "a", 0)
-	assert.Equal(t, "mydeviceset-a-0", id)
+	id, err := deviceSetPVCID("mydeviceset-making-the-length-of-pvc-id-greater-than-the-limit-63", "a", 0)
+	assert.Error(t, err)
+	assert.Equal(t, "", id)
 
-	id = deviceSetPVCID("mydeviceset", "a", 10)
+	id, err = deviceSetPVCID("mydeviceset", "a", 10)
+	assert.NoError(t, err)
 	assert.Equal(t, "mydeviceset-a-10", id)
 
-	id = deviceSetPVCID("device-set", "a", 10)
+	id, err = deviceSetPVCID("device-set", "a", 10)
+	assert.NoError(t, err)
 	assert.Equal(t, "device-set-a-10", id)
 
-	id = deviceSetPVCID("device.set.with.dots", "b", 10)
+	id, err = deviceSetPVCID("device.set.with.dots", "b", 10)
+	assert.NoError(t, err)
 	assert.Equal(t, "device-set-with-dots-b-10", id)
 }