Skip to content

Commit ce759b1

Browse files
committed
fix(arc): leave the runners a pod capacity bounds out of the safety margin
The safety margin covers how far a node's cpu and memory estimate can be off, other workloads on the node growing included. A node's pod capacity is a hard limit, not an estimate, so scaling the runners it bounds by the margin gave away slots the node can serve, two of seven on a pool where two on-prem nodes were capped at two pods each. The runners a node holds because of its pod capacity are now counted in full, and only the rest are scaled by the margin.
1 parent dc855aa commit ce759b1

3 files changed

Lines changed: 19 additions & 2 deletions

File tree

‎plugins/filter/arc.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,12 +398,14 @@ def arc_measured_max_runners(sizing: Mapping[str, Any], nodes: Sequence[Mapping[
398398
if errors:
399399
return {"errors": errors, "nodes": eligible, "skipped_nodes": skipped, "theoretical": None, "max_runners": None}
400400
theoretical = sum(node["runners"] for node in eligible)
401+
# The safety margin covers how far a node's cpu and memory estimate can be off, other workloads on it growing included. A pod capacity is not an estimate: it is a hard limit, so the runners a node holds because of it are counted in full, and only the rest are scaled by the margin.
402+
pod_bound = sum(node["runners"] for node in eligible if node["by_pods"] is not None and node["by_pods"] < min(node["by_cpu"], node["by_memory"]))
401403
return {
402404
"errors": [],
403405
"nodes": eligible,
404406
"skipped_nodes": skipped,
405407
"theoretical": theoretical,
406-
"max_runners": _floor(Decimal(theoretical) * Decimal(str(checked["safety_margin"]))),
408+
"max_runners": pod_bound + _floor(Decimal(theoretical - pod_bound) * Decimal(str(checked["safety_margin"]))),
407409
}
408410

409411

‎roles/github_runner_arc/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ sizing:
7272
safety_margin: 0.8 # optional, default 1
7373
```
7474
75-
That assumes the nodes are alike. Where their free capacity differs a lot, `measured: true` works it out from the cluster instead, each time the role runs. It reads every node the runner pods may be placed on: every node carrying the eligibility label and the profile's `node_selector`, with no `NoSchedule` or `NoExecute` taint other than those Kubernetes adds for a passing condition. The ceiling lasts until the next run, so a node that is not ready, unreachable or cordoned when it is measured still counts, as it will once it is back. For each one it takes the node's allocatable CPU and memory, subtracts what the pods already on it request and the per-node reserve, and divides what is left by one runner pod's requests; the tighter of the two gives the runners that fit there. The sum over the nodes, scaled by the safety margin and rounded down, is the ceiling. Pods in any scale set's namespace are left out, so running runners never lower the ceiling they are sized by, as are pods that have finished. The reserve covers what requests do not show, such as a workload that uses more than it requests. The kubeconfig needs cluster-wide read access to nodes and pods. The role prints each node's figures when it runs. A node's pod capacity also bounds it: the kubelet's `max-pods` (`allocatable.pods`) less the pods already on it that are not runner pods, so a node whose pod limit is set below what its CPU and memory would hold, to keep a node with a slow disk from running more jobs at once than it can serve, is measured at that limit.
75+
That assumes the nodes are alike. Where their free capacity differs a lot, `measured: true` works it out from the cluster instead, each time the role runs. It reads every node the runner pods may be placed on: every node carrying the eligibility label and the profile's `node_selector`, with no `NoSchedule` or `NoExecute` taint other than those Kubernetes adds for a passing condition. The ceiling lasts until the next run, so a node that is not ready, unreachable or cordoned when it is measured still counts, as it will once it is back. For each one it takes the node's allocatable CPU and memory, subtracts what the pods already on it request and the per-node reserve, and divides what is left by one runner pod's requests; the tighter of the two gives the runners that fit there. The sum over the nodes, scaled by the safety margin and rounded down, is the ceiling. Pods in any scale set's namespace are left out, so running runners never lower the ceiling they are sized by, as are pods that have finished. The reserve covers what requests do not show, such as a workload that uses more than it requests. The kubeconfig needs cluster-wide read access to nodes and pods. The role prints each node's figures when it runs. A node's pod capacity also bounds it: the kubelet's `max-pods` (`allocatable.pods`) less the pods already on it that are not runner pods, so a node whose pod limit is set below what its CPU and memory would hold, to keep a node with a slow disk from running more jobs at once than it can serve, is measured at that limit, and the runners it holds because of it are counted in full: the safety margin covers estimates, and a pod capacity is not one.
7676
7777
```yaml
7878
sizing:

‎tests/unit/test_arc_filters.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,21 @@ def test_pods_that_are_not_runners_use_up_a_nodes_pod_capacity(self) -> None:
360360
# Three slots, one held by the kube-system pod; the runner pod is left out, so running jobs never lower the ceiling.
361361
self.assertEqual(result["nodes"][0]["by_pods"], 2)
362362

363+
def test_the_safety_margin_is_not_applied_to_runners_a_pod_capacity_bounds(self) -> None:
364+
sizing = {**MEASURED, "safety_margin": 0.5}
365+
capped = node("capped", "16", "64Gi")
366+
capped["status"]["allocatable"]["pods"] = "2"
367+
result = arc.arc_measured_max_runners(sizing, [capped, node("free", "4", "8Gi")], [])
368+
# capped holds 2 because of its pod capacity, counted in full; free holds 4 by cpu and memory, halved by the margin.
369+
self.assertEqual((result["theoretical"], result["max_runners"]), (6, 2 + 2))
370+
371+
def test_a_pod_capacity_above_what_cpu_and_memory_hold_leaves_the_margin_applied(self) -> None:
372+
sizing = {**MEASURED, "safety_margin": 0.5}
373+
roomy = node("roomy", "4", "8Gi")
374+
roomy["status"]["allocatable"]["pods"] = "110"
375+
result = arc.arc_measured_max_runners(sizing, [roomy], [])
376+
self.assertEqual((result["theoretical"], result["max_runners"]), (4, 2))
377+
363378
def test_a_node_with_no_pod_slots_left_contributes_nothing(self) -> None:
364379
full = node("full", "16", "64Gi")
365380
full["status"]["allocatable"]["pods"] = "1"

0 commit comments

Comments
 (0)