[cinder-csi-plugin] Rationalise all things topology

pkg/csi/cinder/controllerserver.go

@@ -86,22 +86,22 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	// Volume Type
 	volType := volParams["type"]
 
+	// Volume AZ
+
+	accessibleTopologyReq := req.GetAccessibilityRequirements()
+	ignoreVolumeAZ := cloud.GetBlockStorageOpts().IgnoreVolumeAZ
+
+	// First check if volAvailability is already specified, if not get preferred from Topology
+	// Required, in case vol AZ is different from node AZ
 	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 = sharedcsi.GetAZFromTopology(topologyKey, accessibleTopologyReq)
-			}
+		if volParams["availability"] != "" {
+			volAvailability = volParams["availability"]
+		} else if accessibleTopologyReq != nil {
+			volAvailability = sharedcsi.GetAZFromTopology(topologyKey, accessibleTopologyReq)
 		}
 	}
 
-	ignoreVolumeAZ := cloud.GetBlockStorageOpts().IgnoreVolumeAZ
-
 	// get the PVC annotation
 	pvcAnnotations := sharedcsi.GetPVCAnnotations(cs.Driver.pvcLister, volParams)
 	for k, v := range pvcAnnotations {
@@ -120,8 +120,11 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 			return nil, status.Error(codes.AlreadyExists, "Volume Already exists with same name and different capacity")
 		}
 		klog.V(4).Infof("Volume %s already exists in Availability Zone: %s of size %d GiB", vols[0].ID, vols[0].AvailabilityZone, vols[0].Size)
-		return getCreateVolumeResponse(&vols[0], nil, ignoreVolumeAZ, req.GetAccessibilityRequirements()), nil
-	} else if len(vols) > 1 {
+		accessibleTopology := getTopology(&vols[0], accessibleTopologyReq, ignoreVolumeAZ)
+		return getCreateVolumeResponse(&vols[0], nil, accessibleTopology), nil
+	}
+
+	if len(vols) > 1 {
 		klog.V(3).Infof("found multiple existing volumes with selected name (%s) during create", volName)
 		return nil, status.Error(codes.Internal, "Multiple volumes reported by Cinder with same name")
 	}
@@ -247,7 +250,9 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 
 	klog.V(4).Infof("CreateVolume: Successfully created volume %s in Availability Zone: %s of size %d GiB", vol.ID, vol.AvailabilityZone, vol.Size)
 
-	return getCreateVolumeResponse(vol, volCtx, ignoreVolumeAZ, req.GetAccessibilityRequirements()), nil
+	accessibleTopology := getTopology(vol, accessibleTopologyReq, ignoreVolumeAZ)
+
+	return getCreateVolumeResponse(vol, volCtx, accessibleTopology), nil
 }
 
 func (d *controllerServer) ControllerModifyVolume(ctx context.Context, req *csi.ControllerModifyVolumeRequest) (*csi.ControllerModifyVolumeResponse, error) {
@@ -1035,7 +1040,24 @@ func (cs *controllerServer) ControllerExpandVolume(ctx context.Context, req *csi
 	}, nil
 }
 
-func getCreateVolumeResponse(vol *volumes.Volume, volCtx map[string]string, ignoreVolumeAZ bool, accessibleTopologyReq *csi.TopologyRequirement) *csi.CreateVolumeResponse {
+func getTopology(vol *volumes.Volume, topologyReq *csi.TopologyRequirement, ignoreVolumeAZ bool) []*csi.Topology {
+	var accessibleTopology []*csi.Topology
+	if ignoreVolumeAZ {
+		if topologyReq != nil {
+			accessibleTopology = topologyReq.GetPreferred()
+		}
+	} else if vol.AvailabilityZone != "" {
+		// NOTE(stephenfin): We retrieve the AZ from the created volume rather than
+		// using the value we provided in our request since these can differ due to
+		// Cinder's '[DEFAULT] allow_availability_zone_fallback' option.
+		accessibleTopology = []*csi.Topology{
+			{Segments: map[string]string{topologyKey: vol.AvailabilityZone}},
+		}
+	}
+	return accessibleTopology
+}
+
+func getCreateVolumeResponse(vol *volumes.Volume, volCtx map[string]string, accessibleTopology []*csi.Topology) *csi.CreateVolumeResponse {
 	var volsrc *csi.VolumeContentSource
 	volCnx := map[string]string{}
 
@@ -1075,21 +1097,6 @@ func getCreateVolumeResponse(vol *volumes.Volume, volCtx map[string]string, igno
 		}
 	}
 
-	var accessibleTopology []*csi.Topology
-	// If ignore-volume-az is true , dont set the accessible topology to volume az,
-	// use from preferred topologies instead.
-	if ignoreVolumeAZ {
-		if accessibleTopologyReq != nil {
-			accessibleTopology = accessibleTopologyReq.GetPreferred()
-		}
-	} else {
-		accessibleTopology = []*csi.Topology{
-			{
-				Segments: map[string]string{topologyKey: vol.AvailabilityZone},
-			},
-		}
-	}
-
 	resp := &csi.CreateVolumeResponse{
 		Volume: &csi.Volume{
 			VolumeId:           vol.ID,