subvolumegroup: add support for size and datapool #14036

Madhu-1 · 2024-04-05T10:09:59Z

cephfs subvolumegroup supports creating svg with size and the datapool, This PR adds the support for the same.

Resolves #14004

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

pkg/operator/ceph/file/filesystem.go

travisn · 2024-04-05T14:00:42Z

Documentation/CRDs/Shared-Filesystem/ceph-fs-subvolumegroup-crd.md

@@ -32,6 +32,10 @@ spec:
    distributed: 1            # distributed=<0, 1> (disabled=0)
    # export:                 # export=<0-256> (disabled=-1)
    # random:                 # random=[0.0, 1.0](disabled=0.0)
+  # size of the subvolume group.


"size" is too vague, let's mention it's for a quota

Suggested change

# size of the subvolume group.

# Quota size of the subvolume group.

travisn · 2024-04-05T14:01:10Z

Documentation/CRDs/Shared-Filesystem/ceph-fs-subvolumegroup-crd.md

@@ -32,6 +32,10 @@ spec:
    distributed: 1            # distributed=<0, 1> (disabled=0)
    # export:                 # export=<0-256> (disabled=-1)
    # random:                 # random=[0.0, 1.0](disabled=0.0)
+  # size of the subvolume group.
+  #size: 10G
+  # data pool name for the subvolume group layout.


Suggested change

# data pool name for the subvolume group layout.

# data pool name for the subvolume group layout instead of the default data pool.

travisn · 2024-04-05T14:01:35Z

Documentation/CRDs/Shared-Filesystem/ceph-fs-subvolumegroup-crd.md

-    * `distributed`: Range: <0, 1>, for disabling it set to 0
-    * `export`: Range: <0-256>, for disabling it set to -1
-    * `random`: Range: [0.0, 1.0], for disabling it set to 0.0
+* `size`: Size of the Ceph Filesystem subvolume group.


Suggested change

* `size`: Size of the Ceph Filesystem subvolume group.

* `size`: Quota size of the Ceph Filesystem subvolume group.

travisn · 2024-04-05T14:03:22Z

Documentation/CRDs/Shared-Filesystem/ceph-fs-subvolumegroup-crd.md

+
+* `dataPoolName`: The data pool name for the Ceph Filesystem subvolume group layout.
+
+* `pinning`: To distribute load across MDS ranks in predictable and stable ways. Reference: <https://docs.ceph.com/en/latest/cephfs/fs-volumes/#pinning-subvolumes-and-subvolume-groups>.


Not related to your changes, but let's use a link instead of raw URL.

Suggested change

* `pinning`: To distribute load across MDS ranks in predictable and stable ways. Reference: <https://docs.ceph.com/en/latest/cephfs/fs-volumes/#pinning-subvolumes-and-subvolume-groups>.

