check vol status before trying to attach; abort early if status is not happy

Signed-off-by: Niclas Schad <niclas.schad@stackit.cloud>

Author: nschad

pkg/csi/blockstorage/controllerserver.go

@@ -356,7 +356,7 @@ func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *cs
 		return nil, status.Error(codes.InvalidArgument, "[ControllerPublishVolume] Volume capability must be provided")
 	}
 
-	_, err := cloud.GetVolume(ctx, volumeID)
+	vol, err := cloud.GetVolume(ctx, volumeID)
 	if err != nil {
 		if stackiterrors.IsNotFound(err) {
 			return nil, status.Errorf(codes.NotFound, "[ControllerPublishVolume] Volume %s not found", volumeID)
@@ -372,6 +372,15 @@ func (cs *controllerServer) ControllerPublishVolume(ctx context.Context, req *cs
 		return nil, status.Errorf(codes.Internal, "[ControllerPublishVolume] GetInstanceByID failed with error %v", err)
 	}
 
+	// If Volume is already mounted to target instanceID, return OK
+	if vol.ServerId != nil && *vol.ServerId == instanceID {
+		return &csi.ControllerPublishVolumeResponse{}, nil
+	}
+
+	if vol.GetStatus() != stackitclient.VolumeAvailableStatus {
+		return nil, status.Errorf(codes.Internal, "[ControllerPublishVolume] Volume %s is not in an READY state. Got:%s Want:%s", volumeID, vol.GetStatus(), stackitclient.VolumeAvailableStatus)
+	}
+
 	payload := iaas.AddVolumeToServerPayload{
 		DeleteOnTermination: new(false),
 	}

pkg/csi/blockstorage/controllerserver_test.go

@@ -660,7 +660,7 @@ var _ = Describe("ControllerServer test", Ordered, func() {
 				NodeId:           "fake",
 				VolumeCapability: stdVolCap,
 			}
-			iaasClient.EXPECT().GetVolume(gomock.Any(), req.VolumeId).Return(&iaas.Volume{}, nil)
+			iaasClient.EXPECT().GetVolume(gomock.Any(), req.VolumeId).Return(&iaas.Volume{Status: new("AVAILABLE")}, nil)
 			iaasClient.EXPECT().GetServer(gomock.Any(), "fake").Return(&iaas.Server{}, nil)
 			iaasClient.EXPECT().AttachVolume(gomock.Any(), req.NodeId, req.VolumeId, gomock.Any()).Return(req.VolumeId, nil)
 			iaasClient.EXPECT().WaitDiskAttached(gomock.Any(), req.NodeId, req.VolumeId).Return(nil)
@@ -674,7 +674,7 @@ var _ = Describe("ControllerServer test", Ordered, func() {
 				NodeId:           "fake",
 				VolumeCapability: stdVolCap,
 			}
-			iaasClient.EXPECT().GetVolume(gomock.Any(), req.VolumeId).Return(&iaas.Volume{}, nil)
+			iaasClient.EXPECT().GetVolume(gomock.Any(), req.VolumeId).Return(&iaas.Volume{Status: new("AVAILABLE")}, nil)
 			iaasClient.EXPECT().GetServer(gomock.Any(), req.NodeId).Return(&iaas.Server{}, nil)
 			iaasClient.EXPECT().AttachVolume(gomock.Any(), req.NodeId, req.VolumeId, gomock.Any()).Return("", &oapierror.GenericOpenAPIError{
 				StatusCode: http.StatusForbidden,

pkg/csi/blockstorage/sanity_test.go

@@ -263,7 +263,7 @@ var _ = Describe("CSI sanity test", Ordered, func() {
 				newBackup := &iaas.Backup{
 					Id:         new(uuid.New().String()),
 					Name:       new(name),
-					Status:     new("available"),
+					Status:     new("AVAILABLE"),
 					VolumeId:   new(volID),
 					SnapshotId: new(snapshotID),
 					CreatedAt:  new(time.Now()),
@@ -329,7 +329,7 @@ var _ = Describe("CSI sanity test", Ordered, func() {
 					return "", &oapierror.GenericOpenAPIError{StatusCode: http.StatusNotFound}
 				}
 				vol.ServerId = new(instanceID)
-				vol.Status = new("attached")
+				vol.Status = new("ATTACHED")
 				return *vol.Id, nil
 			}).AnyTimes()
 
@@ -343,7 +343,15 @@ var _ = Describe("CSI sanity test", Ordered, func() {
 				gomock.Any(), // context
 				gomock.Any(), // instanceID
 				gomock.Any(), // volumeID
-			).Return(nil).AnyTimes()
+			).DoAndReturn(func(_ context.Context, _, volumeID string) error {
+				vol, ok := createdVolumes[volumeID]
+				if !ok {
+					return &oapierror.GenericOpenAPIError{StatusCode: http.StatusNotFound}
+				}
+				vol.ServerId = nil
+				vol.Status = new("AVAILABLE")
+				return nil
+			}).AnyTimes()
 
 			iaasClient.EXPECT().WaitDiskDetached(
 				gomock.Any(), // context