rbd: VolumeGroupReplicationContent controller to regenerate the OMAP data #4750

iPraveenParihar · 2024-08-06T06:35:59Z

Describe what this PR does

This commit adds new controller that watches for the
VolumeGroupReplicationContent and regenerates the OMAP data if
it doesn't exists.

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!)

nixpanic · 2024-08-06T07:13:57Z

internal/rbd/group/volume_group.go

-type volumeGroup struct {
-	// id is a unique value for this volume group in the Ceph cluster, it
+// VolumeGroup handles all requests for 'rbd group' operations.
+type VolumeGroup struct {


Don't export these, they are private intentional. They implement the interface in the internal/rbd/types/*.go files. You can use rbd.NewManager() to get a manager struct and use the Manager API to get the VolumeGroup.

If you are referring to GetVolumeGroupByID, then It doesn't suite well to use it here. Because in GetVolumeGroup we have GetPoolName by LocationID (which is PoolID) and PoolID may be different in secondary cluster.

ceph-csi/internal/rbd/group/volume_group.go

Lines 106 to 109 in 130e8b4

pool, err := util.GetPoolName(mons, creds, csiID.LocationID)

if err != nil {

return nil, fmt.Errorf("failed to get pool for volume group id %q: %w", id, err)

}

Else we can do it by passing an optional parameter PoolName for GetVolumeGroup ?
If it is passed as empty, then call GetPoolName else use the PoolName passed?

You may want to add more functions to the manager and other types. Ideally the API that the manager provides stays simple to use.

iPraveenParihar · 2024-08-06T12:12:56Z

Initial Test results -

E0806 10:45:48.804306       1 omap.go:80] omap not found (pool="replicapool", namespace="", name="csi.groups.default"): rados: ret=-2, No such file or directory
I0806 10:45:48.804338       1 rbd_journal.go:764] the journal does not contain a reservation for a volume group with name "vgrcontent-88b1d055-4c66-4218-a81c-487ba0b58ccf" yet
I0806 10:45:48.811316       1 omap.go:159] set omap keys (pool="replicapool", namespace="", name="csi.groups.default"): map[csi.volume.group.vgrcontent-88b1d055-4c66-4218-a81c-487ba0b58ccf:88b1d055-4c66-4218-a81c-487ba0b58ccf])
I0806 10:45:48.814878       1 omap.go:159] set omap keys (pool="replicapool", namespace="", name="csi.volume.group.88b1d055-4c66-4218-a81c-487ba0b58ccf"): map[csi.groupname:csi-vol-group-88b1d055-4c66-4218-a81c-487ba0b58ccf csi.volname:vgrcontent-88b1d055-4c66-4218-a81c-487ba0b58ccf])
I0806 10:45:48.816837       1 omap.go:89] got omap values: (pool="replicapool", namespace="", name="csi.volume.5bfefce1-6427-45a3-b088-6d43d19b1e9c"): map[csi.imageid:288758945f2b8 csi.imagename:csi-vol-5bfefce1-6427-45a3-b088-6d43d19b1e9c csi.volname:pvc-34dc4369-2eb3-492a-88aa-5cd37b557154 csi.volume.owner:rook-ceph]
I0806 10:45:48.838187       1 omap.go:89] got omap values: (pool="replicapool", namespace="", name="csi.volume.896c6912-604b-43db-916c-1e386a71973f"): map[csi.imageid:216b4d44baf60 csi.imagename:csi-vol-896c6912-604b-43db-916c-1e386a71973f csi.volname:pvc-2506eb3c-69e9-4054-8b8d-405de060c0f4 csi.volume.owner:rook-ceph]
I0806 10:45:48.856998       1 rbd_journal.go:799] adding volume mapping for volume "0001-0009-rook-ceph-0000000000000001-5bfefce1-6427-45a3-b088-6d43d19b1e9c" to volume group "csi-vol-group-88b1d055-4c66-4218-a81c-487ba0b58ccf"
I0806 10:45:48.860180       1 omap.go:159] set omap keys (pool="replicapool", namespace="", name="csi.volume.group.88b1d055-4c66-4218-a81c-487ba0b58ccf"): map[0001-0009-rook-ceph-0000000000000001-5bfefce1-6427-45a3-b088-6d43d19b1e9c:])
I0806 10:45:48.860287       1 rbd_journal.go:799] adding volume mapping for volume "0001-0009-rook-ceph-0000000000000001-896c6912-604b-43db-916c-1e386a71973f" to volume group "csi-vol-group-88b1d055-4c66-4218-a81c-487ba0b58ccf"
I0806 10:45:48.863734       1 omap.go:159] set omap keys (pool="replicapool", namespace="", name="csi.volume.group.88b1d055-4c66-4218-a81c-487ba0b58ccf"): map[0001-0009-rook-ceph-0000000000000001-896c6912-604b-43db-916c-1e386a71973f:])
I0806 10:45:48.863972       1 rbd_journal.go:816] re-generated Group ID (0001-0009-rook-ceph-0000000000000001-88b1d055-4c66-4218-a81c-487ba0b58ccf) and Group Name (csi-vol-group-88b1d055-4c66-4218-a81c-487ba0b58ccf) for request name (vgrcontent-88b1d055-4c66-4218-a81c-487ba0b58ccf)

