Skip to content
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

add azure disk WriteAccelerator support #87945

Merged
merged 1 commit into from Feb 13, 2020

Conversation

@andyzhangx
Copy link
Member

andyzhangx commented Feb 8, 2020

What type of PR is this?
/kind bug

What this PR does / why we need it:
This PR adds azure disk WriteAccelerator support, added a new parameter(writeAcceleratorEnabled case insensitive) in azure disk storage class, for static provisioning, user needs to add a new tag(writeacceleratorenabled case sensitive) on azure disk before using as disk PV.

kind: StorageClass
apiVersion: storage.k8s.io/v1
metadata:
  name: ssd
provisioner: kubernetes.io/azure-disk
parameters:
  skuname: Premium_LRS
  writeAcceleratorEnabled: "true"
  • background
    For high performance applications, like databases, using Write Accelerator on disks where the database log is stored can improve performance. If customers are already using M or Mv2 VM types, they can use Write Accelerator at no additional costs.

if writeAcceleratorEnabled is enabled on a non-supported VM type, the error would be like following:

  Warning  FailedAttachVolume  5s (x5 over 13s)  attachdetach-controller  AttachVolume.Attach failed for volume "pvc-71e22652-77a3-46f7-a414-e1c3f3e56548" : rpc error: code = Unknown desc = Attach volume "/subscriptions/xxx/resourceGroups/xxx/providers/Microsoft.Compute/disks/pvc-71e22652-77a3-46f7-a414-e1c3f3e56548" to instance "k8s-agentpool-42751077-0" failed with Retriable: false, RetryAfter: 0s, HTTPStatusCode: 400, RawError: Retriable: false, RetryAfter: 0s, HTTPStatusCode: 400, RawError: {
  "error": {
    "code": "InvalidParameter",
    "message": "Write Accelerator is supported with Premium_LRS storage account type only. Please use correct storage account type for disk dataDisk(lun4).",
    "target": "writeAcceleratorEnabled"
  }
}

Which issue(s) this PR fixes:

Fixes #82781

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

add azure disk WriteAccelerator support

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


cc @MSSedusch

/kind bug
/assign @feiskyer
/priority important-soon
/sig cloud-provider
/area provider/azure

@andyzhangx

This comment has been minimized.

Copy link
Member Author

andyzhangx commented Feb 8, 2020

/test pull-kubernetes-e2e-kind-ipv6

Copy link
Member

feiskyer left a comment

/lgtm
/approve
/retest

@k8s-ci-robot k8s-ci-robot added the lgtm label Feb 8, 2020
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Feb 8, 2020

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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

@andyzhangx

This comment has been minimized.

Copy link
Member Author

andyzhangx commented Feb 8, 2020

/hold
hold a while for @MSSedusch to take a look

@andyzhangx

This comment has been minimized.

Copy link
Member Author

andyzhangx commented Feb 9, 2020

/test pull-kubernetes-e2e-gce

@andyzhangx

This comment has been minimized.

Copy link
Member Author

andyzhangx commented Feb 10, 2020

/hold cancel

@MSSedusch

This comment has been minimized.

Copy link

MSSedusch commented Feb 10, 2020

I am testing it right now. Not sure how long it will take me but if you want to hold on for a few days I will try to run some tests

@andyzhangx

This comment has been minimized.

Copy link
Member Author

andyzhangx commented Feb 10, 2020

I am testing it right now. Not sure how long it will take me but if you want to hold on for a few days I will try to run some tests

thanks, will hold this PR for a few more days
/hold

@andyzhangx

This comment has been minimized.

Copy link
Member Author

andyzhangx commented Feb 10, 2020

@MSSedusch after you provision a new disk with writeAcceleratorEnabled: true in storage class, there will be a new tag in disk: writeacceleratorenabled: true, you could easily find that tag in azure portal.

@MSSedusch

This comment has been minimized.

Copy link

MSSedusch commented Feb 12, 2020

I tested the PR. I saw the disk with the writeAcceleratorEnabled: true tag. The performance tests with dd also showed the improved latency, e.g.

dd if=/dev/zero of=/mount/wadisk/test.dd bs=4k count=50000 oflag=direct

was executed in 21 seconds which is similar to what you would directly see on the host.

Thanks @andyzhangx !

@MSSedusch

This comment has been minimized.

Copy link

MSSedusch commented Feb 12, 2020

Would be great to add this new property to the Azure Storage Class documentation as well

@aramase

This comment has been minimized.

Copy link
Member

aramase commented Feb 12, 2020

/cc

@k8s-ci-robot k8s-ci-robot requested a review from aramase Feb 12, 2020
@andyzhangx

This comment has been minimized.

Copy link
Member Author

andyzhangx commented Feb 13, 2020

/hold cancel
@MSSedusch do you want this fix in 1.17.x?

@k8s-ci-robot k8s-ci-robot merged commit 177506d into kubernetes:master Feb 13, 2020
23 checks passed
23 checks passed
cla/linuxfoundation andyzhangx authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-aks-engine-azure Job succeeded.
Details
pull-kubernetes-e2e-azure-disk Job succeeded.
Details
pull-kubernetes-e2e-azure-disk-vmss Job succeeded.
Details
pull-kubernetes-e2e-azure-file Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-storage-slow Job succeeded.
Details
pull-kubernetes-e2e-gce-storage-snapshot Job succeeded.
Details
pull-kubernetes-e2e-kind Job succeeded.
Details
pull-kubernetes-e2e-kind-ipv6 Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Feb 13, 2020
@MSSedusch

This comment has been minimized.

Copy link

MSSedusch commented Feb 13, 2020

for now, 1.18 is fine. we will discuss it internally and if this feature is required in 1.17, I'll let this PR know. :)
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.