add rpc call for RevokeBlockPoolPeering #2493

rewantsoni · 2024-03-05T09:14:01Z

Add a new RPC call RevokeBlockPoolPeering to disable one-way mirroring between the block pools.

The rpc call is initiated from the primary cluster which wants to disable mirroring of the pool which was already mirrored, the server receives the request and does the following:

Remove the bootstrap secret ref on the BlockPool and delete the bootstrap secret.
Disable Mirroring on the Block Pool in the request if no bootstrap secret ref is set.
Disable RBD Mirror Daemon if no bootstrap secret is set for any of the block pools in the cluster.

Depends On:

openshift-ci · 2024-03-05T09:14:07Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

openshift-ci · 2024-03-05T09:14:09Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rewantsoni
Once this PR has been reviewed and has the lgtm label, please assign umangachapagain for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rewantsoni · 2024-03-06T06:17:29Z

Testing:
Before

[rewantsoni ] grpcurl -insecure -d '{"secretName":"ocs-storagecluster-cephblockpool-a96546a7-5e86-4d6e-ad","pool":"......=","token":"........"}' localhost:8881 provider.OCSProvider/PeerBlockPool

[rewantsoni ] date; oc get cephblockpool -oyaml; oc get cephrbdmirror
Tue Mar  5 17:02:34 IST 2024
apiVersion: v1
items:
- apiVersion: ceph.rook.io/v1
  kind: CephBlockPool
  metadata:
    name: ocs-storagecluster-cephblockpool
    namespace: openshift-storage
  spec:
    enableRBDStats: true
    erasureCoded:
      codingChunks: 0
      dataChunks: 0
    failureDomain: zone
    mirroring:
      enabled: true
      mode: image
      peers:
        secretNames:
        - ocs-storagecluster-cephblockpool-a96546a7-5e86-4d6e-ad
    quotas: {}
    replicated:
      replicasPerFailureDomain: 1
      size: 3
      targetSizeRatio: 0.49
    statusCheck:
      mirror: {}
  status:
    info:
      rbdMirrorBootstrapPeerSecretName: pool-peer-token-ocs-storagecluster-cephblockpool
    mirroringInfo:
      lastChecked: "2024-03-05T11:32:01Z"
      mode: image
      site_name: 4a8a20b2-760c-4621-820f-2ca8058ee2b7
    mirroringStatus:
      lastChecked: "2024-03-05T11:32:01Z"
      summary:
        daemon_health: ERROR
        health: ERROR
        image_health: OK
        states: {}
    observedGeneration: 12
    phase: Ready
    snapshotScheduleStatus: {}
kind: List
metadata:
  resourceVersion: ""
NAME         PHASE
rbd-mirror   Ready

Request:

[rewantsoni ] grpcurl -insecure -d '{"secretName":"ocs-storagecluster-cephblockpool-a96546a7-5e86-4d6e-ad","pool":"b2NzLXN0b3JhZ2VjbHVzdGVyLWNlcGhibG9ja3Bvb2w="}' localhost:8881 provider.OCSProvider/RevokeBlockPoolPeering
{}

After:

[rewantsoni ] date; oc get cephblockpool -oyaml; oc get cephrbdmirror
Tue Mar  5 17:05:59 IST 2024
apiVersion: v1
items:
- apiVersion: ceph.rook.io/v1
  kind: CephBlockPool
  metadata:
    name: ocs-storagecluster-cephblockpool
    namespace: openshift-storage
  spec:
    enableRBDStats: true
    erasureCoded:
      codingChunks: 0
      dataChunks: 0
    failureDomain: zone
    mirroring:
      peers: {}
    quotas: {}
    replicated:
      replicasPerFailureDomain: 1
      size: 3
      targetSizeRatio: 0.49
    statusCheck:
      mirror: {}
  status:
    info:
      rbdMirrorBootstrapPeerSecretName: pool-peer-token-ocs-storagecluster-cephblockpool
    mirroringInfo:
      lastChanged: "2024-03-05T11:34:02Z"
      lastChecked: "2024-03-05T11:35:02Z"
      mode: image
      site_name: 4a8a20b2-760c-4621-820f-2ca8058ee2b7
    mirroringStatus:
      lastChecked: "2024-03-05T11:35:02Z"
      summary:
        daemon_health: WARNING
        health: WARNING
        image_health: OK
        states: {}
    observedGeneration: 12
    phase: Ready
    snapshotScheduleStatus: {}
