WIP: Design of local storage dynamic provisioning #1914
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-github-robot
added
kind/design
|
FYI: @dhirajh |
| which depends on the newly introduced changes in [Volume Topology-aware Scheduling](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/storage/volume-topology-scheduling.md), | ||
| that is: | ||
|
|
||
| 1. When feature `DynamicProvisioningScheduling` enabled, scheduler would verify a pod's volume requirement, |
There was a problem hiding this comment.
Reword:
When the feature gate DynamicProvisioningScheduling is enabled, the scheduler will verify a pod's volume requirements,
| that is: | ||
|
|
||
| 1. When feature `DynamicProvisioningScheduling` enabled, scheduler would verify a pod's volume requirement, | ||
| and set `annScheduledNode` on PVCs that need provisioning. |
There was a problem hiding this comment.
/and set/and set an annotation/
|
|
||
| 1. When feature `DynamicProvisioningScheduling` enabled, scheduler would verify a pod's volume requirement, | ||
| and set `annScheduledNode` on PVCs that need provisioning. | ||
| 2. New fields would be added in `StorageClass` to allow storage providers to report |
There was a problem hiding this comment.
What new fields need to be added?
| 2. New fields would be added in `StorageClass` to allow storage providers to report | ||
| capacity limits per topology domain. | ||
|
|
||
| The local volume provisioner need to be updated in several aspects to support the scenario. |
There was a problem hiding this comment.
nit: /need/needs/
nit: /aspects/areas/
nit: /the scenario/dynamic provisioning/
|
|
||
| ### Phase 3: 1.11 alpha | ||
|
|
||
| #### Dynamic provisioning |
There was a problem hiding this comment.
Please capitalize all words in headings ###, ####, and #####.
| - "/dev/sdb" | ||
| - "/dev/sdc" | ||
| ``` | ||
| For LVM, its items are disk partitions, and for other mechanisms, they could be other things. |
There was a problem hiding this comment.
For example for an LVM StorageManager, the list of storageSource could contain disk partitions. For other StorageManagers, the list could contain other storage sources (e.g. unpartitioned disks).
| - "/dev/sdc" | ||
| ``` | ||
| For LVM, its items are disk partitions, and for other mechanisms, they could be other things. | ||
| StorageManager should know the scope and what to do with the input. |
There was a problem hiding this comment.
/StorageManager/Each StorageManager/
| * As provisioners running on all nodes shall a same configuration ConfigMap, | ||
| storage sources of their nodes are required to be configured in the same way. | ||
| Take LVM as an example, for configmap above, disks used for provisioning are required | ||
| to be partitioned as "/dev/sdb" and "/dev/sdc". |
There was a problem hiding this comment.
I think this situation could be handled via multiple means.
Cluster operators could leverage multiple dameonset manifests and various selectors. For example, if you had two different types of hardware with a different number of partitions and different partition locations, then you could label the nodes accordingly and deploy two daemonsets where each daemonset config was tailored toward the appropriately labelled nodes.
Alternatively, another earlier process in the node's lifecycle (e.g. nodeprep - https://engineering.salesforce.com/provisioning-kubernetes-local-persistent-volumes-61a82d1d06b0) could perform operations before the lv dynamic provisioner is used, and the node preparation could make nodes fit the common configuration via symlinks or bindmounts.
There was a problem hiding this comment.
Agree, we should support the use case of different number and path to disks on every node, along with disks being dynamically added and removed. Maybe another discovery directory or local file is needed.
There was a problem hiding this comment.
Supporting wildcards or regex in the storageSources listed could provide some additional flexibility to support multiple node's and their individual configurations.
There was a problem hiding this comment.
Really appreciate for you guys suggestions, I will look into that.
| hostDir: "/mnt/hdds" | ||
| local_dynamic: | ||
| storageSource: | ||
| - "/dev/sdb" |
There was a problem hiding this comment.
I recommend that people use filesystems bind mounted by referencing their by-uuid or raw block devices symlinks by referencing their by-id, and that we do something similar in our examples. https://wiki.archlinux.org/index.php/persistent_block_device_naming
This scheme helps prevent some problems that could happen if you access volumes via names that remain static, when the underlying volumes can move.
There was a problem hiding this comment.
Carefully consider the case where a disk might be added to the system and on reboot, the ordering in /dev changes. Would be good to add a workflow detailing adding/removing capacity, and how to replace a failed disk.
There was a problem hiding this comment.
Another use case to consider is configuring raid underneath. We should decide where to draw the line for the provisioner. Should it:
- manage raid? The lvm input could just be the raid device path instead of the physical disks
- manage lvm? The storage source could just take the vg name
|
|
||
| ##### Backend storage management | ||
|
|
||
| Mechanisms to manage local storage sources are varied. They could be LVM, Ceph, or something like that. |
There was a problem hiding this comment.
For Ceph, why can't the Ceph plugin be used?
| ##### Change in populator | ||
|
|
||
| In addition to PV, the `populator` will also be extended to watch PVC objects of all namespaces. | ||
| When it finds that a PVC is newly annotated with `annScheduledNode`, and its node name match the value |
There was a problem hiding this comment.
It also needs to watch for the provisioner name to match itself.
There was a problem hiding this comment.
Oh, you are right, at the first beginning, I thought the annotation of provisioner is set by the provisioner itself, and forgot to correct it here later when I realize the fact that it's set by the controller.
The permissions thing below is for the same reason
|
|
||
| For this purpose, the following permissions are required: | ||
|
|
||
| * update/list/get PersistentVolumeClaims |
|
|
||
| 2. Check if the Physical volume exists in Volume group exStorageGroup, if not, add it: | ||
| ``` | ||
| vgextend exStorageGroup /dev/new |
There was a problem hiding this comment.
Still the storageManager of LVM, when a new source disk comes, it should check if related vg exists, and create one first if not.
The description here is indeed not detailed enough, will have an update
| hostDir: "/mnt/hdds" | ||
| local_dynamic: | ||
| storageSource: | ||
| - "/dev/sdb" |
There was a problem hiding this comment.
Carefully consider the case where a disk might be added to the system and on reboot, the ordering in /dev changes. Would be good to add a workflow detailing adding/removing capacity, and how to replace a failed disk.
| * As provisioners running on all nodes shall a same configuration ConfigMap, | ||
| storage sources of their nodes are required to be configured in the same way. | ||
| Take LVM as an example, for configmap above, disks used for provisioning are required | ||
| to be partitioned as "/dev/sdb" and "/dev/sdc". |
There was a problem hiding this comment.
Agree, we should support the use case of different number and path to disks on every node, along with disks being dynamically added and removed. Maybe another discovery directory or local file is needed.
| hostDir: "/mnt/hdds" | ||
| local_dynamic: | ||
| storageSource: | ||
| - "/dev/sdb" |
There was a problem hiding this comment.
Another use case to consider is configuring raid underneath. We should decide where to draw the line for the provisioner. Should it:
- manage raid? The lvm input could just be the raid device path instead of the physical disks
- manage lvm? The storage source could just take the vg name
| local-slow: | ||
| hostDir: "/mnt/hdds" | ||
| local_dynamic: | ||
| storageSource: |
There was a problem hiding this comment.
Should storageSource be storageSources, since there is a list below it?
|
@ianchakeres Comments on expression addressed, thanks for your patience |
@msau42 I hope to manage raid. say |
Yes, failure recovery is an issue that we haven't considered well in our internal release of HUAWEI. |
|
|
||
| ##### Backend storage management | ||
|
|
||
| Mechanisms to manage local storage sources are varied. They could be LVM, Ceph, or something like that. |
| `storageManager` is a general interface, each kind of storage should have its own implementation. | ||
| During start-up, a new environment variable `STORAGE_BACKEND` is set in the provisioner DaemonSet, | ||
| to determine which StorageManager instance to start. | ||
| (TBD: Should we support the scenario of multi backends on a node?) |
There was a problem hiding this comment.
Wouldn't it be much simpler if a cluster operator just deploys multiple DaemonSets, each for a different STORAGE_BACKEND?
There was a problem hiding this comment.
Yes, I didn't describe it clearly, I mean support multi backends in a same plugin, which seems no need :)
| Take LVM as an example, when a new source disk "/dev/new" is added to its StorageClass `exStorage`, | ||
| the StorageManager should: | ||
|
|
||
| 1. Check if related PV (physical volume in LVM) exists, if not, create one with command: |
There was a problem hiding this comment.
IMHO it's problematic if the provisioner is responsible for this. As mentioned below, disk naming etc could be problematic. Instead, I believe storage pool provisioning is a responsibility of the cluster operator (using e.g. Ansible or whatever), only volume provisioning should be performed by the Kubernetes infrastructure.
As such, in the LVM case: the Kubernetes provider creates and manages LVs in a given and pre-existing VG, an admin creates PVs and VGs using whichever mechanism seems suitable.
There was a problem hiding this comment.
Agree, there are too many ways an admin can configure and manage vgs. It will be hard to support all of them.
There was a problem hiding this comment.
This will also simplify the issue where all the disks on every node must be named the same.
There was a problem hiding this comment.
Seems you guys have a strong intention on this point.
We made it this way in our internal release, for we mainly use it in shared clusters. And we aimed to make things simpler for users/admins, and our use case may be much simpler: only physical disks are used for now, and the machines are created in batch with the same specifications.
Anyway, since we have seen several defects in it as a general solution, I’ll take it as the first choice to manage vgs directly in the doc, and make existing one alternative for more feedback
There was a problem hiding this comment.
@msau42 @NicolasT By the way, this way, do you think it necessary to support a list of VGs, or a unique VG should be enough?
If we have more than one VG, then we might meet some edge-cases, for example:
If we have two VGs on a node, VG1 and VG2, they each has 10G left, then the reported capacity would be 20G, but a pod with storage request of 15G could not be satisfied indeed
|
|
||
| 2. Check if volume group exStorageGroup exists, if not, create it: | ||
| ``` | ||
| vgcreate exStorageGroup /dev/new |
There was a problem hiding this comment.
See above. If the VG doesn't exist, the provisioner should bail out IMHO.
| 3. If VG exists, check if the physical volume exists in volume group exStorageGroup, | ||
| if not, add it: | ||
| ``` | ||
| vgextend exStorageGroup /dev/new |
| local-slow: | ||
| hostDir: "/mnt/hdds" | ||
| local_dynamic: | ||
| storageSources: |
There was a problem hiding this comment.
I'd say only the name of a VG should be specified for things to work. Nothing more required.
| vgextend exStorageGroup /dev/new | ||
| ``` | ||
|
|
||
| 4. Update capacity in StorageClass if there is a change in VG size. |
There was a problem hiding this comment.
How does this work for thinly-provisioned VGs?
There was a problem hiding this comment.
It becomes complicated if we take overcommit into consideration. I'd like to leave it for future releases
@msau42 What's your opinion?
|
Forget that, what I described is about the traditional volume binding behavior, that volume controller is responsible for it. |
|
Hi @cpfeiffer it sounds a little bit like you don't need the persistence part of the feature, ie you want to be sticky to the data. If all you need is ephemeral local storage, then you can use emptydir |
|
hmm.... actually, thinking about it a little bit, while emptyDir seems to be a good fit for @cpfeiffer's stated issue, I think there is one case it doesn't fit. The dynamic provisioning mentioned here can allocate local volumes as needed in order to not over allocate host storage. emptyDir currently doesn't guarantee an amount of storage. Maybe emptydir should be reimplemented on top of local dynamic storage, so that it can reserve an amount of storage for its lifecycle? or alternately, maybe a second, sizedEmptyDir (localEmptyDir?) driver is added that does this instead. |
|
You can request a limit on emptydir size now, and I believe we do soft eviction if you exceed. I think there are plans to support project quota for hard enforcement too. The other use case that @cpfeiffer mentioned was to avoid having to load data into the volume everytime, but still have the flexibility to schedule on different nodes. In that case, then hostpath may be the simplest solution here. |
|
ah, I didn't realize it did that yet. thanks for the info. :) |
|
@msau42 emptyDir doesn't fit us because we want to keep the contents for longer than just a single run. And indeed hostpath is the best we figured out so far. I just thought that dynamic provisioning might save us from manually provisioning the host volumes. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: If they are not already assigned, you can assign the PR to them by writing 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 |
k8s-ci-robot
added
size/M
|
@lichuqiang: The following test failed, say
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. |
lichuqiang
changed the title
k8s-ci-robot
added
the
do-not-merge/work-in-progress
|
Update to list the thoughts inflight, to avoid readers being misled by the old information. It's far from a complete deisgn, but more of rough thoughts on which we hope to receive feedback from readers. |
|
|
||
| ##### CSI Driver Details | ||
|
|
||
| Different from other storages, we'll need to deploy a set of CSI provisioner on each of the nodes. We'll rely on feature volumeScheduling as a dependency. For local storage, the provisioner is only responsible for PVCs marked with "related plugin name + selected node annotation". That is, it only privision the pvcs that "scheduled" to its node. |
There was a problem hiding this comment.
Will all these provisioners list/watch all PV/PVCs? If so, maybe we need to take care of the pressure on apiserver/etcd.
There was a problem hiding this comment.
The existing static discovery plugin also behavior like this. So I suppose this acceptable.
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
k8s-ci-robot
added
the
lifecycle/stale
|
Hows this progressing.. sure would be nice with automatic LVM provisioning for local storage :) |
|
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
k8s-ci-robot
added
lifecycle/rotten
|
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
|
@fejta-bot: Closed this PR. In response to this:
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. |
Still in progress, more details need to be added
The change relies on #1857
@msau42 @NicolasT