bash-5.1$ rados listomapvals csi.groups.default -p replicapool
csi.volume.group.vgrcontent-88b1d055-4c66-4218-a81c-487ba0b58ccf
value (36 bytes) :
00000000  38 38 62 31 64 30 35 35  2d 34 63 36 36 2d 34 32  |88b1d055-4c66-42|
00000010  31 38 2d 61 38 31 63 2d  34 38 37 62 61 30 62 35  |18-a81c-487ba0b5|
00000020  38 63 63 66                                       |8ccf|
00000024

bash-5.1$ rados listomapvals csi.volume.group.88b1d055-4c66-4218-a81c-487ba0b58ccf -p replicapool
0001-0009-rook-ceph-0000000000000001-5bfefce1-6427-45a3-b088-6d43d19b1e9c
value (0 bytes) :

0001-0009-rook-ceph-0000000000000001-896c6912-604b-43db-916c-1e386a71973f
value (0 bytes) :

csi.groupname
value (50 bytes) :
00000000  63 73 69 2d 76 6f 6c 2d  67 72 6f 75 70 2d 38 38  |csi-vol-group-88|
00000010  62 31 64 30 35 35 2d 34  63 36 36 2d 34 32 31 38  |b1d055-4c66-4218|
00000020  2d 61 38 31 63 2d 34 38  37 62 61 30 62 35 38 63  |-a81c-487ba0b58c|
00000030  63 66                                             |cf|
00000032

csi.volname
value (47 bytes) :
00000000  76 67 72 63 6f 6e 74 65  6e 74 2d 38 38 62 31 64  |vgrcontent-88b1d|
00000010  30 35 35 2d 34 63 36 36  2d 34 32 31 38 2d 61 38  |055-4c66-4218-a8|
00000020  31 63 2d 34 38 37 62 61  30 62 35 38 63 63 66     |1c-487ba0b58ccf|

mergify · 2024-08-06T14:38:30Z

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

internal/rbd/manager.go

mergify · 2024-08-09T10:07:10Z

The PR description contains the unsupported [skip ci] command, please update the description and mark the PR ready for review again.

internal/rbd/rbd_journal.go

mergify · 2024-08-20T08:18:06Z

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

internal/controller/volumegroup/volumegroupreplicationcontent.go

internal/journal/volumegroupjournal.go

internal/rbd/manager.go

nixpanic

Just a nit, but should get corrected never the less.

internal/rbd/manager.go

internal/rbd/types/manager.go

mergify · 2024-08-27T13:31:35Z

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

github-actions · 2024-09-26T21:01:21Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

iPraveenParihar · 2024-11-06T12:43:37Z

@nixpanic, I have addressed your requested changes and rebased.

nixpanic

Thanks!

Looks like there is a conflict in go.mod, that needs addressing before this can be merged.

mergify · 2024-11-22T13:48:28Z

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

Pull request has been modified.

iPraveenParihar · 2024-11-25T07:38:02Z

Thanks!

Looks like there is a conflict in go.mod, that needs addressing before this can be merged.

Thanks, rebased now

Madhu-1

@iPraveenParihar i hope you already tested this change and we are able to regenerate and able to find the group with newly generated omap.

Madhu-1 · 2024-11-25T09:02:19Z

go.mod

 )

+require github.com/go-task/slim-sprig/v3 v3.0.0 // indirect


we already have require section for indirect dep, this should be added there, please check

Madhu-1 · 2024-11-25T09:05:43Z

internal/rbd/manager.go

+	journalPool = mgr.parameters["journalPool"]
+	if journalPool == "" {
+		journalPool = pool
+	}


Do we have journalPool in volumegroup?

CreateVolumeGroup has it -

ceph-csi/internal/rbd/manager.go

Lines 254 to 258 in a32ba13

// journalPool is an optional parameter, use pool if it is not set

