rbd: migration of replication service from controller server #3924

riya-singhal31 · 2023-06-20T09:46:18Z

this commit migrates the replication service from controller server in rbd and adds it to csi-addons.

Rakshith-R

pr title still contains WIP.

Can you test it once to make sure nothing is broken ?
https://github.com/rook/rook/blob/master/Documentation/Storage-Configuration/Ceph-CSI/ceph-csi-drivers.md#csi-addons-controller

Rakshith-R · 2023-06-20T11:37:39Z

internal/csi-addons/rbd/replication.go

+// NewReplicationServer creates a new IdentityServer which handles
+// the Identity Service requests from the CSI-Addons specification.


This is wrong definition.

Yeah, updated. Thanks.

internal/rbd/driver/driver.go

nixpanic

looks good to me, except for the comment above the NewReplicationServer() function.

internal/rbd/driver/driver.go

riya-singhal31 · 2023-06-21T06:22:43Z

pr title still contains WIP.

Can you test it once to make sure nothing is broken ? https://github.com/rook/rook/blob/master/Documentation/Storage-Configuration/Ceph-CSI/ceph-csi-drivers.md#csi-addons-controller

Yes, testing it.

Madhu-1 · 2023-06-21T07:34:25Z

internal/csi-addons/rbd/replication.go

+// NewReplicationServer creates a new ReplicationServer which handles
+// the Replication Service requests from the CSI-Addons specification.
+func NewReplicationServer() *ReplicationServer {
+	return &ReplicationServer{}
+}


Suggested change

// NewReplicationServer creates a new ReplicationServer which handles

// the Replication Service requests from the CSI-Addons specification.

func NewReplicationServer() *ReplicationServer {

return &ReplicationServer{}

}

// NewReplicationServer creates a new ReplicationServer which handles

// the Replication Service requests from the CSI-Addons specification.

func NewReplicationServer(c *corerbd.ControllerServer) *ReplicationServer {

return &ReplicationServer{ControllerServer: c}

}

Without the above change, we cannot use the internal locking mechanism. please check this change once again.

Thanks for pointing out madhu !

volume locks are initialised way later in New[Controller|Node]Server() calls.

We'll need to move this call

ceph-csi/internal/rbd/driver/driver.go

Lines 108 to 112 in 306bfa2

// configre CSI-Addons server and components

err = r.setupCSIAddonsServer(conf)

if err != nil {

log.FatalLogMsg(err.Error())

}

to just before this call to make sure everything is initialized.

ceph-csi/internal/rbd/driver/driver.go

Lines 182 to 183 in 306bfa2

s := csicommon.NewNonBlockingGRPCServer()

@riya-singhal31 Please make this change too.

Updated, thanks!

Pull request has been modified.

Madhu-1 · 2023-06-22T09:40:37Z

@riya-singhal31 Hope this PR is already tested and working as expected?

riya-singhal31 · 2023-06-22T10:16:58Z

@riya-singhal31 Hope this PR is already tested and working as expected?

I'm on it.

riya-singhal31 · 2023-06-22T16:34:53Z

Tested.
Requesting the reviews.

riya-singhal31 · 2023-06-23T05:43:34Z

csiaddons server is running as expected and receiving the requests:

1 replication.go:117] ID: 27 Req-ID: 0001-0011-openshift-storage-0000000000000001-e69eee00-3d47-46b5-9e1f-86f3361b16ac mirroringMode is not set in parameters, setting to mirroringMode to default (snapshot)
E0622 16:28:42.441503 1 replication.go:269] ID: 27 Req-ID: 0001-0011-openshift-storage-0000000000000001-e69eee00-3d47-46b5-9e1f-86f3361b16ac failed to enable mirroring on "ocs-storagecluster-cephblockpool/csi-vol-e69eee00-3d47-46b5-9e1f-86f3361b16ac" with error: rbd: ret=-22, Invalid argument
E0622 16:28:42.441559 1 utils.go:210] ID: 27 Req-ID: 0001-0011-openshift-storage-0000000000000001-e69eee00-3d47-46b5-9e1f-86f3361b16ac GRPC error: rpc error: code = Internal desc = failed to enable mirroring on "ocs-storagecluster-cephblockpool/csi-vol-e69eee00-3d47-46b5-9e1f-86f3361b16ac" with error: rbd: ret=-22, Invalid argument
I0622 16:29:02.942045 1 utils.go:195] ID: 28 Req-ID: 0001-0011-openshift-storage-0000000000000001-e69eee00-3d47-46b5-9e1f-86f3361b16ac GRPC call: /replication.Controller/EnableVolumeReplication
I0622 16:29:02.942155 1 utils.go:204] ID: 28 Req-ID: 0001-0011-openshift-storage-0000000000000001-e69eee00-3d47-46b5-9e1f-86f3361b16ac GRPC request: {"parameters":{"schedulingInterval":"1m"},"replication_id":"replicationHandle","secrets":"stripped","volume_id":"0001-0011-openshift-storage-0000000000000001-e69eee00-3d47-46b5-9e1f-86f3361b16ac"}
I0622 16:29:02.946989 1 omap.go:88] ID: 28 Req-ID: 0001-0011-openshift-storage-0000000000000001-e69eee00-3d47-46b5-9e1f-86f3361b16ac got omap values: (pool="ocs-storagecluster-cephblockpool", namespace="", name="csi.volume.e69eee00-3d47-46b5-9e1f-86f3361b16ac"): map[csi.imageid:3ad28208e123 csi.imagename:csi-vol-e69eee00-3d47-46b5-9e1f-86f3361b16ac csi.volname:pvc-5bc286d7-19c1-4e24-9ca6-508183d6d585 csi.volume.owner:default]
W0622 16:29:02.972414 1 replication.go:117] ID: 28 Req-ID: 0001-0011-openshift-storage-0000000000000001-e69eee00-3d47-46b5-9e1f-86f3361b16ac mirroringMode is not set in parameters, setting to mirroringMode to default (snapshot)
E0622 16:29:03.020184 1 replication.go:269] ID: 28 Req-ID: 0001-0011-openshift-storage-0000000000000001-e69eee00-3d47-46b5-9e1f-86f3361b16ac failed to enable mirroring on "ocs-storagecluster-cephblockpool/csi-vol-e69eee00-3d47-46b5-9e1f-86f3361b16ac" with error: rbd: ret=-22, Invalid argument
E0622 16:29:03.020236 1 utils.go:210] ID: 28 Req-ID: 0001-0011-openshift-storage-0000000000000001-e69eee00-3d47-46b5-9e1f-86f3361b16ac GRPC error: rpc error: code = Internal desc = failed to enable mirroring on "ocs-storagecluster-cephblockpool/csi-vol-e69eee00-3d47-46b5-9e1f-86f3361b16ac" with error: rbd: ret=-22, Invalid argument

Rakshith-R · 2023-06-23T05:49:39Z

@Mergifyio queue

mergify · 2023-06-23T05:49:41Z

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at dbdb908

this commit migrates the replication controller server from internal/rbd and adds it to csi-addons. Signed-off-by: riya-singhal31 <rsinghal@redhat.com>

ceph-csi-bot · 2023-06-23T05:50:09Z

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

ceph-csi-bot · 2023-06-23T05:50:10Z

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

ceph-csi-bot · 2023-06-23T05:50:10Z

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

ceph-csi-bot · 2023-06-23T05:50:11Z

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

ceph-csi-bot · 2023-06-23T05:50:12Z

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

ceph-csi-bot · 2023-06-23T05:50:13Z

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

ceph-csi-bot · 2023-06-23T05:50:14Z

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

ceph-csi-bot · 2023-06-23T05:50:15Z

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

ceph-csi-bot · 2023-06-23T05:50:15Z

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

ceph-csi-bot · 2023-06-23T05:50:16Z

/test ci/centos/upgrade-tests-cephfs

ceph-csi-bot · 2023-06-23T05:50:17Z

/test ci/centos/upgrade-tests-rbd

mergify bot added the component/rbd Issues related to RBD label Jun 20, 2023

riya-singhal31 changed the title ~~[WIP]rbd: migration of replication controller server~~ [WIP]rbd: migration of replication service from controller server Jun 20, 2023

