search

container-storage-interface / spec

Container Storage Interface (CSI) Specification.
Apache License 2.0
1.36k stars 373 forks source link

Add controller capability to advertise whether mount, block, or both types of Volumes may be created #565

Open ebblake opened 5 months ago

ebblake commented 5 months ago

Is your feature request related to a problem?/Why is this needed

While implementing a new CSI driver (KubeSAN), we initially implemented a block-only setup (that is, any VolumeCapability used to create a volume must include access_type.block; because the driver rejects access_type.mount fails with InvalidArgument).

The csi-sanity testsuite defaults to Mount (filesystem) volumes, but supports --csi-testvolumeaccesstype block to switch the testing to just creation of block volumes instead. But it would be nice if it could determine which type(s) are supported by the driver, and test the correct one(s) automatically; in particular, it would be nice for csi-sanity to prove that it is not possible to create a volume clone where the clone requested VolumeCapability.access_type.mount but the source snapshot or volume had access_type.block, and vice versa.

See also https://github.com/kubernetes-csi/csi-test/issues/541

Describe the solution you'd like in detail

My initial idea: in ControllerGetCapabilities, expand the exisitng

message ControllerServiceCapability {
...
  oneof type {
    // RPC that the controller supports.
    RPC rpc = 1;
  }
}

to add a new capability-with-value that advertises the type(s) of volumes that the driver is capable of creating. Something like:

message ControllerServiceCapability {
...
  // Optional, although recommended if the controller advertises CREATE_DELETE_VOLUME.
  // If advertised, the CO must assume that any volume creation attempt not using one of the advertised types will fail with INVALID_ARGUMENT
  message SupportedVolumeAccessTypes {
    enum Type {
      // The controller supports VolumeCapability.access_type.block
      block = 1;
      // The controller supports VolumeCapability.access_type.mount
      mount = 2;
    }

    repeated Type type = 1;
  }
  oneof type {
    // RPC that the controller supports.
    RPC rpc = 1;
    // Set of volume types explicitly supported by the controller.
    SupportedVolumeAccessTypes supported_volume_access_types = 2;
  }
}

Describe alternatives you've considered

Right now, the CO has to know in advance which type(s) of volumes may be handled, and merely deal with the InvalidArgument errors returned when the CreateVolume or similar request passes in an unsupported type.

Additional context

Here's a link to KubeSAN trying to implement Mount support: https://gitlab.com/kubesan/kubesan/-/merge_requests/45

jdef commented 5 months ago

It sounds like the use case here is to simplify automated testing in k8s land. Or is there another fish to fry?

On Sat, Jun 22, 2024, 2:35 PM Eric Blake @.***> wrote:

Is your feature request related to a problem?/Why is this needed

While implementing a new CSI driver (KubeSAN), we initially implemented a block-only setup (that is, any VolumeCapability used to create a volume must include access_type.block; because the driver rejects access_type.mount fails with InvalidArgument).

The csi-sanity testsuite defaults to Mount (filesystem) volumes, but supports --csi-testvolumeaccesstype block to switch the testing to just creation of block volumes instead. But it would be nice if it could determine which type(s) are supported by the driver, and test the correct one(s) automatically; in particular, it would be nice for csi-sanity to prove that it is not possible to create a volume clone where the clone requested VolumeCapability.access_type.mount but the source snapshot or volume had access_type.block, and vice versa.

See also kubernetes-csi/csi-test#541 https://github.com/kubernetes-csi/csi-test/issues/541

Describe the solution you'd like in detail

My initial idea: in ControllerGetCapabilities, expand the exisitng

message ControllerServiceCapability { ... oneof type { // RPC that the controller supports. RPC rpc = 1; } }

to add a new capability-with-value that advertises the type(s) of volumes that the driver is capable of creating. Something like:

message ControllerServiceCapability { ... // Optional, although recommended if the controller advertises CREATE_DELETE_VOLUME. // If advertised, the CO must assume that any volume creation attempt not using one of the advertised types will fail with INVALID_ARGUMENT message SupportedVolumeAccessTypes { enum Type { // The controller supports VolumeCapability.access_type.block block = 1; // The controller supports VolumeCapability.access_type.mount mount = 2; }

repeated Type type = 1;

} oneof type { // RPC that the controller supports. RPC rpc = 1; // Set of volume types explicitly supported by the controller. SupportedVolumeAccessTypes supported_volume_access_types = 2; } }

Describe alternatives you've considered

Right now, the CO has to know in advance which type(s) of volumes may be handled, and merely deal with the InvalidArgument errors returned when the CreateVolume or similar request passes in an unsupported type.

Additional context

Here's a link to KubeSAN trying to implement Mount support: https://gitlab.com/kubesan/kubesan/-/merge_requests/45

— Reply to this email directly, view it on GitHub https://github.com/container-storage-interface/spec/issues/565, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR5KLFFBGVB3NLWESUPDUDZIW7VJAVCNFSM6AAAAABJXUNOSSVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM3DOOJWGI3TSMY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

ebblake commented 1 month ago

It sounds like the use case here is to simplify automated testing in k8s land. Or is there another fish to fry?

[Sorry for my delay in responding]

I just noticed this code: https://github.com/kubernetes-csi/external-provisioner/blob/0f8b1662e25f37675da1f82acb78cd820b359419/pkg/controller/controller.go#L1385

func (p *csiProvisioner) SupportsBlock(ctx context.Context) bool {
        // SupportsBlock always return true, because current CSI spec doesn't allow checking
        // drivers' capability of block volume before creating volume.
        // Drivers that don't support block volume should return error for CreateVolume called
        // by Provision if block AccessType is specified.
        return true
}

That is, if the CSI spec made it possible for drivers to advertise which type(s) of volume creation they support, the kubernetes CSI client would be able to take advantage of that instead of relying on getting an error message from the driver.