kind: List
metadata:
  resourceVersion: ""
No resources found in openshift-storage namespace.

rewantsoni · 2024-03-06T08:28:54Z

/test ocs-operator-bundle-e2e-aws

leelavg · 2024-03-06T11:56:11Z

services/provider/server/cephrbdmirror.go

+	err := c.client.Delete(ctx, cephRBDMirrorObj)
+	if err == nil {
+		return err
+	}
+	return nil


just club everything, return c.client.Delete(ctx, cephRBDMirrorObj)

leelavg · 2024-03-06T11:58:14Z

services/provider/server/cephblockpool.go

+		if cephBlockPool.Spec.Mirroring.Peers != nil && len(cephBlockPool.Spec.Mirroring.Peers.SecretNames) > 0 {
+			if len(cephBlockPool.Spec.Mirroring.Peers.SecretNames) > 0 {
+				return true, nil
+			}


there seems to be a redundant check here?

yes, removed it

leelavg · 2024-03-11T06:50:18Z

services/provider/server/cephblockpool.go

+
+func (c *cephBlockPoolManager) DisableBlockPoolMirroring(ctx context.Context, blockPoolName string, cephBlockPool *rookCephv1.CephBlockPool) error {
+
+	_, err := ctrl.CreateOrUpdate(ctx, c.client, cephBlockPool, func() error {


@rewantsoni could you pls bring this topic in today's team meeting for technical discussion, basically 1. should we use createorupdate on the resources that we didn't create, 2. should be use declarative style while servicing gRPC?

As per discussion offline, updated to use Update rather than createOrUpdate for the resources we are not creating

leelavg · 2024-04-29T07:21:32Z

services/provider/server/server.go

+	err = s.cephBlockPoolManager.UnSetBootstrapSecretRef(ctx, req.SecretName, cephBlockPool)
+	// there might be a case where the bootstrap secret was deleted but request failed after this and there was a retry,
+	// if error is IsNotFound, that means it is safe to proceed as we have deleted the bootstrap secret
+	if err != nil && !kerrors.IsNotFound(err) {


better handle the notfound on delete inside the func itself (same for the rest). Current func is doing two things, maybe state the same in the func name?

leelavg · 2024-04-29T07:25:05Z

services/provider/server/server.go

+		return nil, status.Errorf(codes.Internal, "Failed to get if rbd mirror is required: %v,", err)
+	}
+
+	if !isRBDMirrorRequired {


mirroring requires rbd-mirror and so you are deleting rbd-mirror if none of the cbp requires it, why not make the peering also consistent w/ this approach, like, client asks for peering but not explicitly enable mirroring as the latter is a hard requirement for the former?

I am not sure I understand what you meant. For one-way mirroring to be set up, we need the following:

Side A (primary) should enable mirroring on their blockpool to have the bootstrap secret generated which will be used by Side B (secondary)

Side B enables the mirroring on their blockpool and set the bootstrap secret ref from side A

Side B deploys the rbd mirror

Signed-off-by: Rewant Soni <[email protected]>

rewantsoni · 2024-07-29T16:05:04Z

Closing this in favour of #2671

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 5, 2024

rewantsoni force-pushed the server-client-uninstall branch from 5e98e4f to 0ec534a Compare March 5, 2024 13:12

rewantsoni mentioned this pull request Mar 5, 2024

Storageclusterpeer #2466

Closed

rewantsoni marked this pull request as ready for review March 6, 2024 06:17

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 6, 2024

leelavg reviewed Mar 6, 2024

View reviewed changes

rewantsoni force-pushed the server-client-uninstall branch from 0ec534a to 165b207 Compare March 6, 2024 13:43

leelavg reviewed Mar 11, 2024

View reviewed changes

rewantsoni force-pushed the server-client-uninstall branch from 165b207 to 0591fc5 Compare March 11, 2024 11:46

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 14, 2024

rewantsoni force-pushed the server-client-uninstall branch from 0591fc5 to 6d38236 Compare March 18, 2024 11:09

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 18, 2024

rewantsoni force-pushed the server-client-uninstall branch from 6d38236 to 8778fc3 Compare March 18, 2024 11:14

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 22, 2024

rewantsoni force-pushed the server-client-uninstall branch from 8778fc3 to 1e5b762 Compare March 26, 2024 05:44

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 26, 2024

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 3, 2024

rewantsoni force-pushed the server-client-uninstall branch from 1e5b762 to 65234ab Compare April 17, 2024 09:14

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 17, 2024

rewantsoni force-pushed the server-client-uninstall branch from 65234ab to 8296f9d Compare April 17, 2024 09:16

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 20, 2024

leelavg reviewed Apr 29, 2024

View reviewed changes

rewantsoni added 3 commits April 29, 2024 13:56

add rpc call for peerBlockPool

df9b868

Signed-off-by: Rewant Soni <[email protected]>

add client implementation for the peerblockpool rpc call

7a480fb

Signed-off-by: Rewant Soni <[email protected]>

add server implementation for peerblockpool rpc call

c681a2b

Signed-off-by: Rewant Soni <[email protected]>

rewantsoni force-pushed the server-client-uninstall branch from 5845593 to bb9d7f3 Compare April 29, 2024 09:07

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 29, 2024

rewantsoni added 2 commits May 9, 2024 14:27

add rpc call for revokeBlockPoolPeering

323c924

Signed-off-by: Rewant Soni <[email protected]>

add client implementation for revokeBlockPoolPeering rpc call

1ef4e9b

Signed-off-by: Rewant Soni <[email protected]>

rewantsoni force-pushed the server-client-uninstall branch from bb9d7f3 to 9a08e45 Compare May 9, 2024 08:58

add server implementation for revokeBlockPoolPeering rpc call

45253cf

Signed-off-by: Rewant Soni <[email protected]>

rewantsoni force-pushed the server-client-uninstall branch from 9a08e45 to 45253cf Compare May 9, 2024 09:02

rewantsoni marked this pull request as draft June 20, 2024 08:15

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 20, 2024

rewantsoni closed this Jul 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add rpc call for RevokeBlockPoolPeering #2493

add rpc call for RevokeBlockPoolPeering #2493

rewantsoni commented Mar 5, 2024 •

edited

Loading

openshift-ci bot commented Mar 5, 2024

openshift-ci bot commented Mar 5, 2024

rewantsoni commented Mar 6, 2024

rewantsoni commented Mar 6, 2024

leelavg Mar 6, 2024

leelavg Mar 6, 2024

rewantsoni Mar 6, 2024

leelavg Mar 11, 2024

rewantsoni Apr 17, 2024

leelavg Apr 29, 2024

leelavg Apr 29, 2024

rewantsoni Apr 29, 2024

rewantsoni commented Jul 29, 2024


		func (c cephBlockPoolManager) DisableBlockPoolMirroring(ctx context.Context, blockPoolName string, cephBlockPool rookCephv1.CephBlockPool) error {

		_, err := ctrl.CreateOrUpdate(ctx, c.client, cephBlockPool, func() error {

add rpc call for RevokeBlockPoolPeering #2493

add rpc call for RevokeBlockPoolPeering #2493

Conversation

rewantsoni commented Mar 5, 2024 • edited Loading

openshift-ci bot commented Mar 5, 2024

openshift-ci bot commented Mar 5, 2024

rewantsoni commented Mar 6, 2024

rewantsoni commented Mar 6, 2024

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

rewantsoni commented Jul 29, 2024

rewantsoni commented Mar 5, 2024 •

edited

Loading