cephblockpool: allow updation of mirroring field from different components

[rewantsoni]

When ocs-operator reconciles the cephblockpool object, it resets the mirroring field irrespective of whether mirroring is being used in the storageCluster API. This PR aims to modify the storageCluster API and allow us to enable mirroring on cephblockpool from StorageClusterPeer/API Server (required for RDR for Provider Mode).

After this PR,
When the mirroring object is nil, other components can set/unset the mirroringSpec on the cephBlockPool.
When the mirroring object is not nil, mirroring on the pools will be set based on the value of MirroringSpec in the storageCluster.

[openshift-ci]

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]

[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 agarwal-mudit 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]

/retest

[leelavg]

if existing.Spec is being replaced by all these conditional entries, what happens if some new field gets added to cephBlockPool and we failed to update that in here?

but again, this is only true if the initial assumption is, this piece doesn't require an update if rook api gets updated.

[leelavg]

I only see code changes wrt api changes, however may I know who sets this (mirroringSpec) in the first place?

if that's from UI and has a default, can the field be nullable?

[rewantsoni]

MirroringSpec is set by the MCO operator

[umangachapagain]

Default CephBlockPool is managed by StorageCluster. So, it is expected that all changes to default pool is made through it. We do not allow other components to change our managed resources by design. We should not have 2 ocs-operator APIs managing the same resource.

[leelavg]

@rewantsoni wrt #2480 (review) should this PR be reviewed or not? if not, pls mention what alternatives were considered to move forward, thanks.

[rewantsoni]

For RDR, we will allow users to use both default block pools and any additional blockpool created by them. The default blockpool is created and managed by the storageCluster whereas the user will create any additional blockpool from the UI.

The Mirroring field in the blockpool CR needs to be updated from both StorageClusterPeer and OCS API Server (Provider API Server) ref design doc for more details. If we don't allow the updation of the mirroring field from a different entity,

1. We would need duplication of code
2. StorageClusterPeer and OCS API Server would need to update storageCluster CR, which owns and manages the default blockpool.

As a trade-off, we are allowing an update of the cephblockpool mirroring field from different components.

@leelavg The PR can be reviewed, I have synced with Umanga and updated the PR.

[leelavg]

this is a bit confusing, is this what you are trying to achieve?

Suggested change

	existingMirroringSpec := existing.Spec.Mirroring
	existing.Spec = cephBlockPool.Spec
	// If mirroring is not set on storageCluster instance, don't update it
	if instance.Spec.Mirroring == nil {
	existing.Spec.Mirroring = existingMirroringSpec
	}
	// If mirroring is not set on storageCluster instance, don't update it
	if instance.Spec.Mirroring == nil {
	cephBlockPool.Spec.Mirroring = nil
	}
	existing.Spec = cephBlockPool.Spec

[rewantsoni]

I want to allow the updation of the mirroring field on the cephBlockPool instance from other components if the mirroring field is not set on storageCluster CR.

Initially, I was updating each field under spec one by one, if a new field is added it might be missed here and lead to bugs. For that, I am creating a temporary copy of the mirroring field from cephblockpool CR and mirroring is not set on storageCluster CR I am setting the value to the temporary variable. So, it will not update the mirroring field on cephblockpool when we don't enable mirroring from storageCluster CR.

The suggestion you made, will update the mirroring field on cephblockpool to be nil if mirroring is not set on storageCluster CR