From 1dcc99ec1d899429259ba00157a78fcf33602681 Mon Sep 17 00:00:00 2001 From: subhamkrai Date: Thu, 20 Jun 2024 10:51:14 +0530 Subject: [PATCH 1/4] core: increase storage deviceset name limit to 50 initially, we have the limit of 40 char set on storageclassdeviceset.Name but seems like that is not enough and upgrade clusters can face issue with this. So, increasing the limit to 50. Signed-off-by: subhamkrai --- deploy/charts/rook-ceph/templates/resources.yaml | 2 +- deploy/examples/crds.yaml | 2 +- pkg/apis/ceph.rook.io/v1/types.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml index da00db82867a..6727b5ba7d08 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -3556,7 +3556,7 @@ spec: type: boolean name: description: Name is a unique identifier for the set - maxLength: 40 + maxLength: 50 type: string placement: nullable: true diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml index fe48be2f60ed..70dfe726e35f 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -3554,7 +3554,7 @@ spec: type: boolean name: description: Name is a unique identifier for the set - maxLength: 40 + maxLength: 50 type: string placement: nullable: true diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index d55ae3ccff20..abd64d7c4743 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -2968,7 +2968,7 @@ type PriorityClassNamesSpec map[KeyType]string // +nullable type StorageClassDeviceSet struct { // Name is a unique identifier for the set - // +kubebuilder:validation:MaxLength=40 + // +kubebuilder:validation:MaxLength=50 Name string `json:"name"` // Count is the number of devices in this set // +kubebuilder:validation:Minimum=1 From c7d07562c4b7fb77215aca84add06ff9f6766409 Mon Sep 17 00:00:00 2001 From: subhamkrai Date: Fri, 21 Jun 2024 11:20:25 +0530 Subject: [PATCH 2/4] core: revert increase storage deviceset name limit to 50 This reverts commit 1dcc99ec1d899429259ba00157a78fcf33602681. Signed-off-by: subhamkrai --- deploy/charts/rook-ceph/templates/resources.yaml | 2 +- deploy/examples/crds.yaml | 2 +- pkg/apis/ceph.rook.io/v1/types.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml index 6727b5ba7d08..da00db82867a 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -3556,7 +3556,7 @@ spec: type: boolean name: description: Name is a unique identifier for the set - maxLength: 50 + maxLength: 40 type: string placement: nullable: true diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml index 70dfe726e35f..fe48be2f60ed 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -3554,7 +3554,7 @@ spec: type: boolean name: description: Name is a unique identifier for the set - maxLength: 50 + maxLength: 40 type: string placement: nullable: true diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index abd64d7c4743..d55ae3ccff20 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -2968,7 +2968,7 @@ type PriorityClassNamesSpec map[KeyType]string // +nullable type StorageClassDeviceSet struct { // Name is a unique identifier for the set - // +kubebuilder:validation:MaxLength=50 + // +kubebuilder:validation:MaxLength=40 Name string `json:"name"` // Count is the number of devices in this set // +kubebuilder:validation:Minimum=1 From e8aeb0d2633473dc97b956e451e71ab0e148d046 Mon Sep 17 00:00:00 2001 From: subhamkrai Date: Fri, 21 Jun 2024 11:20:30 +0530 Subject: [PATCH 3/4] osd: revert limit storageClassDeviceSets to 63 chars This reverts commit 6f5cc9b01dd78924dc6bb8cff1f7215e6286aa0c. Signed-off-by: subhamkrai --- deploy/charts/rook-ceph/templates/resources.yaml | 1 - deploy/examples/crds.yaml | 1 - pkg/apis/ceph.rook.io/v1/types.go | 1 - pkg/operator/ceph/cluster/osd/deviceSet.go | 16 ++++------------ pkg/operator/ceph/cluster/osd/deviceset_test.go | 14 +++++--------- 5 files changed, 9 insertions(+), 24 deletions(-) diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml index da00db82867a..b0b909bd726b 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -3556,7 +3556,6 @@ spec: type: boolean name: description: Name is a unique identifier for the set - maxLength: 40 type: string placement: nullable: true diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml index fe48be2f60ed..bde3e25ef972 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -3554,7 +3554,6 @@ spec: type: boolean name: description: Name is a unique identifier for the set - maxLength: 40 type: string placement: nullable: true diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index d55ae3ccff20..8a376257334e 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -2968,7 +2968,6 @@ 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 diff --git a/pkg/operator/ceph/cluster/osd/deviceSet.go b/pkg/operator/ceph/cluster/osd/deviceSet.go index fbbfa9080d5f..0b987212b2a5 100644 --- a/pkg/operator/ceph/cluster/osd/deviceSet.go +++ b/pkg/operator/ceph/cluster/osd/deviceSet.go @@ -205,20 +205,16 @@ 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, err = deviceSetPVCID(deviceSetName, pvcTemplate.GetName(), setIndex) - if err != nil { - return nil, err - } + pvcID = deviceSetPVCID(deviceSetName, pvcTemplate.GetName(), setIndex) 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) } @@ -295,14 +291,10 @@ 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, error) { +func deviceSetPVCID(deviceSetName, pvcTemplateName string, setIndex int) string { cleanName := strings.Replace(pvcTemplateName, " ", "-", -1) deviceSetName = strings.Replace(deviceSetName, ".", "-", -1) - 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 + return fmt.Sprintf("%s-%s-%d", deviceSetName, cleanName, setIndex) } func createValidImageVersionLabel(image string) string { diff --git a/pkg/operator/ceph/cluster/osd/deviceset_test.go b/pkg/operator/ceph/cluster/osd/deviceset_test.go index 8f3d0c55076f..cbffcbdc4ece 100644 --- a/pkg/operator/ceph/cluster/osd/deviceset_test.go +++ b/pkg/operator/ceph/cluster/osd/deviceset_test.go @@ -280,20 +280,16 @@ func TestPrepareDeviceSetsWithCrushParams(t *testing.T) { } func TestPVCName(t *testing.T) { - 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", 0) + assert.Equal(t, "mydeviceset-a-0", id) - id, err = deviceSetPVCID("mydeviceset", "a", 10) - assert.NoError(t, err) + id = deviceSetPVCID("mydeviceset", "a", 10) assert.Equal(t, "mydeviceset-a-10", id) - id, err = deviceSetPVCID("device-set", "a", 10) - assert.NoError(t, err) + id = deviceSetPVCID("device-set", "a", 10) assert.Equal(t, "device-set-a-10", id) - id, err = deviceSetPVCID("device.set.with.dots", "b", 10) - assert.NoError(t, err) + id = deviceSetPVCID("device.set.with.dots", "b", 10) assert.Equal(t, "device-set-with-dots-b-10", id) } From eafb58a8a52851bb614433f30367800d31680d1a Mon Sep 17 00:00:00 2001 From: Ceph Jenkins Date: Sun, 23 Jun 2024 04:06:12 -0400 Subject: [PATCH 4/4] csv: add additional csv changes that other commits bring add generated csv changes Signed-off-by: Ceph Jenkins --- build/csv/ceph/ceph.rook.io_cephclusters.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/build/csv/ceph/ceph.rook.io_cephclusters.yaml b/build/csv/ceph/ceph.rook.io_cephclusters.yaml index 4e7b1f2b4d1e..9b8f4ad51ea3 100644 --- a/build/csv/ceph/ceph.rook.io_cephclusters.yaml +++ b/build/csv/ceph/ceph.rook.io_cephclusters.yaml @@ -1742,7 +1742,6 @@ spec: encrypted: type: boolean name: - maxLength: 40 type: string placement: nullable: true