[cinder-csi-plugin] Allow passing multiple config files

[mdbooth]

We would like to give the user the ability to pass arbitrary arguments to cinder-csi-plugin beyond those specified by the operator. Currently the operator specifies a 'base' configuration in a configmap, which is essentially necessary for the correct operation of the plugin in the cluster. This is currently:

[Global]
use-clouds = true
clouds-file = /etc/kubernetes/secret/clouds.yaml
cloud = openstack

The immediate driver is to give the user the ability to specify ignore-volume-az = true. There's an open PR adding that to openstack-cinder-csi-driver-operator here: openshift/openstack-cinder-csi-driver-operator#29

The PR adds a second, user-owned configmap. As currently written the operator will simply pass this user-owned config in place of the 'base' config if it exists. However, there are 2 major problems with this approach:

• It requires the user to specify the base configuration as well as any configuration they need to add, which is both more complex and more error-prone for the user
• It requires the user to 'fork' the base configuration, which makes it very difficult to update should it ever change.

We considered several other approaches to the problem:

• Add parameters to the operator's config object, and have the operator generate a complete configuration

This is certainly viable. The issue is, looking at other projects which have taken this approach, e.g. libvirt for qemu, and TripleO for openstack, the result is that all options must be implemented in 2 places before they can be exposed to the user. It can also result in user confusion when the user finds the online documentation for the service they are trying to configure, but they can't directly use they find.

• Have the operator compose the base and custom configmaps into a new configmap which is used by cinder-csi-plugin

We didn't like this due to the potential confusion for the user of having 2 additional configmaps, only one of which they're supposed to edit.

• Compose the base and custom configmaps in an init container before starting cinder-csi-plugin

This is a viable option, and there's a chance we may do this as an interim solution. The issue is that it feels more complicated than necessary. As well as the additional step, and therefore potential for user/maintainer confusion, we need to package a tool for composing ini files in an image somewhere.

The solution of passing multiple configuration files directly to cinder-csi-plugin has a number of advantages:

• It's very simple to understand
• It's a common pattern, e.g. it is used by all OpenStack services
• It's very simple to implement. I have submitted a PR here: [cinder-csi-plugin] Allow multiple config files #1476
• There's no potential for incompatibility by having config files parsed by different tools

It would be useful to OpenShift's use case, but the simplicity of it should make it useful for any deployment tooling which may want to compose cinder-csi-plugin configuration. The requirement for a 'base' configuration specific to a cluster is likely universal. Potentially layered configuration may include directly user-specified configuration as in this case, or 'zoned' configuration where certain workers have different configuration based on connectivity.

It may also make sense to make the change consistent across all services provided by CPO.

/kind feature

[jichenjc]

I think it's reasonable request

only a question looks like the change apply to other projects as well (otherwise it will be cinder CSI only feature...)

[mdbooth]

I think it's reasonable request

only a question looks like the change apply to other projects as well (otherwise it will be cinder CSI only feature...)

Although I didn't check, I assumed this would apply to all the other projects. I agree we should make them consistent, although probably best to do them individually.

[jichenjc]

yes, not asking for that in this issue,PR, just want to share some thoughts on this

this come to requirement to ask all project use same set of openstack shim layer (create client, API call etc), it might need further effort ,

[mdbooth]

I looked at the existing documentation and it's not 100% clear to me where best to document this feature. The existing docs on how to use cinder csi plugin refer to the CLOUD_CONFIG environment variable, which is a feature of the static deployment in the manifests. This feature is probably of most use to folks who generate that dynamically. I didn't want to add it to the existing manifest because I didn't want to complicate it.

I suggest I could add a new directory in examples containing an alternate deployment which specifies 2 configmaps instead of one, and provides both of those configmaps. I could then reference them in a new section in using-cinder-csi-plugin.md called something like 'Using a multi-layered cloud-config'. Would that work? I could add that as a separate commit in this PR, or a new PR.

[jichenjc]

I suggest I could add a new directory in examples containing an alternate deployment which specifies 2 configmaps instead of one, and provides both of those configmaps. I could then reference them in a new section in using-cinder-csi-plugin.md called something like 'Using a multi-layered cloud-config'. Would that work

I think a seperate PR is totally fine , and I agree move to a new folder/example should be very helpful as well
to avoid confuse people who don't need that feature, thanks for the followup

[ramineni]

