Skip to content

rbd: use volumeattributesclass feature implement rbd volume qos#6160

Open
YiteGu wants to merge 11 commits intoceph:develfrom
YiteGu:support-vac-feature-for-rbd
Open

rbd: use volumeattributesclass feature implement rbd volume qos#6160
YiteGu wants to merge 11 commits intoceph:develfrom
YiteGu:support-vac-feature-for-rbd

Conversation

@YiteGu
Copy link
Member

@YiteGu YiteGu commented Mar 6, 2026

Describe what this PR does

  1. The solution using storageclass to implement RBD volume QoS has been cancelled because the storageclass parameters cannot be modified. The volumeattributesclass solution is more flexible because it supports mutable parameters.
  2. Add the new parameter: volumeAttributesClassName when defining the PVC. For example:
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: vac-pvc-1
  namespace: rook-ceph
spec:
  accessModes:
  - ReadWriteOnce
  resources:
    requests:
      storage: 30Gi
  storageClassName: rook-ceph-block
  volumeMode: Block
  volumeAttributesClassName: silver
  1. Implemented the ControllerModifyVolume interface. Users can upgrade their QoS by modifying the volumeAttributesClassName parameter of an existing PVC.

ref: https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/

Is there anything that requires special attention

Do you have any questions?

Is the change backward compatible?

Are there concerns around backward compatibility?

Provide any external context for the change, if any.

For example:

  • Kubernetes links that explain why the change is required
  • CSI spec related changes/catch-up that necessitates this patch
  • golang related practices that necessitates this change

Related issues

Mention any github issues relevant to this PR. Adding below line
will help to auto close the issue once the PR is merged.

Fixes: #issue_number

Future concerns

List items that are not part of the PR and do not impact it's
functionality, but are work items that can be taken up subsequently.

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 major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@mergify mergify bot added the component/rbd Issues related to RBD label Mar 6, 2026
@YiteGu YiteGu force-pushed the support-vac-feature-for-rbd branch 3 times, most recently from b78efe5 to 36c1c2f Compare March 6, 2026 11:43
@YiteGu YiteGu force-pushed the support-vac-feature-for-rbd branch 4 times, most recently from c1e4f0a to dffa21e Compare March 9, 2026 13:41
Copy link
Member

@nixpanic nixpanic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a user visible change. Put a note in PendingReleaseNotes.md for this too. Maybe add a note in the main README.md for support of this as well, update other documentation?

@mergify
Copy link
Contributor

mergify bot commented Mar 10, 2026

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

@YiteGu YiteGu force-pushed the support-vac-feature-for-rbd branch 3 times, most recently from cb885ba to a8df95a Compare March 11, 2026 05:13
@mergify
Copy link
Contributor

mergify bot commented Mar 11, 2026

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

@YiteGu YiteGu force-pushed the support-vac-feature-for-rbd branch 2 times, most recently from 43fa44e to 1a4ade8 Compare March 11, 2026 06:25
@YiteGu YiteGu requested a review from nixpanic March 11, 2026 07:36
@YiteGu YiteGu force-pushed the support-vac-feature-for-rbd branch from f7b6685 to e786b28 Compare March 11, 2026 07:44
Copy link
Member

@nixpanic nixpanic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor things that caught the eye of Claude Code, and look like good recommendations to me.

@YiteGu
Copy link
Member Author

YiteGu commented Mar 11, 2026

/test ci/centos/mini-e2e/k8s-1.34

@YiteGu YiteGu force-pushed the support-vac-feature-for-rbd branch from e786b28 to 650a107 Compare March 11, 2026 11:46
Copy link
Member

@nixpanic nixpanic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another round of review (now completed by me, not Claude).

These are the only changes I would like to see, everything else looks good to me. Thanks!

@YiteGu YiteGu force-pushed the support-vac-feature-for-rbd branch 2 times, most recently from b2d68cb to a79ce62 Compare March 16, 2026 02:52
@YiteGu YiteGu requested a review from nixpanic March 16, 2026 03:34
Copy link
Member

@nixpanic nixpanic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me.

@nixpanic nixpanic requested a review from a team March 16, 2026 09:05
@nixpanic nixpanic added the enhancement New feature or request label Mar 16, 2026
@YiteGu YiteGu force-pushed the support-vac-feature-for-rbd branch from 19d57ab to b6400da Compare March 19, 2026 12:13
@YiteGu
Copy link
Member Author

YiteGu commented Mar 19, 2026

In the logs of the csi-resizer sidecar of the provisioner:

  I0319 08:16:18.213061       1 event.go:389] "Event occurred" object="rbd-930/rbd-pvc" fieldPath="" kind="PersistentVolumeClaim" apiVersion="v1" type="Normal" reason="VolumeModify" message="external resizer is modifying volume rbd-pvc with vac gold"
  E0319 08:16:18.214624       1 controller.go:254] "Error syncing PVC" err="cannot get error status from modify volume err: error getting secret csi-rbd-secret in namespace default: secrets \"csi-rbd-secret\" not found"

The StorageClass probably points to the wrong secrets?

I checked the StorageClass's secret and found no problems.

   # controller-modify-secret
   csi.storage.k8s.io/controller-modify-secret-name: csi-rbd-secret
   csi.storage.k8s.io/controller-modify-secret-namespace: default

@YiteGu
Copy link
Member Author

YiteGu commented Mar 19, 2026

