From 303701c5a84c84084bb3eb1a0d4c3d8fc085b13f Mon Sep 17 00:00:00 2001
From: Meinhard Zhou <zhouenhua@bytedance.com>
Date: Mon, 13 Jun 2022 21:08:06 +0800
Subject: [PATCH] fix: add creating/deleting check to ensure idemponent

Signed-off-by: Meinhard Zhou <zhouenhua@bytedance.com>
---
 pkg/nvmf/controllerserver.go | 31 +++++++++++++++++++++++++++++--
 pkg/nvmf/volume.go           | 15 ---------------
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/pkg/nvmf/controllerserver.go b/pkg/nvmf/controllerserver.go
index 8341ad2..7f9ca5e 100644
--- a/pkg/nvmf/controllerserver.go
+++ b/pkg/nvmf/controllerserver.go
@@ -33,6 +33,12 @@ import (
 // the map of VolumeName to PvName for idempotency.
 var createdVolumeMap = map[string]*csi.Volume{}
 
+// the map of DeleteVolumeReq for idempotency
+var deletingVolumeReqMap = map[string]*csi.DeleteVolumeRequest{}
+
+// the map of CreateVolumeReq for idempotency
+var creatingVolumeReqMap = map[string]*csi.CreateVolumeRequest{}
+
 type ControllerServer struct {
 	Driver *driver
 }
@@ -84,7 +90,17 @@ func (c *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu
 		}, nil
 	}
 
-	// 3. if not created, request backend controller to create a new volume
+	// 3. if creating, return error
+	_, ok = creatingVolumeReqMap[req.GetName()]
+	if ok {
+		klog.Warningf("CreateVolume: volume %s has been creating in other req", req.GetName())
+		//maybe return err?
+		return &csi.CreateVolumeResponse{}, nil
+	}
+
+	creatingVolumeReqMap[req.GetName()] = req
+
+	// 4. if not created, request backend controller to create a new volume
 	createVolReq.Name = req.GetName()
 	createVolReq.SizeByte = req.GetCapacityRange().RequiredBytes
 
@@ -124,6 +140,7 @@ func (c *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu
 	if err != nil {
 		klog.Warningf("CreateVolume: create volume %s success, but persistent error: %s", req.GetName(), err)
 	}
+	delete(creatingVolumeReqMap, req.GetName())
 
 	return &csi.CreateVolumeResponse{Volume: tmpVolume}, nil
 }
@@ -147,7 +164,16 @@ func (c *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVolu
 
 	volumeMapFilePath := filepath.Join(c.Driver.volumeMapDir, req.GetVolumeId())
 
-	volume, ok := createdVolumeMap[req.GetVolumeId()]
+	// 2. if deleting, return error
+	_, ok := deletingVolumeReqMap[req.GetVolumeId()]
+	if ok {
+		klog.Warningf("DeleteVolume: vol-%s has been deleting in other req", req.GetVolumeId())
+		// maybe return error?
+		return &csi.DeleteVolumeResponse{}, nil
+	}
+	deletingVolumeReqMap[req.GetVolumeId()] = req
+
+	volume, ok = createdVolumeMap[req.GetVolumeId()]
 	if !ok {
 		klog.Errorf("DeleteVolume: can't find the vol-%s in driver cache", req.GetVolumeId())
 		volume, err = GetVolumeInfoFromFile(volumeMapFilePath)
@@ -183,6 +209,7 @@ func (c *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVolu
 		klog.Warningf("DeleteVolume: can't remove vol-%s mapping file %s for error: %s.", volume.VolumeId, volumeMapFilePath, err)
 	}
 
+	delete(deletingVolumeReqMap, volume.GetVolumeId())
 	klog.Infof("DeleteVolume: delete vol-%s success.", volume.VolumeId)
 
 	return &csi.DeleteVolumeResponse{}, nil
diff --git a/pkg/nvmf/volume.go b/pkg/nvmf/volume.go
index 9b21edc..589298b 100644
--- a/pkg/nvmf/volume.go
+++ b/pkg/nvmf/volume.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"strings"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"github.com/kubernetes-csi/csi-driver-nvmf/pkg/client"
@@ -16,7 +15,6 @@ type CreateVolumeRequest struct {
 	Name     string
 	SizeByte int64
 	// AllowHostNqn string
-	ReadOnly bool
 }
 
 type DeleteVolumeRequest struct {
@@ -26,19 +24,6 @@ type DeleteVolumeRequest struct {
 func ParseCreateVolumeParameters(parameters map[string]string) (volReq *CreateVolumeRequest, err error) {
 	//todo: need more parameters for nvmf
 
-	// readonly
-	readonly, ok := parameters["readonly"]
-	if !ok {
-		volReq.ReadOnly = false
-	} else {
-		readonly = strings.ToLower(readonly)
-		if readonly == "yes" || readonly == "true" || readonly == "1" {
-			volReq.ReadOnly = true
-		} else {
-			volReq.ReadOnly = false
-		}
-	}
-
 	return volReq, nil
 }