Windows disk ID validation range should be increased

kubernetes / mount-utils

Package mount defines an interface to mounting filesystems.

Apache License 2.0

62 stars 33 forks source link

Windows disk ID validation range should be increased #6

Closed mthorson closed 2 years ago

mthorson commented 3 years ago

Hello

I'm reviewing an issue currently where a customer of mine is seeing MountVolume.MountDevice failed for volume "pvc-" : disk number out of range: "104"

In the k8s source code, the disk number is checked for validity by seeing if it's between 0 and 99 https://github.com/kubernetes/mount-utils/blob/master/mount_helper_windows.go#L93

However, the GCP Persistent Disk limits for predefined machine types are 128. https://cloud.google.com/compute/docs/disks/#pdnumberlimits

The k8s documentation also confirms this for google compute engine. https://kubernetes.io/docs/concepts/storage/storage-limits/#dynamic-volume-limits

So I believe the check should be modified to see if the volume ID is in the range of 0-127 instead.

lizhuqi commented 3 years ago

Andy added this limitation because in the past, every disk mount would be assigned a drive name. It is okay to remove such restriction now.

Andy already came out a PR to remove this restriction.

lizhuqi commented 3 years ago

/assign @andyzhangx

lizhuqi commented 3 years ago

PRs are all merged: Master: https://github.com/kubernetes/kubernetes/pull/105673 1.22: https://github.com/kubernetes/kubernetes/pull/105692 1.21: https://github.com/kubernetes/kubernetes/pull/105693 1.20: https://github.com/kubernetes/kubernetes/pull/106171

The fix will be in k8s v1.20.13, v1.21.7, v1.22.4 and v1.23.0-alpha.4 releases.

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

andyzhangx commented 2 years ago

/close

k8s-ci-robot commented 2 years ago

@andyzhangx: Closing this issue.

In response to [this](https://github.com/kubernetes/mount-utils/issues/6#issuecomment-1038757271): >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.