Add increaseTooLowIOPS parameter

kubernetes-sigs · Mar 26, 2021 · 0ecc0e9 · 0ecc0e9
1 parent d5b4c05
commit 0ecc0e9
Show file tree

Hide file tree

Showing 5 changed files with 137 additions and 72 deletions.
diff --git a/docs/README.md b/docs/README.md
@@ -35,7 +35,8 @@ There are several optional parameters that could be passed into `CreateVolumeReq
 |-----------------------------|----------------------------------------|----------|---------------------|
 | "csi.storage.k8s.io/fsType" | xfs, ext2, ext3, ext4                  | ext4     | File system type that will be formatted during volume creation |
 | "type"                      | io1, io2, gp2, gp3, sc1, st1,standard  | gp3*     | EBS volume type     |
-| "iopsPerGB"                 |                                        |          | I/O operations per second per GiB. Required when io1 or io2 volume type is specified. If this value multiplied by the size of a requested volume produces a value below the minimum or above the maximum IOPs allowed for the volume type, as documented [here](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ebs-volume-types.html), the driver will increase or reduce the volume IOPS to fit into the AWS IOPS limits. |
+| "iopsPerGB"                 |                                        |          | I/O operations per second per GiB. Required when io1 or io2 volume type is specified. If this value multiplied by the size of a requested volume produces a value above the maximum IOPs allowed for the volume type, as documented [here](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ebs-volume-types.html), AWS will cap the IOPS to maximum supported value. If the value is lower than minimal supported IOPS value per volume, either error is returned (the default behavior) or the value is increased to fit into the supported range when `allowautoiopspergbincrease` is `"true"`.|
+| "allowAutoIOPSPerGBIncrease"| true, false                            | false    | When `"true"`, the CSI driver increases IOPS for a volume when `iopsPerGB * <volume size>` is too low to fit into IOPS range supported by AWS. This allows dynamic provisioning to always succeed, even when user specifies too small PVC capacity or `iopsPerGB` value. On the other hand, it may introduce additional costs, as such volumes have higher IOPS than requested in `iopsPerGB`.|
 | "iops"                      |                                        | 3000     | I/O operations per second. Only effetive when gp3 volume type is specified. If empty, it will set to 3000 as documented [here](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ebs-volume-types.html). |
 | "throughput"                |                                        | 125      | Throughput in MiB/s. Only effective when gp3 volume type is specified. If empty, it will set to 125MiB/s as documented [here](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ebs-volume-types.html). |
 | "encrypted"                 |                                        |          | Whether the volume should be encrypted or not. Valid values are "true" or "false" |

diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go
@@ -153,15 +153,16 @@ type Disk struct {
 
 // DiskOptions represents parameters to create an EBS volume
 type DiskOptions struct {
-	CapacityBytes    int64
-	Tags             map[string]string
-	VolumeType       string
-	IOPSPerGB        int
-	IOPS             int
-	Throughput       int
-	AvailabilityZone string
-	OutpostArn       string
-	Encrypted        bool
+	CapacityBytes          int64
+	Tags                   map[string]string
+	VolumeType             string
+	IOPSPerGB              int
+	AllowIOPSPerGBIncrease bool
+	IOPS                   int
+	Throughput             int
+	AvailabilityZone       string
+	OutpostArn             string
+	Encrypted              bool
 	// KmsKeyID represents a fully qualified resource name to the key to use for encryption.
 	// example: arn:aws:kms:us-east-1:012345678910:key/abcd1234-a123-456a-a12b-a123b4cd56ef
 	KmsKeyID   string
@@ -267,6 +268,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
 		createType string
 		iops       int64
 		throughput int64
+		err        error
 	)
 	capacityGiB := util.BytesToGiB(diskOptions.CapacityBytes)
 
@@ -275,10 +277,16 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
 		createType = diskOptions.VolumeType
 	case VolumeTypeIO1:
 		createType = diskOptions.VolumeType
-		iops = capIOPS(diskOptions.VolumeType, capacityGiB, int64(diskOptions.IOPSPerGB), io1MinTotalIOPS, io1MaxTotalIOPS, io1MaxIOPSPerGB)
+		iops, err = capIOPS(diskOptions.VolumeType, capacityGiB, int64(diskOptions.IOPSPerGB), io1MinTotalIOPS, io1MaxTotalIOPS, io1MaxIOPSPerGB, diskOptions.AllowIOPSPerGBIncrease)
+		if err != nil {
+			return nil, err
+		}
 	case VolumeTypeIO2:
 		createType = diskOptions.VolumeType
-		iops = capIOPS(diskOptions.VolumeType, capacityGiB, int64(diskOptions.IOPSPerGB), io2MinTotalIOPS, io2MaxTotalIOPS, io2MaxIOPSPerGB)
+		iops, err = capIOPS(diskOptions.VolumeType, capacityGiB, int64(diskOptions.IOPSPerGB), io2MinTotalIOPS, io2MaxTotalIOPS, io2MaxIOPSPerGB, diskOptions.AllowIOPSPerGBIncrease)
+		if err != nil {
+			return nil, err
+		}
 	case VolumeTypeGP3:
 		createType = diskOptions.VolumeType
 		iops = int64(diskOptions.IOPS)
@@ -1092,12 +1100,16 @@ func getVolumeAttachmentsList(volume *ec2.Volume) []string {
 // Using requstedIOPSPerGB allows users to create a "fast" storage class
 // (requstedIOPSPerGB = 50 for io1), which can provide the maximum iops
 // that AWS supports for any requestedCapacityGiB.
-func capIOPS(volumeType string, requestedCapacityGiB int64, requstedIOPSPerGB, minTotalIOPS, maxTotalIOPS, maxIOPSPerGB int64) int64 {
+func capIOPS(volumeType string, requestedCapacityGiB int64, requstedIOPSPerGB, minTotalIOPS, maxTotalIOPS, maxIOPSPerGB int64, allowIncrease bool) (int64, error) {
 	iops := requestedCapacityGiB * requstedIOPSPerGB
 
 	if iops < minTotalIOPS {
-		iops = minTotalIOPS
-		klog.V(5).Infof("Increased IOPS for %s %d GB volume to the min supported limit: %d", volumeType, requestedCapacityGiB, iops)
+		if allowIncrease {
+			iops = minTotalIOPS
+			klog.V(5).Infof("Increased IOPS for %s %d GB volume to the min supported limit: %d", volumeType, requestedCapacityGiB, iops)
+		} else {
+			return 0, fmt.Errorf("invalid combination of volume size %d GB and iopsPerGB %d: the resulting IOPS %d is too low for AWS, it must be at least %d", requestedCapacityGiB, requstedIOPSPerGB, iops, minTotalIOPS)
+		}
 	}
 	if iops > maxTotalIOPS {
 		iops = maxTotalIOPS
@@ -1107,5 +1119,5 @@ func capIOPS(volumeType string, requestedCapacityGiB int64, requstedIOPSPerGB, m
 		iops = maxIOPSPerGB * requestedCapacityGiB
 		klog.V(5).Infof("Capped IOPS for %s %d GB volume at %d IOPS/GB: %d", volumeType, requestedCapacityGiB, maxIOPSPerGB, iops)
 	}
-	return iops
+	return iops, nil
 }
diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go
@@ -59,6 +59,7 @@ func TestCreateDisk(t *testing.T) {
 				CapacityBytes: util.GiBToBytes(1),
 				Tags:          map[string]string{VolumeNameTagKey: "vol-test"},
 			},
+			expCreateVolumeInput: &ec2.CreateVolumeInput{},
 			expDisk: &Disk{
 				VolumeID:         "vol-test",
 				CapacityGiB:      1,
@@ -118,7 +119,8 @@ func TestCreateDisk(t *testing.T) {
 				CapacityGiB:      1,
 				AvailabilityZone: expZone,
 			},
-			expErr: nil,
+			expCreateVolumeInput: &ec2.CreateVolumeInput{},
+			expErr:               nil,
 		},
 		{
 			name:       "success: normal with encrypted volume",
@@ -135,7 +137,8 @@ func TestCreateDisk(t *testing.T) {
 				CapacityGiB:      1,
 				AvailabilityZone: expZone,
 			},
-			expErr: nil,
+			expCreateVolumeInput: &ec2.CreateVolumeInput{},
+			expErr:               nil,
 		},
 		{
 			name:       "success: outpost volume",
@@ -152,7 +155,8 @@ func TestCreateDisk(t *testing.T) {
 				AvailabilityZone: expZone,
 				OutpostArn:       "arn:aws:outposts:us-west-2:111111111111:outpost/op-0aaa000a0aaaa00a0",
 			},
-			expErr: nil,
+			expCreateVolumeInput: &ec2.CreateVolumeInput{},
+			expErr:               nil,
 		},
 		{
 			name:       "success: empty outpost arn",
@@ -168,7 +172,8 @@ func TestCreateDisk(t *testing.T) {
 				AvailabilityZone: expZone,
 				OutpostArn:       "",
 			},
-			expErr: nil,
+			expCreateVolumeInput: &ec2.CreateVolumeInput{},
+			expErr:               nil,
 		},
 		{
 			name:       "fail: ec2.CreateVolume returned CreateVolume error",
@@ -178,8 +183,9 @@ func TestCreateDisk(t *testing.T) {
 				Tags:             map[string]string{VolumeNameTagKey: "vol-test"},
 				AvailabilityZone: expZone,
 			},
-			expErr:             fmt.Errorf("could not create volume in EC2: CreateVolume generic error"),
-			expCreateVolumeErr: fmt.Errorf("CreateVolume generic error"),
+			expCreateVolumeInput: &ec2.CreateVolumeInput{},
+			expErr:               fmt.Errorf("could not create volume in EC2: CreateVolume generic error"),
+			expCreateVolumeErr:   fmt.Errorf("CreateVolume generic error"),
 		},
 		{
 			name:       "fail: ec2.DescribeVolumes error after volume created",
@@ -190,9 +196,10 @@ func TestCreateDisk(t *testing.T) {
 				Tags:             map[string]string{VolumeNameTagKey: "vol-test"},
 				AvailabilityZone: "",
 			},
-			expErr:              fmt.Errorf("failed to get an available volume in EC2: DescribeVolumes generic error"),
-			expDescVolumeErr:    fmt.Errorf("DescribeVolumes generic error"),
-			cleanUpFailedVolume: true,
+			expCreateVolumeInput: &ec2.CreateVolumeInput{},
+			expErr:               fmt.Errorf("failed to get an available volume in EC2: DescribeVolumes generic error"),
+			expDescVolumeErr:     fmt.Errorf("DescribeVolumes generic error"),
+			cleanUpFailedVolume:  true,
 		},
 		{
 			name:       "fail: Volume is not ready to use, volume stuck in creating status and controller timed out waiting for the condition",
@@ -203,8 +210,9 @@ func TestCreateDisk(t *testing.T) {
 				Tags:             map[string]string{VolumeNameTagKey: "vol-test"},
 				AvailabilityZone: "",
 			},
-			cleanUpFailedVolume: true,
-			expErr:              fmt.Errorf("failed to get an available volume in EC2: timed out waiting for the condition"),
+			cleanUpFailedVolume:  true,
+			expCreateVolumeInput: &ec2.CreateVolumeInput{},
+			expErr:               fmt.Errorf("failed to get an available volume in EC2: timed out waiting for the condition"),
 		},
 		{
 			name:       "success: normal from snapshot",
@@ -220,10 +228,31 @@ func TestCreateDisk(t *testing.T) {
 				CapacityGiB:      1,
 				AvailabilityZone: expZone,
 			},
+			expCreateVolumeInput: &ec2.CreateVolumeInput{},
+			expErr:               nil,
+		},
+		{
+			name:       "success: io1 with too low iopsPerGB and AllowIOPSPerGBIncrease",
+			volumeName: "vol-test-name",
+			diskOptions: &DiskOptions{
+				CapacityBytes:          util.GiBToBytes(4),
+				Tags:                   map[string]string{VolumeNameTagKey: "vol-test"},
+				VolumeType:             VolumeTypeIO1,
+				IOPSPerGB:              1,
+				AllowIOPSPerGBIncrease: true,
+			},
+			expDisk: &Disk{
+				VolumeID:         "vol-test",
+				CapacityGiB:      4,
+				AvailabilityZone: defaultZone,
+			},
+			expCreateVolumeInput: &ec2.CreateVolumeInput{
+				Iops: aws.Int64(100),
+			},
 			expErr: nil,
 		},
 		{
-			name:       "success: io1 with too low iopsPerGB",
+			name:       "fail: io1 with too low iopsPerGB",
 			volumeName: "vol-test-name",
 			diskOptions: &DiskOptions{
 				CapacityBytes: util.GiBToBytes(4),
@@ -236,10 +265,8 @@ func TestCreateDisk(t *testing.T) {
 				CapacityGiB:      4,
 				AvailabilityZone: defaultZone,
 			},
-			expCreateVolumeInput: &ec2.CreateVolumeInput{
-				Iops: aws.Int64(100),
-			},
-			expErr: nil,
+			expCreateVolumeInput: nil,
+			expErr:               fmt.Errorf("invalid combination of volume size 4 GB and iopsPerGB 1: the resulting IOPS 4 is too low for AWS, it must be at least 100"),
 		},
 		{
 			name:       "success: small io1 with too high iopsPerGB",
@@ -280,13 +307,14 @@ func TestCreateDisk(t *testing.T) {
 			expErr: nil,
 		},
 		{
-			name:       "success: io2 with too low iopsPerGB",
+			name:       "success: io2 with too low iopsPerGB and AllowIOPSPerGBIncrease",
 			volumeName: "vol-test-name",
 			diskOptions: &DiskOptions{
-				CapacityBytes: util.GiBToBytes(4),
-				Tags:          map[string]string{VolumeNameTagKey: "vol-test"},
-				VolumeType:    VolumeTypeIO2,
-				IOPSPerGB:     1,
+				CapacityBytes:          util.GiBToBytes(4),
+				Tags:                   map[string]string{VolumeNameTagKey: "vol-test"},
+				VolumeType:             VolumeTypeIO2,
+				IOPSPerGB:              1,
+				AllowIOPSPerGBIncrease: true,
 			},
 			expDisk: &Disk{
 				VolumeID:         "vol-test",
@@ -298,6 +326,23 @@ func TestCreateDisk(t *testing.T) {
 			},
 			expErr: nil,
 		},
+		{
+			name:       "fail: io2 with too low iopsPerGB",
+			volumeName: "vol-test-name",
+			diskOptions: &DiskOptions{
+				CapacityBytes: util.GiBToBytes(4),
+				Tags:          map[string]string{VolumeNameTagKey: "vol-test"},
+				VolumeType:    VolumeTypeIO2,
+				IOPSPerGB:     1,
+			},
+			expDisk: &Disk{
+				VolumeID:         "vol-test",
+				CapacityGiB:      4,
+				AvailabilityZone: defaultZone,
+			},
+			expCreateVolumeInput: nil,
+			expErr:               fmt.Errorf("invalid combination of volume size 4 GB and iopsPerGB 1: the resulting IOPS 4 is too low for AWS, it must be at least 100"),
+		},
 		{
 			name:       "success: small io2 with too high iopsPerGB",
 			volumeName: "vol-test-name",
@@ -362,24 +407,24 @@ func TestCreateDisk(t *testing.T) {
 				State:      aws.String("completed"),
 			}
 			ctx := context.Background()
-			matcher := gomock.Any()
+
 			if tc.expCreateVolumeInput != nil {
-				matcher = eqCreateVolume(tc.expCreateVolumeInput)
-			}
-			mockEC2.EXPECT().CreateVolumeWithContext(gomock.Eq(ctx), matcher).Return(vol, tc.expCreateVolumeErr)
-			mockEC2.EXPECT().DescribeVolumesWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeVolumesOutput{Volumes: []*ec2.Volume{vol}}, tc.expDescVolumeErr).AnyTimes()
-			if len(tc.diskOptions.SnapshotID) > 0 {
-				mockEC2.EXPECT().DescribeSnapshotsWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{Snapshots: []*ec2.Snapshot{snapshot}}, nil).AnyTimes()
-			}
-			if tc.cleanUpFailedVolume == true {
-				mockEC2.EXPECT().DeleteVolumeWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DeleteVolumeOutput{}, nil)
-			}
-			if len(tc.diskOptions.AvailabilityZone) == 0 {
-				mockEC2.EXPECT().DescribeAvailabilityZonesWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeAvailabilityZonesOutput{
-					AvailabilityZones: []*ec2.AvailabilityZone{
-						{ZoneName: aws.String(defaultZone)},
-					},
-				}, nil)
+				matcher := eqCreateVolume(tc.expCreateVolumeInput)
+				mockEC2.EXPECT().CreateVolumeWithContext(gomock.Eq(ctx), matcher).Return(vol, tc.expCreateVolumeErr)
+				mockEC2.EXPECT().DescribeVolumesWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeVolumesOutput{Volumes: []*ec2.Volume{vol}}, tc.expDescVolumeErr).AnyTimes()
+				if len(tc.diskOptions.SnapshotID) > 0 {
+					mockEC2.EXPECT().DescribeSnapshotsWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeSnapshotsOutput{Snapshots: []*ec2.Snapshot{snapshot}}, nil).AnyTimes()
+				}
+				if tc.cleanUpFailedVolume == true {
+					mockEC2.EXPECT().DeleteVolumeWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DeleteVolumeOutput{}, nil)
+				}
+				if len(tc.diskOptions.AvailabilityZone) == 0 {
+					mockEC2.EXPECT().DescribeAvailabilityZonesWithContext(gomock.Eq(ctx), gomock.Any()).Return(&ec2.DescribeAvailabilityZonesOutput{
+						AvailabilityZones: []*ec2.AvailabilityZone{
+							{ZoneName: aws.String(defaultZone)},
+						},
+					}, nil)
+				}
 			}
 
 			disk, err := c.CreateDisk(ctx, tc.volumeName, tc.diskOptions)

diff --git a/pkg/driver/constants.go b/pkg/driver/constants.go
@@ -31,6 +31,9 @@ const (
 	// IopsPerGBKey represents key for IOPS per GB
 	IopsPerGBKey = "iopspergb"
 
+	// AllowAutoIOPSPerGBIncreaseKey represents key for allowing automatic increase of IOPS
+	AllowAutoIOPSPerGBIncreaseKey = "allowautoiopspergbincrease"
+
 	// Iops represents key for IOPS for volume
 	IopsKey = "iops"
 

diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go
@@ -132,13 +132,14 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 	}
 
 	var (
-		volumeType  string
-		iopsPerGB   int
-		iops        int
-		throughput  int
-		isEncrypted bool
-		kmsKeyID    string
-		volumeTags  = map[string]string{
+		volumeType             string
+		iopsPerGB              int
+		allowIOPSPerGBIncrease bool
+		iops                   int
+		throughput             int
+		isEncrypted            bool
+		kmsKeyID               string
+		volumeTags             = map[string]string{
 			cloud.VolumeNameTagKey: volName,
 		}
 	)
@@ -154,6 +155,8 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 			if err != nil {
 				return nil, status.Errorf(codes.InvalidArgument, "Could not parse invalid iopsPerGB: %v", err)
 			}
+		case AllowAutoIOPSPerGBIncreaseKey:
+			allowIOPSPerGBIncrease = value == "true"
 		case IopsKey:
 			iops, err = strconv.Atoi(value)
 			if err != nil {
@@ -232,17 +235,18 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol
 	}
 
 	opts := &cloud.DiskOptions{
-		CapacityBytes:    volSizeBytes,
-		Tags:             volumeTags,
-		VolumeType:       volumeType,
-		IOPSPerGB:        iopsPerGB,
-		IOPS:             iops,
-		Throughput:       throughput,
-		AvailabilityZone: zone,
-		OutpostArn:       outpostArn,
-		Encrypted:        isEncrypted,
-		KmsKeyID:         kmsKeyID,
-		SnapshotID:       snapshotID,
+		CapacityBytes:          volSizeBytes,
+		Tags:                   volumeTags,
+		VolumeType:             volumeType,
+		IOPSPerGB:              iopsPerGB,
+		AllowIOPSPerGBIncrease: allowIOPSPerGBIncrease,
+		IOPS:                   iops,
+		Throughput:             throughput,
+		AvailabilityZone:       zone,
+		OutpostArn:             outpostArn,
+		Encrypted:              isEncrypted,
+		KmsKeyID:               kmsKeyID,
+		SnapshotID:             snapshotID,
 	}
 
 	disk, err = d.cloud.CreateDisk(ctx, volName, opts)