journalPool, ok := mgr.parameters["journalPool"]

if !ok || journalPool == "" {

journalPool = pool

}

okay, i dont know in which case we will have journalPool, i think we removed it as its was not required. cc @nixpanic

Madhu-1 · 2024-11-25T09:06:33Z

internal/rbd/manager.go

+	}
+	defer vgJournal.Destroy()
+
+	namePrefix = mgr.parameters["volumeNamePrefix"]


do we have volumeNamePrefix in group?

actually, it should be groupNamePrefix. CreateVolumeGroupSnapshot uses groupNamePrefix.
But CreateVolumeGroup uses volumeNamePrefix

ceph-csi/internal/rbd/manager.go

Lines 260 to 262 in a32ba13

// volumeNamePrefix is an optional parameter, can be an empty string

prefix := mgr.parameters["volumeNamePrefix"]

Will change it to groupNamePrefix ?

yes lets stick to one groupNamePrefix for both as they need to work together

This commit adds groupUUID param for `ReserveName` to be used for OMAP name reserve instead of auto-generating. This is useful for mirroring and metro-DR ensuring that mirrored resources have consistent OMAP names across mirrored clusters. Signed-off-by: Praveen M <[email protected]>

This commit adds `RegenerateVolumeGroupJournal` to Manager interface. RegenerateVolumeGroupJournal regenerate the omap data for the volume group. This performs the following operations: - extracts clusterID and Mons from the cluster mapping - Retrieves pool and journalPool parameters from the VolumeGroupReplicationClass - Reserves omap data - Add volumeIDs mapping to the reserved volume group omap object - Generate new volume group handle Returns the generated volume group handler. Signed-off-by: Praveen M <[email protected]>

This commit adds new controller that watches for the VolumeGroupReplicationContent and regenerates the OMAP data if it doesn't exists. Signed-off-by: Praveen M <[email protected]>

Nikhil-Ladha · 2024-12-05T09:14:21Z

internal/rbd/manager.go

+		return "", fmt.Errorf("failed to get poolID for %q: %w", groupUUID, err)
+	}
+
+	groupHandle, err := util.GenerateVolID(ctx, monitors, mgr.creds, poolID, pool, clusterID, groupUUID)


Will it return an updated handle when we modify the VolumeGroup? If yes, then we need to check for the VG response always while calling the ModifyVolumeGroup RPC and update the VGRContent CR as well, if the handle has changed.

Based on the params passed to the function, I am assuming if the handle is already created it should not get updated as the params would remain the same 🤔

iPraveenParihar self-assigned this Aug 6, 2024

mergify bot added the component/rbd Issues related to RBD label Aug 6, 2024

nixpanic reviewed Aug 6, 2024

View reviewed changes

iPraveenParihar force-pushed the vg/replication-omap-regenerate branch 4 times, most recently from 2a75eec to 6d757b8 Compare August 6, 2024 11:26

iPraveenParihar marked this pull request as ready for review August 6, 2024 11:31

iPraveenParihar requested review from Madhu-1 and Rakshith-R August 6, 2024 11:37

iPraveenParihar force-pushed the vg/replication-omap-regenerate branch 4 times, most recently from 0290729 to 628b00b Compare August 6, 2024 13:35

nixpanic requested changes Aug 6, 2024

View reviewed changes

internal/rbd/manager.go Outdated Show resolved Hide resolved

iPraveenParihar force-pushed the vg/replication-omap-regenerate branch from 628b00b to 3088e07 Compare August 9, 2024 06:51

nixpanic mentioned this pull request Aug 9, 2024

journal: store CreationTime for VolumeGroups #4753

Merged

mergify bot marked this pull request as draft August 9, 2024 10:07

iPraveenParihar marked this pull request as ready for review August 12, 2024 04:15

iPraveenParihar force-pushed the vg/replication-omap-regenerate branch 5 times, most recently from 17e3418 to 62e2f4a Compare August 12, 2024 13:42

nixpanic reviewed Aug 13, 2024

View reviewed changes

internal/rbd/rbd_journal.go Outdated Show resolved Hide resolved

internal/rbd/rbd_journal.go Outdated Show resolved Hide resolved

iPraveenParihar force-pushed the vg/replication-omap-regenerate branch 2 times, most recently from 87e64d7 to 01ca0f3 Compare August 14, 2024 11:40

iPraveenParihar force-pushed the vg/replication-omap-regenerate branch 2 times, most recently from d017163 to 7baabbf Compare August 19, 2024 12:00

Madhu-1 reviewed Aug 20, 2024

View reviewed changes

