cinder-csi-plugin: Add --with-topology option (#2743) (#2746)

Many clouds do not have a 1:1 mapping of compute and storage AZs. As we generate topology information from the metadata service on the node (i.e. a compute AZ), this can prevent us from being able to schedule VMs. Add a new boolean, '--with-topology', to allow users to disable the topology feature where it does not make sense. An identical option already exists for the Manila CSI driver. However, unlike that option, this one defaults to 'true' to retain current behavior. Signed-off-by: Stephen Finucane <[email protected]> (cherry picked from commit ac23a1e) Co-authored-by: Stephen Finucane <[email protected]>
kubernetes · Dec 12, 2024 · 6cb478d · 6cb478d
1 parent 6f1d78d
commit 6cb478d
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 31 deletions.
diff --git a/cmd/cinder-csi-plugin/main.go b/cmd/cinder-csi-plugin/main.go
@@ -41,6 +41,7 @@ var (
 	provideControllerService bool
 	provideNodeService       bool
 	noClient                 bool
+	withTopology             bool
 )
 
 func main() {
@@ -68,6 +69,8 @@ func main() {
 		klog.Fatalf("Unable to mark flag cloud-config to be required: %v", err)
 	}
 
+	cmd.PersistentFlags().BoolVar(&withTopology, "with-topology", true, "cluster is topology-aware")
+
 	cmd.PersistentFlags().StringSliceVar(&cloudNames, "cloud-name", []string{""}, "Cloud name to instruct CSI driver to read additional OpenStack cloud credentials from the configuration subsections. This option can be specified multiple times to manage multiple OpenStack clouds.")
 	cmd.PersistentFlags().StringToStringVar(&additionalTopologies, "additional-topology", map[string]string{}, "Additional CSI driver topology keys, for example topology.kubernetes.io/region=REGION1. This option can be specified multiple times to add multiple additional topology keys.")
 
@@ -86,7 +89,11 @@ func main() {
 
 func handle() {
 	// Initialize cloud
-	d := cinder.NewDriver(&cinder.DriverOpts{Endpoint: endpoint, ClusterID: cluster})
+	d := cinder.NewDriver(&cinder.DriverOpts{
+		Endpoint:     endpoint,
+		ClusterID:    cluster,
+		WithTopology: withTopology,
+	})
 
 	openstack.InitOpenStackProvider(cloudConfig, httpEndpoint)
 

diff --git a/docs/cinder-csi-plugin/using-cinder-csi-plugin.md b/docs/cinder-csi-plugin/using-cinder-csi-plugin.md
@@ -69,6 +69,13 @@ In addition to the standard set of klog flags, `cinder-csi-plugin` accepts the f
   The manifests default this to `unix://csi/csi.sock`, which is supplied via the `CSI_ENDPOINT` environment variable.
   </dd>
 
+  <dt>--with-topology &lt;enabled&gt;</dt>
+  <dd>
+  If set to true then the CSI driver reports topology information and the controller respects it.
+
+  Defaults to `true` (enabled).
+  </dd>
+
   <dt>--cloud-config &lt;config file&gt; [--cloud-config &lt;config file&gt; ...]</dt>
   <dd>
   This argument must be given at least once.
@@ -100,23 +107,23 @@ In addition to the standard set of klog flags, `cinder-csi-plugin` accepts the f
 
   <dt>--provide-controller-service &lt;enabled&gt;</dt>
   <dd>
-  If set to true then the CSI driver does provide the controller service.
+  If set to true then the CSI driver provides the controller service.
 
-  The default is to provide the controller service.
+  Defaults to `true` (enabled).
   </dd>
 
   <dt>--provide-node-service &lt;enabled&gt;</dt>
   <dd>
-  If set to true then the CSI driver does provide the node service.
+  If set to true then the CSI driver provides the node service.
 
-  The default is to provide the node service.
+  Defaults to `true` (enabled).
   </dd>
 
   <dt>--node-service-no-os-client &lt;disabled&gt;</dt>
   <dd>
   If set to true then the CSI driver does not provide the OpenStack client in the node service.
 
-  The default is to provide the OpenStack client in the node service.
+  Defaults to `false` (disabled).
   </dd>
 </dl>
 

diff --git a/pkg/csi/cinder/controllerserver.go b/pkg/csi/cinder/controllerserver.go
@@ -61,6 +61,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	// Volume Name
 	volName := req.GetName()
 	volCapabilities := req.GetVolumeCapabilities()
+	volParams := req.GetParameters()
 
 	if len(volName) == 0 {
 		return nil, status.Error(codes.InvalidArgument, "[CreateVolume] missing Volume Name")
@@ -80,13 +81,17 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	// Volume Type
 	volType := req.GetParameters()["type"]
 
-	// First check if volAvailability is already specified, if not get preferred from Topology
-	// Required, incase vol AZ is different from node AZ
-	volAvailability := req.GetParameters()["availability"]
-	if volAvailability == "" {
-		// Check from Topology
-		if req.GetAccessibilityRequirements() != nil {
-			volAvailability = util.GetAZFromTopology(topologyKey, req.GetAccessibilityRequirements())
+	var volAvailability string
+	if cs.Driver.withTopology {
+		// First check if volAvailability is already specified, if not get preferred from Topology
+		// Required, incase vol AZ is different from node AZ
+		volAvailability = volParams["availability"]
+		if volAvailability == "" {
+			accessibleTopologyReq := req.GetAccessibilityRequirements()
+			// Check from Topology
+			if accessibleTopologyReq != nil {
+				volAvailability = util.GetAZFromTopology(topologyKey, accessibleTopologyReq)
+			}
 		}
 	}
 

diff --git a/pkg/csi/cinder/controllerserver_test.go b/pkg/csi/cinder/controllerserver_test.go
@@ -38,7 +38,7 @@ func init() {
 		osmock = new(openstack.OpenStackMock)
 		osmockRegionX = new(openstack.OpenStackMock)
 
-		d := NewDriver(&DriverOpts{Endpoint: FakeEndpoint, ClusterID: FakeCluster})
+		d := NewDriver(&DriverOpts{Endpoint: FakeEndpoint, ClusterID: FakeCluster, WithTopology: true})
 
 		fakeCs = NewControllerServer(d, map[string]openstack.IOpenStack{
 			"": osmock,

diff --git a/pkg/csi/cinder/driver.go b/pkg/csi/cinder/driver.go
@@ -56,10 +56,11 @@ type CinderDriver = Driver
 //revive:enable:exported
 
 type Driver struct {
-	name      string
-	fqVersion string //Fully qualified version in format {Version}@{CPO version}
-	endpoint  string
-	cluster   string
+	name         string
+	fqVersion    string //Fully qualified version in format {Version}@{CPO version}
+	endpoint     string
+	cluster      string
+	withTopology bool
 
 	ids *identityServer
 	cs  *controllerServer
@@ -71,8 +72,9 @@ type Driver struct {
 }
 
 type DriverOpts struct {
-	ClusterID string
-	Endpoint  string
+	ClusterID    string
+	Endpoint     string
+	WithTopology bool
 }
 
 func NewDriver(o *DriverOpts) *Driver {
@@ -81,10 +83,12 @@ func NewDriver(o *DriverOpts) *Driver {
 	d.fqVersion = fmt.Sprintf("%s@%s", Version, version.Version)
 	d.endpoint = o.Endpoint
 	d.cluster = o.ClusterID
+	d.withTopology = o.WithTopology
 
 	klog.Info("Driver: ", d.name)
 	klog.Info("Driver version: ", d.fqVersion)
 	klog.Info("CSI Spec version: ", specVersion)
+	klog.Infof("Topology awareness: %T", d.withTopology)
 
 	d.AddControllerServiceCapabilities(
 		[]csi.ControllerServiceCapability_RPC_Type{

diff --git a/pkg/csi/cinder/nodeserver.go b/pkg/csi/cinder/nodeserver.go
@@ -359,12 +359,21 @@ func (ns *nodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag
 }
 
 func (ns *nodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoRequest) (*csi.NodeGetInfoResponse, error) {
-
 	nodeID, err := ns.Metadata.GetInstanceID()
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "[NodeGetInfo] unable to retrieve instance id of node %v", err)
 	}
 
+	maxVolume := ns.Cloud.GetMaxVolLimit()
+	nodeInfo := &csi.NodeGetInfoResponse{
+		NodeId:            nodeID,
+		MaxVolumesPerNode: maxVolume,
+	}
+
+	if !ns.Driver.withTopology {
+		return nodeInfo, nil
+	}
+
 	zone, err := ns.Metadata.GetAvailabilityZone()
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "[NodeGetInfo] Unable to retrieve availability zone of node %v", err)
@@ -374,15 +383,9 @@ func (ns *nodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoReque
 	for k, v := range ns.Topologies {
 		topologyMap[k] = v
 	}
-	topology := &csi.Topology{Segments: topologyMap}
-
-	maxVolume := ns.Cloud.GetMaxVolLimit()
+	nodeInfo.AccessibleTopology = &csi.Topology{Segments: topologyMap}
 
-	return &csi.NodeGetInfoResponse{
-		NodeId:             nodeID,
-		AccessibleTopology: topology,
-		MaxVolumesPerNode:  maxVolume,
-	}, nil
+	return nodeInfo, nil
 }
 
 func (ns *nodeServer) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetCapabilitiesRequest) (*csi.NodeGetCapabilitiesResponse, error) {

diff --git a/pkg/csi/cinder/nodeserver_test.go b/pkg/csi/cinder/nodeserver_test.go
@@ -40,7 +40,7 @@ var omock *openstack.OpenStackMock
 func init() {
 	if fakeNs == nil {
 
-		d := NewDriver(&DriverOpts{Endpoint: FakeEndpoint, ClusterID: FakeCluster})
+		d := NewDriver(&DriverOpts{Endpoint: FakeEndpoint, ClusterID: FakeCluster, WithTopology: true})
 
 		// mock MountMock
 		mmock = new(mount.MountMock)

diff --git a/tests/sanity/cinder/sanity_test.go b/tests/sanity/cinder/sanity_test.go
@@ -19,7 +19,7 @@ func TestDriver(t *testing.T) {
 	endpoint := "unix://" + socket
 	cluster := "kubernetes"
 
-	d := cinder.NewDriver(&cinder.DriverOpts{Endpoint: endpoint, ClusterID: cluster})
+	d := cinder.NewDriver(&cinder.DriverOpts{Endpoint: endpoint, ClusterID: cluster, WithTopology: true})
 
 	fakecloudprovider := getfakecloud()
 	openstack.OsInstances = map[string]openstack.IOpenStack{