[Proposal] Gate-Controlled Scheduling for Cluster Autoscalers Compatibility#4727
[Proposal] Gate-Controlled Scheduling for Cluster Autoscalers Compatibility#4727devzizu wants to merge 26 commits intovolcano-sh:masterfrom
Conversation
|
Welcome @devzizu! It looks like this is your first PR to volcano-sh/volcano 🎉 |
c7f8885 to
a58e67a
Compare
a58e67a to
5f0ed53
Compare
|
@kingeasternsun @hajnalmt Do you have time help @devzizu to take a look at this proposal? |
|
/cc @hajnalmt Sure I am starting to review it 👍 |
There was a problem hiding this comment.
Thank you for this great proposal @devzizu.
I am looking forward to the implementation, great idea and solid design change.
I had mostly minor remarks except the capacity plugin change.
What I see is that the pods that are marked scheduling gated by volcano after this change with the annotation they can maybe considered to be inqueue but straight out to be considered allocated seems a too significant change.
Wouldn’t it be sufficient to adjust the DeductSchGatedResources function so that the tasks that are only volcano scheduling gated won't be deducted when hasOnlyVolcanoSchedulingGate returns true? This approach would also maintain compatibility with the overcommit plugin, which I’m slightly concerned about since it heavily depends on scheduling gate behavior.
--- I want to remark that this would mean that these annotated pods would move to inqueue and would never be in the scope of the enqueue action if every other scheduling gate is removed, which is probably what we want.
Thank you for the contribution and the idea once more!
61ee592 to
1f0df13
Compare
hajnalmt
left a comment
There was a problem hiding this comment.
We are getting there! Keep up the great work 😊
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Great Work!
Please squash your commits, and sign the DCO! I think this is quite ready.
Do you need any help with the implementation? We will need to add an e2e test for this too.
I think the it shall happen in a different PR though as the design is quite big.
03bfa14 to
42003fa
Compare
|
/cc @JesseStutler I think we can pull this in release-1.15 |
42003fa to
e035957
Compare
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Co-authored-by: João Soares <36493199+jmgsoares@users.noreply.github.com> Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
7426729 to
7b8f691
Compare
Signed-off-by: devzizu <jazevedo960@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive design proposal for addressing a critical compatibility issue between Volcano scheduler and cluster autoscalers (CA/Karpenter). The proposal leverages Kubernetes scheduling gates to prevent unnecessary cluster scale-ups when pods are waiting for queue capacity rather than node capacity.
Changes:
- Adds a detailed design document proposing an opt-in feature using scheduling gates to differentiate between queue capacity constraints and cluster capacity constraints
- Introduces queue reservation mechanism to prevent race conditions when ungated pods remain unscheduled
- Proposes extensions to the admission webhook, scheduler actions, and capacity plugin to implement the gate-controlled scheduling
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ```go | ||
| type capacityPlugin struct { | ||
| // ... existing fields ... | ||
|
|
||
| // queueGateReservedTasks tracks tasks that passed capacity checks but cannot be scheduled | ||
| // These tasks reserve queue capacity to prevent other tasks from consuming it | ||
| // Rebuilt fresh at the start of each scheduling cycle in OnSessionOpen | ||
| queueGateReservedTasks map[api.QueueID]map[api.TaskID]*api.TaskInfo | ||
| } | ||
| ``` | ||
|
|
||
| ###### Capacity Check with Reserved Resources | ||
|
|
||
| The `queueAllocatableWithReserved` method performs capacity checks including reserved resources: | ||
|
|
||
| ```go | ||
| func (cp *capacityPlugin) queueAllocatableWithReserved(attr *queueAttr, candidate *api.TaskInfo, queue *api.QueueInfo) bool { |
There was a problem hiding this comment.
The reserved cache accounting could lead to resource starvation in certain scenarios. Consider: if pod-A fails to schedule due to node constraints and gets its gate removed, it reserves queue capacity. If new pods (pod-B, pod-C) are created that could actually schedule, they will be blocked by pod-A's reservation even though pod-A can't make progress until the autoscaler provisions new nodes. The proposal should discuss whether there should be a timeout or mechanism to release reservations for pods that have been ungated but haven't made progress for an extended period, preventing indefinite queue capacity blocking.
There was a problem hiding this comment.
This is actually intended design, but the concern is a valid point: an ungated-but-unschedulable pod can hold queue capacity and block other pods that could schedule. We could add a timeout or similar mechanism to release reservations for pods that stay unschedulable for a long time, but that would need a clear policy and might over-commit the queue when those pods eventually get nodes.
@JesseStutler @hajnalmt – Do you think we should address this in the initial implementation (e.g. document the tradeoff and/or add a reservation timeout), or treat it as follow-up work once we see how it behaves in practice?
There was a problem hiding this comment.
I think this is fine and this is the "standard" way of things happening. I am fine with it.
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
Signed-off-by: devzizu <jazevedo960@gmail.com>
60b9d34 to
2c67c87
Compare
Signed-off-by: devzizu <jazevedo960@gmail.com>
What type of PR is this?
/kind documentation
/kind feature
What this PR does / why we need it:
This design proposal addresses an issue where Volcano incorrectly signals cluster autoscalers (e.g., CA or Karpenter) to scale up nodes even when pods are only waiting for queue capacity, not cluster resources. Currently, Volcano marks all unallocated pods as
Unschedulableregardless of the reason, causing autoscalers to interpret queue constraints as insufficient node capacity.This proposal introduces an opt-in feature using Kubernetes
schedulingGatesto hide queue-constrained pods from autoscalers, ensuring scale-up operations only trigger for legitimate node-fit failures. This aims to prevent unnecessary infrastructure costs and resource waste.Which issue(s) this PR fixes:
Fixes #4710