riya-singhal31 marked this pull request as ready for review June 20, 2023 11:12

riya-singhal31 requested a review from nixpanic June 20, 2023 11:12

Rakshith-R reviewed Jun 20, 2023

View reviewed changes

riya-singhal31 changed the title ~~[WIP]rbd: migration of replication service from controller server~~ rbd: migration of replication service from controller server Jun 20, 2023

nixpanic reviewed Jun 20, 2023

View reviewed changes

internal/rbd/driver/driver.go Show resolved Hide resolved

Madhu-1 reviewed Jun 21, 2023

View reviewed changes

nixpanic previously approved these changes Jun 21, 2023

View reviewed changes

nixpanic approved these changes Jun 22, 2023

View reviewed changes

Madhu-1 requested a review from Rakshith-R June 22, 2023 09:40

riya-singhal31 requested a review from Madhu-1 June 22, 2023 16:35

Rakshith-R approved these changes Jun 23, 2023

View reviewed changes

Rakshith-R added ci/skip/e2e skip running e2e CI jobs ci/skip/multi-arch-build skip building on multiple architectures labels Jun 23, 2023

rbd: migration of replication controller server

17c1b04

this commit migrates the replication controller server from internal/rbd and adds it to csi-addons. Signed-off-by: riya-singhal31 <rsinghal@redhat.com>

Rakshith-R linked an issue Jun 23, 2023 that may be closed by this pull request

rbd: complete migration of volrep to csi-addons #3314

Closed

5 tasks

mergify bot added the ok-to-test Label to trigger E2E tests label Jun 23, 2023

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

mergify bot merged commit dbdb908 into ceph:devel Jun 23, 2023
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rbd: migration of replication service from controller server #3924

rbd: migration of replication service from controller server #3924

riya-singhal31 commented Jun 20, 2023 •

edited

Loading

Rakshith-R left a comment

Rakshith-R Jun 20, 2023

riya-singhal31 Jun 21, 2023

nixpanic left a comment

riya-singhal31 commented Jun 21, 2023

Madhu-1 Jun 21, 2023

Rakshith-R Jun 22, 2023

riya-singhal31 Jun 22, 2023

Madhu-1 commented Jun 22, 2023

riya-singhal31 commented Jun 22, 2023

riya-singhal31 commented Jun 22, 2023 •

edited

Loading

riya-singhal31 commented Jun 23, 2023

Rakshith-R commented Jun 23, 2023

mergify bot commented Jun 23, 2023 •

edited

Loading

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

		// NewReplicationServer creates a new IdentityServer which handles
		// the Identity Service requests from the CSI-Addons specification.

	// configre CSI-Addons server and components
	err = r.setupCSIAddonsServer(conf)
	if err != nil {
	log.FatalLogMsg(err.Error())
	}

rbd: migration of replication service from controller server #3924

rbd: migration of replication service from controller server #3924

Conversation

riya-singhal31 commented Jun 20, 2023 • edited Loading

Rakshith-R left a comment

Choose a reason for hiding this comment

Rakshith-R Jun 20, 2023

Choose a reason for hiding this comment

riya-singhal31 Jun 21, 2023

Choose a reason for hiding this comment

nixpanic left a comment

Choose a reason for hiding this comment

riya-singhal31 commented Jun 21, 2023

Madhu-1 Jun 21, 2023

Choose a reason for hiding this comment

Rakshith-R Jun 22, 2023

Choose a reason for hiding this comment

riya-singhal31 Jun 22, 2023

Choose a reason for hiding this comment

Madhu-1 commented Jun 22, 2023

riya-singhal31 commented Jun 22, 2023

riya-singhal31 commented Jun 22, 2023 • edited Loading

riya-singhal31 commented Jun 23, 2023

Rakshith-R commented Jun 23, 2023

mergify bot commented Jun 23, 2023 • edited Loading

✅ The pull request has been merged automatically

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

ceph-csi-bot commented Jun 23, 2023

riya-singhal31 commented Jun 20, 2023 •

edited

Loading

riya-singhal31 commented Jun 22, 2023 •

edited

Loading

mergify bot commented Jun 23, 2023 •

edited

Loading