In the logs of the csi-resizer sidecar of the provisioner:

  I0319 08:16:18.213061       1 event.go:389] "Event occurred" object="rbd-930/rbd-pvc" fieldPath="" kind="PersistentVolumeClaim" apiVersion="v1" type="Normal" reason="VolumeModify" message="external resizer is modifying volume rbd-pvc with vac gold"
  E0319 08:16:18.214624       1 controller.go:254] "Error syncing PVC" err="cannot get error status from modify volume err: error getting secret csi-rbd-secret in namespace default: secrets \"csi-rbd-secret\" not found"

The StorageClass probably points to the wrong secrets?

I checked the StorageClass's secret and found no problems.

   # controller-modify-secret
   csi.storage.k8s.io/controller-modify-secret-name: csi-rbd-secret
   csi.storage.k8s.io/controller-modify-secret-namespace: default

Are there any other investigation approaches?

@nixpanic
Copy link
Member

In createRBDStorageClass there is this, and it needs extending for the new StoragClass parameters:

ceph-csi/e2e/rbd_helper.go

Lines 136 to 146 in 9af58c4

sc.Parameters["csi.storage.k8s.io/provisioner-secret-namespace"] = cephCSINamespace
sc.Parameters["csi.storage.k8s.io/provisioner-secret-name"] = rbdProvisionerSecretName
sc.Parameters["csi.storage.k8s.io/controller-expand-secret-namespace"] = cephCSINamespace
sc.Parameters["csi.storage.k8s.io/controller-expand-secret-name"] = rbdProvisionerSecretName
sc.Parameters["csi.storage.k8s.io/controller-publish-secret-namespace"] = cephCSINamespace
sc.Parameters["csi.storage.k8s.io/controller-publish-secret-name"] = rbdProvisionerSecretName
sc.Parameters["csi.storage.k8s.io/node-stage-secret-namespace"] = cephCSINamespace
sc.Parameters["csi.storage.k8s.io/node-stage-secret-name"] = rbdNodePluginSecretName

@nixpanic nixpanic force-pushed the support-vac-feature-for-rbd branch from b6400da to f838c0b Compare March 19, 2026 15:28
@nixpanic
Copy link
Member

/test ci/centos/mini-e2e/k8s-1.34/rbd

@nixpanic nixpanic force-pushed the support-vac-feature-for-rbd branch from f838c0b to c059c35 Compare March 19, 2026 15:42
@nixpanic
Copy link
Member

@YiteGu , I've updated the commit e2e: add e2e test for rbd volume volumeattributesclass to include the new parameters. With these changes it should all be good 🤞

@nixpanic
Copy link
Member

/test ci/centos/mini-e2e/k8s-1.34/rbd

@nixpanic
Copy link
Member

/test ci/centos/mini-e2e/k8s-1.33/rbd

nixpanic
nixpanic previously approved these changes Mar 19, 2026
@nixpanic nixpanic requested a review from a team March 19, 2026 15:44
@YiteGu
Copy link
Member Author

YiteGu commented Mar 19, 2026

@YiteGu , I've updated the commit e2e: add e2e test for rbd volume volumeattributesclass to include the new parameters. With these changes it should all be good 🤞

Nice, I've already returned home from the company, and I'll continue working tomorrow.

@YiteGu
Copy link
Member Author

YiteGu commented Mar 20, 2026

@nixpanic @iPraveenParihar e2e test successful, I will execu a final push, remove the diagnostic log I had added.

@YiteGu YiteGu force-pushed the support-vac-feature-for-rbd branch from c059c35 to 137bf95 Compare March 20, 2026 02:58
@mergify mergify bot dismissed nixpanic’s stale review March 20, 2026 02:59

Pull request has been modified.

YiteGu and others added 9 commits March 20, 2026 11:03
1. Create a VolumeAttributesClass with a QoS parameter.
   Create a PVC with a VolumeAttributesClassName parameter.
   Verify the QoS results of the RBD image.

2. Modify the VolumeAttributesClassName of the PVC and
   verify the change in the QoS of the RBD image.

3. Create a VolumeAttributesClass that supports capacity-based QoS
   and verify the QoS results of the RBD image.
   Create a cloned volume of the PVC and verify the QoS results of
   the cloned volume.

Signed-off-by: Yite Gu <guyite@bytedance.com>
Co-authored-by: Niels de Vos <ndevos@ibm.com>
The addition of a new op caused the tryAcquire function
to become too complex and fail CI checks. The tryAcquire function
implementation was refactored, and the conflictMatrix design
was introduced, which greatly reduced the complexity.

Signed-off-by: Yite Gu <guyite@bytedance.com>
storageclass is no longer used as the qos parameters carrier
for RBD volumes; these parameters are now defined in volumeattribetesclass.

Signed-off-by: Yite Gu <guyite@bytedance.com>
Signed-off-by: Yite Gu <guyite@bytedance.com>
Signed-off-by: Yite Gu <guyite@bytedance.com>
Signed-off-by: Yite Gu <guyite@bytedance.com>
Signed-off-by: Yite Gu <guyite@bytedance.com>
Signed-off-by: Yite Gu <guyite@bytedance.com>
Signed-off-by: Yite Gu <guyite@bytedance.com>
@YiteGu YiteGu force-pushed the support-vac-feature-for-rbd branch from 137bf95 to ff95af7 Compare March 20, 2026 03:04
@YiteGu
Copy link
Member Author

YiteGu commented Mar 20, 2026

/test ci/centos/mini-e2e/k8s-1.34/rbd

@YiteGu
Copy link
Member Author

YiteGu commented Mar 20, 2026

/test ci/centos/mini-e2e/k8s-1.33/rbd

@nixpanic nixpanic requested a review from a team March 20, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/rbd Issues related to RBD enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants