fix: allow resizing to min required size for performance plus disks #1950

RomanBednar · 2023-08-29T11:09:42Z

What type of PR is this?
/kind bug
/kind feature

What this PR does / why we need it:

See #1949 for detailed description. This patch would reduce the number of errors users might get when creating performancePlus disks.

Which issue(s) this PR fixes:

Fixes #1949

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
tested upgrade from previous version

Special notes for your reviewer:

Patch verified with:

Create StorageClass:

allowVolumeExpansion: true
apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  name: csi-perfplus
parameters:
  skuname: Premium_LRS
  enablePerformancePlus: "true"
provisioner: disk.csi.azure.com
reclaimPolicy: Delete
volumeBindingMode: Immediate

Create PVC:

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: mypvc-enableperformanceplus
spec:
  accessModes:
  - ReadWriteOnce
  resources:
    requests:
      storage: 5Gi
  storageClassName: "csi-perfplus"

Warning log message:

W0829 10:46:37.457074 1 controllerserver.go:103] using PerformancePlus, increasing requested disk size from 5GiB to 513GiB (minimal size for PerformancePlus feature)

Check disk was created with performancePlus enabled:

$ az disk show --resource-group rbednar-mycluster-01-4c6xn-rg --name `oc get pv/pvc-3795b4cd-c105-462d-9a95-99e6ce35282b -o custom-columns=:.metadata.name --no-headers` | jq '.creationData.performancePlus'
true

Release note:

When `enablePerformancePlus` parameter is set to `"true"` volumes will be automatically resized to 513Gi if requested size was smaller.

andyzhangx · 2023-08-29T14:22:08Z

pkg/azuredisk/controllerserver.go

@@ -98,7 +98,13 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 	capacityBytes := req.GetCapacityRange().GetRequiredBytes()
 	volSizeBytes := int64(capacityBytes)
 	requestGiB := int(volumehelper.RoundUpGiB(volSizeBytes))
+
+	if diskParams.PerformancePlus != nil && *diskParams.PerformancePlus {


if ... && requestGiB < consts.PerformancePlusMinimumDiskSizeGiB

Ah, sure, fixed.

still not fixed?

sorry, now it should be ok

RomanBednar · 2023-09-19T07:40:59Z

/assign @andyzhangx for review

andyzhangx · 2023-09-19T11:08:19Z

pkg/azuredisk/controllerserver.go

 	if requestGiB < consts.MinimumDiskSizeGiB {
+		klog.Infof("increasing requested disk size from %vGiB to %vGiB (minimal disk size)")


append requestGiB, consts.MinimumDiskSizeGiB

andyzhangx · 2023-09-19T11:08:40Z

pkg/azuredisk/controllerserver_v2.go

 	if requestGiB < consts.MinimumDiskSizeGiB {
+		klog.Infof("increasing requested disk size from %vGiB to %vGiB (minimal disk size)")


append requestGiB, consts.MinimumDiskSizeGiB

andyzhangx · 2023-09-22T11:58:00Z

pkg/azuredisk/controllerserver.go

 	if requestGiB < consts.MinimumDiskSizeGiB {
+		klog.Infof("increasing requested disk size from %vGiB to %vGiB (minimal disk size)", requestGiB, consts.PerformancePlusMinimumDiskSizeGiB)


it should be MinimumDiskSizeGiB

andyzhangx · 2023-09-22T11:58:09Z

pkg/azuredisk/controllerserver_v2.go

 	if requestGiB < consts.MinimumDiskSizeGiB {
+		klog.Infof("increasing requested disk size from %vGiB to %vGiB (minimal disk size)", requestGiB, consts.PerformancePlusMinimumDiskSizeGiB)


it should be MinimumDiskSizeGiB

andyzhangx

/lgtm
/retest

k8s-ci-robot · 2023-09-30T04:26:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, RomanBednar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [andyzhangx]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2023-09-30T05:44:42Z

@RomanBednar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-azuredisk-csi-driver-e2e-capz-windows-2022-hostprocess	`6f8c1a2`	link	true	`/test pull-azuredisk-csi-driver-e2e-capz-windows-2022-hostprocess`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

andyzhangx · 2023-09-30T14:05:35Z

/cherrypick release-1.29

k8s-infra-cherrypick-robot · 2023-09-30T14:06:27Z

@andyzhangx: new pull request created: #1986

In response to this:

/cherrypick release-1.29

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

andyzhangx · 2023-09-30T14:07:18Z

/cherrypick release-1.28

k8s-infra-cherrypick-robot · 2023-09-30T14:07:55Z

@andyzhangx: new pull request created: #1987

In response to this:

/cherrypick release-1.28

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

andyzhangx · 2023-09-30T14:08:46Z

/cherrypick release-1.27

k8s-infra-cherrypick-robot · 2023-09-30T14:09:24Z

@andyzhangx: #1950 failed to apply on top of branch "release-1.27":

Applying: allow resizing to min required size for performance plus disks
Using index info to reconstruct a base tree...
M	pkg/azureconstants/azure_constants.go
M	pkg/azuredisk/controllerserver.go
M	pkg/azuredisk/controllerserver_test.go
M	pkg/azuredisk/controllerserver_v2.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/azuredisk/controllerserver_v2.go
Auto-merging pkg/azuredisk/controllerserver_test.go
Auto-merging pkg/azuredisk/controllerserver.go
Auto-merging pkg/azureconstants/azure_constants.go
CONFLICT (content): Merge conflict in pkg/azureconstants/azure_constants.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 allow resizing to min required size for performance plus disks
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-1.27

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 29, 2023

k8s-ci-robot requested review from edreed and ZeroMagic August 29, 2023 11:10

RomanBednar force-pushed the perfplus-disk branch from 4c9cfbe to 95642a9 Compare August 29, 2023 11:20

RomanBednar mentioned this pull request Aug 29, 2023

misleading message for performancePlus disk size #1949

Closed

RomanBednar force-pushed the perfplus-disk branch from 95642a9 to 75c3426 Compare August 29, 2023 12:30

andyzhangx reviewed Aug 29, 2023

View reviewed changes

RomanBednar force-pushed the perfplus-disk branch from 75c3426 to 2ed798d Compare August 30, 2023 08:11

RomanBednar force-pushed the perfplus-disk branch from 2ed798d to 389893e Compare September 19, 2023 08:21

andyzhangx reviewed Sep 19, 2023

View reviewed changes

RomanBednar force-pushed the perfplus-disk branch from 389893e to 72f743b Compare September 22, 2023 08:21

andyzhangx reviewed Sep 22, 2023

View reviewed changes

allow resizing to min required size for performance plus disks

6f8c1a2

RomanBednar force-pushed the perfplus-disk branch from 72f743b to 6f8c1a2 Compare September 26, 2023 09:40

andyzhangx approved these changes Sep 30, 2023

View reviewed changes

k8s-ci-robot assigned andyzhangx Sep 30, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 30, 2023

andyzhangx changed the title ~~allow resizing to min required size for performance plus disks~~ fix: allow resizing to min required size for performance plus disks Sep 30, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 30, 2023

andyzhangx merged commit 582d6ef into kubernetes-sigs:master Sep 30, 2023
10 checks passed

k8s-infra-cherrypick-robot mentioned this pull request Sep 30, 2023

[release-1.29] fix: allow resizing to min required size for performance plus disks #1986

Merged

k8s-infra-cherrypick-robot mentioned this pull request Sep 30, 2023

[release-1.28] fix: allow resizing to min required size for performance plus disks #1987

Merged

This was referenced Oct 11, 2023

STOR-1394: Rebase v1.29.0 for OCP 4.15 openshift/azure-disk-csi-driver#51

Merged

OCPBUGS-17542: allow resizing to min required size for performance plus disks openshift/azure-disk-csi-driver#52

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: allow resizing to min required size for performance plus disks #1950

fix: allow resizing to min required size for performance plus disks #1950

RomanBednar commented Aug 29, 2023

andyzhangx Aug 29, 2023

RomanBednar Aug 30, 2023

andyzhangx Sep 19, 2023

RomanBednar Sep 19, 2023

RomanBednar commented Sep 19, 2023

andyzhangx Sep 19, 2023

RomanBednar Sep 22, 2023

andyzhangx Sep 19, 2023

RomanBednar Sep 22, 2023

andyzhangx Sep 22, 2023

andyzhangx Sep 22, 2023

andyzhangx left a comment

k8s-ci-robot commented Sep 30, 2023

k8s-ci-robot commented Sep 30, 2023 •

edited

Loading

andyzhangx commented Sep 30, 2023

k8s-infra-cherrypick-robot commented Sep 30, 2023

andyzhangx commented Sep 30, 2023

k8s-infra-cherrypick-robot commented Sep 30, 2023

andyzhangx commented Sep 30, 2023

k8s-infra-cherrypick-robot commented Sep 30, 2023

		if requestGiB < consts.MinimumDiskSizeGiB {
		klog.Infof("increasing requested disk size from %vGiB to %vGiB (minimal disk size)")

fix: allow resizing to min required size for performance plus disks #1950

fix: allow resizing to min required size for performance plus disks #1950

Conversation

RomanBednar commented Aug 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RomanBednar commented Sep 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andyzhangx left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Sep 30, 2023

k8s-ci-robot commented Sep 30, 2023 • edited Loading

andyzhangx commented Sep 30, 2023

k8s-infra-cherrypick-robot commented Sep 30, 2023

andyzhangx commented Sep 30, 2023

k8s-infra-cherrypick-robot commented Sep 30, 2023

andyzhangx commented Sep 30, 2023

k8s-infra-cherrypick-robot commented Sep 30, 2023

k8s-ci-robot commented Sep 30, 2023 •

edited

Loading