mirroring: disable mirroring when secondary cluster is down

[rewantsoni]

Refactor the mirroring controller to disable mirroring on cephblockpool and rns when the secondary cluster is down.

Fixes: DFBUGS-2957, DFBUGS-3638

[openshift-ci]

[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 iamniting for approval. For more information see the Code Review Process.

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

Details

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]

/hold for testing

[nb-ohad]

@rewantsoni This refactor introduce a lot of noise into the code base. Although the shouldMirror is only calculated once, the decision on how to act on it is spread all over the reconcile function and even being passed downward into other sub functions. Preferably the decision need to be made as early as possible in the reconcile flow then a reconcile path is being choosed based on the result. Doing it this way will benefit the code base, the readability and the ability to reason about the flow.

[rewantsoni]

@rewantsoni This refactor introduce a lot of noise into the code base. Although the shouldMirror is only calculated once, the decision on how to act on it is spread all over the reconcile function and even being passed downward into other sub functions. Preferably the decision need to be made as early as possible in the reconcile flow then a reconcile path is being choosed based on the result. Doing it this way will benefit the code base, the readability and the ability to reason about the flow.

@nb-ohad The problem is that we need to reconcile RBDMirror, CephBlockPools, and Radosnamespace in both cases. When shouldMirror is true we need to deploy rbdMirror, enable mirroring on cephblockpools and enable mirroring on radosnamespace as well. When it is set to false, we need to delete RBDMirror, disable mirroring on cephBlockPool and disable mirroring on radosnamespace as well. Along with this we also need to make sure that if a blockpool is labeled with forbid Mirroring label we probably should disable the mirroring on it as well.

What I can do to increase readability is move the rpc calls inside shouldMirror and then call reconcile RBDMirror, CephBlockPools, and Radosnamespace in both cases to enable/disable mirroring based on the in-memory map that we create from the rpc responses.

[rewantsoni]

/cherry-pick release-4.19

[openshift-cherrypick-robot]

@rewantsoni: once the present PR merges, I will cherry-pick it on top of release-4.19 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.19

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[rewantsoni]

Tested it on a cluster, mirroring is disabled for cephblockpool and rns.
/unhold

[rewantsoni]

/cherry-pick release-4.20

[openshift-cherrypick-robot]

@rewantsoni: once the present PR merges, I will cherry-pick it on top of release-4.20 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.20

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.