From f26515f356ef432eed42774cf539e6d98f18d2cd Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 24 Mar 2025 14:13:20 +0000 Subject: [PATCH 1/3] cinder-csi-plugin, manila-csi-plugin: Correct formatting character %T prints the type. %t prints the word true/false, which is what we want. [1] [1] https://pkg.go.dev/fmt Conflicts: pkg/csi/manila/driver.go NOTE(stephenfin): Merge conflicts are due to the absence of PR #2734, which we don't want to backport. Signed-off-by: Stephen Finucane --- pkg/csi/cinder/driver.go | 2 +- pkg/csi/manila/driver.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/csi/cinder/driver.go b/pkg/csi/cinder/driver.go index 08da8d5080..f3b4613aad 100644 --- a/pkg/csi/cinder/driver.go +++ b/pkg/csi/cinder/driver.go @@ -88,7 +88,7 @@ func NewDriver(o *DriverOpts) *Driver { klog.Info("Driver: ", d.name) klog.Info("Driver version: ", d.fqVersion) klog.Info("CSI Spec version: ", specVersion) - klog.Infof("Topology awareness: %T", d.withTopology) + klog.Infof("Topology awareness: %t", d.withTopology) d.AddControllerServiceCapabilities( []csi.ControllerServiceCapability_RPC_Type{ diff --git a/pkg/csi/manila/driver.go b/pkg/csi/manila/driver.go index 80e3d849aa..c46822ea51 100644 --- a/pkg/csi/manila/driver.go +++ b/pkg/csi/manila/driver.go @@ -127,6 +127,7 @@ func NewDriver(o *DriverOpts) (*Driver, error) { klog.Info("Driver: ", d.name) klog.Info("Driver version: ", d.fqVersion) klog.Info("CSI spec version: ", specVersion) + klog.Infof("Topology awareness: %t", d.withTopology) getShareAdapter(d.shareProto) // The program will terminate with a non-zero exit code if the share protocol selector is wrong klog.Infof("Operating on %s shares", d.shareProto) From df4364865daa380e74fa288978f9be69d328bac2 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 24 Mar 2025 12:40:04 +0000 Subject: [PATCH 2/3] cinder-csi-plugin: Don't report topology capability when --with-topology=False The 'PluginCapability_Service_VOLUME_ACCESSIBILITY_CONSTRAINTS' capability flag determines whether the provisioner attempts to look up topology information from the node or not [1][2]. If we report it but don't return topology information from the node, we end up with failures to provision [3]. Fix the issue by optionally reporting the capability, like Manila already does. [1] https://github.com/kubernetes-csi/external-provisioner/blob/17e2429e9f/pkg/controller/controller.go#L685-L700 [2] https://github.com/kubernetes-csi/external-provisioner/blob/17e2429e9f/pkg/controller/controller.go#L994-L996 [3] https://github.com/kubernetes-csi/external-provisioner/blob/17e2429e9f/pkg/controller/topology.go#L177 Signed-off-by: Stephen Finucane --- pkg/csi/cinder/identityserver.go | 51 +++++++++++++++++--------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/pkg/csi/cinder/identityserver.go b/pkg/csi/cinder/identityserver.go index 7a73ac9870..dbd62917fc 100644 --- a/pkg/csi/cinder/identityserver.go +++ b/pkg/csi/cinder/identityserver.go @@ -52,36 +52,39 @@ func (ids *identityServer) Probe(ctx context.Context, req *csi.ProbeRequest) (*c func (ids *identityServer) GetPluginCapabilities(ctx context.Context, req *csi.GetPluginCapabilitiesRequest) (*csi.GetPluginCapabilitiesResponse, error) { klog.V(5).Infof("GetPluginCapabilities called with req %+v", req) - return &csi.GetPluginCapabilitiesResponse{ - Capabilities: []*csi.PluginCapability{ - { - Type: &csi.PluginCapability_Service_{ - Service: &csi.PluginCapability_Service{ - Type: csi.PluginCapability_Service_CONTROLLER_SERVICE, - }, + caps := []*csi.PluginCapability{ + { + Type: &csi.PluginCapability_Service_{ + Service: &csi.PluginCapability_Service{ + Type: csi.PluginCapability_Service_CONTROLLER_SERVICE, }, }, - { - Type: &csi.PluginCapability_Service_{ - Service: &csi.PluginCapability_Service{ - Type: csi.PluginCapability_Service_VOLUME_ACCESSIBILITY_CONSTRAINTS, - }, + }, + { + Type: &csi.PluginCapability_VolumeExpansion_{ + VolumeExpansion: &csi.PluginCapability_VolumeExpansion{ + Type: csi.PluginCapability_VolumeExpansion_ONLINE, }, }, - { - Type: &csi.PluginCapability_VolumeExpansion_{ - VolumeExpansion: &csi.PluginCapability_VolumeExpansion{ - Type: csi.PluginCapability_VolumeExpansion_ONLINE, - }, + }, + { + Type: &csi.PluginCapability_VolumeExpansion_{ + VolumeExpansion: &csi.PluginCapability_VolumeExpansion{ + Type: csi.PluginCapability_VolumeExpansion_OFFLINE, }, }, - { - Type: &csi.PluginCapability_VolumeExpansion_{ - VolumeExpansion: &csi.PluginCapability_VolumeExpansion{ - Type: csi.PluginCapability_VolumeExpansion_OFFLINE, - }, + }, + } + + if ids.Driver.withTopology { + caps = append(caps, &csi.PluginCapability{ + Type: &csi.PluginCapability_Service_{ + Service: &csi.PluginCapability_Service{ + Type: csi.PluginCapability_Service_VOLUME_ACCESSIBILITY_CONSTRAINTS, }, }, - }, - }, nil + }) + } + + return &csi.GetPluginCapabilitiesResponse{Capabilities: caps}, nil } From 8d2d8a74bdb747f9756ef5e1208ee2f5ebccfa33 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 24 Mar 2025 12:45:58 +0000 Subject: [PATCH 3/3] manila-csi-plugin: Unify some logging between cinder, manila Signed-off-by: Stephen Finucane --- pkg/csi/manila/identityserver.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/csi/manila/identityserver.go b/pkg/csi/manila/identityserver.go index dae37ca98c..10b11505a6 100644 --- a/pkg/csi/manila/identityserver.go +++ b/pkg/csi/manila/identityserver.go @@ -22,6 +22,7 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "k8s.io/klog/v2" ) type identityServer struct { @@ -29,10 +30,16 @@ type identityServer struct { } func (ids *identityServer) GetPluginInfo(ctx context.Context, req *csi.GetPluginInfoRequest) (*csi.GetPluginInfoResponse, error) { + klog.V(5).Infof("Using default GetPluginInfo") + if ids.d.name == "" { return nil, status.Error(codes.Unavailable, "Driver name not configured") } + if ids.d.fqVersion == "" { + return nil, status.Error(codes.Unavailable, "Driver is missing version") + } + return &csi.GetPluginInfoResponse{ Name: ids.d.name, VendorVersion: ids.d.fqVersion, @@ -50,6 +57,7 @@ func (ids *identityServer) Probe(ctx context.Context, req *csi.ProbeRequest) (*c } func (ids *identityServer) GetPluginCapabilities(ctx context.Context, req *csi.GetPluginCapabilitiesRequest) (*csi.GetPluginCapabilitiesResponse, error) { + klog.V(5).Infof("GetPluginCapabilities called with req %+v", req) caps := []*csi.PluginCapability{ { Type: &csi.PluginCapability_Service_{