Skip to content

Commit

Permalink
osd: limit storageClassDeviceSets to 63 chars
Browse files Browse the repository at this point in the history
there are some cases where storageClassDeviceSets length
are more 63 chars and osd provisioned failed due to this.
with error
```
 failed to run osd provisioning job for PVC "ocs-deviceset-*-local-volume-storageclass-0-data-*": Job.batch "rook-ceph-osd-prepare-*" is invalid: [spec.template.spec.volumes[10].name: Invalid value: "ocs-deviceset-*-local-volume-storageclass-0-data-*": must be no more than 63 characters, spec.template.spec.containers[0].volumeMounts[9].name: Not found: "ocs-deviceset-*-local-volume-storageclass-0-data-*",
```

Using x-validator to limit the maxLength to the deviceClassSet.Name to
40 chars and adding code check that overall pvc name should be less than
63 chars.

Signed-off-by: subhamkrai <[email protected]>
  • Loading branch information
subhamkrai committed May 3, 2024
1 parent 381556a commit 6f5cc9b
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 9 deletions.
1 change: 1 addition & 0 deletions deploy/charts/rook-ceph/templates/resources.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3523,6 +3523,7 @@ spec:
type: boolean
name:
description: Name is a unique identifier for the set
maxLength: 40
type: string
placement:
nullable: true
Expand Down
1 change: 1 addition & 0 deletions deploy/examples/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3521,6 +3521,7 @@ spec:
type: boolean
name:
description: Name is a unique identifier for the set
maxLength: 40
type: string
placement:
nullable: true
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/ceph.rook.io/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 12 additions & 4 deletions pkg/operator/ceph/cluster/osd/deviceSet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 9 additions & 5 deletions pkg/operator/ceph/cluster/osd/deviceset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down

0 comments on commit 6f5cc9b

Please sign in to comment.