Add support for volume/share name resolve

Author: kayrus

docs/cinder-csi-plugin/examples.md

@@ -481,7 +481,7 @@ metadata:
   name: csi-pvc-cinderplugin
   annotations:
     cinder.csi.openstack.org/affinity: "1b4e28ba-2fa1-11ec-8d3d-0242ac130003"
-    cinder.csi.openstack.org/anti-affinity: "1b4e28ba-2fa1-11ec-8d3d-0242ac130004,1b4e28ba-2fa1-11ec-8d3d-0242ac130005"
+    cinder.csi.openstack.org/anti-affinity: "1b4e28ba-2fa1-11ec-8d3d-0242ac130004,pv-k8s--cluster-1b5f47bf-0119-442e-8529-254c36e43644"
 spec:
   accessModes:
     - ReadWriteOnce
@@ -493,12 +493,12 @@ spec:
 
 where `1b4e28ba-2fa1-11ec-8d3d-0242ac130003`,
 `1b4e28ba-2fa1-11ec-8d3d-0242ac130004` and
-`1b4e28ba-2fa1-11ec-8d3d-0242ac130005` are the UUIDs of the already provisioned
-volumes in the OpenStack cloud. The scheduler will try to place the volume in
-the same block storage server as the volume with UUID
-`1b4e28ba-2fa1-11ec-8d3d-0242ac130003` and avoid placing the volume in the same
-block storage server as the volumes with UUIDs
-`1b4e28ba-2fa1-11ec-8d3d-0242ac130004` and
-`1b4e28ba-2fa1-11ec-8d3d-0242ac130005`. If the scheduler hints are not
-satisfied, the volume will not be provisioned with an error message in the
-controller logs.
+`pv-k8s--cluster-1b5f47bf-0119-442e-8529-254c36e43644` are names or UUIDs of
+the already provisioned volumes in the OpenStack cloud. The scheduler will try
+to place the volume in the same block storage server as the volume with the
+`1b4e28ba-2fa1-11ec-8d3d-0242ac130003` UUID and avoid placing the volume in the
+same block storage server as the volumes with the
+`1b4e28ba-2fa1-11ec-8d3d-0242ac130004` UUID and the
+`pv-k8s--cluster-1b5f47bf-0119-442e-8529-254c36e43644` volume name. If the
+scheduler hints are not satisfied, the volume will not be provisioned with an
+error message in the controller logs.

docs/cinder-csi-plugin/using-cinder-csi-plugin.md

@@ -291,14 +291,14 @@ following PVC annotations are supported:
 
 | Annotation Name            | Description      | Example |
 |-------------------------   |-----------------|----------|
-| `cinder.csi.openstack.org/affinity` | Volume affinity to existing volume or volumes UUIDs. The value should be a comma-separated list of volume UUIDs. | `cinder.csi.openstack.org/affinity: "1b4e28ba-2fa1-11ec-8d3d-0242ac130003"` |
-| `cinder.csi.openstack.org/anti-affinity` | Volume anti-affinity to existing volume or volumes UUIDs. The value should be a comma-separated list of volume UUIDs. | `cinder.csi.openstack.org/anti-affinity: "1b4e28ba-2fa1-11ec-8d3d-0242ac130004,1b4e28ba-2fa1-11ec-8d3d-0242ac130005"` |
+| `cinder.csi.openstack.org/affinity` | Volume affinity to existing volume or volumes names/UUIDs. The value should be a comma-separated list of volume names/UUIDs. | `cinder.csi.openstack.org/affinity: "1b4e28ba-2fa1-11ec-8d3d-0242ac130003"` |
+| `cinder.csi.openstack.org/anti-affinity` | Volume anti-affinity to existing volume or volumes names/UUIDs. The value should be a comma-separated list of volume names/UUIDs. | `cinder.csi.openstack.org/anti-affinity: "1b4e28ba-2fa1-11ec-8d3d-0242ac130004,pv-k8s--cluster-1b5f47bf-0119-442e-8529-254c36e43644"` |
 
 If the PVC annotation is set, the volume will be created according to the
-existing volume UUIDs placements, i.e. on the same host as the
+existing volume names/UUIDs placements, i.e. on the same host as the
 `1b4e28ba-2fa1-11ec-8d3d-0242ac130003` volume and not on the same host as the
 `1b4e28ba-2fa1-11ec-8d3d-0242ac130004` and
-`1b4e28ba-2fa1-11ec-8d3d-0242ac130005` volumes.
+`pv-k8s--cluster-1b5f47bf-0119-442e-8529-254c36e43644` volumes.
 
 ## Local Development
 

docs/manila-csi-plugin/using-manila-csi-plugin.md

@@ -286,20 +286,19 @@ following PVC annotations are supported:
 
 | Annotation Name            | Description      | Example |
 |-------------------------   |-----------------|----------|
-| `manila.csi.openstack.org/affinity` | Share affinity to existing share or shares UUIDs. The value should be a comma-separated list of share UUIDs. | `manila.csi.openstack.org/affinity: "1b4e28ba-2fa1-11ec-8d3d-0242ac130003"` |
-| `manila.csi.openstack.org/anti-affinity` | Share anti-affinity to existing share or shares UUIDs. The value should be a comma-separated list of share UUIDs. | `manila.csi.openstack.org/anti-affinity: "1b4e28ba-2fa1-11ec-8d3d-0242ac130004,1b4e28ba-2fa1-11ec-8d3d-0242ac130005"` |
+| `manila.csi.openstack.org/affinity` | Share affinity to existing share or shares names/UUIDs. The value should be a comma-separated list of share names or UUIDs. | `manila.csi.openstack.org/affinity: "1b4e28ba-2fa1-11ec-8d3d-0242ac130003"` |
+| `manila.csi.openstack.org/anti-affinity` | Share anti-affinity to existing share or shares names/UUIDs. The value should be a comma-separated list of share names/UUIDs. | `manila.csi.openstack.org/anti-affinity: "1b4e28ba-2fa1-11ec-8d3d-0242ac130004,pv-default-50c5a3b3-e0b5-48d6-a163-4e68956aeb54"` |
 | `manila.csi.openstack.org/group-id` | The UUID of the share group to which the provisioned share must belong. The share group must be created before the PVC is created. | `manila.csi.openstack.org/group-id: "1b4e28ba-2fa1-11ec-8d3d-0242ac130006"` |
 
 If the PVC annotation is set, the share will be created according to the
-existing share UUIDs placements, i.e. on the same host as the
+existing share names/UUIDs placements, i.e. on the same host as the
 `1b4e28ba-2fa1-11ec-8d3d-0242ac130003` share and not on the same host as the
 `1b4e28ba-2fa1-11ec-8d3d-0242ac130004` and
-`1b4e28ba-2fa1-11ec-8d3d-0242ac130005` shares.
+`pv-default-50c5a3b3-e0b5-48d6-a163-4e68956aeb54` shares.
 
 The `manila.csi.openstack.org/group-id` annotation value overrides the storage
 class `groupID` parameter if both are set.
 
 ## For developers
 
 If you'd like to contribute to CSI Manila, check out `docs/manila-csi-plugin/developers-csi-manila.md` to get you started.
-

go.mod

@@ -5,6 +5,7 @@ go 1.22.0
 require (
 	github.com/container-storage-interface/spec v1.9.0
 	github.com/go-chi/chi/v5 v5.0.8
+	github.com/google/uuid v1.6.0
 	github.com/gophercloud/gophercloud/v2 v2.1.2-0.20241016132526-1c4dd03733fe
 	github.com/gophercloud/utils/v2 v2.0.0-20240701101423-2401526caee5
 	github.com/hashicorp/go-version v1.6.0
@@ -91,7 +92,6 @@ require (
 	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af // indirect
-	github.com/google/uuid v1.6.0 // indirect
 	github.com/gorilla/websocket v1.5.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0 // indirect

pkg/csi/cinder/controllerserver.go

@@ -200,16 +200,29 @@ 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.SchedulerHintOptsBuilder
 	var volCtx map[string]string
-	affinity := util.SplitTrimJoin(pvcAnnotations[affinityKey], ',')
-	antiAffinity := util.SplitTrimJoin(pvcAnnotations[antiAffinityKey], ',')
+	affinity := pvcAnnotations[affinityKey]
+	antiAffinity := pvcAnnotations[antiAffinityKey]
 	if affinity != "" || antiAffinity != "" {
+		klog.V(4).Infof("CreateVolume: Getting scheduler hints: affinity=%s, anti-affinity=%s", affinity, antiAffinity)
+
+		// resolve volume names to UUIDs
+		affinity, err = cloud.ResolveVolumeListToUUIDs(affinity)
+		if err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "failed to resolve affinity volume UUIDs: %v", err)
+		}
+		antiAffinity, err = cloud.ResolveVolumeListToUUIDs(antiAffinity)
+		if err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "failed to resolve anti-affinity volume UUIDs: %v", err)
+		}
+
 		volCtx = util.SetMapIfNotEmpty(volCtx, "affinity", affinity)
 		volCtx = util.SetMapIfNotEmpty(volCtx, "anti-affinity", antiAffinity)
 		schedulerHints = &volumes.SchedulerHintOpts{
 			SameHost:      util.SplitTrim(affinity, ','),
 			DifferentHost: util.SplitTrim(antiAffinity, ','),
 		}
-		klog.V(4).Infof("CreateVolume: Setting scheduler hints: affinity=%s, anti-affinity=%s", affinity, antiAffinity)
+
+		klog.V(4).Infof("CreateVolume: Resolved scheduler hints: affinity=%s, anti-affinity=%s", affinity, antiAffinity)
 	}
 
 	vol, err := cloud.CreateVolume(opts, schedulerHints)

pkg/csi/cinder/openstack/openstack.go

