osd: limit storageClassDeviceSets to 63 chars #14134
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
testing result ka cluster-on-pvc.yaml
The CephCluster "rook-ceph" is invalid:
* spec.storage.storageClassDeviceSets[0].name: Too long: may not be longer than 63
* <nil>: Invalid value: "null": some validation rules were not checked because the object was invalid; correct the existing errors to complete validation |
travisn
reviewed
Apr 29, 2024
pkg/apis/ceph.rook.io/v1/types.go
Outdated
| @@ -2937,6 +2937,7 @@ type PriorityClassNamesSpec map[KeyType]string | |||
| // +nullable | |||
| type StorageClassDeviceSet struct { | |||
| // Name is a unique identifier for the set | |||
| // +kubebuilder:validation:MaxLength=63 | |||
There was a problem hiding this comment.
Is 63 the length that actually causes the issue? Or is it actually shorter because the name of the device set is concatenated with something else to create the resource name?
There was a problem hiding this comment.
that true it adds the *-local-volume-storageclass-0-data-* which should have a check on total length too
There was a problem hiding this comment.
That quite a lot of characters added. Are the asterisks also added? I'm a bit confused. I don't think asterisks are allowed characters.
There was a problem hiding this comment.
Actually asterisks are placeholder here for the real names
subhamkrai
force-pushed
the
add-validation-scds
branch
from
874bd4e
to
8276378
Compare
travisn
requested changes
May 2, 2024
| 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("length of generate pvc should be less the 63 as the 'volumeMount[*].Name' has limit of 63 char. Current length' %d", len(pvcID)) |
There was a problem hiding this comment.
How about this message so it is clear what they need to change?
Suggested change
| return "", fmt.Errorf("length of generate pvc should be less the 63 as the 'volumeMount[*].Name' has limit of 63 char. Current length' %d", len(pvcID)) | |
| 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)) |
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]>
subhamkrai
force-pushed
the
add-validation-scds
branch
from
8276378
to
6f5cc9b
Compare
travisn
approved these changes
May 3, 2024
6 tasks
travisn
added a commit
that referenced
this pull request
May 3, 2024
osd: limit storageClassDeviceSets to 63 chars (backport #14134)
|
Is a "deviceset" a Rook-internal thing? |
@anthonyeleven yess |
|
@anthonyeleven For an example of the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
there are some cases where storageClassDeviceSets length are more 63 chars and osd provisioned failed due to this. with error
Using x-validator to limit the maxLength to 63 chars.
Checklist: