Add controller publish secrets to default RBD/CephFS StorageClasses

[iPraveenParihar]

Ceph-CSI will be implementing ControllerPublishVolume()/ControllerUnpublishVolume() which requires
controller-publish-secret from StorageClass. The secret is needed to access the Ceph cluster for [add|remove] metadata/blocklist operations.

The following parameters to be added in default RBD/CephFS StorageClasses created by ocs-operator.

csi.storage.k8s.io/controller-publish-secret-name
csi.storage.k8s.io/controller-publish-secret-namespace

Story: https://issues.redhat.com/browse/RHSTOR-7598

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: iPraveenParihar
Once this PR has been reviewed and has the lgtm label, please assign nbalacha for approval. For more information see the 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

[iPraveenParihar]

cc @Madhu-1 @Rakshith-R

[Madhu-1]

@iPraveenParihar How about implementing the changes in ceph-csi first and later we can add the required parameters to the SC later?

[iPraveenParihar]

@iPraveenParihar How about implementing the changes in ceph-csi first and later we can add the required parameters to the SC later?

Since theses changes are not dependent, thought raising the PR for reviews.
Anyway, these changes will not harm existing functionalities?

[Madhu-1]

@iPraveenParihar How about implementing the changes in ceph-csi first and later we can add the required parameters to the SC later?

Since theses changes are not dependent, thought raising the PR for reviews. Anyway, these changes will not harm existing functionalities?

Even we might not need this change if we just update the csi configmap with the right secrets. If this PR gets merged first, not have the cephcsi PR merged on time. we might end up having extra secrets in the storageclass but not the functionality.

[iPraveenParihar]

@Madhu-1, since the ceph-csi PR is merged. Can we check on this?
Are you suggesting to not add controller secrets to StorageClass and add directly in ceph-csi-config Map?

[Madhu-1]

No harm in setting this one.

@iPraveenParihar if we set it in the Clientprofile CR, we dont need this change right? what is the benefit of setting it in both places?

@nb-ohad WDYT?

[iPraveenParihar]

@iPraveenParihar if we set it in the Clientprofile CR, we dont need this change right? what is the benefit of setting it in both places?

If the controller secrets are not present in StorageClass, then Ceph-CSI will attempt to fetch by making a k8s API call.
For now, ceph-csi doesn't have a cache mechanism so it would be a load on k8s API server making a call for each PV.

Its just having controller-publish-secret{name|namespace} parameter in StorageClass, should not harm?

[Madhu-1]

@iPraveenParihar if we set it in the Clientprofile CR, we dont need this change right? what is the benefit of setting it in both places?

If the controller secrets are not present in StorageClass, then Ceph-CSI will attempt to fetch by making a k8s API call. For now, ceph-csi doesn't have a cache mechanism so it would be a load on k8s API server making a call for each PV.

If the secrets are present its the same. it will be fetched by the external attacher itself, i dont think it uses cache client. Does it uses cache client?

Its just having controller-publish-secret{name|namespace} parameter in StorageClass, should not harm?

No harm, just waiting for confirmation, what i heard was the ODF has plan to move to configmap not to add more secrets to the storageclass. tagged Ohad to confirm on that one.

[iPraveenParihar]

If the secrets are present its the same. it will be fetched by the external attacher itself, i dont think it uses cache client. Does it uses cache client?

My bad, I was assuming it uses cache client. Then it should be fine.

[openshift-merge-robot]

PR needs rebase.

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.