doc: add design doc for RBD QoS #4089

Madhu-1 · 2023-08-29T10:15:40Z

Add design doc for QoS for rbd devices mapped with both krbd and rbd-nbd

closes: #521

Thanks to @idryomov for helping me to understand and experiment on this one 🎉

Madhu-1 · 2023-08-29T10:16:11Z

nixpanic · 2023-08-29T12:08:42Z

docs/design/proposals/rbd-qos.md

@@ -0,0 +1,269 @@
+# Design Doc for RBD QOS using cgroup v2


QoS is probably the more common abbreviation?

nixpanic · 2023-08-29T12:10:04Z

docs/design/proposals/rbd-qos.md

+cgroup.stat  cpuset.cpus.effective io.pressure        memory.pressure  sys-kernel-config.mount
+```
+
+`kubepods.slice`` is the starting point and it contains multiple hierarchy


one ` too many

nixpanic · 2023-08-29T12:10:57Z

docs/design/proposals/rbd-qos.md

+Now go back to the application pod and check if we have the right limit set
+
+```bash
+[$]oc exec: oc exec -it csi-rbd-demo-pod -- sh


use kubectl examples?

nixpanic · 2023-08-29T12:12:23Z

docs/design/proposals/rbd-qos.md

+
+|  Parameter     |  Description     |
+|  ---  |  ---  |
+|  MaxReadIOOperationsPerSecond     | Max read IO operations per second      |


just use IOPS, that is common enough, IOOperationsPerSecond sounds very weird.

nixpanic · 2023-08-29T12:13:06Z

docs/design/proposals/rbd-qos.md

+
+## Different approaches
+
+The above solution can be implemented in CephCSI using 3 different approaches.


the name of the project is Ceph-CSI 😉

removed it now :D

nixpanic · 2023-08-29T12:17:46Z

docs/design/proposals/rbd-qos.md

+
+Both options 2 and 3 are contingent upon changes to the CSI spec and Kubernetes
+support. Additionally, VolumeAttributeClass is currently being developed within
+the Kubernetes realm and will initially be in the Alpha stage. Consequently, it


add inline link to the KEP please

nixpanic · 2023-08-29T12:19:23Z

docs/design/proposals/rbd-qos.md

+
+#### Implementation of VolumeAttributeClass QOS
+
+1. Modify CSIDriver object to pass pod details to the NodePublish Volume RPC


maybe: ... NodePublishVolume CSI procedure.

nixpanic · 2023-08-29T12:19:39Z

docs/design/proposals/rbd-qos.md

+#### Implementation of VolumeAttributeClass QOS
+
+1. Modify CSIDriver object to pass pod details to the NodePublish Volume RPC
+1. Add support in cephCSI to expose ModifyVolume RPC


cephCSI -> Ceph-CSI

nixpanic · 2023-08-29T12:22:13Z

docs/design/proposals/rbd-qos.md

+
+Considering the advantages and drawbacks, we can use StorageClass and
+VolumeAttributeClass to support QoS, with VolumeAttributeClass taking
+precedence over StorageClass.


Did you consider adding something like VolumeAttributeClass to CSI-Addons? That would allow changing the parameters similar to ModifyVolume already.

@nixpanic not as of today as kubernetes is working on it, if not we can look for adding it in future

I've now checked the KEP and it seems to have noted Alpha status for Kubernetes 1.28. I am not sure implementing additional custom parameters in the StorageClass is the right approach if the feature lands in Kubernetes soon. Having the parameters in the StorageClass means maintaining support for at least 2 configuration directives, which adds work on the testing and maintenance.

Ideally we only need to implement and maintain the VolumeAttributeClass CSI procedures that Kubernetes is implementing. A single standard mechanism benefits us as well as Ceph-CSI users. Putting efforts in getting the Kubernetes work done is important, and if the Ceph-CSI contributors can assists with that, all the better!

@nixpanic this is deferred for Kubernetes 1.29 kubernetes/enhancements#3751 (comment) and am not sure it will happen (hope it will happen), and as it goes with Alpha, Beta Feature cycle it takes more time for users to use this in production and not everyone will agree to upgrade to latest kubernetes for this one. I think it better to have both solutions and IMO it is not a maintenance and testing burden for this one as its minimal one, later once VolumeAttributeClass is stable we can deprecate the support for storageclass Qos.

