[csi-plugins] add support for PVC annotations

[kayrus]

What this PR does / why we need it:

This PR adds an ability to use PVC annotations to create persistent volumes with extra options.

For manila-csi-plugin a new groupID option is added allowing the share to be created in a specific group.

Which issue this PR fixes(if applicable): fixes #2685

Special notes for reviewers:

This is a fraft, since it's depends on:

• https://github.com/gophercloud/gophercloud/pull/3211
• https://github.com/gophercloud/gophercloud/pull/3205

Release note:

[csi-plugins] add support for PVC annotations
[manila-csi-plugin] add support for share group id

[k8s-ci-robot]

Skipping CI for Draft Pull Request. If you want CI signal for your change, please convert it to an actual PR. You can still manually trigger a test run with /test all

[kayrus]

@zetaab @dulek @jichenjc ready for review

[kayrus]

I just discovered that var *volumes.SchedulerHintOpts is not nil, when the func receives the interface type as an arg.

if schedulerHints != nil {
   schedulerHints.ToSchedulerHintsMap()
}

this will cause a panic: value method openstack/blockstorage/v3/volumes.SchedulerHintOpts.ToSchedulerHintsMap called using nil *SchedulerHintOpts pointer

According to go, the passed var *volumes.SchedulerHintOpts is (*volumes.SchedulerHintOpts)(nil), and passed var volumes.SchedulerHintOptsBuilder interface is <nil>

So I had to push this:

diff --git a/pkg/csi/cinder/controllerserver.go b/pkg/csi/cinder/controllerserver.go
index a96b817a..dd630ba8 100644
--- a/pkg/csi/cinder/controllerserver.go
+++ b/pkg/csi/cinder/controllerserver.go
@@ -198,7 +198,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
        }

        // Set scheduler hints if affinity or anti-affinity is set in PVC annotations
-       var schedulerHints *volumes.SchedulerHintOpts
+       var schedulerHints volumes.SchedulerHintOptsBuilder
        var volCtx map[string]string
        affinity := util.SplitTrimJoin(pvcAnnotations[affinityKey], ',')
        antiAffinity := util.SplitTrimJoin(pvcAnnotations[antiAffinityKey], ',')
