[release-v1.31] Drop CSI with-topology and cleanups (#68)

* Drop CSI with-topology and cleanups

* drop additional-topology and more cleanups

* fix

* fix

* fix shellcheck and cleanup

* fix

---------

Co-authored-by: Robert Kaussow <[email protected]>

Author: stackit-ske

cmd/stackit-csi-plugin/main.go

@@ -20,12 +20,10 @@ import (
 var (
 	endpoint                 string
 	cloudConfig              string
-	additionalTopologies     map[string]string
 	cluster                  string
 	httpEndpoint             string
 	provideControllerService bool
 	provideNodeService       bool
-	withTopology             bool
 )
 
 func main() {
@@ -65,12 +63,6 @@ func main() {
 
 	cmd.Flags().StringVar(&cloudConfig, "cloud-config", "", "CSI driver cloud config. This option can be given multiple times")
 
-	cmd.PersistentFlags().BoolVar(&withTopology, "with-topology", true, "cluster is topology-aware")
-
-	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.")
-
 	cmd.PersistentFlags().StringVar(&cluster, "cluster", "", "The identifier of the cluster that the plugin is running in.")
 	cmd.PersistentFlags().StringVar(&httpEndpoint, "http-endpoint", "",
 		"The TCP network address where the HTTP server for providing metrics for diagnostics, will listen (example: `:8080`)."+
@@ -90,20 +82,19 @@ func main() {
 func handle() {
 	// Initialize cloud
 	d := blockstorage.NewDriver(&blockstorage.DriverOpts{
-		Endpoint:     endpoint,
-		ClusterID:    cluster,
-		PVCLister:    csi.GetPVCLister(),
-		WithTopology: withTopology,
+		Endpoint:  endpoint,
+		ClusterID: cluster,
+		PVCLister: csi.GetPVCLister(),
 	})
 
 	if provideControllerService {
 		var err error
-		cfg, err := stackit.GetConfigForFile(cloudConfig)
+		cfg, err := stackit.GetConfigFromFile(cloudConfig)
 		if err != nil {
 			klog.Fatal(err)
 		}
 
-		iaasClient, err := stackit.CreateIAASClient(&cfg)
+		iaasClient, err := stackit.CreateIaaSClient(&cfg)
 		if err != nil {
 			klog.Fatalf("Failed to create IaaS client: %v", err)
 		}
@@ -120,15 +111,15 @@ func handle() {
 		// Initialize mount
 		mountProvider := mount.GetMountProvider()
 
-		cfg, err := stackit.GetConfigForFile(cloudConfig)
+		cfg, err := stackit.GetConfigFromFile(cloudConfig)
 		if err != nil {
 			klog.Fatal(err)
 		}
 
 		// Initialize Metadata
 		metadataProvider := metadata.GetMetadataProvider(fmt.Sprintf("%s,%s", metadata.MetadataID, metadata.ConfigDriveID))
 
-		d.SetupNodeService(mountProvider, metadataProvider, cfg.BlockStorage, additionalTopologies)
+		d.SetupNodeService(mountProvider, metadataProvider, cfg.BlockStorage)
 	}
 
 	d.Run()

deploy/deployment.yaml

@@ -24,7 +24,7 @@ spec:
       terminationGracePeriodSeconds: 30
       containers:
       - name: stackit-cloud-controller-manager
-        image: ghcr.io/stackitcloud/cloud-provider-stackit:latest
+        image: ghcr.io/stackitcloud/cloud-provider-stackit/cloud-controller-manager:release-v1.33
         args:
         - --cloud-provider=stackit
         - --webhook-secure-port=0

docs/csi-driver.md

@@ -89,7 +89,7 @@ storageClass:
   volumeBindingMode: WaitForFirstConsumer
   allowedTopologies:
     - matchLabelExpressions:
-        - key: topology.kubernetes.io/zone
+        - key: topology.block-storage.csi.stackit.cloud/zone
           values:
             - zone1
             - zone2

docs/deployment.md

@@ -11,7 +11,6 @@
 5. [Example Deployment](#example-deployment)
 6. [Configuration Options](#configuration-options)
    - [Cloud Configuration](#cloud-configuration)
-   - [Topology Configuration](#topology-configuration)
 7. [Monitoring and Logging](#monitoring-and-logging)
    - [Metrics](#metrics)
    - [Logs](#logs)
@@ -46,8 +45,6 @@ The deployment can be customized using the following flags:
 
 - `--endpoint`: CSI endpoint URL
 - `--cloud-config`: Path to cloud configuration file
-- `--with-topology`: Enable topology awareness (default: true)
-- `--additional-topology`: Additional topology keys for volume placement
 - `--cluster`: Cluster identifier
 - `--http-endpoint`: HTTP server endpoint for metrics
 - `--provide-controller-service`: Enable controller service (default: true)
@@ -84,7 +81,7 @@ spec:
       serviceAccountName: stackit-cloud-controller-manager
       containers:
       - name: stackit-cloud-controller-manager
-        image: ghcr.io/stackitcloud/cloud-provider-stackit:latest
+        image: ghcr.io/stackitcloud/cloud-provider-stackit/cloud-controller-manager:release-v1.33
         args:
         # CCM flags
         - --cloud-provider=stackit
@@ -97,8 +94,6 @@ spec:
         # CSI flags
         - --endpoint=unix:///csi/csi.sock
         - --cloud-config=/etc/stackit/cloud-config.yaml
-        - --with-topology=true
-        - --additional-topology=topology.kubernetes.io/region=REGION1
         - --cluster=my-cluster-id
         - --provide-controller-service=true
         - --provide-node-service=true
@@ -137,17 +132,6 @@ loadBalancerApi:
   url: https://load-balancer.api.eu01.stackit.cloud
 ```
 
-### Topology Configuration
-
-The driver supports topology-aware volume placement. Configure topology using the `--with-topology` and `--additional-topology` flags.
-
-Example with multiple topology keys:
-
-```bash
---with-topology=true
---additional-topology=topology.kubernetes.io/region=REGION1,topology.kubernetes.io/zone=ZONE1
-```
-
 ## Monitoring and Logging
 
 ### Metrics

hack/common.sh

@@ -3,28 +3,30 @@
 # See https://misc.flogisoft.com/bash/tip_colors_and_formatting
 
 color-green() { # Green
-  echo -e "\x1B[1;32m${@}\x1B[0m"
+  echo -e "\x1B[1;32m${*}\x1B[0m"
 }
 
 color-step() { # Yellow
-  echo -e "\x1B[1;33m${@}\x1B[0m"
+  echo -e "\x1B[1;33m${*}\x1B[0m"
 }
 
 color-context() { # Bold blue
-  echo -e "\x1B[1;34m${@}\x1B[0m"
+  echo -e "\x1B[1;34m${*}\x1B[0m"
 }
 
 color-missing() { # Yellow
-  echo -e "\x1B[1;33m${@}\x1B[0m"
+  echo -e "\x1B[1;33m${*}\x1B[0m"
 }
 
 prepare_token() {
   local namespace="$1"
   local name="$2"
-  local rolekind="$(echo "$3" | tr '[:upper:]' '[:lower:]')"
   local role="$4"
   local duration="${5:-10m}"
 
+  local rolekind
+  rolekind="$(echo "$3" | tr '[:upper:]' '[:lower:]')"
+
   result="$(kubectl -n "$namespace" create serviceaccount "$name" 2>&1)"
   # ignore "already exists" errors
   if ! [[ "$result" == *"created"* ]] && ! [[ "$result" == *"already exists"* ]] ; then
@@ -44,10 +46,12 @@ prepare_token() {
 prepare_access() {
   local namespace="$1"
   local name="$2"
-  local rolekind="$(echo "$3" | tr '[:upper:]' '[:lower:]')"
   local role="$4"
   local tmp_kubeconfig="$5"
 
+  local rolekind
+  rolekind="$(echo "$3" | tr '[:upper:]' '[:lower:]')"
+
   color-step "Preparing access for $name"
 
   local token
@@ -71,10 +75,12 @@ prepare_access() {
 cleanup_access() {
   local namespace="$1"
   local name="$2"
-  local rolekind="$(echo "$3" | tr '[:upper:]' '[:lower:]')"
   local role="$4"
   local tmp_kubeconfig="$5"
 
+  local rolekind
+  rolekind="$(echo "$3" | tr '[:upper:]' '[:lower:]')"
+
   color-step "Cleaning up cluster access for $name"
 
   rm "$tmp_kubeconfig"

hack/test.sh

@@ -4,22 +4,24 @@ set -o errexit
 set -o nounset
 set -o pipefail
 
+# shellcheck disable=SC1091
 source "$(dirname "$0")/common.sh"
 
 echo "> Unit Tests"
 
-test_flags=
+test_flags=()
 # If running in Prow, we want to generate a machine-readable output file under the location specified via $ARTIFACTS.
 # This will add a JUnit view above the build log that shows an overview over successful and failed test cases.
 if [ -n "${CI:-}" ] && [ -n "${ARTIFACTS:-}" ]; then
   mkdir -p "$ARTIFACTS/junit"
+  # shellcheck disable=SC2064
   trap "collect_reports \"$ARTIFACTS/junit\"" EXIT
-  test_flags="--ginkgo.junit-report=junit.xml"
+  test_flags+=(--ginkgo.junit-report=junit.xml)
   # Use Ginkgo timeout in Prow to print everything that is buffered in GinkgoWriter.
-  test_flags+=" --ginkgo.timeout=2m"
+  test_flags+=(--ginkgo.timeout=2m)
 else
   # We don't want Ginkgo's timeout flag locally because it causes skipping the test cache.
   timeout_flag="-timeout=2m"
 fi
 
-go test ${timeout_flag:-} "$@" $test_flags
+go test ${timeout_flag:+"$timeout_flag"} "$@" "${test_flags[@]}"

pkg/csi/blockstorage/controllerserver.go

@@ -105,16 +105,14 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	volSizeGB := util.RoundUpSize(volSizeBytes, util.GIBIBYTE)
 
 	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 = ptr.Deref(volParams.AvailabilityZone, "")
-		if volAvailability == "" {
-			accessibleTopologyReq := req.GetAccessibilityRequirements()
-			// Check from Topology
-			if accessibleTopologyReq != nil {
-				volAvailability = sharedcsi.GetAZFromTopology(topologyKey, accessibleTopologyReq)
-			}
+	// First check if volAvailability is already specified, if not, get preferred from topology
+	// Required, in case volume AZ is different from node AZ
+	volAvailability = ptr.Deref(volParams.AvailabilityZone, "")
+	if volAvailability == "" {
+		accessibleTopologyReq := req.GetAccessibilityRequirements()
+		// Check from topology
+		if accessibleTopologyReq != nil {
+			volAvailability = sharedcsi.GetAZFromTopology(topologyKey, accessibleTopologyReq)
 		}
 	}
 

pkg/csi/blockstorage/controllerserver_test.go

@@ -49,7 +49,7 @@ var _ = Describe("ControllerServer test", Ordered, func() {
 	)
 
 	BeforeEach(func() {
-		d := NewDriver(&DriverOpts{Endpoint: FakeEndpoint, ClusterID: FakeCluster, WithTopology: true})
+		d := NewDriver(&DriverOpts{Endpoint: FakeEndpoint, ClusterID: FakeCluster})
 
 		mockCtrl := gomock.NewController(GinkgoT())
 		iaasClient = stackit.NewMockIaasClient(mockCtrl)

pkg/csi/blockstorage/driver.go

@@ -28,11 +28,10 @@ var (
 )
 
 type Driver struct {
-	name         string
-	fqVersion    string // Fully qualified version in format {Version}@{CPO version}
-	endpoint     string
-	clusterID    string
-	withTopology bool
+	name      string
+	fqVersion string // Fully qualified version in format {Version}@{CPO version}
+	endpoint  string
+	clusterID string
 
 	ids *identityServer
 	cs  *controllerServer
@@ -47,27 +46,24 @@ type Driver struct {
 }
 
 type DriverOpts struct {
-	ClusterID    string
-	Endpoint     string
-	WithTopology bool
+	ClusterID string
+	Endpoint  string
 
 	PVCLister corev1.PersistentVolumeClaimLister
 }
 
 func NewDriver(o *DriverOpts) *Driver {
 	d := &Driver{
-		name:         driverName,
-		fqVersion:    fmt.Sprintf("%s@%s", Version, version.Version),
-		endpoint:     o.Endpoint,
-		clusterID:    o.ClusterID,
-		withTopology: o.WithTopology,
-		pvcLister:    o.PVCLister,
+		name:      driverName,
+		fqVersion: fmt.Sprintf("%s@%s", Version, version.Version),
+		endpoint:  o.Endpoint,
+		clusterID: o.ClusterID,
+		pvcLister: o.PVCLister,
 	}
 
 	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{
@@ -142,9 +138,9 @@ func (d *Driver) SetupControllerService(instance stackit.IaasClient) {
 	d.cs = NewControllerServer(d, instance)
 }
 
-func (d *Driver) SetupNodeService(mountProvider mount.IMount, metadataProvider metadata.IMetadata, opts stackit.BlockStorageOpts, topologies map[string]string) {
+func (d *Driver) SetupNodeService(mountProvider mount.IMount, metadataProvider metadata.IMetadata, opts stackit.BlockStorageOpts) {
 	klog.Info("Providing node service")
-	d.ns = NewNodeServer(d, mountProvider, metadataProvider, opts, topologies)
+	d.ns = NewNodeServer(d, mountProvider, metadataProvider, opts)
 }
 
 func (d *Driver) Run() {

pkg/csi/blockstorage/nodeserver.go

@@ -40,11 +40,10 @@ import (
 )
 
 type nodeServer struct {
-	Driver     *Driver
-	Mount      mount.IMount
-	Metadata   metadata.IMetadata
-	Opts       stackit.BlockStorageOpts
-	Topologies map[string]string
+	Driver   *Driver
+	Mount    mount.IMount
+	Metadata metadata.IMetadata
+	Opts     stackit.BlockStorageOpts
 	csi.UnimplementedNodeServer
 }
 
@@ -318,20 +317,17 @@ func (ns *nodeServer) NodeGetInfo(ctx context.Context, _ *csi.NodeGetInfoRequest
 		MaxVolumesPerNode: maxVolumesPerNode,
 	}
 
-	if !ns.Driver.withTopology {
-		return nodeInfo, nil
-	}
-
 	zone, err := ns.Metadata.GetAvailabilityZone(ctx)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "[NodeGetInfo] Unable to retrieve availability zone of node %v", err)
 	}
-	topologyMap := make(map[string]string, len(ns.Topologies)+1)
-	topologyMap[topologyKey] = zone
-	for k, v := range ns.Topologies {
-		topologyMap[k] = v
+
+	//TODO: support well-known topology key "topology.kubernetes.io/zone"
+	segments := map[string]string{
+		topologyKey: zone,
 	}
-	nodeInfo.AccessibleTopology = &csi.Topology{Segments: topologyMap}
+
+	nodeInfo.AccessibleTopology = &csi.Topology{Segments: segments}
 
 	return nodeInfo, nil
 }