support custom imagePullSecrets in all ServiceAccounts

[leelavg]

sample usage:
IMAGE_PULL_SECRETS="secret-1" make build-installer
helm template csi deploy/charts/ceph-csi-drivers --set 'imagePullSecrets={secret-1}'
helm template csi deploy/charts/ceph-csi-operator --set 'imagePullSecrets={secret-1}'

fixes: #305

[nb-ohad]

@leelavg I am not sure how does a change to the make file solves the issue. If I understand correctly it will require to build a custom version of the operator. Or at least run some make commends locally.

[leelavg]

the description says if one wants to attach a new pullsecret to the service account then can do so via the commands mentioned there.

to accommodate the request we require the change in Makefile and helm which this PR implements.

pls note the ask is for CSI images behind a private registry not the operator itself.

[travisn]

@leelavg In such environments, the imagePullSecrets are necessary for all images, or else they cannot be pulled, so the request would also include the operator.

The image pull secrets are needed for 1) helm installs, as well as for 2) the manifest install. For the manifest install, note that Rook commits the latest csi operator in a single csi-operator.yaml. In that manifest, only a placeholder is needed so it's easy to search and replace before installing. For example, see here where we have added the imagePullSecrets comment to a service account, and documented instructions here. FYI @subhamkrai in case we need customization for generating that manifest for rook.

[leelavg]

so the request would also include the operator.

• ack, another variable can be created explicitly for operator

1. helm installs

• already covered, we can supply the var while deployed

2. the manifest install

• I'm aware of how rook directly adds the commented out lines in the manifest file, however I don't know if we can or should take that path, if that is the requirement I can update the PR.

[travisn]

2. the manifest install

• I'm aware of how rook directly adds the commented out lines in the manifest file, however I don't know if we can or should take that path, if that is the requirement I can update the PR.

Actually it's not important to add a comment for the manifest. The users have full access to edit it directly if needed, so that's fine if only the helm chart is updated in this PR.

[leelavg]

Ok, I'll change the variable name and get it added to all serviceAccounts.

[leelavg]

updated.

I realized we have two sets of helms charts, csi-op as a superset of csi-drivers w/ duplication, need to know the reason from @Madhu-1 maybe we could just club them using subcharts depedency 🤔

[leelavg]

We are providing 2 helm charts, ceph-csi-operator (auto generated) and ceph-csi-drivers (manual afaics), the former is generated by helmify which doesn't provide an option to add imagePullSecrets to ServiceAccounts. Need to wait for Madhu to have more info why this is setup in such a way and to proceed w/ fixing the failures.

[Madhu-1]

We are providing 2 helm charts, ceph-csi-operator (auto generated) and ceph-csi-drivers (manual afaics), the former is generated by helmify which doesn't provide an option to add imagePullSecrets to ServiceAccounts. Need to wait for Madhu to have more info why this is setup in such a way and to proceed w/ fixing the failures.

yes this be as per one of the standards for the helm to separate out the CRD and the CRs its own charts. for operator helimfy provides an option to include the image pull secrets for the operator and for the drivers. we can make it as a library and use it to keep the changes very minimal

[Madhu-1]

@leelavg i got a chance to looked the code and at high level it looks good. As you have noticed that the problem is with the helmify i think we should take alternative approach to fix this problem (only temporary) and get the fix in helmify or move to some other tool (later)

• Generate the helmify as it is for today
• Create a new script that will go through all the helm templates and patch the service account yaml with the pull secrets and also the values.yaml file

With this CI will be happy and also we can keep using helmify to generate the charts and verify that charts are having latest content in the CI by default and we will apply the extra later once its done. WDYT?

[leelavg]

I'm awaiting to get #321 merged to take in the split, csi-rbac is already covered and for csi-op I'll invoke helmify with -image-pull-secrets which adds the field directly to the deployment and creates a configurable field in values.yaml.

[Madhu-1]

Closing this one as its fixed now.