iPraveenParihar force-pushed the vg/replication-omap-regenerate branch 3 times, most recently from 4348949 to c744ae4 Compare August 21, 2024 07:21

nixpanic requested changes Aug 22, 2024

View reviewed changes

internal/rbd/manager.go Outdated Show resolved Hide resolved

internal/rbd/types/manager.go Outdated Show resolved Hide resolved

github-actions bot added the stale label Sep 26, 2024

iPraveenParihar added keepalive This label can be used to disable stale bot activiity in the repo and removed stale labels Sep 27, 2024

iPraveenParihar force-pushed the vg/replication-omap-regenerate branch 3 times, most recently from 98a23cd to 8a72e9c Compare November 6, 2024 12:40

iPraveenParihar force-pushed the vg/replication-omap-regenerate branch from 8a72e9c to 0927a48 Compare November 6, 2024 12:46

nixpanic previously approved these changes Nov 22, 2024

View reviewed changes

nixpanic requested a review from Madhu-1 November 22, 2024 13:48

iPraveenParihar force-pushed the vg/replication-omap-regenerate branch from 0927a48 to aa7cf8a Compare November 25, 2024 07:37

Madhu-1 reviewed Nov 25, 2024

View reviewed changes

iPraveenParihar added 3 commits December 4, 2024 13:04

rbd: controller to regenerate volume group omap data

149ce56

This commit adds new controller that watches for the VolumeGroupReplicationContent and regenerates the OMAP data if it doesn't exists. Signed-off-by: Praveen M <[email protected]>

iPraveenParihar force-pushed the vg/replication-omap-regenerate branch from aa7cf8a to 149ce56 Compare December 4, 2024 07:34

Nikhil-Ladha reviewed Dec 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rbd: VolumeGroupReplicationContent controller to regenerate the OMAP data #4750

rbd: VolumeGroupReplicationContent controller to regenerate the OMAP data #4750

iPraveenParihar commented Aug 6, 2024 •

edited

Loading

nixpanic Aug 6, 2024

iPraveenParihar Aug 6, 2024

nixpanic Aug 6, 2024

iPraveenParihar commented Aug 6, 2024

mergify bot commented Aug 6, 2024

mergify bot commented Aug 9, 2024

mergify bot commented Aug 20, 2024

nixpanic left a comment

mergify bot commented Aug 27, 2024

github-actions bot commented Sep 26, 2024

iPraveenParihar commented Nov 6, 2024

nixpanic left a comment

mergify bot commented Nov 22, 2024

iPraveenParihar commented Nov 25, 2024

Madhu-1 left a comment

Madhu-1 Nov 25, 2024

Madhu-1 Nov 25, 2024

iPraveenParihar Dec 5, 2024

Madhu-1 Dec 5, 2024

Madhu-1 Nov 25, 2024

iPraveenParihar Dec 5, 2024

Madhu-1 Dec 5, 2024

Nikhil-Ladha Dec 5, 2024

Nikhil-Ladha Dec 5, 2024

	pool, err := util.GetPoolName(mons, creds, csiID.LocationID)
	if err != nil {
	return nil, fmt.Errorf("failed to get pool for volume group id %q: %w", id, err)
	}

		)

		require github.com/go-task/slim-sprig/v3 v3.0.0 // indirect

	// journalPool is an optional parameter, use pool if it is not set
	journalPool, ok := mgr.parameters["journalPool"]
	if !ok \|\| journalPool == "" {
	journalPool = pool
	}

	// volumeNamePrefix is an optional parameter, can be an empty string
	prefix := mgr.parameters["volumeNamePrefix"]

rbd: VolumeGroupReplicationContent controller to regenerate the OMAP data #4750

Are you sure you want to change the base?

rbd: VolumeGroupReplicationContent controller to regenerate the OMAP data #4750

Conversation

iPraveenParihar commented Aug 6, 2024 • edited Loading

Describe what this PR does

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iPraveenParihar commented Aug 6, 2024

Initial Test results -

mergify bot commented Aug 6, 2024

mergify bot commented Aug 9, 2024

mergify bot commented Aug 20, 2024

nixpanic left a comment

Choose a reason for hiding this comment

mergify bot commented Aug 27, 2024

github-actions bot commented Sep 26, 2024

iPraveenParihar commented Nov 6, 2024

nixpanic left a comment

Choose a reason for hiding this comment

mergify bot commented Nov 22, 2024

iPraveenParihar commented Nov 25, 2024

Madhu-1 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

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

iPraveenParihar commented Aug 6, 2024 •

edited

Loading