fix(arc): leave the runners a pod capacity bounds out of the safety margin

plugins/filter/arc.py

@@ -398,12 +398,14 @@ def arc_measured_max_runners(sizing: Mapping[str, Any], nodes: Sequence[Mapping[
     if errors:
         return {"errors": errors, "nodes": eligible, "skipped_nodes": skipped, "theoretical": None, "max_runners": None}
     theoretical = sum(node["runners"] for node in eligible)
+    # 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.
+    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"]))
     return {
         "errors": [],
         "nodes": eligible,
         "skipped_nodes": skipped,
         "theoretical": theoretical,
-        "max_runners": _floor(Decimal(theoretical) * Decimal(str(checked["safety_margin"]))),
+        "max_runners": pod_bound + _floor(Decimal(theoretical - pod_bound) * Decimal(str(checked["safety_margin"]))),
     }
 
 

roles/github_runner_arc/README.md

@@ -72,7 +72,7 @@ sizing:
   safety_margin: 0.8                # optional, default 1
 ```
 
-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.
+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.
 
 ```yaml
 sizing:

tests/unit/test_arc_filters.py

@@ -360,6 +360,21 @@ def test_pods_that_are_not_runners_use_up_a_nodes_pod_capacity(self) -> None:
         # Three slots, one held by the kube-system pod; the runner pod is left out, so running jobs never lower the ceiling.
         self.assertEqual(result["nodes"][0]["by_pods"], 2)
 
+    def test_the_safety_margin_is_not_applied_to_runners_a_pod_capacity_bounds(self) -> None:
+        sizing = {**MEASURED, "safety_margin": 0.5}
+        capped = node("capped", "16", "64Gi")
+        capped["status"]["allocatable"]["pods"] = "2"
+        result = arc.arc_measured_max_runners(sizing, [capped, node("free", "4", "8Gi")], [])
+        # capped holds 2 because of its pod capacity, counted in full; free holds 4 by cpu and memory, halved by the margin.
+        self.assertEqual((result["theoretical"], result["max_runners"]), (6, 2 + 2))
+
+    def test_a_pod_capacity_above_what_cpu_and_memory_hold_leaves_the_margin_applied(self) -> None:
+        sizing = {**MEASURED, "safety_margin": 0.5}
+        roomy = node("roomy", "4", "8Gi")
+        roomy["status"]["allocatable"]["pods"] = "110"
+        result = arc.arc_measured_max_runners(sizing, [roomy], [])
+        self.assertEqual((result["theoretical"], result["max_runners"]), (4, 2))
+
     def test_a_node_with_no_pod_slots_left_contributes_nothing(self) -> None:
         full = node("full", "16", "64Gi")
         full["status"]["allocatable"]["pods"] = "1"