@nixpanic this is deferred for Kubernetes 1.29 kubernetes/enhancements#3751 (comment) and am not sure it will happen (hope it will happen), and as it goes with Alpha, Beta Feature cycle it takes more time for users to use this in production and not everyone will agree to upgrade to latest kubernetes for this one. I think it better to have both solutions and IMO it is not a maintenance and testing burden for this one as its minimal one, later once VolumeAttributeClass is stable we can deprecate the support for storageclass Qos.

+1

nixpanic · 2023-08-29T12:23:07Z

docs/design/proposals/rbd-qos.md

+
+### References
+
+* <https://kubernetes.io/docs/concepts/architecture/cgroups/>


please link with a subject/description of what is at the URL

@nixpanic any specific format you are looking at, provide a suggestion.

something like

* [Kubernetes cgroup v2 Architecture](https://kubernetes.io/docs/concepts/architecture/cgroups/)

nixpanic · 2023-08-29T12:48:24Z

docs/design/proposals/rbd-qos.md

+
+### References
+
+* <https://kubernetes.io/docs/concepts/architecture/cgroups/>


something like

* [Kubernetes cgroup v2 Architecture](https://kubernetes.io/docs/concepts/architecture/cgroups/)

nixpanic · 2023-08-29T12:50:55Z

docs/design/proposals/rbd-qos.md

+
+Both options 2 and 3 are contingent upon changes to the CSI spec and Kubernetes
+support. Additionally,
+[VolumeAttributeClass](https://github.com/kubernetes/enhancements/pull/3780) is


a link to the actual doc is probably easier for people to read https://github.com/kubernetes/enhancements/blob/master/keps/sig-storage/3751-volume-attributes-class/README.md

nixpanic · 2023-08-29T12:57:47Z

docs/design/proposals/rbd-qos.md

+
+Considering the advantages and drawbacks, we can use StorageClass and
+VolumeAttributeClass to support QoS, with VolumeAttributeClass taking
+precedence over StorageClass.


I've now checked the KEP and it seems to have noted Alpha status for Kubernetes 1.28. I am not sure implementing additional custom parameters in the StorageClass is the right approach if the feature lands in Kubernetes soon. Having the parameters in the StorageClass means maintaining support for at least 2 configuration directives, which adds work on the testing and maintenance.

Ideally we only need to implement and maintain the VolumeAttributeClass CSI procedures that Kubernetes is implementing. A single standard mechanism benefits us as well as Ceph-CSI users. Putting efforts in getting the Kubernetes work done is important, and if the Ceph-CSI contributors can assists with that, all the better!

Rakshith-R

Thanks !
Left a few comments to highlight options and small nits.

The design doc looks very complete and captures everything perfectly to me.

Rakshith-R · 2023-09-05T09:45:25Z

docs/design/proposals/rbd-qos.md

+
+The above solution can be implemented using 3 different approaches.
+
+### QoS using new parameters in RBD StorageClass


Suggested change

### QoS using new parameters in RBD StorageClass

### 1. QoS using new parameters in RBD StorageClass

Rakshith-R · 2023-09-05T09:46:03Z

docs/design/proposals/rbd-qos.md

+1. Need to take a backup and restore to New QoS StorageClass
+1. Delete and Recreate the PV object
+
+### Add support for VolumeAttributeClass


Suggested change

### Add support for VolumeAttributeClass

### 2. QoS using parameters in VolumeAttributeClass

Rakshith-R · 2023-09-05T09:46:35Z

docs/design/proposals/rbd-qos.md

+up to get the new QoS value even though its changed in the PVC object, this is
+sometime impossible as it will have downtime.
+
+### VolumeAttributeClass with NodePublish Secret


Suggested change

### VolumeAttributeClass with NodePublish Secret

### 3. QoS using parameters in VolumeAttributeClass with NodePublish Secret

docs/design/proposals/rbd-qos.md

Rakshith-R · 2023-09-05T09:50:32Z

docs/design/proposals/rbd-qos.md

+in the Alpha stage. Consequently, it will be disabled by default, but it does
+offer the following advantages.


Suggested change

in the Alpha stage. Consequently, it will be disabled by default, but it does

offer the following advantages.

in the Alpha stage. Consequently, it will be disabled by default.

#### Advantages of QoS using VolumeAttributeClass

Let's highlight by adding another heading here.

docs/design/proposals/rbd-qos.md

Pull request has been modified.

idryomov

Just a few nits, otherwise LGTM!

docs/design/proposals/rbd-qos.md

travisn · 2023-09-06T20:59:52Z

docs/design/proposals/rbd-qos.md

+
+### Hybrid Approach
+
+Considering the advantages and drawbacks, we can use StorageClass and


Is it possible for users to override the QoS in the storage class with the VolumeAttributeClass settings? This sounds good assuming the admin controls which VolumeAttributeClass is available for the user to specify in their PVC. Otherwise, the user could override admin settings in the storage class.

both storageclass and VolumeAttributeClass are admin created ones and the user will not have access to create one, Yes users can update the VolumeAttributeClass in the PVC later creation but he/she should get the name of the VolumeAttributeClass from the admin.

Madhu-1 · 2023-09-07T13:19:16Z

@Mergifyio queue

mergify · 2023-09-07T13:19:21Z

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 7c0ce53

Add design doc for QoS for rbd devices mapped with both krbd and rbd-nbd closes: ceph#521 Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

ceph-csi-bot · 2023-09-07T13:20:34Z

/test ci/centos/upgrade-tests-cephfs

ceph-csi-bot · 2023-09-07T13:20:35Z

/test ci/centos/k8s-e2e-external-storage/1.26

ceph-csi-bot · 2023-09-07T13:20:35Z

/test ci/centos/upgrade-tests-rbd

ceph-csi-bot · 2023-09-07T13:20:36Z

/test ci/centos/k8s-e2e-external-storage/1.28

ceph-csi-bot · 2023-09-07T13:20:36Z

/test ci/centos/mini-e2e-helm/k8s-1.26

ceph-csi-bot · 2023-09-07T13:20:37Z

/test ci/centos/mini-e2e-helm/k8s-1.28

ceph-csi-bot · 2023-09-07T13:20:37Z

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

ceph-csi-bot · 2023-09-07T13:20:37Z

/test ci/centos/k8s-e2e-external-storage/1.27

ceph-csi-bot · 2023-09-07T13:20:38Z

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

ceph-csi-bot · 2023-09-07T13:20:38Z

/test ci/centos/mini-e2e-helm/k8s-1.27

ceph-csi-bot · 2023-09-07T13:20:39Z

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

Madhu-1 requested review from idryomov and a team August 29, 2023 10:15

mergify bot added ci/skip/e2e skip running e2e CI jobs ci/skip/multi-arch-build skip building on multiple architectures component/docs Issues and PRs related to documentation labels Aug 29, 2023

Madhu-1 force-pushed the rbd-qos branch from d3b4ace to 1146486 Compare August 29, 2023 10:19

nixpanic reviewed Aug 29, 2023

View reviewed changes

Madhu-1 force-pushed the rbd-qos branch from 1146486 to 083a973 Compare August 29, 2023 12:36

Madhu-1 requested a review from nixpanic August 29, 2023 12:36

Madhu-1 force-pushed the rbd-qos branch 2 times, most recently from 544ebb1 to 6c3462a Compare August 29, 2023 12:45

Madhu-1 changed the title ~~doc: add design doc for RBD QOS~~ doc: add design doc for RBD QoS Aug 29, 2023

Madhu-1 force-pushed the rbd-qos branch from 6c3462a to a01bdf2 Compare August 29, 2023 12:46

nixpanic reviewed Aug 29, 2023

View reviewed changes

Madhu-1 force-pushed the rbd-qos branch from a01bdf2 to 5960e10 Compare August 29, 2023 13:25

Madhu-1 requested a review from nixpanic September 1, 2023 06:58

Rakshith-R reviewed Sep 5, 2023

View reviewed changes

Madhu-1 force-pushed the rbd-qos branch 2 times, most recently from 7da0a8c to 7bdff7d Compare September 5, 2023 10:10

Madhu-1 requested a review from Rakshith-R September 5, 2023 10:10

Rakshith-R previously approved these changes Sep 5, 2023

View reviewed changes

idryomov reviewed Sep 5, 2023

View reviewed changes

docs/design/proposals/rbd-qos.md Outdated Show resolved Hide resolved

docs/design/proposals/rbd-qos.md Outdated Show resolved Hide resolved

docs/design/proposals/rbd-qos.md Outdated Show resolved Hide resolved

Madhu-1 force-pushed the rbd-qos branch from 7bdff7d to e2095bc Compare September 5, 2023 13:04

Madhu-1 requested a review from idryomov September 5, 2023 13:05

Madhu-1 force-pushed the rbd-qos branch from e2095bc to eebdf42 Compare September 5, 2023 13:08

idryomov reviewed Sep 5, 2023

View reviewed changes

Madhu-1 force-pushed the rbd-qos branch from eebdf42 to 2c139cb Compare September 6, 2023 05:31

Madhu-1 requested a review from idryomov September 6, 2023 05:31

idryomov reviewed Sep 6, 2023

View reviewed changes

docs/design/proposals/rbd-qos.md Outdated Show resolved Hide resolved

Madhu-1 force-pushed the rbd-qos branch from 2c139cb to b564d35 Compare September 6, 2023 10:23

Madhu-1 requested a review from idryomov September 6, 2023 10:23

idryomov approved these changes Sep 6, 2023

View reviewed changes

travisn reviewed Sep 6, 2023

View reviewed changes

Madhu-1 requested review from Rakshith-R and travisn September 7, 2023 06:40

Rakshith-R approved these changes Sep 7, 2023

View reviewed changes

nixpanic approved these changes Sep 7, 2023

View reviewed changes

travisn approved these changes Sep 7, 2023

View reviewed changes

doc: add design doc for QoS

b74ba28

Add design doc for QoS for rbd devices mapped with both krbd and rbd-nbd closes: ceph#521 Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

Madhu-1 force-pushed the rbd-qos branch from b564d35 to b74ba28 Compare September 7, 2023 13:19

mergify bot added the ok-to-test Label to trigger E2E tests label Sep 7, 2023

ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Sep 7, 2023

mergify bot merged commit 7c0ce53 into ceph:devel Sep 7, 2023
34 checks passed


		## Different approaches

		The above solution can be implemented in CephCSI using 3 different approaches.


		#### Implementation of VolumeAttributeClass QOS

		1. Modify CSIDriver object to pass pod details to the NodePublish Volume RPC


		### References

		* <https://kubernetes.io/docs/concepts/architecture/cgroups/>


		The above solution can be implemented using 3 different approaches.

		### QoS using new parameters in RBD StorageClass

	### QoS using new parameters in RBD StorageClass
	### 1. QoS using new parameters in RBD StorageClass

	### Add support for VolumeAttributeClass
	### 2. QoS using parameters in VolumeAttributeClass

	### VolumeAttributeClass with NodePublish Secret
	### 3. QoS using parameters in VolumeAttributeClass with NodePublish Secret

		in the Alpha stage. Consequently, it will be disabled by default, but it does
		offer the following advantages.


		### Hybrid Approach

		Considering the advantages and drawbacks, we can use StorageClass and

doc: add design doc for RBD QoS #4089

doc: add design doc for RBD QoS #4089

Conversation

Madhu-1 commented Aug 29, 2023 • edited

Madhu-1 commented Aug 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rakshith-R left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

idryomov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Madhu-1 commented Sep 7, 2023

mergify bot commented Sep 7, 2023 • edited

✅ The pull request has been merged automatically

ceph-csi-bot commented Sep 7, 2023

ceph-csi-bot commented Sep 7, 2023

ceph-csi-bot commented Sep 7, 2023

ceph-csi-bot commented Sep 7, 2023

ceph-csi-bot commented Sep 7, 2023

ceph-csi-bot commented Sep 7, 2023

ceph-csi-bot commented Sep 7, 2023

ceph-csi-bot commented Sep 7, 2023

ceph-csi-bot commented Sep 7, 2023

ceph-csi-bot commented Sep 7, 2023

ceph-csi-bot commented Sep 7, 2023

Madhu-1 commented Aug 29, 2023 •

edited

mergify bot commented Sep 7, 2023 •

edited