Add "auto" provisioning type. #120
Conversation
There was a problem hiding this comment.
Hi!
Many thanks in your interest and investing time into this.
Overall implementation looks okay, though there are some smaller improvements.
I haven't tested it yet.
Please also have a look at the test files, in CI they are failing. There is also supposed to be some integration tests, maybe you can use this to verify the intended behavior.
If you don't mind, would you write some documentation in the README? If not, I could add it later on by myself while testing, just let me know.
Hmm, I have some concerns regarding this side effect. It sounds like it'll become effectively much less deterministic regarding which type is used, especially if the first Pod lands on the ZFS host and prevents other Pods from running. This will lead to issues and questions eventually. In that sense, I don't think naming this method |
|
Since I've merged #126 , you should probably rebase your PR. |
jp39
force-pushed
the
hostpath-auto
branch
3 times, most recently
from
9b0e209
to
d70e9b7
Compare
|
Thanks for your review. I've addressed most of your comments, and I rebased the branch on top of the most recent master. I still need to complete the README file. Also I'm still looking for a good enough name to replace the "auto" type. |
There was a problem hiding this comment.
Thanks for working on it!
Regarding the readme, if applicable I would refer to https://kubernetes.io/docs/tasks/configure-pod-container/assign-pods-nodes-using-node-affinity/#schedule-a-pod-using-preferred-node-affinity to better control the side effect
My expertise in Kubernetes is actually still fairly limited. But after reading this https://kubernetes.io/docs/concepts/storage/persistent-volumes/#access-modes, it looks like pods running on multiple nodes accessing the same volume is not something that is commonly supported anyway. I think it would be reasonable to force the NFS volume source to be used instead of HostPath, whenever the volume claim contains the Then in this case, maybe it would actually make sense to call this mode "Auto". What do you think? |
jp39
force-pushed
the
hostpath-auto
branch
2 times, most recently
from
2f4a71e
to
825b946
Compare
|
You have a point regarding the access modes. |
|
Hi @ccremer, I've added a few words in the README file. Let me know if that suits you. Thanks. |
Automatically use hostpath if pod is on same host as zfs pool. This addresses ccremer#85. When storage class type is set to auto, automatically create a hostpath volume when the scheduler selects the specified node to run the pod, otherwise fallback to using NFS. Note this only works when volumeBindingMode is set to WaitForFirstConsumer in the storage class. Otherwise, when set to Immediate, volumes will be pre-provisioned before the scheduler selects a node for the pod consuming the volume, and options.SelectedNode will be unset.
|
I also fixed lint errors and failing tests. Thanks. |
|
Hi, I've added the changes you suggested in a second commit so you can see more easily what's been changed here. |
Force HostPath to be used preemptively instead of letting the scheduler choose where to place the first pod. Also add support for ReadWriteOncePod.
|
Thanks for your PR! Merged it |
Automatically use hostpath if pod is on same host as zfs pool.
This addresses #85. When storage class type is set to auto, automatically create a hostpath volume when the scheduler selects the specified node to run the pod, otherwise fallback to using NFS.
Note this only works when volumeBindingMode is set to WaitForFirstConsumer in the storage class. Otherwise, when set to Immediate, volumes will be pre-provisioned before the scheduler selects a node for the pod consuming the volume, and options.SelectedNode will be unset.
Note that there could be unintended side-effects if multiple pods using the volume claim are scheduled on different nodes, depending on which pods gets scheduled first. If the first pod gets scheduled on the ZFS host, it will automatically use a hostpath volume and node affinity will be set so that other pods will be prevented from running on other nodes. On the other hand, if the second pod gets scheduled on a node which is not the ZFS host, it will use a NFS volume, and subsequent pods will also use NFS, even if scheduled to run on the ZFS host.