* `pinning`: To distribute load across MDS ranks in predictable and stable ways. See the Ceph doc for [Pinning subvolume groups](https://docs.ceph.com/en/latest/cephfs/fs-volumes/#pinning-subvolumes-and-subvolume-groups).

travisn · 2024-04-05T14:04:06Z

Documentation/CRDs/Shared-Filesystem/ceph-fs-subvolumegroup-crd.md

-    * `random`: Range: [0.0, 1.0], for disabling it set to 0.0
+* `size`: Size of the Ceph Filesystem subvolume group.
+
+* `dataPoolName`: The data pool name for the Ceph Filesystem subvolume group layout.


Suggested change

* `dataPoolName`: The data pool name for the Ceph Filesystem subvolume group layout.

* `dataPoolName`: The data pool name for the Ceph Filesystem subvolume group layout, if the default CephFS pool is not desired.

travisn · 2024-04-05T14:05:11Z

pkg/daemon/ceph/client/subvolumegroup.go

 	args := []string{"fs", "subvolumegroup", "create", volName, groupName}
+	if svgSpec != nil {
+		if svgSpec.Size != nil {


If the svg already exists and the CR is edited with these new properties, will the svg be updated? Or will this create command be skipped or not apply the new settings?

it will be updates as per the user requirement

travisn · 2024-04-05T14:05:29Z

pkg/daemon/ceph/client/subvolumegroup.go

+	if svgSpec != nil {
+		if svgSpec.Size != nil {
+			// convert the size to bytes as ceph expect the size in bytes
+			args = append(args, fmt.Sprintf("--size=%d", svgSpec.Size.Value()))


If they edit the size in the CR, we need a different command to resize the svg, right?

No, we have only one command which takes care of everything we don't have resize command in cephfs

pkg/daemon/ceph/client/subvolumegroup.go

travisn · 2024-04-08T18:45:20Z

Documentation/CRDs/Shared-Filesystem/ceph-fs-subvolumegroup-crd.md

+* `dataPoolName`: The data pool name for the subvolume group layout instead of the default data pool.
+
+* `pinning`: To distribute load across MDS ranks in predictable and stable ways. See the Ceph doc for [Pinning subvolume groups](https://docs.ceph.com/en/latest/cephfs/fs-volumes/#pinning-subvolumes-and-subvolume-groups).
+  * `distributed`: Range: <0, 1>, for disabling it set to 0


Did your editor autoformat to only have two leading spaces? IIRC the formatted docs will not have proper indentation if there are not at least three spaces.

yes, fixed it now.

Did you push the fix? It looks the same.

somehow it got reverted but its present in latest push

pkg/daemon/ceph/client/subvolumegroup.go

travisn · 2024-04-10T13:40:14Z

Documentation/CRDs/Shared-Filesystem/ceph-fs-subvolumegroup-crd.md

+* `dataPoolName`: The data pool name for the subvolume group layout instead of the default data pool.
+
+* `pinning`: To distribute load across MDS ranks in predictable and stable ways. See the Ceph doc for [Pinning subvolume groups](https://docs.ceph.com/en/latest/cephfs/fs-volumes/#pinning-subvolumes-and-subvolume-groups).
+  * `distributed`: Range: <0, 1>, for disabling it set to 0


Did you push the fix? It looks the same.

travisn · 2024-04-10T13:43:40Z

pkg/daemon/ceph/client/subvolumegroup.go

+		err = resizeCephFSSubVolumeGroup(context, clusterInfo, volName, groupName, svgSpec)
+		if err != nil {
+			return errors.Wrapf(err, "failed to create subvolume group %q in filesystem %q", groupName, volName)
+		}


Should it return here after a successful resize? Otherwise it will continue and call create, which would allow the shrink?

as the resize was successful create wont shrink and also we need to call create to update the pool_layout (which is allowed in ceph fs)

What happens if the size was reduced by the user? Wouldn't the create apply it even though the resize ignored it?

The resize will fail if a user is trying to reduce the size to less than the size of the stored data.

Perfect, thanks

pkg/daemon/ceph/client/subvolumegroup.go

mergify · 2024-04-10T13:51:15Z

This pull request has merge conflicts that must be resolved before it can be merged. @Madhu-1 please rebase it. https://rook.io/docs/rook/latest/Contributing/development-flow/#updating-your-fork

cephfs subvolumegroup supports creating svg with quota and the datapool, This PR adds the support for the same. Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

travisn · 2024-04-10T14:51:13Z

pkg/daemon/ceph/client/subvolumegroup.go

+		err = resizeCephFSSubVolumeGroup(context, clusterInfo, volName, groupName, svgSpec)
+		if err != nil {
+			return errors.Wrapf(err, "failed to create subvolume group %q in filesystem %q", groupName, volName)
+		}


Perfect, thanks

subvolumegroup: add support for size and datapool (backport #14036)

Madhu-1 requested a review from travisn April 5, 2024 10:10

Madhu-1 force-pushed the fix-14004 branch from 45c4da6 to 4a68aa8 Compare April 5, 2024 10:14

travisn requested changes Apr 5, 2024

View reviewed changes

Madhu-1 requested a review from travisn April 8, 2024 06:43

Madhu-1 force-pushed the fix-14004 branch 2 times, most recently from a4f62b7 to 5c5b055 Compare April 8, 2024 06:45

travisn reviewed Apr 8, 2024

View reviewed changes

Madhu-1 force-pushed the fix-14004 branch 3 times, most recently from daef154 to fc76802 Compare April 10, 2024 11:50

travisn requested changes Apr 10, 2024

View reviewed changes

Madhu-1 force-pushed the fix-14004 branch 2 times, most recently from 198ca5d to 4f1aa3a Compare April 10, 2024 13:49

Madhu-1 force-pushed the fix-14004 branch from 4f1aa3a to 6c92ffd Compare April 10, 2024 13:51

Madhu-1 requested a review from travisn April 10, 2024 13:52

subvolumegroup: add support for size and datapool

f57a8b6

cephfs subvolumegroup supports creating svg with quota and the datapool, This PR adds the support for the same. Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>

Madhu-1 force-pushed the fix-14004 branch from 6c92ffd to f57a8b6 Compare April 10, 2024 14:41

travisn approved these changes Apr 10, 2024

View reviewed changes

travisn added the backport-release-1.14 label Apr 10, 2024

travisn merged commit f0a793c into rook:master Apr 10, 2024
53 checks passed

mergify bot mentioned this pull request Apr 10, 2024

subvolumegroup: add support for size and datapool (backport #14036) #14055

Merged

6 tasks

travisn added a commit that referenced this pull request Apr 11, 2024

Merge pull request #14055 from rook/mergify/bp/release-1.14/pr-14036

cfe6173

subvolumegroup: add support for size and datapool (backport #14036)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

subvolumegroup: add support for size and datapool #14036

subvolumegroup: add support for size and datapool #14036

Madhu-1 commented Apr 5, 2024 •

edited

travisn Apr 5, 2024

travisn Apr 5, 2024

travisn Apr 5, 2024

travisn Apr 5, 2024

travisn Apr 5, 2024

travisn Apr 5, 2024

Madhu-1 Apr 8, 2024

travisn Apr 5, 2024

Madhu-1 Apr 8, 2024

travisn Apr 8, 2024

Madhu-1 Apr 10, 2024

travisn Apr 10, 2024

Madhu-1 Apr 10, 2024

travisn Apr 10, 2024

travisn Apr 10, 2024

Madhu-1 Apr 10, 2024

travisn Apr 10, 2024

Madhu-1 Apr 10, 2024

travisn Apr 10, 2024

mergify bot commented Apr 10, 2024

travisn Apr 10, 2024

	# size of the subvolume group.
	# Quota size of the subvolume group.

	# data pool name for the subvolume group layout.
	# data pool name for the subvolume group layout instead of the default data pool.

	* `size`: Size of the Ceph Filesystem subvolume group.
	* `size`: Quota size of the Ceph Filesystem subvolume group.


		* `dataPoolName`: The data pool name for the Ceph Filesystem subvolume group layout.

		* `pinning`: To distribute load across MDS ranks in predictable and stable ways. Reference: <https://docs.ceph.com/en/latest/cephfs/fs-volumes/#pinning-subvolumes-and-subvolume-groups>.

subvolumegroup: add support for size and datapool #14036

subvolumegroup: add support for size and datapool #14036

Conversation

Madhu-1 commented Apr 5, 2024 • edited

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

mergify bot commented Apr 10, 2024

Choose a reason for hiding this comment

Madhu-1 commented Apr 5, 2024 •

edited