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

WIP: Design of local storage dynamic provisioning #1914

Open
wants to merge 8 commits into
base: master
from

Conversation

@lichuqiang
Copy link
Member

lichuqiang commented Mar 9, 2018

Still in progress, more details need to be added
The change relies on #1857

@msau42 @NicolasT

@ianchakeres

This comment has been minimized.

Copy link
Member

ianchakeres commented Mar 9, 2018

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,

This comment has been minimized.

@ianchakeres

ianchakeres Mar 9, 2018

Member

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.

This comment has been minimized.

@ianchakeres

ianchakeres Mar 9, 2018

Member

/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

This comment has been minimized.

@ianchakeres

ianchakeres Mar 9, 2018

Member

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.

This comment has been minimized.

@ianchakeres

ianchakeres Mar 9, 2018

Member

nit: /need/needs/
nit: /aspects/areas/
nit: /the scenario/dynamic provisioning/


### Phase 3: 1.11 alpha

#### Dynamic provisioning

This comment has been minimized.

@ianchakeres

ianchakeres Mar 9, 2018

Member

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.

This comment has been minimized.

@ianchakeres

ianchakeres Mar 9, 2018

Member

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.

This comment has been minimized.

@ianchakeres

ianchakeres Mar 9, 2018

Member

/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".

This comment has been minimized.

@ianchakeres

ianchakeres Mar 9, 2018

Member

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.

This comment has been minimized.

@msau42

msau42 Mar 10, 2018

Member

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.

This comment has been minimized.

@ianchakeres

ianchakeres Mar 10, 2018

Member

Supporting wildcards or regex in the storageSources listed could provide some additional flexibility to support multiple node's and their individual configurations.

This comment has been minimized.

@lichuqiang

lichuqiang Mar 12, 2018

Author Member

Really appreciate for you guys suggestions, I will look into that.

hostDir: "/mnt/hdds"
local_dynamic:
storageSource:
- "/dev/sdb"

This comment has been minimized.

@ianchakeres

ianchakeres Mar 9, 2018

Member

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.

This comment has been minimized.

@msau42

msau42 Mar 10, 2018

Member

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.

This comment has been minimized.

@msau42

msau42 Mar 10, 2018

Member

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.

This comment has been minimized.

@msau42

msau42 Mar 10, 2018

Member

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

This comment has been minimized.

@msau42

msau42 Mar 10, 2018

Member

It also needs to watch for the provisioner name to match itself.

This comment has been minimized.

@lichuqiang

lichuqiang Mar 12, 2018

Author Member

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

This comment has been minimized.

@msau42

msau42 Mar 10, 2018

Member

Why does it need to update PVC?


2. Check if the Physical volume exists in Volume group exStorageGroup, if not, add it:
```
vgextend exStorageGroup /dev/new

This comment has been minimized.

@msau42

msau42 Mar 10, 2018

Member

Who creates/configures the vg?

This comment has been minimized.

@lichuqiang

lichuqiang Mar 12, 2018

Author Member

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"

This comment has been minimized.

@msau42

msau42 Mar 10, 2018

Member

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".

This comment has been minimized.

@msau42

msau42 Mar 10, 2018

Member

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"

This comment has been minimized.

@msau42

msau42 Mar 10, 2018

Member

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:

This comment has been minimized.

@ianchakeres

ianchakeres Mar 10, 2018

Member

Should storageSource be storageSources, since there is a list below it?

@lichuqiang

This comment has been minimized.

Copy link
Member Author

lichuqiang commented Mar 12, 2018

@ianchakeres Comments on expression addressed, thanks for your patience

@lichuqiang

This comment has been minimized.

Copy link
Member Author

lichuqiang commented Mar 12, 2018

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

@msau42 I hope to manage raid. say pvcreate /dev/md0 and add it to existing VG.
This way, the behavior could be the same, no matter the path passed in is of physical disks or of raids.
But as I don't have enough knowledge on RAID, could you help clarify whether it possible to mix physical disks and raids in a VG?

@lichuqiang

This comment has been minimized.

Copy link
Member Author

lichuqiang commented Mar 12, 2018

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.

Yes, failure recovery is an issue that we haven't considered well in our internal release of HUAWEI.
For example, if we have three disks /dev/sda, /dev/sdb and /dev/sdc in the VG, if /dev/sdc failed, what do you think users would like to see?
Update its capcacity to 0 to inform admin for recovery, or just remove the failed device from the VG, and leave the remaining two disks for provisioning?


##### Backend storage management

Mechanisms to manage local storage sources are varied. They could be LVM, Ceph, or something like that.

This comment has been minimized.

@NicolasT

NicolasT Mar 12, 2018

Why would Ceph be 'local storage'?

`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?)

This comment has been minimized.

@NicolasT

NicolasT Mar 12, 2018

Wouldn't it be much simpler if a cluster operator just deploys multiple DaemonSets, each for a different STORAGE_BACKEND?

This comment has been minimized.

@lichuqiang

lichuqiang Mar 13, 2018

Author Member

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:

This comment has been minimized.

@NicolasT

NicolasT Mar 12, 2018

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.

This comment has been minimized.

@msau42

msau42 Mar 12, 2018

Member

Agree, there are too many ways an admin can configure and manage vgs. It will be hard to support all of them.

This comment has been minimized.

@msau42

msau42 Mar 12, 2018

Member

This will also simplify the issue where all the disks on every node must be named the same.

This comment has been minimized.

@lichuqiang

lichuqiang Mar 13, 2018

Author Member

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

This comment has been minimized.

@lichuqiang

lichuqiang Mar 13, 2018

Author Member

@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

This comment has been minimized.

@NicolasT

NicolasT Mar 12, 2018

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

This comment has been minimized.

@NicolasT
local-slow:
hostDir: "/mnt/hdds"
local_dynamic:
storageSources:

This comment has been minimized.

@NicolasT

NicolasT Mar 12, 2018

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.

This comment has been minimized.

@NicolasT

NicolasT Mar 12, 2018

How does this work for thinly-provisioned VGs?

This comment has been minimized.

@lichuqiang

lichuqiang Mar 13, 2018

Author Member

It becomes complicated if we take overcommit into consideration. I'd like to leave it for future releases
@msau42 What's your opinion?

@lichuqiang

This comment has been minimized.

Copy link
Member Author

lichuqiang commented Sep 10, 2018

How is this faring.. does it work?
Not really, see kubernetes-incubator/external-storage#743 (comment)

@cpfeiffer

This comment has been minimized.

Copy link

cpfeiffer commented Sep 18, 2018

One thing is not clear to me from reading the design document. Our use case is a build cluster for Jenkins jobs, where a workload (job) would be scheduled on any node in the cluster (depending on resource constraints). The workload should get a local persistent volume, which is either already available on the node, or dynamically created. It is not desired to schedule the same workload always on the same node (e.g. when it is already overloaded), just because its local PV lives there.

IOW: the workload should be scheduled to a node by resource constraints and then get a dynamically provisioned local PV on that node, even if another node already has a PV for it.

@msau42

This comment has been minimized.

Copy link
Member

msau42 commented Sep 18, 2018

@cpfeiffer do the pods have anti-affinity for nodes? If so, then this design will handle that. Even if there is a local PV already available on node A, if there is a Pod A already running there and your Pod B has anti-affinity to Pod A, then we will not choose the PV that's already on Node A

@cpfeiffer

This comment has been minimized.

Copy link

cpfeiffer commented Sep 19, 2018

@msau42 No, I mean just Pod A that should be scheduled to either Node A or Node B depending on resource availability. If Pod A stops on Node A (= Jenkins build job has finished), then it should be possible to get Pod A restarted on Node B when the next build is started.

It's not necessary to have the same volume contents (= build artifacts etc.) on Node A and Node B. The container will make sure it gets all that is needed. We just want to achieve that it won't have to retrieve everything again and again, while being able to use fast local storage.

@lichuqiang

This comment has been minimized.

Copy link
Member Author

lichuqiang commented Sep 19, 2018

It is not desired to schedule the same workload always on the same node (e.g. when it is already overloaded), just because its local PV lives there.

It's an storage mechanism issue, but not limited to local pv.
during volume binding, we'll try to find available pv for a pvc first, and will not trigger dynamic provision if there exists available pv, even if its node is already of high load.
And for your case, if you expect pod to be scheduled to node B, make sure there are available pvs on the node as well

@cpfeiffer

This comment has been minimized.

Copy link

cpfeiffer commented Sep 19, 2018

@lichuqiang OK, if the PVs need to be available on all nodes in advance, would the dynamic provisioner be able to create them on all nodes at once?

@lichuqiang

This comment has been minimized.

Copy link
Member Author

lichuqiang commented Sep 19, 2018

With existing static discovery pattern, PV will be created once you expose volumes to the plugin;
And for the upcoming dynamic provisioning pattern, as I described above, we'll not trigger provision if there exists available pv.

So for your case, you can
1. pre-create PVs across the nodes (static discovery pattern makes sense for this)
2. ensure no unbound PVs left in the cluster, and rely on the dynamic plugin to provision on demand.

Forget that, what I described is about the traditional volume binding behavior, that volume controller is responsible for it.
But if you need dynamic provision for local pv, you'll need to enable the volume scheduling feature.
This way, scheduler is responsible for triggering volume binding/provision. And for your case, it will schedule the pod to node B, and trigger volume provision, instead of bound the pvc to the pv on node A.

@msau42

This comment has been minimized.

Copy link
Member

msau42 commented Sep 19, 2018

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

@kfox1111

This comment has been minimized.

Copy link

kfox1111 commented Sep 19, 2018

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.

@msau42

This comment has been minimized.

Copy link
Member

msau42 commented Sep 19, 2018

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.

@kfox1111

This comment has been minimized.

Copy link

kfox1111 commented Sep 19, 2018

ah, I didn't realize it did that yet. thanks for the info. :)

@cpfeiffer

This comment has been minimized.

Copy link

cpfeiffer commented Sep 19, 2018

@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.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Oct 13, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: saad-ali

If they are not already assigned, you can assign the PR to them by writing /assign @saad-ali in a comment when ready.

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

@k8s-ci-robot k8s-ci-robot added size/M and removed size/L labels Oct 13, 2018

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Oct 13, 2018

@lichuqiang: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-community-verify 4ff4283 link /test pull-community-verify

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 lichuqiang changed the title Design of local storage dynamic provisioning WIP: Design of local storage dynamic provisioning Oct 13, 2018

@lichuqiang

This comment has been minimized.

Copy link
Member Author

lichuqiang commented Oct 13, 2018

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.

This comment has been minimized.

@mlmhl

mlmhl Oct 17, 2018

Will all these provisioners list/watch all PV/PVCs? If so, maybe we need to take care of the pressure on apiserver/etcd.

This comment has been minimized.

@lichuqiang

lichuqiang Oct 26, 2018

Author Member

The existing static discovery plugin also behavior like this. So I suppose this acceptable.

@fejta-bot

This comment has been minimized.

Copy link

fejta-bot commented Jan 24, 2019

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@KlavsKlavsen

This comment has been minimized.

Copy link

KlavsKlavsen commented Jan 24, 2019

Hows this progressing.. sure would be nice with automatic LVM provisioning for local storage :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment