Merge pull request #882 from k8s-infra-cherrypick-robot/cherry-pick-8…

…73-to-release-1.6 [release-1.6] fix tier capacity check for expandVolume
kubernetes-sigs · Jun 11, 2024 · cdaa196 · cdaa196
2 parents 0a7029d + b2f4f2b
commit cdaa196
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 5 deletions.
diff --git a/pkg/csi_driver/controller.go b/pkg/csi_driver/controller.go
@@ -542,6 +542,7 @@ func provisionableCapacityForTier(tier string) *capacityRangeForTier {
 		basicHDDTier:   defaultRange, //these two are aliases
 	}
 
+	tier = strings.ToLower(tier)
 	validRange, ok := provisionableCapacityForTier[tier]
 	if !ok {
 		validRange = provisionableCapacityForTier[defaultTier]
@@ -715,11 +716,6 @@ func (s *controllerServer) ControllerExpandVolume(ctx context.Context, req *csi.
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
-	reqBytes, err := getRequestCapacity(req.GetCapacityRange(), filer.Tier)
-	if err != nil {
-		return nil, status.Error(codes.InvalidArgument, err.Error())
-	}
-
 	filer.Project = s.config.cloud.Project
 	filer, err = s.config.fileService.GetInstance(ctx, filer)
 	if err != nil {
@@ -729,6 +725,12 @@ func (s *controllerServer) ControllerExpandVolume(ctx context.Context, req *csi.
 		return nil, fmt.Errorf("lolume %q is not yet ready, current state %q", volumeID, filer.State)
 	}
 
+	// getFileInstanceFromID doesn't have tier info set, we have to check the range after GetInstance call
+	reqBytes, err := getRequestCapacity(req.GetCapacityRange(), filer.Tier)
+	if err != nil {
+		return nil, status.Error(codes.InvalidArgument, err.Error())
+	}
+
 	if util.BytesToGb(reqBytes) <= util.BytesToGb(filer.Volume.SizeBytes) {
 		klog.Infof("Controller expand volume succeeded for volume %v, existing size(bytes): %v", volumeID, filer.Volume.SizeBytes)
 		return &csi.ControllerExpandVolumeResponse{

diff --git a/pkg/csi_driver/controller_test.go b/pkg/csi_driver/controller_test.go
@@ -937,6 +937,31 @@ func TestGetRequestCapacity(t *testing.T) {
 			tier:  basicHDDTier,
 			bytes: 1 * util.Tb,
 		},
+		{
+			name: "required in range ZONAL all cap",
+			capRange: &csi.CapacityRange{
+				RequiredBytes: 100 * util.Tb,
+			},
+			tier:  "ZONAL",
+			bytes: 100 * util.Tb,
+		},
+		{
+			name: "required above max BASIC_SSD all cap",
+			capRange: &csi.CapacityRange{
+				RequiredBytes: 70 * util.Tb,
+			},
+			tier:          "BASIC_SSD",
+			errorExpected: true,
+		},
+		{
+			name: "required and limit both in range BASIC_SSD all cap",
+			capRange: &csi.CapacityRange{
+				RequiredBytes: 3 * util.Tb,
+				LimitBytes:    60 * util.Tb,
+			},
+			tier:  "BASIC_SSD",
+			bytes: 3 * util.Tb,
+		},
 	}
 
 	for _, tc := range cases {