@@ -56,6 +56,7 @@ type IOpenStack interface {
 	GetAttachmentDiskPath(instanceID, volumeID string) (string, error)
 	GetVolume(volumeID string) (*volumes.Volume, error)
 	GetVolumesByName(name string) ([]volumes.Volume, error)
+	GetVolumeByName(name string) (*volumes.Volume, error)
 	CreateSnapshot(name, volID string, tags map[string]string) (*snapshots.Snapshot, error)
 	ListSnapshots(filters map[string]string) ([]snapshots.Snapshot, string, error)
 	DeleteSnapshot(snapID string) error
@@ -72,6 +73,7 @@ type IOpenStack interface {
 	GetMaxVolLimit() int64
 	GetMetadataOpts() metadata.Opts
 	GetBlockStorageOpts() BlockStorageOpts
+	ResolveVolumeListToUUIDs(volumes string) (string, error)
 }
 
 type OpenStack struct {

pkg/csi/cinder/openstack/openstack_mock.go

@@ -17,11 +17,14 @@ limitations under the License.
 package openstack
 
 import (
+	"fmt"
+
 	"github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/backups"
 	"github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/snapshots"
 	"github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes"
 	"github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers"
 	"github.com/stretchr/testify/mock"
+	"k8s.io/cloud-provider-openstack/pkg/util/errors"
 	"k8s.io/cloud-provider-openstack/pkg/util/metadata"
 )
 
@@ -234,6 +237,24 @@ func (_m *OpenStackMock) GetVolumesByName(name string) ([]volumes.Volume, error)
 	return r0, r1
 }
 
+// GetVolumeByName provides a mock function with given fields: name
+func (_m *OpenStackMock) GetVolumeByName(name string) (*volumes.Volume, error) {
+	vols, err := _m.GetVolumesByName(name)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(vols) == 0 {
+		return nil, errors.ErrNotFound
+	}
+
+	if len(vols) > 1 {
+		return nil, fmt.Errorf("found %d volumes with name %q", len(vols), name)
+	}
+
+	return &vols[0], nil
+}
+
 // ListSnapshots provides a mock function with given fields: limit, offset, filters
 func (_m *OpenStackMock) ListSnapshots(filters map[string]string) ([]snapshots.Snapshot, string, error) {
 	ret := _m.Called(filters)
@@ -501,3 +522,8 @@ func (_m *OpenStackMock) GetMetadataOpts() metadata.Opts {
 func (_m *OpenStackMock) GetBlockStorageOpts() BlockStorageOpts {
 	return BlockStorageOpts{}
 }
+
+// ResolveVolumeListToUUIDs provides a mock function to return volume UUIDs
+func (_m *OpenStackMock) ResolveVolumeListToUUIDs(v string) (string, error) {
+	return v, nil
+}

pkg/csi/cinder/openstack/openstack_volumes.go

@@ -20,14 +20,18 @@ import (
 	"context"
 	"fmt"
 	"net/url"
+	"strings"
 	"time"
 
 	"github.com/gophercloud/gophercloud/v2/openstack"
 	"github.com/gophercloud/gophercloud/v2/openstack/blockstorage/v3/volumes"
 	"github.com/gophercloud/gophercloud/v2/openstack/compute/v2/volumeattach"
 	"github.com/gophercloud/gophercloud/v2/pagination"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/cloud-provider-openstack/pkg/metrics"
+	"k8s.io/cloud-provider-openstack/pkg/util"
 	cpoerrors "k8s.io/cloud-provider-openstack/pkg/util/errors"
 
 	"k8s.io/klog/v2"
@@ -140,6 +144,25 @@ func (os *OpenStack) GetVolumesByName(n string) ([]volumes.Volume, error) {
 	return vols, nil
 }
 
+// GetVolumeByName is a wrapper around GetVolumesByName that returns a single Volume reference
+// with the specified name
+func (os *OpenStack) GetVolumeByName(n string) (*volumes.Volume, error) {
+	vols, err := os.GetVolumesByName(n)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(vols) == 0 {
+		return nil, cpoerrors.ErrNotFound
+	}
+
+	if len(vols) > 1 {
+		return nil, fmt.Errorf("found %d volumes with name %q", len(vols), n)
+	}
+
+	return &vols[0], nil
+}
+
 // DeleteVolume delete a volume
 func (os *OpenStack) DeleteVolume(volumeID string) error {
 	used, err := os.diskIsUsed(volumeID)
@@ -414,3 +437,39 @@ func (os *OpenStack) diskIsUsed(volumeID string) (bool, error) {
 func (os *OpenStack) GetBlockStorageOpts() BlockStorageOpts {
 	return os.bsOpts
 }
+
+// ResolveVolumeListToUUIDs resolves a list of volume names or UUIDs to a
+// string of UUIDs
+func (os *OpenStack) ResolveVolumeListToUUIDs(affinityList string) (string, error) {
+	list := util.SplitTrim(affinityList, ',')
+	if len(list) == 0 {
+		return "", nil
+	}
+
+	affinityUUIDs := make([]string, 0, len(list))
+	for _, v := range list {
+		var volume *volumes.Volume
+		var err error
+
+		if id, e := util.UUID(v); e == nil {
+			// First try to get volume by ID
+			volume, err = os.GetVolume(id)
+			if err != nil && cpoerrors.IsNotFound(err) {
+				volume, err = os.GetVolumeByName(v)
+			}
+		} else {
+			// If not a UUID, try to get volume by name
+			volume, err = os.GetVolumeByName(v)
+		}
+		if err != nil {
+			if cpoerrors.IsNotFound(err) {
+				return "", status.Errorf(codes.NotFound, "referenced volume %s not found: %v", v, err)
+			}
+			return "", status.Errorf(codes.Internal, "failed to resolve volume %s: %v", v, err)
+		}
+
+		affinityUUIDs = append(affinityUUIDs, volume.ID)
+	}
+
+	return strings.Join(affinityUUIDs, ","), nil
+}

pkg/csi/manila/controllerserver.go

@@ -148,10 +148,22 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	for k, v := range pvcAnnotations {
 		klog.V(4).Infof("CreateVolume: retrieved %q pvc annotation: %s: %s", k, v, shareName)
 	}
-	shareOpts.Affinity = util.SplitTrimJoin(pvcAnnotations[affinityKey], ',')
-	shareOpts.AntiAffinity = util.SplitTrimJoin(pvcAnnotations[antiAffinityKey], ',')
-	if shareOpts.Affinity != "" || shareOpts.AntiAffinity != "" {
-		klog.V(4).Infof("CreateVolume: Setting scheduler hints: affinity=%s, anti-affinity=%s", shareOpts.Affinity, shareOpts.AntiAffinity)
+	affinity := pvcAnnotations[affinityKey]
+	antiAffinity := pvcAnnotations[antiAffinityKey]
+	if affinity != "" || antiAffinity != "" {
+		klog.V(4).Infof("CreateVolume: Getting scheduler hints: affinity=%s, anti-affinity=%s", affinity, antiAffinity)
+
+		// resolve share names to UUIDs
+		shareOpts.Affinity, err = resolveShareListToUUIDs(manilaClient, affinity)
+		if err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "failed to resolve affinity share UUIDs: %v", err)
+		}
+		shareOpts.AntiAffinity, err = resolveShareListToUUIDs(manilaClient, antiAffinity)
+		if err != nil {
+			return nil, status.Errorf(codes.InvalidArgument, "failed to resolve anti-affinity share UUIDs: %v", err)
+		}
+
+		klog.V(4).Infof("CreateVolume: Resolved scheduler hints: affinity=%v, anti-affinity=%v", shareOpts.Affinity, shareOpts.AntiAffinity)
 	}
 
 	// override the storage class group ID if it is set in the PVC annotation

pkg/csi/manila/share.go

@@ -18,13 +18,15 @@ package manila
 
 import (
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/gophercloud/gophercloud/v2/openstack/sharedfilesystems/v2/shares"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient"
+	"k8s.io/cloud-provider-openstack/pkg/util"
 	clouderrors "k8s.io/cloud-provider-openstack/pkg/util/errors"
 	"k8s.io/klog/v2"
 )
@@ -197,3 +199,38 @@ func waitForShareStatus(manilaClient manilaclient.Interface, shareID string, val
 		return false, fmt.Errorf("share %s is in an unexpected state: wanted either %v or %s, got %s", shareID, validTransientStates, desiredStatus, share.Status)
 	})
 }
+
+func resolveShareListToUUIDs(manilaClient manilaclient.Interface, affinityList string) (string, error) {
+	list := util.SplitTrim(affinityList, ',')
+	if len(list) == 0 {
+		return "", nil
+	}
+
+	affinityUUIDs := make([]string, 0, len(list))
+	for _, v := range list {
+		var share *shares.Share
+		var err error
+
+		if id, e := util.UUID(v); e == nil {
+			// First try to get share by ID
+			share, err = manilaClient.GetShareByID(id)
+			if err != nil && clouderrors.IsNotFound(err) {
+				// If not found by ID, try to get share by ID as name
+				share, err = manilaClient.GetShareByName(v)
+			}
+		} else {
+			// If not a UUID, try to get share by name
+			share, err = manilaClient.GetShareByName(v)
+		}
+		if err != nil {
+			if clouderrors.IsNotFound(err) {
+				return "", status.Errorf(codes.NotFound, "referenced share %s not found: %v", v, err)
+			}
+			return "", status.Errorf(codes.Internal, "failed to resolve share %s: %v", v, err)
+		}
+
+		affinityUUIDs = append(affinityUUIDs, share.ID)
+	}
+
+	return strings.Join(affinityUUIDs, ","), nil
+}