Skip to content

Add provisionedIops for pd-extreme #1079

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,14 @@ See Github [Issues](https://github.com/kubernetes-sigs/gcp-compute-persistent-di

### CreateVolume Parameters

| Parameter | Values | Default | Description |
|------------------|---------------------------|---------------|----------------------------------------------------------------------------------------------------|
| type | Any PD type (see [GCP documentation](https://cloud.google.com/compute/docs/disks#disk-types)), eg `pd-ssd` `pd-balanced` | `pd-standard` | Type allows you to choose between standard Persistent Disks or Solid State Drive Persistent Disks |
| replication-type | `none` OR `regional-pd` | `none` | Replication type allows you to choose between Zonal Persistent Disks or Regional Persistent Disks |
| disk-encryption-kms-key | Fully qualified resource identifier for the key to use to encrypt new disks. | Empty string. | Encrypt disk using Customer Managed Encryption Key (CMEK). See [GKE Docs](https://cloud.google.com/kubernetes-engine/docs/how-to/using-cmek#create_a_cmek_protected_attached_disk) for details. |
| labels | `key1=value1,key2=value2` | | Labels allow you to assign custom [GCE Disk labels](https://cloud.google.com/compute/docs/labeling-resources). |
| Parameter | Values | Default | Description |
|-----------------------------|---------------------------|---------------|----------------------------------------------------------------------------------------------------|
| type | Any PD type (see [GCP documentation](https://cloud.google.com/compute/docs/disks#disk-types)), eg `pd-ssd` `pd-balanced` | `pd-standard` | Type allows you to choose between standard Persistent Disks or Solid State Drive Persistent Disks |
| replication-type | `none` OR `regional-pd` | `none` | Replication type allows you to choose between Zonal Persistent Disks or Regional Persistent Disks |
| disk-encryption-kms-key | Fully qualified resource identifier for the key to use to encrypt new disks. | Empty string. | Encrypt disk using Customer Managed Encryption Key (CMEK). See [GKE Docs](https://cloud.google.com/kubernetes-engine/docs/how-to/using-cmek#create_a_cmek_protected_attached_disk) for details. |
| labels | `key1=value1,key2=value2` | | Labels allow you to assign custom [GCE Disk labels](https://cloud.google.com/compute/docs/labeling-resources). |
| provisioned-iops-on-create | string (int64 format). Values typically between 10,000 and 120,000 | | Indicates how many IOPS to provision for the disk. See the [Extreme persistent disk documentation](https://cloud.google.com/compute/docs/disks/extreme-persistent-disk) for details, including valid ranges for IOPS. |


### Topology

Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ require (
gopkg.in/gcfg.v1 v1.2.3
k8s.io/apimachinery v0.24.1
k8s.io/client-go v11.0.1-0.20190805182717-6502b5e7b1b5+incompatible
k8s.io/cloud-provider v0.24.1
k8s.io/component-base v0.24.1
k8s.io/klog/v2 v2.60.1
k8s.io/kubernetes v1.24.1
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2432,6 +2432,7 @@ k8s.io/apiserver v0.24.1/go.mod h1:dQWNMx15S8NqJMp0gpYfssyvhYnkilc1LpExd/dkLh0=
k8s.io/cli-runtime v0.24.1/go.mod h1:14aVvCTqkA7dNXY51N/6hRY3GUjchyWDOwW84qmR3bs=
k8s.io/client-go v0.24.1 h1:w1hNdI9PFrzu3OlovVeTnf4oHDt+FJLd9Ndluvnb42E=
k8s.io/client-go v0.24.1/go.mod h1:f1kIDqcEYmwXS/vTbbhopMUbhKp2JhOeVTfxgaCIlF8=
k8s.io/cloud-provider v0.24.1 h1:SaQNq2Ax+epdY9wFngwN9GWpOVnM72hUqr2qy20cOvg=
k8s.io/cloud-provider v0.24.1/go.mod h1:h5m/KIiwiQ76hpUBsgrwm/rxteIfJG9kJQ/+/w1as2M=
k8s.io/cluster-bootstrap v0.24.1/go.mod h1:uq2PiYfKh8ZLb6DBU/3/2Z1DkMqXkTOHLemalC4tOgE=
k8s.io/code-generator v0.24.1/go.mod h1:dpVhs00hTuTdTY6jvVxvTFCk6gSMrtfRydbhZwHI15w=
Expand Down
18 changes: 14 additions & 4 deletions pkg/common/parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ import (

const (
// Parameters for StorageClass
ParameterKeyType = "type"
ParameterKeyReplicationType = "replication-type"
ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key"
ParameterKeyLabels = "labels"
ParameterKeyType = "type"
ParameterKeyReplicationType = "replication-type"
ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key"
ParameterKeyLabels = "labels"
ParameterKeyProvisionedIOPSOnCreate = "provisioned-iops-on-create"

// Parameters for VolumeSnapshotClass
ParameterKeyStorageLocations = "storage-locations"
Expand Down Expand Up @@ -80,6 +81,9 @@ type DiskParameters struct {
// Values: {map[string]string}
// Default: ""
Labels map[string]string
// Values: {int64}
// Default: none
ProvisionedIOPSOnCreate int64
}

// SnapshotParameters contains normalized and defaulted parameters for snapshots
Expand Down Expand Up @@ -143,6 +147,12 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
for labelKey, labelValue := range paramLabels {
p.Labels[labelKey] = labelValue
}
case ParameterKeyProvisionedIOPSOnCreate:
paramProvisionedIOPSOnCreate, err := ConvertGiBStringToInt64(v)
if err != nil {
return p, fmt.Errorf("parameters contain invalid provisionedIOPSOnCreate parameter: %w", err)
}
p.ProvisionedIOPSOnCreate = paramProvisionedIOPSOnCreate
default:
return p, fmt.Errorf("parameters contains invalid option %q", k)
}
Expand Down
16 changes: 16 additions & 0 deletions pkg/common/parameters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,22 @@ func TestExtractAndDefaultParameters(t *testing.T) {
},
},
},
{
name: "values from parameters, checking pd-extreme",
parameters: map[string]string{ParameterKeyType: "pd-extreme", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyProvisionedIOPSOnCreate: "10000Gi"},
labels: map[string]string{},
expectParams: DiskParameters{
DiskType: "pd-extreme",
ReplicationType: "none",
DiskEncryptionKMSKey: "foo/key",
Tags: map[string]string{},
Labels: map[string]string{
"key1": "value1",
"key2": "value2",
},
ProvisionedIOPSOnCreate: 10000,
},
},
{
name: "values from parameters, checking balanced pd",
parameters: map[string]string{ParameterKeyType: "pd-balanced", ParameterKeyReplicationType: "regional-pd", ParameterKeyDiskEncryptionKmsKey: "foo/key"},
Expand Down
13 changes: 13 additions & 0 deletions pkg/common/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import (
"strings"

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/util/sets"
volumehelpers "k8s.io/cloud-provider/volume/helpers"
)

const (
Expand Down Expand Up @@ -248,3 +250,14 @@ func ValidateSnapshotType(snapshotType string) error {
return fmt.Errorf("invalid snapshot type %s", snapshotType)
}
}

// ConvertGiBStringToInt64 converts a GiB string to int64
func ConvertGiBStringToInt64(str string) (int64, error) {
// Verify regex before
match, _ := regexp.MatchString("^([+-]?[0-9.]+)([eEinumkKMGTP]*[-+]?[0-9]*)$", str)
if !match {
return 0, fmt.Errorf("invalid string %s", str)
}
quantity := resource.MustParse(str)
return volumehelpers.RoundUpToGiB(quantity)
}
90 changes: 90 additions & 0 deletions pkg/common/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,3 +577,93 @@ func TestSnapshotStorageLocations(t *testing.T) {
})
}
}

func TestConvertGiBStringToInt64(t *testing.T) {
tests := []struct {
desc string
inputStr string
expInt64 int64
expectError bool
}{
{
"valid number string",
"10000",
1,
false,
},
{
"round Ki to GiB",
"1000Ki",
1,
false,
},
{
"round k to GiB",
"1000k",
1,
false,
},
{
"round Mi to GiB",
"1000Mi",
1,
false,
},
{
"round M to GiB",
"1000M",
1,
false,
},
{
"round G to GiB",
"1000G",
932,
false,
},
{
"round Gi to GiB",
"10000Gi",
10000,
false,
},
{
"round decimal to GiB",
"1.2Gi",
2,
false,
},
{
"round big value to GiB",
"8191Pi",
8588886016,
false,
},
{
"invalid empty string",
"",
10000,
true,
},
{
"invalid string",
"ew%65",
10000,
true,
},
}
for _, tc := range tests {
t.Run(tc.desc, func(t *testing.T) {
actualInt64, err := ConvertGiBStringToInt64(tc.inputStr)
if err != nil && !tc.expectError {
t.Errorf("Got error %v converting string to int64 %s; expect no error", err, tc.inputStr)
}
if err == nil && tc.expectError {
t.Errorf("Got no error converting string to int64 %s; expect an error", tc.inputStr)
}
if err == nil && actualInt64 != tc.expInt64 {
t.Errorf("Got %d for converting string to int64; expect %d", actualInt64, tc.expInt64)
}
})
}
}
15 changes: 8 additions & 7 deletions pkg/gce-cloud-provider/compute/fake-gce.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,13 +196,14 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, project string,
}

computeDisk := &computev1.Disk{
Name: volKey.Name,
SizeGb: common.BytesToGbRoundUp(capBytes),
Description: "Disk created by GCE-PD CSI Driver",
Type: cloud.GetDiskTypeURI(project, volKey, params.DiskType),
SourceDiskId: volumeContentSourceVolumeID,
Status: cloud.mockDiskStatus,
Labels: params.Labels,
Name: volKey.Name,
SizeGb: common.BytesToGbRoundUp(capBytes),
Description: "Disk created by GCE-PD CSI Driver",
Type: cloud.GetDiskTypeURI(project, volKey, params.DiskType),
SourceDiskId: volumeContentSourceVolumeID,
Status: cloud.mockDiskStatus,
Labels: params.Labels,
ProvisionedIops: params.ProvisionedIOPSOnCreate,
}

if snapshotID != "" {
Expand Down
11 changes: 6 additions & 5 deletions pkg/gce-cloud-provider/compute/gce-compute.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,11 +429,12 @@ func (cloud *CloudProvider) insertRegionalDisk(
}

diskToCreate := &computev1.Disk{
Name: volKey.Name,
SizeGb: common.BytesToGbRoundUp(capBytes),
Description: description,
Type: cloud.GetDiskTypeURI(cloud.project, volKey, params.DiskType),
Labels: params.Labels,
Name: volKey.Name,
SizeGb: common.BytesToGbRoundUp(capBytes),
Description: description,
Type: cloud.GetDiskTypeURI(cloud.project, volKey, params.DiskType),
Labels: params.Labels,
ProvisionedIops: params.ProvisionedIOPSOnCreate,
}
if snapshotID != "" {
_, snapshotType, _, err := common.SnapshotIDToProjectKey(snapshotID)
Expand Down
25 changes: 25 additions & 0 deletions pkg/gce-pd-csi-driver/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,31 @@ func TestCreateVolumeArguments(t *testing.T) {
},
expErrCode: codes.InvalidArgument,
},
{
name: "success with provisionedIops parameter",
req: &csi.CreateVolumeRequest{
Name: name,
CapacityRange: stdCapRange,
VolumeCapabilities: stdVolCaps,
Parameters: map[string]string{"labels": "key1=value1,key2=value2", "provisioned-iops-on-create": "10000"},
},
expVol: &csi.Volume{
CapacityBytes: common.GbToBytes(20),
VolumeId: testVolumeID,
VolumeContext: nil,
AccessibleTopology: stdTopology,
},
},
{
name: "fail with malformed provisionedIops parameter",
req: &csi.CreateVolumeRequest{
Name: name,
CapacityRange: stdCapRange,
VolumeCapabilities: stdVolCaps,
Parameters: map[string]string{"labels": "key1=value1,key2=value2", "provisioned-iops-on-create": "dsfo3"},
},
expErrCode: codes.InvalidArgument,
},
}

// Run test cases
Expand Down
Loading