diff --git a/pkg/csi/cinder/openstack/openstack.go b/pkg/csi/cinder/openstack/openstack.go
index cc733690..46116ecb 100644
--- a/pkg/csi/cinder/openstack/openstack.go
+++ b/pkg/csi/cinder/openstack/openstack.go
@@ -45,7 +45,7 @@ func AddExtraFlags(fs *pflag.FlagSet) {
 }

 type IOpenStack interface {
-       CreateVolume(*volumes.CreateOpts, *volumes.SchedulerHintOpts) (*volumes.Volume, error)
+       CreateVolume(*volumes.CreateOpts, volumes.SchedulerHintOptsBuilder) (*volumes.Volume, error)
        DeleteVolume(volumeID string) error
        AttachVolume(instanceID, volumeID string) (string, error)
        ListVolumes(limit int, startingToken string) ([]volumes.Volume, string, error)
diff --git a/pkg/csi/cinder/openstack/openstack_mock.go b/pkg/csi/cinder/openstack/openstack_mock.go
index b35d923c..89f0d3dc 100644
--- a/pkg/csi/cinder/openstack/openstack_mock.go
+++ b/pkg/csi/cinder/openstack/openstack_mock.go
@@ -85,7 +85,7 @@ func (_m *OpenStackMock) AttachVolume(instanceID string, volumeID string) (strin
 }

 // CreateVolume provides a mock function with given fields: name, size, vtype, availability, tags
-func (_m *OpenStackMock) CreateVolume(opts *volumes.CreateOpts, _ *volumes.SchedulerHintOpts) (*volumes.Volume, error) {
+func (_m *OpenStackMock) CreateVolume(opts *volumes.CreateOpts, _ volumes.SchedulerHintOptsBuilder) (*volumes.Volume, error) {
        name := opts.Name
        size := opts.Size
        vtype := opts.VolumeType
diff --git a/pkg/csi/cinder/openstack/openstack_volumes.go b/pkg/csi/cinder/openstack/openstack_volumes.go
index f7f6be95..b3d90230 100644
--- a/pkg/csi/cinder/openstack/openstack_volumes.go
+++ b/pkg/csi/cinder/openstack/openstack_volumes.go
@@ -51,7 +51,7 @@ const (
 var volumeErrorStates = [...]string{"error", "error_extending", "error_deleting"}

 // CreateVolume creates a volume of given size
-func (os *OpenStack) CreateVolume(opts *volumes.CreateOpts, schedulerHints *volumes.SchedulerHintOpts) (*volumes.Volume, error) {
+func (os *OpenStack) CreateVolume(opts *volumes.CreateOpts, schedulerHints volumes.SchedulerHintOptsBuilder) (*volumes.Volume, error) {
        blockstorageClient, err := openstack.NewBlockStorageV3(os.blockstorage.ProviderClient, os.epOpts)
        if err != nil {
                return nil, err
diff --git a/tests/sanity/cinder/fakecloud.go b/tests/sanity/cinder/fakecloud.go
index a02a8e77..56ee04ad 100644
--- a/tests/sanity/cinder/fakecloud.go
+++ b/tests/sanity/cinder/fakecloud.go
@@ -34,7 +34,7 @@ func getfakecloud() *cloud {
 var _ openstack.IOpenStack = &cloud{}

 // Fake Cloud
-func (cloud *cloud) CreateVolume(opts *volumes.CreateOpts, _ *volumes.SchedulerHintOpts) (*volumes.Volume, error) {
+func (cloud *cloud) CreateVolume(opts *volumes.CreateOpts, _ volumes.SchedulerHintOptsBuilder) (*volumes.Volume, error) {
        vol := &volumes.Volume{
                ID:               randString(10),
                Name:             opts.Name,

to fix the:

panic: value method github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes.SchedulerHintOpts.ToSchedulerHintsMap called using nil *SchedulerHintOpts pointer

goroutine 62 [running]:
github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes.(*SchedulerHintOpts).ToSchedulerHintsMap(0xd?)
    <autogenerated>:1 +0x8c
github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes.Create({0x22647c8, 0x3383fc0}, 0xc000233180, {0x2241820, 0xc00002e370}, {0x2241840, 0x0})
    github.com/gophercloud/gophercloud/v2@v2.1.2-0.20241016132526-1c4dd03733fe/openstack/blockstorage/v3/volumes/requests.go:154 +0xf7
k8s.io/cloud-provider-openstack/pkg/csi/cinder/openstack.(*OpenStack).CreateVolume(0xc0001a6c80, 0xc00002e370, 0x0)
    k8s.io/cloud-provider-openstack/pkg/csi/cinder/openstack/openstack_volumes.go:68 +0x245
k8s.io/cloud-provider-openstack/pkg/csi/cinder.(*controllerServer).CreateVolume(0xc00051a030, {0x1ec1a20?, 0x20?}, 0xc0000c6800)
    k8s.io/cloud-provider-openstack/pkg/csi/cinder/controllerserver.go:215 +0x1caf
github.com/container-storage-interface/spec/lib/go/csi._Controller_CreateVolume_Handler.func1({0x2264800?, 0xc0003c3500?}, {0x1e83680?, 0xc0000c6800?})
    github.com/container-storage-interface/spec@v1.9.0/lib/go/csi/csi.pb.go:6587 +0xcb
k8s.io/cloud-provider-openstack/pkg/csi/cinder.logGRPC({0x2264800, 0xc0003c3500}, {0x1e83680, 0xc0000c6800}, 0xc000307240, 0xc000415c38)
    k8s.io/cloud-provider-openstack/pkg/csi/cinder/utils.go:97 +0x2f5
github.com/container-storage-interface/spec/lib/go/csi._Controller_CreateVolume_Handler({0x1ec1a20, 0xc00051a030}, {0x2264800, 0xc0003c3500}, 0xc0000c6700, 0x2089b60)
    github.com/container-storage-interface/spec@v1.9.0/lib/go/csi/csi.pb.go:6589 +0x143
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0001a3000, {0x2264800, 0xc0003c3470}, {0x2270ae0, 0xc0004e9680}, 0xc00017b200, 0xc000502b40, 0x3306020, 0x0)
    google.golang.org/grpc@v1.65.0/server.go:1379 +0xdf8
google.golang.org/grpc.(*Server).handleStream(0xc0001a3000, {0x2270ae0, 0xc0004e9680}, 0xc00017b200)
    google.golang.org/grpc@v1.65.0/server.go:1790 +0xe8b
google.golang.org/grpc.(*Server).serveStreams.func2.1()
    google.golang.org/grpc@v1.65.0/server.go:1029 +0x8b
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 60
    google.golang.org/grpc@v1.65.0/server.go:104

[kayrus]

@zetaab @dulek @jichenjc kindly ping

[chuan137]

@freeformz @gnufied @ncdc @dulek @jichenjc Hey guys, could you have a look at this PR? I am going to discuss adding affinity feature to share groups in the coming openstack PTG meeting with upstream. This PR allows us to provision group of PVCs with affinity relationships. It would be nice to have your feedback. Cheers, Chuan

[kayrus]

@zetaab @dulek @jichenjc do you have any updates?

[kayrus]

@dulek

Do I correctly understand that this requires knowing the volume or share ID upfront before creating a PVC? This is totally not a K8s pattern, as to e.g. deploy three volumes with anti-affinity you need to create one, wait for ID to be populated, create second, wait for its ID and only then create the third, right?

Right. Unfortunately I don't see any other way to distribute shares across multiple backends. There will be a feature, based on the share group id, but I suppose it will be implemented mid next year, that's why I introduced a group ID support in advance.

the value must be the share name or id

Interesting finding, but it doesn't work with API. I tested this locally and got: {"badRequest": {"code": 400, "message": "123a is not a valid uuid."}}. Do you think it's worth adding the resolving share ID by name like it's done in user CLI tool?

[dulek]

Alright, so we get what we can here.

/lgtm /approve /hold for @kayrus to decide when this is ready.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dulek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes/cloud-provider-openstack/blob/master/OWNERS)~~ [dulek] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

[kayrus]

@dulek I decided to add name resolving.

[kayrus]

@dulek waiting for a second lgtm

[kayrus]

@dulek @zetaab kindly ping

[dulek]

/lgtm

[kayrus]

/unhold

[dulek]

/lgtm

[kayrus]

/test openstack-cloud-csi-cinder-e2e-test

[kayrus]

/test openstack-cloud-csi-manila-e2e-test