Merge pull request #878 from huww98/disk-type-as-slice

store disk type and performance level as slice
kubernetes-sigs · Jan 15, 2024 · 4c27ce4 · 4c27ce4
2 parents 1fe6c50 + d6800b5
commit 4c27ce4
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 46 deletions.
diff --git a/pkg/disk/cloud.go b/pkg/disk/cloud.go
@@ -999,7 +999,7 @@ func getDiskType(diskVol *diskVolumeArgs) ([]string, []string, error) {
 	}
 
 	provisionDiskTypes := []string{}
-	allTypes := deleteEmpty(strings.Split(diskVol.Type, ","))
+	allTypes := deleteEmpty(diskVol.Type)
 	if len(nodeSupportDiskType) != 0 {
 		provisionDiskTypes = intersect(nodeSupportDiskType, allTypes)
 		if len(provisionDiskTypes) == 0 {
@@ -1009,10 +1009,7 @@ func getDiskType(diskVol *diskVolumeArgs) ([]string, []string, error) {
 	} else {
 		provisionDiskTypes = allTypes
 	}
-	provisionPerformanceLevel := []string{}
-	if diskVol.PerformanceLevel != "" {
-		provisionPerformanceLevel = strings.Split(diskVol.PerformanceLevel, ",")
-	}
+	provisionPerformanceLevel := diskVol.PerformanceLevel
 	return provisionDiskTypes, provisionPerformanceLevel, nil
 }
 

diff --git a/pkg/disk/controllerserver.go b/pkg/disk/controllerserver.go
@@ -60,24 +60,24 @@ type controllerServer struct {
 
 // Alicloud disk parameters
 type diskVolumeArgs struct {
-	Type                    string              `json:"type"`
-	RegionID                string              `json:"regionId"`
-	ZoneID                  string              `json:"zoneId"`
-	FsType                  string              `json:"fsType"`
-	ReadOnly                bool                `json:"readOnly"`
-	MultiAttach             string              `json:"multiAttach"`
-	Encrypted               bool                `json:"encrypted"`
-	KMSKeyID                string              `json:"kmsKeyId"`
-	PerformanceLevel        string              `json:"performanceLevel"`
-	ResourceGroupID         string              `json:"resourceGroupId"`
-	StorageClusterID        string              `json:"storageClusterId"`
-	DiskTags                map[string]string   `json:"diskTags"`
-	NodeSelected            string              `json:"nodeSelected"`
-	DelAutoSnap             string              `json:"delAutoSnap"`
-	ARN                     []ecs.CreateDiskArn `json:"arn"`
-	VolumeSizeAutoAvailable bool                `json:"volumeSizeAutoAvailable"`
-	ProvisionedIops         int                 `json:"provisionedIops"`
-	BurstingEnabled         bool                `json:"burstingEnabled"`
+	Type                    []string
+	RegionID                string
+	ZoneID                  string
+	FsType                  string
+	ReadOnly                bool
+	MultiAttach             string
+	Encrypted               bool
+	KMSKeyID                string
+	PerformanceLevel        []string
+	ResourceGroupID         string
+	StorageClusterID        string
+	DiskTags                map[string]string
+	NodeSelected            string
+	DelAutoSnap             string
+	ARN                     []ecs.CreateDiskArn
+	VolumeSizeAutoAvailable bool
+	ProvisionedIops         int
+	BurstingEnabled         bool
 }
 
 var veasp = struct {
@@ -200,7 +200,7 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		requestGB = MinimumDiskSizeInGB
 		volSizeBytes = MinimumDiskSizeInBytes
 	}
-	sharedDisk := diskVol.Type == DiskSharedEfficiency || diskVol.Type == DiskSharedSSD
+	sharedDisk := len(diskVol.Type) == 1 && (diskVol.Type[0] == DiskSharedEfficiency || diskVol.Type[0] == DiskSharedSSD)
 
 	diskType, diskID, diskPL, err := createDisk(req.GetName(), snapshotID, requestGB, diskVol, req.Parameters[TenantUserUID])
 	if err != nil {

diff --git a/pkg/disk/utils.go b/pkg/disk/utils.go
@@ -871,7 +871,7 @@ func getDiskVolumeOptions(req *csi.CreateVolumeRequest) (*diskVolumeArgs, error)
 	// disk Type
 	diskType, err := validateDiskType(volOptions)
 	if err != nil {
-		return nil, fmt.Errorf("Illegal required parameter type: " + diskVolArgs.Type)
+		return nil, fmt.Errorf("Illegal required parameter type: " + volOptions["type"])
 	}
 	diskVolArgs.Type = diskType
 	pls, err := validateDiskPerformanceLevel(volOptions)
@@ -963,7 +963,7 @@ func getDiskVolumeOptions(req *csi.CreateVolumeRequest) (*diskVolumeArgs, error)
 	}
 
 	if diskVolArgs.StorageClusterID != "" {
-		if diskVolArgs.PerformanceLevel == "" {
+		if len(diskVolArgs.PerformanceLevel) == 0 {
 			return nil, fmt.Errorf("performanceLevel is necessary when storageClusterID: '%s' specified", diskVolArgs.StorageClusterID)
 		}
 	}
@@ -1019,9 +1019,9 @@ func getDiskVolumeOptions(req *csi.CreateVolumeRequest) (*diskVolumeArgs, error)
 	return diskVolArgs, nil
 }
 
-func validateDiskType(opts map[string]string) (diskType string, err error) {
+func validateDiskType(opts map[string]string) (diskType []string, err error) {
 	if value, ok := opts["type"]; !ok || (ok && value == DiskHighAvail) {
-		diskType = strings.Join([]string{DiskSSD, DiskEfficiency}, ",")
+		diskType = []string{DiskSSD, DiskEfficiency}
 		return
 	}
 	if strings.Contains(opts["type"], ",") {
@@ -1033,33 +1033,32 @@ func validateDiskType(opts map[string]string) (diskType string, err error) {
 				return diskType, fmt.Errorf("Illegal required parameter type: " + cusType)
 			}
 		}
-		diskType = strings.Join(orderedList, ",")
+		diskType = orderedList
 		return
 	}
 	t := opts["type"]
 	if AvailableDiskTypes.Has(t) {
-		diskType = t
+		diskType = []string{t}
 	}
-	if diskType == "" {
+	if len(diskType) == 0 {
 		return diskType, fmt.Errorf("Illegal required parameter type: " + opts["type"])
 	}
 	return
 }
 
-func validateDiskPerformanceLevel(opts map[string]string) (performanceLevel string, err error) {
+func validateDiskPerformanceLevel(opts map[string]string) (performanceLevel []string, err error) {
 	pl, ok := opts[ESSD_PERFORMANCE_LEVEL]
 	if !ok || pl == "" {
-		return "", nil
+		return
 	}
 	log.Infof("validateDiskPerformanceLevel: pl: %v", pl)
-	if strings.Contains(pl, ",") {
-		for _, cusPer := range strings.Split(pl, ",") {
-			if _, ok := CustomDiskPerfermance[cusPer]; !ok {
-				return "", fmt.Errorf("illegal performance level type: %s", cusPer)
-			}
+	performanceLevel = strings.Split(pl, ",")
+	for _, cusPer := range performanceLevel {
+		if _, ok := CustomDiskPerfermance[cusPer]; !ok {
+			return nil, fmt.Errorf("illegal performance level type: %s", cusPer)
 		}
 	}
-	return pl, nil
+	return
 }
 
 func checkDeviceAvailable(devicePath, volumeID, targetPath string) error {

diff --git a/pkg/disk/utils_test.go b/pkg/disk/utils_test.go
@@ -50,15 +50,18 @@ func TestIsFileExisting(t *testing.T) {
 }
 
 func TestValidateDiskType(t *testing.T) {
-	examples := []map[string]string{
-		{"result": "cloud_ssd,cloud_efficiency"},
-		{"type": "a,b,c", "result": ""},
-		{"type": "available", "result": "cloud_ssd,cloud_efficiency"},
-		{"type": "cloud_ssd,cloud_essd", "result": "cloud_ssd,cloud_essd"},
+	examples := []struct {
+		opts   map[string]string
+		result []string
+	}{
+		{result: []string{"cloud_ssd", "cloud_efficiency"}},
+		{opts: map[string]string{"type": "a,b,c"}, result: nil},
+		{opts: map[string]string{"type": "available"}, result: []string{"cloud_ssd", "cloud_efficiency"}},
+		{opts: map[string]string{"type": "cloud_ssd,cloud_essd"}, result: []string{"cloud_ssd", "cloud_essd"}},
 	}
 	for _, example := range examples {
-		actualResult, _ := validateDiskType(example)
-		assert.Equal(t, example["result"], actualResult)
+		actualResult, _ := validateDiskType(example.opts)
+		assert.Equal(t, example.result, actualResult)
 	}
 }