Skip to content
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

Storage Class parameters during all CSI calls #387

Open
venkatsc opened this issue Sep 3, 2019 · 12 comments
Open

Storage Class parameters during all CSI calls #387

venkatsc opened this issue Sep 3, 2019 · 12 comments

Comments

@venkatsc
Copy link

venkatsc commented Sep 3, 2019

Currently, CSI sends parameters configured in a storage class in CreateVolumeRequest, ValidateVolumeCapabilitiesRequest, GetCapacityRequest, and CreateSnapshotRequest but all other request types are missing the KV pairs configured under StorageClass.parameters.

Use case
We have two storage clusters -- production, testing. To use these both clusters with same CSI driver, we need to deploy the same CSI driver with different provisioner names (quobyte-csi-prod, quobyte-csi-testing) each pointing to their own endpoint.

With missing StorageClass parameters in some CSI calls (DeleteVolume, PublishVolume etc), we have only two options

  1. Deploy multiple instances of the same CSI driver with their own endpoint. Passing parameters during every CSI call will enable single CSI driver instance to talk to multiple endpoints of the same vendor.
  2. Though, not clean, append vendor specific parameters such as endpoint etc to the VolumeHandle: in the vendor CSI driver during CreateVolume for making parameters available during later stages of CSI calls.
@shay-berman
Copy link

Can you elaborate about the endpoint use case. What is this endpoint in your use case? is it the backend storage detail of your CSI driver like address?

Which APIs require the endpoints in your use case? is it only the controller APIs or also the node APIs?

@venkatsc
Copy link
Author

venkatsc commented Sep 4, 2019

Can you elaborate about the endpoint use case. What is this endpoint in your use case? is it the backend storage detail of your CSI driver like address?

Yes, it is the address of the storage API. The volume creation/deletion is achieved through the vendor API calls. Since DeleteVolume is not provided with parameters from storage class, the only way, AFAIU is to start driver with the API endpoint address and that will tie the CSI driver to that storage endpoint.

We make use of NodePublishVolume in our CSI driver. In our storage solution, volumes have both name (does not have to be unique under different storage tenants. Tenants are akin to namespaces in K8S) and UUID (unique throughout storage cluster). While mounting volume, our CSI driver need to resolve the name and tenant combination to UUID by talking to the storage API address. Without parameters from the StorageClass during NodePublishVolume call, storage API address has to be tied to driver deployment.

Which APIs require the endpoints in your use case? is it only the controller APIs or also the node APIs?

As explained above, we have use cases for both types of CSI APIs, particularly, DeleteVolume and NodePublishVolume calls.

@shay-berman
Copy link

for today CSI spec, you will have to add configmap to your driver in order to get this detail in all the calls.
OR you can use the CSI secret keys in the storage class and just put your address inside secret so the driver will have this secret in every CSI call. (I know that address is not really a secret but its an option for you)

@venkatsc
Copy link
Author

venkatsc commented Sep 5, 2019

Yes. As it was mentioned in the description, we deployed two different drivers, each with their own endpoint tied to the driver deployment. This is more of a feature request.

Moreover, the only way for vendor CSI drivers to get configurable information cleanly is with StorageClass parameters and passing these during all CSI calls will enable vendor CSI drivers to get configurable information such as endpoint address.

@jdef
Copy link
Member

jdef commented Sep 10, 2019

In this case it sounds like "API address" is related to the SP's storage topology, and there's a desire to map different classes (ala StorageClass) of storage to different backends/racks (or storage segments). I'm not super familiar with how CSI topology has been integrated into the k8s CSI stack, but if there's topological information passed along to each of the CSI node/controller calls then it could be useful to capture "API address" that way, or at least provide a mapping element from which the proper API address could be derived.

@venkatsc
Copy link
Author

It looks like storage topology is to add constraints within the same storage cluster and may not be suitable for the current requirement.

This feature request is to achieve the below setup (single instance of CSI driver from the storage vendor should be able to talk to multiple storage clusters of the vendor)
CSI endpoint-single-instance

But, CSI driver at the moment can only be deployed as below due to the missing StorageClass.parameters during some CSI API (DeleteVolume, NodePublishVolume etc) calls.

CSI endpoint

@jdef
Copy link
Member

jdef commented Oct 14, 2019

I've given this a little more thought and, you're right, the topology suggestion I made doesn't quite fit.

That said, I'm wondering if you really want a single instance of the driver talking to two different backends. For example, if your different backends are for different tenants, and tenant A repeatedly issues a set of RPCs that chokes the driver instance, then tenant B's use of the same driver is impacted. Another example: you want to upgrade the testing cluster backend, but that requires a driver upgrade as well - but there's risk to production now because bumping the driver may impact the stability of the production cluster (yeah, it's probably low risk, but still).

It sounds like your use case is: "I have multiple storage backends, each for a different tenant, at different endpoints and I want to be able to configure a single CSI driver instance to communicate to both of them depending on the storage class of an existing or to-be-created volume."

CSI hasn't actually tried to resolve tenancy issues - we've typically punted to COs for that. I'm not convinced that we should try to tackle multi-tenancy at this level.

@zhucan
Copy link

zhucan commented Oct 29, 2019

@venkatsc +1

@zhucan
Copy link

zhucan commented Oct 29, 2019

@jdef we hope it can be added that Storage Class parameters during all CSI calls. The url of the storage can be added to the secret is a options, but I think it's not suitable, In all of our use cases, on kubernetes to multi backend storage.

@jdef
Copy link
Member

jdef commented Oct 29, 2019

CSI hasn't actually tried to resolve tenancy issues - we've typically punted to COs for that. I'm not convinced that we should try to tackle multi-tenancy at this level.

Has this been discussed in the k8s sig-storage channel yet? If not, it probably should be. Tenancy is a CO issue.

@venkatsc
Copy link
Author

I'm wondering if you really want a single instance of the driver talking to two different backends. For example, if your different backends are for different tenants, and tenant A repeatedly issues a set of RPCs that chokes the driver instance, then tenant B's use of the same driver is impacted. Another example: you want to upgrade the testing cluster backend, but that requires a driver upgrade as well - but there's risk to production now because bumping the driver may impact the stability of the production cluster (yeah, it's probably low risk, but still).

True, but providing StorageClass parameters does not change the existing behavior. It enables additional flexibility of CSI driver deployment as explained in the scenario above. Storage Vendor and their customers can choose to deploy single instance or multiple instance of their driver depending on their requirement.

Additionally, passing parameters during all CSI calls also gives users an opportunity to configure additional required KVs for aforementioned CSI calls. For example, with our storage, we need volumeID (akin to K8S resource name) and tenant ID (comparable to K8S namespace). With current CSI design, we need to append this information to PV.volumeID during volume creation, as the configured data in StorageClass does not reach beyond CreateVolume CSI call. This would complicate the standard CSI flow for the storage vendor such as using common e2e test cases or deviations from standard K8S resource definitions (example, for pre-provisioned volume, we need user to configure PV.volumeID: "namespace|volume" as Node* CSI calls don't read information from StorageClass).

@jdef
Copy link
Member

jdef commented Oct 29, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants