From fa837091e6e7149514f91d1d7b1ce62dc9a8db02 Mon Sep 17 00:00:00 2001 From: Eddie Torres Date: Wed, 31 Aug 2022 16:22:48 +0000 Subject: [PATCH] Unify IOPS handling across volume types Signed-off-by: Eddie Torres --- docs/parameters.md | 16 ++++-- pkg/cloud/cloud.go | 87 +++++++++++++++++++----------- pkg/cloud/cloud_test.go | 68 ++++++++++++++++++++--- tests/e2e/driver/ebs_csi_driver.go | 5 +- 4 files changed, 132 insertions(+), 44 deletions(-) diff --git a/docs/parameters.md b/docs/parameters.md index 0cc1da51f6..cbbbee0bc0 100644 --- a/docs/parameters.md +++ b/docs/parameters.md @@ -7,13 +7,21 @@ The AWS EBS CSI Driver supports [tagging](tagging.md) through `StorageClass.para |-----------------------------|----------------------------------------|----------|---------------------| | "csi.storage.k8s.io/fstype" | xfs, ext2, ext3, ext4 | ext4 | File system type that will be formatted during volume creation. This parameter is case sensitive! | | "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 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"`.| +| "iopsPerGB" | | | I/O operations per second per GiB. Can be specified for IO1, IO2, and GP3 volumes. | | "allowAutoIOPSPerGBIncrease"| true, false | false | When `"true"`, the CSI driver increases IOPS for a volume when `iopsPerGB * ` 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 effective 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). | +| "iops" | | | I/O operations per second. Can be specified for IO1, IO2, and GP3 volumes. | | "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". | | "kmsKeyId" | | | The full ARN of the key to use when encrypting the volume. If not specified, AWS will use the default KMS key for the region the volume is in. This will be an auto-generated key called `/aws/ebs` if not changed. | -**Notes**: +**Appendix** * `gp3` is currently not supported on outposts. Outpost customers need to use a different type for their volumes. -* Unless explicitly noted, all parameters are case insensitive (e.g. "kmsKeyId", "kmskeyid" and any other combination of upper/lowercase characters can be used). \ No newline at end of file +* Unless explicitly noted, all parameters are case insensitive (e.g. "kmsKeyId", "kmskeyid" and any other combination of upper/lowercase characters can be used). +* If the requested IOPs (either directly from `iops` or from `iopsPerGB` multiplied by the volume's capacity) produces a value above the maximum IOPs allowed for the [volume type](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ebs-volume-types.html), the IOPS will be capped at the maximum value allowed. If the value is lower than the minimal supported IOPS value per volume, either an error is returned (the default behavior), or the value is increased to fit into the supported range when `allowautoiopspergbincrease` is `"true"`. +* You may specify either the "iops" or "iopsPerGb" parameters, not both. Specifying both parameters will result in an invalid StorageClass. + +| Volume Type | Min total IOPS | Max total IOPS | Max IOPS per GB | +|-----------------------------|----------------------------------------|------------------|-------------------| +| IO1 | 100 | 64000 | 50 | +| IO2 | 100 | 64000 | 500 | +| GP3 | 3000 | 16000 | 500 | diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go index 936c5a054d..0d5c216c24 100644 --- a/pkg/cloud/cloud.go +++ b/pkg/cloud/cloud.go @@ -68,6 +68,9 @@ const ( io2MinTotalIOPS = 100 io2MaxTotalIOPS = 64000 io2MaxIOPSPerGB = 500 + gp3MaxTotalIOPS = 16000 + gp3MinTotalIOPS = 3000 + gp3MaxIOPSPerGB = 500 ) var ( @@ -115,8 +118,6 @@ const ( const ( // DefaultVolumeSize represents the default volume size. DefaultVolumeSize int64 = 100 * util.GiB - // DefaultVolumeType specifies which storage to use for newly created Volumes. - DefaultVolumeType = VolumeTypeGP3 ) // Tags @@ -276,40 +277,59 @@ func newEC2Cloud(region string, awsSdkDebugLog bool) (Cloud, error) { func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *DiskOptions) (*Disk, error) { var ( - createType string - iops int64 - throughput int64 - err error + createType string + iops int64 + throughput int64 + err error + maxIops int64 + minIops int64 + maxIopsPerGb int64 + requestedIops int64 ) + capacityGiB := util.BytesToGiB(diskOptions.CapacityBytes) - switch diskOptions.VolumeType { + if diskOptions.IOPS > 0 && diskOptions.IOPSPerGB > 0 { + return nil, fmt.Errorf("invalid StorageClass parameters; specify either IOPS or IOPSPerGb, not both") + } + + createType = diskOptions.VolumeType + // If no volume type is specified, GP3 is used as default for newly created volumes. + if createType == "" { + createType = VolumeTypeGP3 + } + + switch createType { case VolumeTypeGP2, VolumeTypeSC1, VolumeTypeST1, VolumeTypeSBG1, VolumeTypeSBP1, VolumeTypeStandard: - createType = diskOptions.VolumeType case VolumeTypeIO1: - createType = diskOptions.VolumeType - iops, err = capIOPS(diskOptions.VolumeType, capacityGiB, int64(diskOptions.IOPSPerGB), io1MinTotalIOPS, io1MaxTotalIOPS, io1MaxIOPSPerGB, diskOptions.AllowIOPSPerGBIncrease) - if err != nil { - return nil, err - } + maxIops = io1MaxTotalIOPS + minIops = io1MinTotalIOPS + maxIopsPerGb = io1MaxIOPSPerGB case VolumeTypeIO2: - createType = diskOptions.VolumeType - iops, err = capIOPS(diskOptions.VolumeType, capacityGiB, int64(diskOptions.IOPSPerGB), io2MinTotalIOPS, io2MaxTotalIOPS, io2MaxIOPSPerGB, diskOptions.AllowIOPSPerGBIncrease) - if err != nil { - return nil, err - } + maxIops = io2MaxTotalIOPS + minIops = io2MinTotalIOPS + maxIopsPerGb = io2MaxIOPSPerGB case VolumeTypeGP3: - createType = diskOptions.VolumeType - iops = int64(diskOptions.IOPS) - throughput = int64(diskOptions.Throughput) - case "": - createType = DefaultVolumeType - iops = int64(diskOptions.IOPS) + maxIops = gp3MaxTotalIOPS + minIops = gp3MinTotalIOPS + maxIopsPerGb = gp3MaxIOPSPerGB throughput = int64(diskOptions.Throughput) default: return nil, fmt.Errorf("invalid AWS VolumeType %q", diskOptions.VolumeType) } + if maxIops > 0 { + if diskOptions.IOPS > 0 { + requestedIops = int64(diskOptions.IOPS) + } else if diskOptions.IOPSPerGB > 0 { + requestedIops = int64(diskOptions.IOPSPerGB) * capacityGiB + } + iops, err = capIOPS(createType, capacityGiB, requestedIops, minIops, maxIops, maxIopsPerGb, diskOptions.AllowIOPSPerGBIncrease) + if err != nil { + return nil, err + } + } + var tags []*ec2.Tag for key, value := range diskOptions.Tags { copiedKey := key @@ -1221,26 +1241,29 @@ func getVolumeAttachmentsList(volume *ec2.Volume) []string { } // Calculate actual IOPS for a volume and cap it at supported AWS limits. -// 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, allowIncrease bool) (int64, error) { - iops := requestedCapacityGiB * requstedIOPSPerGB +func capIOPS(volumeType string, requestedCapacityGiB int64, requestedIops int64, minTotalIOPS, maxTotalIOPS, maxIOPSPerGB int64, allowIncrease bool) (int64, error) { + // If requestedIops is zero the user did not request a specific amount, and the default will be used instead + if requestedIops == 0 { + return 0, nil + } + + iops := requestedIops if iops < minTotalIOPS { if allowIncrease { iops = minTotalIOPS klog.V(5).Infof("[Debug] 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) + return 0, fmt.Errorf("invalid IOPS: %d is too low, it must be at least %d", iops, minTotalIOPS) } } if iops > maxTotalIOPS { iops = maxTotalIOPS klog.V(5).Infof("[Debug] Capped IOPS for %s %d GB volume at the max supported limit: %d", volumeType, requestedCapacityGiB, iops) } - if iops > maxIOPSPerGB*requestedCapacityGiB { - iops = maxIOPSPerGB * requestedCapacityGiB + maxIopsByCapacity := maxIOPSPerGB * requestedCapacityGiB + if iops > maxIOPSPerGB*requestedCapacityGiB && maxIopsByCapacity >= minTotalIOPS { + iops = maxIopsByCapacity klog.V(5).Infof("[Debug] Capped IOPS for %s %d GB volume at %d IOPS/GB: %d", volumeType, requestedCapacityGiB, maxIOPSPerGB, iops) } return iops, nil diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go index db8b10c82d..68cb9f6ca1 100644 --- a/pkg/cloud/cloud_test.go +++ b/pkg/cloud/cloud_test.go @@ -72,13 +72,13 @@ func TestCreateDisk(t *testing.T) { name: "success: normal with iops", volumeName: "vol-test-name", diskOptions: &DiskOptions{ - CapacityBytes: util.GiBToBytes(1), + CapacityBytes: util.GiBToBytes(500), Tags: map[string]string{VolumeNameTagKey: "vol-test", AwsEbsDriverTagKey: "true"}, IOPS: 6000, }, expDisk: &Disk{ VolumeID: "vol-test", - CapacityGiB: 1, + CapacityGiB: 500, AvailabilityZone: defaultZone, }, expCreateVolumeInput: &ec2.CreateVolumeInput{ @@ -122,18 +122,56 @@ func TestCreateDisk(t *testing.T) { expErr: nil, }, { - name: "success: normal with gp3 options", + name: "success: io1 with IOPS parameter", + volumeName: "vol-test-name", + diskOptions: &DiskOptions{ + CapacityBytes: util.GiBToBytes(200), + Tags: map[string]string{VolumeNameTagKey: "vol-test", AwsEbsDriverTagKey: "true"}, + VolumeType: VolumeTypeIO1, + IOPS: 100, + }, + expDisk: &Disk{ + VolumeID: "vol-test", + CapacityGiB: 200, + AvailabilityZone: defaultZone, + }, + expCreateVolumeInput: &ec2.CreateVolumeInput{ + Iops: aws.Int64(100), + }, + expErr: nil, + }, + { + name: "success: io2 with IOPS parameter", volumeName: "vol-test-name", diskOptions: &DiskOptions{ CapacityBytes: util.GiBToBytes(1), Tags: map[string]string{VolumeNameTagKey: "vol-test", AwsEbsDriverTagKey: "true"}, + VolumeType: VolumeTypeIO2, + IOPS: 100, + }, + expDisk: &Disk{ + VolumeID: "vol-test", + CapacityGiB: 1, + AvailabilityZone: defaultZone, + }, + expCreateVolumeInput: &ec2.CreateVolumeInput{ + Iops: aws.Int64(100), + }, + expErr: nil, + }, + { + name: "success: normal with gp3 options", + volumeName: "vol-test-name", + diskOptions: &DiskOptions{ + CapacityBytes: util.GiBToBytes(400), + Tags: map[string]string{VolumeNameTagKey: "vol-test", AwsEbsDriverTagKey: "true"}, VolumeType: VolumeTypeGP3, IOPS: 3000, Throughput: 125, }, expDisk: &Disk{ VolumeID: "vol-test", - CapacityGiB: 1, + CapacityGiB: 400, AvailabilityZone: defaultZone, }, expCreateVolumeInput: &ec2.CreateVolumeInput{ @@ -310,6 +348,24 @@ func TestCreateDisk(t *testing.T) { }, expErr: nil, }, + { + name: "fail: invalid StorageClass parameters; specified both IOPS and IOPSPerGb", + volumeName: "vol-test-name", + diskOptions: &DiskOptions{ + CapacityBytes: util.GiBToBytes(4), + Tags: map[string]string{VolumeNameTagKey: "vol-test", AwsEbsDriverTagKey: "true"}, + VolumeType: VolumeTypeIO1, + IOPS: 1, + IOPSPerGB: 1, + }, + expDisk: &Disk{ + VolumeID: "vol-test", + CapacityGiB: 4, + AvailabilityZone: defaultZone, + }, + expCreateVolumeInput: nil, + expErr: fmt.Errorf("invalid StorageClass parameters; specify either IOPS or IOPSPerGb, not both"), + }, { name: "fail: io1 with too low iopsPerGB", volumeName: "vol-test-name", @@ -325,7 +381,7 @@ func TestCreateDisk(t *testing.T) { 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"), + expErr: fmt.Errorf("invalid IOPS: 4 is too low, it must be at least 100"), }, { name: "success: small io1 with too high iopsPerGB", @@ -400,7 +456,7 @@ func TestCreateDisk(t *testing.T) { 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"), + expErr: fmt.Errorf("invalid IOPS: 4 is too low, it must be at least 100"), }, { name: "success: small io2 with too high iopsPerGB", diff --git a/tests/e2e/driver/ebs_csi_driver.go b/tests/e2e/driver/ebs_csi_driver.go index b4bbeebe8b..265034dd8b 100644 --- a/tests/e2e/driver/ebs_csi_driver.go +++ b/tests/e2e/driver/ebs_csi_driver.go @@ -160,8 +160,9 @@ func IOPSForVolumeType(volumeType string) string { switch volumeType { case "gp3": // Maximum IOPS for gp3 is 16000. However, maximum IOPS/GB for gp3 is 500. - // Since the tests will run using minimum volume capacity (1GB), set to 500. - return "500" + // Since the tests will run using minimum volume capacity (1GB), set to 3000 + // because minimum IOPS for gp3 is 3000. + return "3000" default: return "" }