-
Notifications
You must be signed in to change notification settings - Fork 559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
util: add support to configure mirror daemon count #4566
Conversation
internal/util/csiconfig.go
Outdated
@@ -45,6 +45,7 @@ const ( | |||
"clusterID": "<cluster-id>", | |||
"rbd": { | |||
"radosNamespace": "<rados-namespace>" | |||
"mirrorDaemonCount": "1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great test case, but unfortunately missing in the unit tests.
Please add a test-case that expects an error when the argument is a number as "string"
(maybe convert it?) or even an invalid number.
Currently we are assuming that only one rbd mirror daemon running on the ceph cluster but that is not true for many cases and it can be more that one, this PR make this as a configurable parameter. fixes: ceph#4312 Signed-off-by: Madhu Rajanna <[email protected]>
@megifyio rebase |
@Mergifyio rebase |
@Mergifyio queue |
☑️ Nothing to do
|
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 4c2d2ca |
/test ci/centos/k8s-e2e-external-storage/1.29 |
/test ci/centos/k8s-e2e-external-storage/1.28 |
/test ci/centos/k8s-e2e-external-storage/1.27 |
/test ci/centos/mini-e2e-helm/k8s-1.29 |
/test ci/centos/mini-e2e-helm/k8s-1.28 |
/test ci/centos/mini-e2e-helm/k8s-1.27 |
/test ci/centos/mini-e2e/k8s-1.29 |
/test ci/centos/mini-e2e/k8s-1.28 |
/test ci/centos/mini-e2e/k8s-1.27 |
/test ci/centos/upgrade-tests-cephfs |
/test ci/centos/upgrade-tests-rbd |
Describe what this PR does
Currently, we are assuming that only one rbd mirror daemon running on the ceph cluster but that is not true for many cases and it can be more than one, this PR makes this a configurable parameter.
Is there anything that requires special attention
Do you have any questions?
Is the change backward compatible?
yes
Are there concerns around backward compatibility?
No
Fixes: #4312