Add slurm node name labeling to k8s nodes for StatefulSet-mode NodeSets for slurm-bridge interoperability - #255
Conversation
|
Thanks for this @arsdragonfly - I'll pull it internally and incorporate the changes. |
|
I'm not totally comfortable with including this change. What we want here is to encode a contract slurm-bridge uses for hybrid nodes. Today this has to be done manually by cluster admins, I'm not sure encoding it inside slurm-operator is the right choice here. The current implementation creates a very contract for the StatefulSet NodeSet - leaking slurm-bridge implementation details into the way the operator works. I do understand the pain point here and would love to find a solution - I wonder if this be done more suitably with an admission controller or some other custom piece of orchestration? |
Not sure why the cluster-admin-tunable label isn't a proper contract already. |
Agreed - but this is a slurm-bridge mapping rather than something related to operator. I'm trying to understand what would be the right approach here - we don't want tight coupling between the two projects without a strong motivation. |
|
@arsdragonfly - can I ask what kind of use case you're trying to unlock here?
|
Yes, that should be a valid scenario.
That's what oversubscribe set to false already supports, yes.
Yes, with particular focus on sane workload-aware scale-down behavior, as stated in the additional context in the beginning. (Scale-up-wise DaemonSet is also doable if you have something like KEDA -> CAPI but its scale-down isn't trivially graceful and workload-aware)
Not sure if that had been a goal at all from other maintainers' perspective. DaemonSet relied on an even more implicit pod hostname - k8s node name matching. |
Publish the effective Slurm node name on Kubernetes Nodes for pinned, non-oversubscribed StatefulSet NodeSets. Track mapping ownership so collisions are rejected and operator-owned labels are cleaned up safely. Signed-off-by: Zheyu Shen <zheyushen@microsoft.com> Changelog: Added - Publish StatefulSet Slurm node names for slurm-bridge mapping.
ee37d5a to
c284b95
Compare
|
I wonder if a simpler solution would be to always set the slurmd node name to the Kubernetes nodeName in this case instead of relying on an override label? This would make StatefulSet slurmd nodes with We could pass the The end result would be StatefulSet nodes with slurmNodeName == kubernetesNodeName. KEDA scheduling should work without any changes, and slurm-bridge should be able to match nodes using the existing flow through nodeName. Does this solution make sense? |
|
It would be doable, though at the cost of mass pod replacement when one flips those flags to that specific combination. |
|
see #264 for the alternative solution |
|
I'm okay with either implementation (#264 is just effectively extending DaemonSet's convention to eligible StatefulSet) |
Summary
Adds slurm node name labeling to k8s nodes for StatefulSet-mode NodeSets for slurm-bridge interoperability. Applicable when pinToNode is true and oversubscription is turned off.
Checklist
CONTRIBUTING.md
and the
Code of Conduct.
Additional Context
An important nuance regarding why StatefulSet-mode NodeSets could be more appealing (even in a slurm-bridge setting), compared to DaemonSet-mode NodeSets, emerges when one considers the scenario of workload-aware scale-in of the k8s cluster itself (think of saving costs on the cloud).
With Karpenter or Cluster Autoscaler, which work via eliminating idle nodes without running pods, StatefulSet-mode NodeSet scale-in is already workload-aware in slurm-operator, after which scale-in would then trivially work in a workload-aware fashion as well. A working example can be found here.
With DaemonSet-mode NodeSet (which, unlike real DaemonSet, is actually also workload-bearing), the decision of "which node to evict during scale-in" no longer has a nice workload-aware k8s-native answer, and one would find oneself needing awkward custom solutions.