I looked at the existing documentation and it's not 100% clear to me where best to document this feature. The existing docs on how to use cinder csi plugin refer to the CLOUD_CONFIG environment variable, which is a feature of the static deployment in the manifests. This feature is probably of most use to folks who generate that dynamically. I didn't want to add it to the existing manifest because I didn't want to complicate it.

I suggest I could add a new directory in examples containing an alternate deployment which specifies 2 configmaps instead of one, and provides both of those configmaps. I could then reference them in a new section in using-cinder-csi-plugin.md called something like 'Using a multi-layered cloud-config'. Would that work? I could add that as a separate commit in this PR, or a new PR.

@mdbooth Are the examples added in PR, I couldnt find it. In current manifests we pass the cloud config as secret not configmap, are you proposing to change that? I think example of alternate deployment which you are proposing would make things easier to understand. Could you add that as well .

[mdbooth]

I didn't add an example in the end, I just added command line documentation. There wasn't any existing documentation for the command line options, so I documented all of them. Incidentally, please do check that the documentation is correct!

I'm not proposing to make any changes to the manifests at all. The default manifests pass a single cloud.conf to the driver in a secret, and that will not change.

In OpenShift we control this deployment with an operator, so we don't use these manifests. We will most likely change the command line passed to the driver based on the existence of an additional, user-supplied configmap. However, from the POV of the command line arguments it makes no difference how the config files are exposed to the container.

The use case for this change is for anybody doing this kind of dynamic deployment. It is not really relevant to static deployments where the user already has responsibility for the entire configuration. In our case we want the operator to have responsibility for the make it work parts of the config, i.e. those parts which describe how to access the cloud credentials. We want the user to have responsibility for only the additional configuration directives they choose to add. Consequently we want the ability to split the configuration into multiple parts. e.g.:

Operator-managed. The operator owns this entire config fragment and can change it if required without worrying about any user additions.

[Global]
use-clouds = true
clouds-file = /etc/kubernetes/secret/clouds.yaml
cloud = openstack

User-managed. The user does not have to know where the deployment mounted the cloud credentials, or what the cloud is called. They are only interested in modifying the behaviour of the driver. The user's config will not break if a cluster-wide change alters the cloud credentials, because it doesn't contain any reference to them.

[BlockStorage]
ignore-volume-az=true

This change doesn't define where these different config fragments live.

[ramineni]

@mdbooth Thanks for the detailed explanation.
I'm in favor of splitting the config files as a default way forward for the project as well, that's the reason asked for examples as it makes sense to split to one secret (cloud credentials) and the config map , which are the config options required for the nodes. That way I guess its easier to change per node config etc.

One more option which was discussed before also sometime back , instead of seperate config file , is to add all the config parameters as command line options instead , so that it could be changed per node basis , but it might have upgrade issues.

From your POV , wdyt?

[mdbooth]

@mdbooth Thanks for the detailed explanation.
I'm in favor of splitting the config files as a default way forward for the project as well, that's the reason asked for examples as it makes sense to split to one secret (cloud credentials) and the config map , which are the config options required for the nodes. That way I guess its easier to change per node config etc.

One more option which was discussed before also sometime back , instead of seperate config file , is to add all the config parameters as command line options instead , so that it could be changed per node basis , but it might have upgrade issues.

From your POV , wdyt?

Ah, ok!

The configmap sounds simpler for an operator to integrate with. Another solution we discussed internally was to expose CSI driver options in our own CRD and have the operator write the resulting complete configuration. However, I didn't like that solution because of my experience with a certain other installer. The major disadvantages I see are:

• If the CSI driver adds a new option, the operator must also add support for that option before the user can use it. When support lags in the operator this causes user frustration. Over time it also causes mess in the operator.

• When the user googles how to achieve X with the driver, they then have to google again to work out how to achieve the same using the operator, and they're never quite sure if it's going to do exactly what they want.

IOW I'm very much in favour of allowing the user to write config directly if they want to.

The problem with individual command line options is that I think it would force us to implement support for them all in our CRD, which would amount to the same problem above I'd like to avoid. I don't think we can reasonably allow the user to pass their own command line.

I can potentially add alternate manifests to examples if you think that's worthwhile.

[ramineni]

@mdbooth Ok, even I'm more inclined towards the configmap. Please add alternate manifests as well, will help to understand better and speed up the review of the PR