kubevirt / csi-driver

KubeVirt CSI driver
Other
34 stars 25 forks source link

Option to use the default infra storageclass automatically #72

Closed davidvossel closed 1 year ago

davidvossel commented 1 year ago

With the useDefaultInfraStorageClass parameter, a storageclass can be created in the guest cluster without having knowledge of what the underlying infra cluster provides. Whatever is the default storageclass on the infra will be used.

NONE
kubevirt-bot commented 1 year ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from davidvossel by writing /assign @davidvossel in a comment. For more information see the Kubernetes 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](https://github.com/kubevirt/csi-driver/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
davidvossel commented 1 year ago

/cc @awels i'd be interested in what you think of this idea. It allows us to create a storageclass in the guest cluster without explicitly mapping to a specific infra storage class.

awels commented 1 year ago

Is this for the 'regular' installation where the controller exists in tenant cluster? Or also for the 'split' configuration? I thought in the split configuration the tenant didn't know anything about the infra storage class already.

davidvossel commented 1 year ago

Is this for the 'regular' installation where the controller exists in tenant cluster? Or also for the 'split' configuration? I thought in the split configuration the tenant didn't know anything about the infra storage class already.

it would be used for either. the storageclass in the guest wouldn't specify an infra storageclass, and the csi controller would create a PVC without the storageclass as well, which would result in the default storageclass being used.

basically, defaults all the way down.

awels commented 1 year ago

Okay I am slightly confused by the fact you say the tenant storage class, specifies the infra storage class to use. Only the controller should care about the infra storage class (so it can generate the DVs) I see no reasonable reason for the infra storage class to be passed as a parameter. It should be an argument to the controller when it is created.

But if we can start by eliminating one argument from the tenant storage class I am all for it.

davidvossel commented 1 year ago

/hold

I want to build of isaac's work here, https://github.com/kubevirt/csi-driver/pull/62, before merging this. We're working in similar areas and I think i can converge some of my work into his storageclass enforcement config options

davidvossel commented 1 year ago

obsoleted by https://github.com/kubevirt/csi-driver/pull/73