Bug 2259847 - RBD Mirror daemon count is not validated in StorageCluster
Summary: RBD Mirror daemon count is not validated in StorageCluster
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat OpenShift Data Foundation
Classification: Red Hat Storage
Component: ocs-operator
Version: 4.15
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: ---
: ODF 4.16.0
Assignee: umanga
QA Contact: Sidhant Agrawal
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2024-01-23 10:53 UTC by umanga
Modified: 2024-07-17 13:12 UTC (History)
5 users (show)

Fixed In Version: 4.16.0-113
Doc Type: No Doc Update
Doc Text:
Clone Of:
Environment:
Last Closed: 2024-07-17 13:12:29 UTC
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Github red-hat-storage ocs-operator pull 2399 0 None Merged RBD mirror daemon count validation 2024-05-06 07:40:47 UTC
Github red-hat-storage ocs-operator pull 2420 0 None Merged Bug 2259847: [release-4.15] RBD mirror daemon count validation 2024-05-06 07:40:48 UTC
Github red-hat-storage ocs-operator pull 2603 0 None Merged Fix rbd daemon count validation 2024-05-13 05:26:25 UTC
Github red-hat-storage ocs-operator pull 2604 0 None Merged Bug 2259847:[release-4.16] Fix rbd daemon count validation 2024-05-31 06:23:30 UTC
Red Hat Product Errata RHSA-2024:4591 0 None None None 2024-07-17 13:12:32 UTC

Description umanga 2024-01-23 10:53:53 UTC
Description of problem (please be detailed as possible and provide log
snippests):

StorageCluster spec allows defining the count of RBD Mirror daemon that should be deployed. But, it does not validate the provided count. This allows the user to provide a random high value which could break functionality.

Version of all relevant components (if applicable):
ocs-operator v4.15.0

Does this issue impact your ability to continue to work with the product
(please explain in detail what is the user impact)?
No

Is there any workaround available to the best of your knowledge?
No

Rate from 1 - 5 the complexity of the scenario you performed that caused this
bug (1 - very simple, 5 - very complex)?
1

Can this issue reproducible?
Yes

Can this issue reproduce from the UI?


If this is a regression, please provide more details to justify this:


Steps to Reproduce:
1.Deploy ODF
2.Set RBDMirror Count to a high value in StorageCluster CR
3.


Actual results:
Any value is allowed.

Expected results:
Only some values should be allowed like 0 or 1.

Additional info:

Comment 7 Mudit Agarwal 2024-03-12 07:19:57 UTC
Not a 4.15 blocker

Comment 12 Malay Kumar parida 2024-04-30 06:05:03 UTC
The fix for the BZ has already been shipped with 4.15 but somehow the BZ was not updated properly.

Comment 13 Malay Kumar parida 2024-04-30 06:07:01 UTC
Missed the comment from Sidhant about this being a Failed QA one, reopening and keeping it in assigned.

Comment 17 umanga 2024-05-06 07:51:30 UTC
```
CephRBDMirror.ceph.rook.io "ocs-storagecluster-cephrbdmirror" is invalid: spec.count: Invalid value: 0: spec.count in body should be greater than or equal to 1'
```

tnielsen Do you know why CephRBDMirror spec.count does not allow "0" value. I was expecting it to allow "0" which would scale down the rbd-mirror daemon.
If it is by design, we need to remove this option in ocs-operator where we are trying to scale down rbd-mirror by setting count to "0".

Comment 18 Travis Nielsen 2024-05-06 18:24:40 UTC
(In reply to umanga from comment #17)
> ```
> CephRBDMirror.ceph.rook.io "ocs-storagecluster-cephrbdmirror" is invalid:
> spec.count: Invalid value: 0: spec.count in body should be greater than or
> equal to 1'
> ```
> 
> tnielsen Do you know why CephRBDMirror spec.count does not allow
> "0" value. I was expecting it to allow "0" which would scale down the
> rbd-mirror daemon.
> If it is by design, we need to remove this option in ocs-operator where we
> are trying to scale down rbd-mirror by setting count to "0".

RBD Mirroring would be disabled if there were 0 daemons running, so I believe it's just an assumption that we always expect at least one daemon to be created. I don't know of a strict reason to prevent that if it's helpful to have for scaling down the rbd mirror daemon temporarily, but I didn't look deeply. If RBD mirroring should be disabled then we expect the mirroring CR to be deleted. What is the need to keep the mirroring enabled even while all RBD mirror daemons are scaled down?

Comment 19 umanga 2024-05-07 08:13:04 UTC
(In reply to Travis Nielsen from comment #18)
> 
> RBD Mirroring would be disabled if there were 0 daemons running, so I
> believe it's just an assumption that we always expect at least one daemon to
> be created. I don't know of a strict reason to prevent that if it's helpful
> to have for scaling down the rbd mirror daemon temporarily, but I didn't
> look deeply. If RBD mirroring should be disabled then we expect the
> mirroring CR to be deleted. What is the need to keep the mirroring enabled
> even while all RBD mirror daemons are scaled down?
>
Makes sense that we delete RBDMirror CR to disable RBD mirroring. However, in
our case, we do not want to disable mirroring. We just need to restart the RBD
mirror daemon. I assumed that setting the count to 0 would achieve that but did
not verify if it works. So, I added a DaemonCount spec in StorageCluster and allow
setting it to 0. And, that fails due to rook validation.

Is it reasonable to support this in rook? When count is set to 0, just scale down
the deployment but do not disable mirroring completely. It is up to the user to bring
it back to 1. As a workaround, today we find the rbd-mirror deployment object and set it's
replica to 0 but we'd like to avoid that.

In worst case, we'll just need to drop this spec from 4.16 and look for our options in 4.17.

Comment 20 Travis Nielsen 2024-05-07 19:02:34 UTC
If the goal is to restart the rbd mirror daemon, how about just delete the rbd mirror pod(s)? Then you don't need to change CR state temporarily and then revert it.

Comment 21 umanga 2024-05-10 09:43:11 UTC
(In reply to Travis Nielsen from comment #20)
> If the goal is to restart the rbd mirror daemon, how about just delete the
> rbd mirror pod(s)? Then you don't need to change CR state temporarily and
> then revert it.

This could be a problem when there are multiple users trying to restart the daemon.
If everyone keeps deleting it without any checks, other users will be impacted.

While we find a solution for this, I'll update the validation to match Rook validations
to maintain consistency. We don't use this feature yet, so there is no impact with this change.

Comment 28 errata-xmlrpc 2024-07-17 13:12:29 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Important: Red Hat OpenShift Data Foundation 4.16.0 security, enhancement & bug fix update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2024:4591


Note You need to log in before you can comment on or make changes to this bug.