From 165dffdf92e30d99a64bf5c6c05506ab44aa89a1 Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Wed, 14 Oct 2026 16:16:42 +0100 Subject: [PATCH] feat(arc): let a profile name tolerations, and count the nodes they let its pods use A node can carry a taint that keeps other workloads off it while the profile's runner pods are meant to run there. Measured sizing took runner pods to tolerate nothing, so it left such a node out of maxRunners even though the pods could be placed on it. The profile now takes tolerations, validated like burst's, placed on the runner pods ahead of burst's, and used by the eligibility check: a node whose only repelling taints they match counts, and a taint they do not match still excludes it. --- plugins/filter/arc.py | 56 ++++++++++++++----- roles/github_runner_arc/README.md | 1 + .../github_runner_arc/meta/argument_specs.yml | 3 +- .../tasks/measure_sizing.yml | 3 +- tests/unit/test_arc_filters.py | 35 ++++++++++++ 5 files changed, 82 insertions(+), 16 deletions(-) diff --git a/plugins/filter/arc.py b/plugins/filter/arc.py index 3a82372..8b7cbd0 100644 --- a/plugins/filter/arc.py +++ b/plugins/filter/arc.py @@ -265,8 +265,35 @@ def _pod_requests(pod: Mapping[str, Any]) -> tuple[Decimal, Decimal]: return totals[0], totals[1] -def _node_eligibility(node: Mapping[str, Any], node_selector: Mapping[str, Any]) -> tuple[str, str]: - """Return why runner pods can never be placed on node (empty when they can), and any passing condition that keeps them off it for now.""" +def _tolerations(value: Any, where: str, errors: list[str]) -> list[dict[str, Any]]: + """Return a list of Kubernetes tolerations as plain dicts, or record why value is not one and return an empty list.""" + tolerations = value or [] + if not isinstance(tolerations, Sequence) or isinstance(tolerations, (str, bytes)) or not all(isinstance(toleration, Mapping) for toleration in tolerations): + errors.append(f"{where} must be a list of Kubernetes tolerations") + return [] + for index, toleration in enumerate(tolerations): + errors.extend(f"{where}[{index}] has an unknown key '{key}'" for key in sorted(set(toleration) - _TOLERATION_KEYS)) + return [dict(toleration) for toleration in tolerations] + + +def _tolerates(tolerations: Sequence[Mapping[str, Any]], taint: Mapping[str, Any]) -> bool: + """Whether one of tolerations matches taint, by the rules of core/v1 Toleration: an empty key with operator Exists matches every taint; otherwise the keys are equal, an empty effect matches any effect, and operator Equal (the default) also needs equal values while Exists ignores them.""" + for toleration in tolerations: + key = toleration.get("key", "") + operator = toleration.get("operator") or "Equal" + if key == "" and operator == "Exists": + return True + if key != taint.get("key"): + continue + if toleration.get("effect") and toleration["effect"] != taint.get("effect"): + continue + if operator == "Exists" or toleration.get("value", "") == taint.get("value", ""): + return True + return False + + +def _node_eligibility(node: Mapping[str, Any], node_selector: Mapping[str, Any], tolerations: Sequence[Mapping[str, Any]] = ()) -> tuple[str, str]: + """Return why runner pods can never be placed on node (empty when they can), and any passing condition that keeps them off it for now. A taint that one of tolerations matches does not keep them off.""" labels = (node.get("metadata") or {}).get("labels") or {} for key, value in node_selector.items(): if labels.get(key) != str(value): @@ -274,7 +301,7 @@ def _node_eligibility(node: Mapping[str, Any], node_selector: Mapping[str, Any]) spec = node.get("spec") or {} conditions = ["is cordoned"] if spec.get("unschedulable") else [] for taint in spec.get("taints") or []: - if taint.get("effect") not in _REPELLING_TAINT_EFFECTS: + if taint.get("effect") not in _REPELLING_TAINT_EFFECTS or _tolerates(tolerations, taint): continue description = f"has the taint {taint.get('key')}:{taint.get('effect')}" if not str(taint.get("key", "")).startswith(_CONDITION_TAINT_PREFIX): @@ -283,7 +310,7 @@ def _node_eligibility(node: Mapping[str, Any], node_selector: Mapping[str, Any]) return "", ", ".join(conditions) -def arc_measured_max_runners(sizing: Mapping[str, Any], nodes: Sequence[Mapping[str, Any]], pods: Sequence[Mapping[str, Any]], node_selector: Mapping[str, Any] | None = None, excluded_namespaces: Sequence[str] = ()) -> dict[str, Any]: +def arc_measured_max_runners(sizing: Mapping[str, Any], nodes: Sequence[Mapping[str, Any]], pods: Sequence[Mapping[str, Any]], node_selector: Mapping[str, Any] | None = None, excluded_namespaces: Sequence[str] = (), tolerations: Sequence[Mapping[str, Any]] = ()) -> dict[str, Any]: """Derive how many runner pods fit on the eligible nodes as they are now. A node is eligible when it carries every label in node_selector and has no NoSchedule or NoExecute taint other than the ones Kubernetes adds for a passing condition (not ready, unreachable, cordoned and the like): the ceiling lasts until the next run, so a node that is only briefly unavailable still counts. For each one, its allocatable CPU and memory, less what the pods already on it request and less the sizing's reserve, is divided by one runner pod's requests; the tighter bound is how many fit there. The sum over the nodes, scaled down by the safety margin, is the result. Pods in excluded_namespaces are not counted, so the runner pods themselves never lower the ceiling they are sized by, and pods that have finished are not counted, as the scheduler does not count them. @@ -294,6 +321,7 @@ def arc_measured_max_runners(sizing: Mapping[str, Any], nodes: Sequence[Mapping[ pods: Pod objects from every namespace. node_selector: labels an eligible node must carry. excluded_namespaces: namespaces whose pods are left out of each node's requests. + tolerations: the runner pods' tolerations; a node whose only repelling taints they match is eligible, as it is for the pods. Returns: A dict with ``errors`` (messages, empty when it could be worked out), ``nodes`` (per eligible node: name, allocatable_cpu, allocatable_memory_gib, requested_cpu, requested_memory_gib, by_cpu, by_memory, runners, and condition, naming any passing condition it was counted despite), ``skipped_nodes`` (name and reason for each node that is not eligible), ``theoretical`` and ``max_runners``, which is None when there are errors. @@ -328,7 +356,7 @@ def arc_measured_max_runners(sizing: Mapping[str, Any], nodes: Sequence[Mapping[ skipped: list[dict[str, str]] = [] for node in sorted(nodes, key=lambda item: (item.get("metadata") or {}).get("name", "")): name = (node.get("metadata") or {}).get("name", "") - reason, condition = _node_eligibility(node, selector) + reason, condition = _node_eligibility(node, selector, tolerations) if reason: skipped.append({"name": name, "reason": reason}) continue @@ -445,12 +473,7 @@ def _burst(profile: Mapping[str, Any], where: str, errors: list[str]) -> dict[st if not isinstance(label_values, Sequence) or isinstance(label_values, (str, bytes)) or not all(isinstance(value, str) and value for value in label_values): errors.append(f"{where}.burst.node_label_values must be a list of non-empty strings (empty or unset accepts any value)") label_values = [] - tolerations = burst.get("tolerations") or [] - if not isinstance(tolerations, Sequence) or isinstance(tolerations, (str, bytes)) or not all(isinstance(toleration, Mapping) for toleration in tolerations): - errors.append(f"{where}.burst.tolerations must be a list of Kubernetes tolerations") - tolerations = [] - for toleration_index, toleration in enumerate(tolerations): - errors.extend(f"{where}.burst.tolerations[{toleration_index}] has an unknown key '{key}'" for key in sorted(set(toleration) - _TOLERATION_KEYS)) + tolerations = _tolerations(burst.get("tolerations"), f"{where}.burst.tolerations", errors) max_runners = _decimal(burst.get("max_runners")) if max_runners is None or max_runners < 0 or max_runners != max_runners.to_integral_value(): errors.append(f"{where}.burst.max_runners must be a whole number of at least 0 (got {burst.get('max_runners')!r})") @@ -478,18 +501,22 @@ def arc_runner_placement(profile: Mapping[str, Any], eligibility: Mapping[str, A """ selector = {key: str(value) for key, value in eligibility.items()} burst = profile["burst"] + own_tolerations = profile["tolerations"] if burst is None: - return {"nodeSelector": {**selector, **profile["node_selector"]}} + placement: dict[str, Any] = {"nodeSelector": {**selector, **profile["node_selector"]}} + if own_tolerations: + placement["tolerations"] = own_tolerations + return placement burst_node = {"key": burst["node_label_key"], "operator": "In", "values": burst["node_label_values"]} if burst["node_label_values"] else {"key": burst["node_label_key"], "operator": "Exists"} # An exclusive profile runs on burst nodes only, whatever the eligibility label, so there is no fixed node to prefer. if burst["exclusive"]: - return {"nodeSelector": dict(profile["node_selector"]), "affinity": {"nodeAffinity": {"requiredDuringSchedulingIgnoredDuringExecution": {"nodeSelectorTerms": [{"matchExpressions": [burst_node]}]}}}, "tolerations": burst["tolerations"]} + return {"nodeSelector": dict(profile["node_selector"]), "affinity": {"nodeAffinity": {"requiredDuringSchedulingIgnoredDuringExecution": {"nodeSelectorTerms": [{"matchExpressions": [burst_node]}]}}}, "tolerations": own_tolerations + burst["tolerations"]} node_affinity: dict[str, Any] = {"preferredDuringSchedulingIgnoredDuringExecution": [{"weight": _PREFER_FIXED_NODES_WEIGHT, "preference": {"matchExpressions": [{"key": burst["node_label_key"], "operator": "DoesNotExist"}]}}]} # With no eligibility label every untainted node is already allowed, and the tolerations alone add the burst nodes. if selector: eligible_node = [{"key": key, "operator": "In", "values": [value]} for key, value in selector.items()] node_affinity["requiredDuringSchedulingIgnoredDuringExecution"] = {"nodeSelectorTerms": [{"matchExpressions": eligible_node}, {"matchExpressions": [burst_node]}]} - return {"nodeSelector": dict(profile["node_selector"]), "affinity": {"nodeAffinity": node_affinity}, "tolerations": burst["tolerations"]} + return {"nodeSelector": dict(profile["node_selector"]), "affinity": {"nodeAffinity": node_affinity}, "tolerations": own_tolerations + burst["tolerations"]} def _expand_profile(org_name: str, app_secret: str, index: int, profile: Any, errors: list[str]) -> dict[str, Any] | None: @@ -545,6 +572,7 @@ def _expand_profile(org_name: str, app_secret: str, index: int, profile: Any, er "node_selector": dict(node_selector), "autoscale": autoscale, "sizing": sizing_result, + "tolerations": _tolerations(profile.get("tolerations"), f"{where}.tolerations", errors), "burst": burst, "burst_runners": burst_runners, "max_runners": max_runners, diff --git a/roles/github_runner_arc/README.md b/roles/github_runner_arc/README.md index bee6192..ab5c730 100644 --- a/roles/github_runner_arc/README.md +++ b/roles/github_runner_arc/README.md @@ -20,6 +20,7 @@ Before touching the cluster the role then checks the secrets it is about to writ - `max_runners`: the release's `maxRunners`. It takes precedence over the values file and over `sizing`; on the autoscaled profile it is the floor the autoscaler starts from. - `runs_on_label`: the extra `scaleSetLabels` entry the runners carry. Default `-runners`, which pools every profile of the org under one `runs-on` label; set a different one to keep a profile out of that pool. - `scale_set_labels`: the whole `scaleSetLabels` list, in place of `runs_on_label`. An empty list sets no `scaleSetLabels`, so jobs target the scale set by its release name. + - `tolerations`: Kubernetes tolerations for the profile's runner pods, for nodes that carry both the eligibility label and a `NoSchedule` or `NoExecute` taint that keeps other workloads off them, such as machines the controller and listener must not run on. Measured sizing counts a node whose only repelling taints these tolerations match; without them it leaves a tainted node out, because it takes runner pods to tolerate nothing. A taint Kubernetes adds for a passing condition (not ready, unreachable, cordoned) still counts as before. With `burst` they come before `burst.tolerations`. - `autoscale`: `true` on at most one profile in the whole inventory makes it the scale set the autoscaler manages. - `sizing`: derive a runner ceiling from node capacity, given as uniform figures or measured from each eligible node (see Sizing). On an ordinary profile without `max_runners` it sets `maxRunners`; on the autoscaled profile it sets the autoscaler's ceiling, leaving `maxRunners` as the floor. - `capabilities`: names of entries in `github_runner_arc_capabilities` that the profile's runner pods carry, in order (see [Capabilities](#capabilities)). diff --git a/roles/github_runner_arc/meta/argument_specs.yml b/roles/github_runner_arc/meta/argument_specs.yml index 305a519..492e4b9 100644 --- a/roles/github_runner_arc/meta/argument_specs.yml +++ b/roles/github_runner_arc/meta/argument_specs.yml @@ -10,9 +10,10 @@ argument_specs: default: [] description: - Orgs whose runner scale sets this host installs. Each entry has name, app_id (needed only when the role writes the App Secret; otherwise the existing Secret supplies it), image, private_key or private_key_op_reference, and optionally installation_id and app_secret_name (the App Secret's name, default -github-app). - - Each entry's scale_set_profiles is a non-empty list of profiles, each with suffix and optionally namespace (default arc-runners-), release_name (default -runners), values_file, max_runners, min_runners, container_mode, resources, node_selector, runs_on_label, scale_set_labels, capabilities, autoscale, sizing and burst. + - Each entry's scale_set_profiles is a non-empty list of profiles, each with suffix and optionally namespace (default arc-runners-), release_name (default -runners), values_file, max_runners, min_runners, container_mode, resources, node_selector, runs_on_label, scale_set_labels, capabilities, tolerations, autoscale, sizing and burst. - max_runners, when set, is the release's maxRunners and takes precedence over its values file and its sizing. - scale_set_labels replaces runs_on_label with the whole list of scaleSetLabels; an empty list sets none, so jobs target the release name. + - tolerations is a list of Kubernetes tolerations for the profile's runner pods, for nodes the eligibility label selects that also carry a taint to keep other workloads off. Measured sizing counts a node whose only repelling taints these match, as the pods can be placed there; without them it leaves such a node out. With burst they come before burst.tolerations. - sizing derives a runner ceiling from uniform node figures, or with measured true from each eligible node's allocatable CPU and memory less its current requests and reserve_cpu and reserve_memory_gib. - capabilities names entries of github_runner_arc_capabilities that the profile's runner pods carry, in order. - burst, on a profile with sizing and without max_runners or autoscale, lets its runner pods overflow onto nodes carrying burst.node_label_key (any value, or one of burst.node_label_values), with burst.tolerations for their taint, and adds burst.max_runners to the sized maxRunners. With burst.exclusive true (and no sizing, max_runners or autoscale) the pods run on burst nodes only and burst.max_runners is the whole maxRunners. diff --git a/roles/github_runner_arc/tasks/measure_sizing.yml b/roles/github_runner_arc/tasks/measure_sizing.yml index f9873f4..a7fe3a3 100644 --- a/roles/github_runner_arc/tasks/measure_sizing.yml +++ b/roles/github_runner_arc/tasks/measure_sizing.yml @@ -26,7 +26,8 @@ github_runner_arc_measure_pods.resources, ({github_runner_arc_node_label_key: github_runner_arc_node_label_value | string} if github_runner_arc_node_label_key | length > 0 else {}) | combine(github_runner_arc_measure.node_selector), - github_runner_arc_fleet.profiles | map(attribute='namespace') | list)}) + github_runner_arc_fleet.profiles | map(attribute='namespace') | list, + github_runner_arc_measure.tolerations)}) }} - name: Fail when the runner ceiling cannot be measured for {{ github_runner_arc_measure.label }} diff --git a/tests/unit/test_arc_filters.py b/tests/unit/test_arc_filters.py index f12f82f..fdea2f7 100644 --- a/tests/unit/test_arc_filters.py +++ b/tests/unit/test_arc_filters.py @@ -343,6 +343,29 @@ def test_invalid_input_is_reported(self) -> None: bad = arc.arc_measured_max_runners(MEASURED, [node("a", cpu="lots")], []) self.assertEqual(bad["errors"], ["node a: not a Kubernetes quantity: 'lots'"]) + def test_a_node_whose_taint_the_profile_tolerates_counts_and_one_it_does_not_still_does_not(self) -> None: + nodes = [ + node("home", labels={"ci": "true"}, taints=[{"key": "example.com/ci-runner-only", "value": "true", "effect": "NoSchedule"}]), + node("other", labels={"ci": "true"}, taints=[{"key": "example.com/dedicated", "effect": "NoSchedule"}]), + node("plain", labels={"ci": "true"}), + ] + tolerations = [{"key": "example.com/ci-runner-only", "operator": "Exists", "effect": "NoSchedule"}] + without = arc.arc_measured_max_runners(MEASURED, nodes, [], {"ci": "true"}) + self.assertEqual([entry["name"] for entry in without["nodes"]], ["plain"]) + result = arc.arc_measured_max_runners(MEASURED, nodes, [], {"ci": "true"}, (), tolerations) + self.assertEqual([entry["name"] for entry in result["nodes"]], ["home", "plain"]) + self.assertEqual([entry["name"] for entry in result["skipped_nodes"]], ["other"]) + + def test_a_toleration_matches_a_taint_by_the_rules_of_a_kubernetes_toleration(self) -> None: + taint = {"key": "k", "value": "v", "effect": "NoSchedule"} + self.assertTrue(arc._tolerates([{"key": "k", "operator": "Exists"}], taint)) + self.assertTrue(arc._tolerates([{"key": "k", "value": "v"}], taint)) + self.assertTrue(arc._tolerates([{"operator": "Exists"}], taint)) + self.assertTrue(arc._tolerates([{"key": "k", "operator": "Exists", "effect": "NoSchedule"}], taint)) + self.assertFalse(arc._tolerates([{"key": "k", "value": "other"}], taint)) + self.assertFalse(arc._tolerates([{"key": "k", "operator": "Exists", "effect": "NoExecute"}], taint)) + self.assertFalse(arc._tolerates([{"key": "other", "operator": "Exists"}], taint)) + self.assertFalse(arc._tolerates([], taint)) BURST = {"node_label_key": "example.com/ci-burst", "tolerations": [{"key": "example.com/ci-burst", "operator": "Exists", "effect": "NoSchedule"}], "max_runners": 6} @@ -442,6 +465,18 @@ def test_burst_without_an_eligibility_label_only_adds_the_tolerations_and_prefer self.assertEqual(placement["tolerations"], BURST["tolerations"]) + def test_a_profile_s_own_tolerations_reach_the_runner_pods_with_or_without_burst(self) -> None: + own = [{"key": "example.com/ci-runner-only", "operator": "Exists", "effect": "NoSchedule"}] + self.assertEqual(self.placement({"tolerations": own}, self.ELIGIBLE)["tolerations"], own) + self.assertEqual(self.placement({"tolerations": own, "burst": BURST}, self.ELIGIBLE)["tolerations"], own + BURST["tolerations"]) + self.assertNotIn("tolerations", self.placement({}, self.ELIGIBLE)) + + def test_a_malformed_toleration_is_reported_with_its_place(self) -> None: + result = arc.arc_profiles([org(profiles=[{"sizing": SIZING, "tolerations": [{"key": "k", "colour": "red"}]}])]) + self.assertEqual(result["errors"], ["Example scale_set_profiles[0].tolerations[0] has an unknown key 'colour'"]) + self.assertIn("scale_set_profiles[0].tolerations must be a list of Kubernetes tolerations", arc.arc_profiles([org(profiles=[{"sizing": SIZING, "tolerations": "nope"}])])["errors"][0]) + + class ImageRefTest(unittest.TestCase): def test_splits_registry_repository_and_tag(self) -> None: cases = {