red-hat-storage / ocs-operator

Operator for RHOCS
Apache License 2.0
85 stars 184 forks source link

rbdmirror: add a blockpool name label in the rados namespace cr #2883

Closed parth-gr closed 1 week ago

parth-gr commented 2 weeks ago

add a blockpool name in the lable so UI can watch for the rados namespace that belongs to the particular blockpool

parth-gr commented 2 weeks ago

I was wondering if ocs-operator is the right place to add this label and if adding the label from Rook would make more sense. If Rook adds the label then, irrespective of the controller that creates the CR, we will always have the label. As an example, for external mode we might also require monitoring and might have to add the label on the CR from ting radosnamespacce where we are creating radosNS CR for external mode. WDYT?

we are only doing mirroring in provider mode for the rados namespace, in future, we can change it to rook but for now, I think we can be specific

rewantsoni commented 2 weeks ago

we are only doing mirroring in provider mode for the rados namespace, in future, we can change it to rook but for now, I think we can be specific

If it is a small fix and we can have it from the rook, I suggest making changes to the rook so that we don't have to change it in the future.

parth-gr commented 2 weeks ago

we are only doing mirroring in provider mode for the rados namespace, in future, we can change it to rook but for now, I think we can be specific

If it is a small fix and we can have it from the rook, I suggest making changes to the rook so that we don't have to change it in the future.

@travisn should we do at rook? As this label would be unnecessary for other deployments, but it wont hurt

travisn commented 2 weeks ago

we are only doing mirroring in provider mode for the rados namespace, in future, we can change it to rook but for now, I think we can be specific

If it is a small fix and we can have it from the rook, I suggest making changes to the rook so that we don't have to change it in the future.

@travisn should we do at rook? As this label would be unnecessary for other deployments, but it wont hurt

Rook only sets labels on the resources that it generates, such as the ceph daemons. On all of the CRs, Rook only updates the status fields, and never touches the metadata (including labels) or the spec. So if the OCS operator is generating these rados namespace CRs, then OCS operator needs to add the labels.

rewantsoni commented 1 week ago

/lgtm

parth-gr commented 1 week ago

@iamniting @malayparida2000 PTAL ^^

malayparida2000 commented 1 week ago

/cc @iamniting

openshift-ci[bot] commented 1 week ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: iamniting, malayparida2000, parth-gr, rewantsoni

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/red-hat-storage/ocs-operator/blob/main/OWNERS)~~ [iamniting] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment