linode / linode-blockstorage-csi-driver

Container Storage Interface (CSI) Driver for Linode Block Storage
Apache License 2.0
64 stars 54 forks source link

LKE-3931: Set maxVolumesPerNode to 7 #152

Closed rsienko closed 7 months ago

rsienko commented 8 months ago

This is a quick fix for the default case when we have 8 slots and one slot is already taken by boot disk. There may be more slots in use by non-PVC volumes such as swap but they are not the default in LKE.

fixes #154

General:

Pull Request Guidelines:

  1. [x] Does your submission pass tests?
  2. [x] Have you added tests?
  3. [x] Are you addressing a single feature in this PR?
  4. [x] Are your commits atomic, addressing one change per commit?
  5. [x] Are you following the conventions of the language?
  6. [x] Have you saved your large formatting changes for a different PR, so we can focus on your work?
  7. [x] Have you explained your rationale for why this feature is needed?
  8. [x] Have you linked your PR to an open issue
luthermonson commented 8 months ago

@kitknox i think you should weigh in on this, there is some logic he suggested we could use to check the disks on the node and make this limit more dynamic. this is probably the quick fix though and we could go with this and ticket the better fix if its too much of a time sink today

kitknox commented 8 months ago

In the current situation where the API attempts to enforce a limit of 8 block devices, this is an improvement. The downside is that work is planned to improve that to align with the host enforced scalable limit at which point this will be worse. Given workloads are already impacted by the current oversubscription issue there is value in getting this out there but we need to ensure we come back to this quickly when the upstream API shifts.