diff --git a/.github/workflows/autoscaler-ceiling.yml b/.github/workflows/autoscaler-ceiling.yml index db94878d..c389cc40 100644 --- a/.github/workflows/autoscaler-ceiling.yml +++ b/.github/workflows/autoscaler-ceiling.yml @@ -9,6 +9,7 @@ on: paths: - scripts/autoscaler.sh - scripts/autoscaler-ceiling.jq + - scripts/autoscaler-node.jq - roles/github_runner_arc/tasks/install_platform.yml - plugins/filter/arc.py - tests/autoscaler_ceiling/** diff --git a/README.md b/README.md index 5347f04f..b1016639 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,7 @@ A scale set's static `maxRunners` has to be safe for the pool's real worst case, Each poll (`AUTOSCALER_POLL_SECONDS`, default 45s): - For every pooled profile, counts its running runners from its `EphemeralRunner` resources (every one that has not finished: a phase other than `Succeeded` or `Failed`, so a pending runner counts, because its pod already holds a slot) and reads its per-pod hard memory limit (`spec.template.spec.containers[0].resources.limits.memory`) straight from its live `AutoscalingRunnerSet`, never a static config value, so both always match what is actually deployed. The running count is not read from the `AutoscalingRunnerSet`'s status: in the ARC release this role pins (0.15) that status carries only `phase` and `observedGeneration`, so a count read from it would always be empty. A failed read of the runners stops the poll instead of counting zero. Sums the running counts into one combined total, and takes the largest of the pod memory limits as the threshold a raise or lower must leave room for. - Sums real, current memory usage of every pooled profile's running runner pods, from the metrics API's pod list for each profile's own namespace (`scripts/autoscaler-usage.jq`); this, not a static per-pod request or limit, is the entire point of "usage-driven". It does not use `kubectl top pod`, which fails a whole namespace while any pod in it has no metrics, as every new runner pod has none until metrics-server first samples it. A runner pod placed on a node, not finished and missing from the list is unmeasured, and assumed to use its own memory limit (its scale set's limit when it sets none), the most it can reach. Lowering is decided on the measured usage, so a new pod never lowers the pool by itself; a raise also needs a pod's room with every unmeasured pod at its limit, and the pool holds until they are measured otherwise. -- Reads cluster-wide memory-availability pressure via `kubectl top nodes`, taking the **worst-case (minimum)** available-memory percentage across the nodes runner pods may use, not an average — a pool average can look healthy while the specific node a new runner pod would actually land on is not, consistent with the algorithm's own bias toward lowering eagerly (lowering never disrupts in-flight jobs). With the node eligibility label set (`github_runner_arc_node_label_key`, passed as `AUTOSCALER_NODE_SELECTOR`) only nodes carrying it are read, otherwise every node: a node outside the label runs no runner pod, so its pressure says nothing about where the next one lands, and when its kubelet drops out its missing metrics would otherwise fail safe on every poll. This is a genuine improvement over the pod's own earlier host-pinned design, which could only ever see one fixed machine's pressure via `/proc/meminfo`, not the whole pool's. **No swap-pressure signal**: metrics-server's API has no swap field at all, and the only alternative (the kubelet's own Summary API) needs a materially broader RBAC grant for a signal most kubelet versions don't even surface unless an off-by-default feature gate is on — dropping swap detection is a deliberate, documented trade-off once the pod is genuinely unpinned to any node, not an oversight; memory-availability pressure remains the dominant signal (the incident the values file's own comments were tuned from). +- Reads cluster-wide memory-availability pressure via `kubectl top nodes`, taking the **worst-case (minimum)** available-memory percentage across the nodes runner pods may use, not an average — a pool average can look healthy while the specific node a new runner pod would actually land on is not, consistent with the algorithm's own bias toward lowering eagerly (lowering never disrupts in-flight jobs). With the node eligibility label set (`github_runner_arc_node_label_key`, passed as `AUTOSCALER_NODE_SELECTOR`) only nodes carrying it are read, otherwise every node: a node outside the label runs no runner pod, so its pressure says nothing about where the next one lands, and when its kubelet drops out its missing metrics would otherwise fail safe on every poll. The same holds for a node inside the label that is not `Ready` (its kubelet stopped reporting or reported a fault): it is left out of this check by the definition the ceiling uses (`scripts/autoscaler-node.jq`), since it places no runner pod and `kubectl top nodes` prints `` for it. This is a genuine improvement over the pod's own earlier host-pinned design, which could only ever see one fixed machine's pressure via `/proc/meminfo`, not the whole pool's. **No swap-pressure signal**: metrics-server's API has no swap field at all, and the only alternative (the kubelet's own Summary API) needs a materially broader RBAC grant for a signal most kubelet versions don't even surface unless an off-by-default feature gate is on — dropping swap detection is a deliberate, documented trade-off once the pod is genuinely unpinned to any node, not an oversight; memory-availability pressure remains the dominant signal (the incident the values file's own comments were tuned from). - Works out the ceiling, how many pooled runner pods the nodes can place now, from the live cluster (`scripts/autoscaler-ceiling.jq`). It reads every pooled profile's runner pod spec from its `AutoscalingRunnerSet` and takes the largest request among them, per resource, as the runner pod (a container that sets only a limit counts at its limit, as it will once its pod is created, and sidecars, init containers and pod overhead count as the scheduler counts them). A node counts when every pooled profile's runner pods could be placed on it now: it carries their `nodeSelector`, is `Ready`, is not cordoned, and their tolerations match each of its `NoSchedule` and `NoExecute` taints; a node that is not ready or unreachable is left out until it is back. On each, allocatable CPU and memory less the requests of the pods on it that are not pooled runner pods, divided by the runner pod's request, and its pod capacity (`allocatable.pods`) less those pods, give how many fit; the smallest of the three, rounded down, is the node's figure, and the ceiling is the sum. The pooled runner pods are left out so that runners already running never lower the ceiling they count towards; every other pod still holding requests counts, a non-pooled scale set's runners included. Node affinity is not read: the role gives an autoscaled profile none, and a pod one would keep off a counted node is caught as unschedulable below. `AUTOSCALER_MAX_CEILING` (`github_runner_arc_autoscaler_max_ceiling`, or, when exactly one pooled profile sets `sizing`, that profile's own derived figure) is an optional upper bound on it; empty leaves it unbounded. The ceiling worked out is kept in the status ConfigMap's `ceiling` key, and `status.json` reports `ceiling`, `ceiling_bound`, `ceiling_measured` and, per node, the runners it places or why it is not counted. When the nodes, the pods or a template cannot be read, or a figure in them is missing or is not a Kubernetes quantity, the poll prints why on stderr, keeps the last ceiling it worked out (or the bound, before it has one) for lowering and rebalancing, and does not raise the pool. Reading every node's pods needs `get`/`list` on pods cluster-wide, which the autoscaler's service account holds for this alone, as the heartbeat's does. - Raises the combined `maxRunners` by exactly +1 per cycle (never straight to a computed target) once headroom has covered a full pod's hard limit for `AUTOSCALER_RAISE_CONFIRM_POLLS` (default 2) consecutive polls, capped at the ceiling, and only in a poll that worked the ceiling out. - Lowers the combined `maxRunners` to the ceiling when the pooled total is above it, even when nothing else is wrong: each profile's own static `maxRunners` can sum to more than the ceiling, every Helm upgrade restores those, and the ceiling falls when a node goes, so the cap is enforced rather than only bounding raises. @@ -171,7 +171,7 @@ Each poll (`AUTOSCALER_POLL_SECONDS`, default 45s): - Also collects the peak memory each job's runner measured for itself: the runner image's job-completed hook (`runner-hooks/job-completed.sh`, set through `ACTIONS_RUNNER_HOOK_JOB_COMPLETED`) reads the container cgroup's `memory.peak`, which a spike shorter than the poll cannot escape, and posts it as a `JobMemoryPeak` Event in the pod's namespace. The autoscaler reads those Events each poll and keeps the maximum per repository, workflow ref and job id under the status ConfigMap's `job-peak` key (`kubectl get configmap autoscaler-status -n github-runner-platform -o jsonpath='{.data.job-peak}'`), in MiB. Keeping the maximum means a forged low figure lowers nothing and re-reading an Event changes nothing. The hook never fails the job it ends. To post an Event the runner pods' service account, which otherwise holds no permission, gets `create` on `events` in its own namespace and nothing else (`github_runner_arc_record_job_peaks`, default on; `false` removes the grant). A job's code runs in the same container and can read the same token, so the worst it can do with that permission is post events in its own runner namespace. - Records, per job, the highest memory a runner pod running it has been seen at, in the status ConfigMap's `job-memory` key (`kubectl get configmap autoscaler-status -n github-runner-platform -o jsonpath='{.data.job-memory}'`), keyed by repository, workflow ref and job name with the peak in MiB, the number of observations and when it was last seen. It is sampled once per poll from the metrics server, so it is a lower bound that a spike shorter than the poll escapes; it shows how far below a pod's request jobs sit, which is what decides whether an accurate in-pod measurement (see the per-job sizing issue) is worth building. Entries unseen for 30 days are dropped, and a failure to keep the record never affects a capacity decision. - Optionally reads per-node pressure from the nodes' own `/proc/pressure` (`github_runner_arc_autoscaler_node_pressure_enabled`). A `psi-reporter` DaemonSet (`scripts/psi-reporter.sh`, in the autoscaler's image) runs on each node runner pods may use, tolerating every taint, and patches that node's own key of one ConfigMap with the `avg10` of its io, cpu and memory pressure. The reading is not taken from the kubelet's summary API, because on a cgroup v1 node that reports every pressure field as 0 whatever the node is doing. The autoscaler lowers the combined `maxRunners` by exactly one each poll, never below the running count, while any node's I/O "full" stall is at or over `AUTOSCALER_IO_PRESSURE_PCT` (default 10) or, when set above 0, its cpu "some" stall is at or over `AUTOSCALER_CPU_PRESSURE_PCT`, and it does not raise the pool while any node has no report newer than three polls, since whether that node can take more is then unknown. This is the signal for a pool whose nodes differ: memory can have room on a node whose disk is already the limit. -- Fails safe on any memory measurement error (`kubectl top nodes` or the metrics API's pod list failing, or a figure that cannot be parsed), but not on a pod that merely has no metrics yet: lowers the combined total toward `AUTOSCALER_FLOOR` (`github_runner_arc_autoscaler_floor`) immediately rather than skipping the cycle silently. A failure reading the scale sets' own state or their pods exits non-zero instead, since fail-safe needs that state to lower anything and the API server is then unreachable anyway. This combined floor need not equal any one profile's own values-file `maxRunners`, since each profile's Helm upgrade reverts to its own static floor independently of how the pool's combined floor is set. +- Fails safe on any memory measurement error (`kubectl top nodes` or the metrics API's pod list failing, or the node list failing, or a `Ready` node's figure that cannot be parsed), but not on a pod that merely has no metrics yet, nor on a node that is not `Ready`: lowers the combined total toward `AUTOSCALER_FLOOR` (`github_runner_arc_autoscaler_floor`) immediately rather than skipping the cycle silently. A failure reading the scale sets' own state or their pods exits non-zero instead, since fail-safe needs that state to lower anything and the API server is then unreachable anyway. This combined floor need not equal any one profile's own values-file `maxRunners`, since each profile's Helm upgrade reverts to its own static floor independently of how the pool's combined floor is set. Because the ceiling is the tighter of CPU and memory on each node, shrinking the runner pod's CPU request raises it only on nodes where CPU is what decides. Halving a 2 CPU, 7Gi pod to 1 CPU was measured on a two-node pool and not taken: memory then decided on both nodes, so it bought one runner (a ceiling of 5 became 6), while every job that keeps more than one core busy would be throttled to one, and the autoscaler records memory only, so there was no CPU history to weigh that against. Revisit it when CPU history exists (the node pressure reporter's cpu figures, or a per-job CPU record), when one more runner is worth that throttling, or when a node with too few cores for the larger pod needs a 1 CPU pod to take work. diff --git a/autoscaler/Dockerfile b/autoscaler/Dockerfile index 6d6eca60..d07a4daf 100644 --- a/autoscaler/Dockerfile +++ b/autoscaler/Dockerfile @@ -21,6 +21,7 @@ COPY scripts/autoscaler.sh /app/scripts/autoscaler.sh COPY scripts/autoscaler-ceiling.jq /app/scripts/autoscaler-ceiling.jq COPY scripts/autoscaler-usage.jq /app/scripts/autoscaler-usage.jq COPY scripts/autoscaler-quantity.jq /app/scripts/autoscaler-quantity.jq +COPY scripts/autoscaler-node.jq /app/scripts/autoscaler-node.jq COPY scripts/psi-reporter.sh /app/scripts/psi-reporter.sh COPY scripts/node-cache-pruner.sh /app/scripts/node-cache-pruner.sh RUN chmod +x /app/loop.sh /app/scripts/autoscaler.sh /app/scripts/psi-reporter.sh /app/scripts/node-cache-pruner.sh diff --git a/scripts/autoscaler-ceiling.jq b/scripts/autoscaler-ceiling.jq index 222787e5..7cb5a007 100644 --- a/scripts/autoscaler-ceiling.jq +++ b/scripts/autoscaler-ceiling.jq @@ -8,6 +8,8 @@ # The quantity parsing shared with the usage program (scripts/autoscaler-quantity.jq), found through jq's -L, which the autoscaler points at its own directory. include "autoscaler-quantity"; +# Which nodes are Ready, shared with the autoscaler's memory check (scripts/autoscaler-node.jq). +include "autoscaler-node"; # A container's request for one resource. A container that sets only a limit is given that limit as its request when its pod is created, which a scale set's template has not been yet; a running pod already carries the request, so the fallback changes nothing there. def container_request($resource): (.resources.requests[$resource] // .resources.limits[$resource] // 0) | amount($resource); @@ -33,9 +35,8 @@ def tolerates($tolerations; $taint): # Why a node cannot take a runner pod of the template $spec now, or "" when it can. def unplaceable($spec): (.metadata.labels // {}) as $labels - | ([.status.conditions[]? | select(.type == "Ready") | .status] | first) as $ready | if any(($spec.nodeSelector // {}) | to_entries[]; $labels[.key] != (.value | tostring)) then "does not carry the runner pods' nodeSelector" - elif $ready != "True" then "is not Ready" + elif is_ready | not then "is not Ready" elif .spec.unschedulable == true then "is cordoned" else [.spec.taints[]? | select(.effect == "NoSchedule" or .effect == "NoExecute") | select(tolerates($spec.tolerations // []; .) | not) | "has the taint \(.key):\(.effect)"] | first // "" end; diff --git a/scripts/autoscaler-node.jq b/scripts/autoscaler-node.jq new file mode 100644 index 00000000..b1970857 --- /dev/null +++ b/scripts/autoscaler-node.jq @@ -0,0 +1,4 @@ +# What the autoscaler's jq programs and shell share about a Node object (scripts/autoscaler-ceiling.jq, and the memory check in scripts/autoscaler.sh), found through jq's -L, which the autoscaler points at its own directory, and pulled in with `include "autoscaler-node";`. + +# Whether the node's Ready condition is True. A node whose kubelet has stopped reporting (Unknown) or has reported a fault (False) is not Ready, and so is a node with no Ready condition at all: the scheduler places no pod on it and metrics-server has no figures for it, so it is left out of every count of where runner pods can go. +def is_ready: ([.status.conditions[]? | select(.type == "Ready") | .status] | first) == "True"; diff --git a/scripts/autoscaler.sh b/scripts/autoscaler.sh index 5a2e550e..1db55464 100755 --- a/scripts/autoscaler.sh +++ b/scripts/autoscaler.sh @@ -249,7 +249,7 @@ record_measured_peaks() { store_job_entries "$JOB_PEAK_KEY" "$jobs" false } -# Works out how many pooled runner pods the nodes can place now (scripts/autoscaler-ceiling.jq has the rule) and prints the program's result. Each pooled scale set's runner pod spec is read from its live AutoscalingRunnerSet, as its memory limit is above, so a change to the pod's request moves the ceiling with it. The lists go through files, never arguments: every pod in the cluster is more than one argument may hold (see configmap_set). Any failed read, or a figure the program cannot use, prints why on stderr and returns non-zero. A subshell, so the files go with it whichever way it ends; errexit does not apply inside a function called as a condition, so each step checks its own status. +# Works out how many pooled runner pods the nodes can place now (scripts/autoscaler-ceiling.jq has the rule) and prints the program's result. Each pooled scale set's runner pod spec is read from its live AutoscalingRunnerSet, as its memory limit is above, so a change to the pod's request moves the ceiling with it. The nodes are the poll's one read of them (NODES_FILE, taken before the memory check), so the ceiling and the memory check agree on which nodes are Ready. The lists go through files, never arguments: every pod in the cluster is more than one argument may hold (see configmap_set). Any failed read, or a figure the program cannot use, prints why on stderr and returns non-zero. A subshell, so the files go with it whichever way it ends; errexit does not apply inside a function called as a condition, so each step checks its own status. measure_ceiling() ( work="$(mktemp -d)" || exit 1 trap 'rm -rf "$work"' EXIT @@ -257,10 +257,9 @@ measure_ceiling() ( kubectl get autoscalingrunnerset "${RELEASES[$i]}" -n "${NAMESPACES[$i]}" -o json | jq -c '.spec.template.spec' >> "$work/templates" \ || { echo "could not read ${TARGETS[$i]}'s runner pod template" >&2; exit 1; } done - kubectl get nodes -o json > "$work/nodes" || { echo "could not list the nodes" >&2; exit 1; } kubectl get pods --all-namespaces --field-selector=status.phase!=Succeeded,status.phase!=Failed -o json > "$work/pods" \ || { echo "could not list the pods that hold requests" >&2; exit 1; } - jq -c --slurpfile templates "$work/templates" --slurpfile nodes "$work/nodes" --argjson pooled "$(printf '%s\n' "${NAMESPACES[@]}" | jq -Rsc 'split("\n") | map(select(length > 0))')" \ + jq -c --slurpfile templates "$work/templates" --slurpfile nodes "$NODES_FILE" --argjson pooled "$(printf '%s\n' "${NAMESPACES[@]}" | jq -Rsc 'split("\n") | map(select(length > 0))')" \ -L "$JQ_MODULES" -f "$CEILING_PROGRAM" "$work/pods" ) @@ -358,7 +357,10 @@ for i in "${!TARGETS[@]}"; do done # Cluster-wide memory-availability pressure via `kubectl top nodes` (metrics-server), not a single node's own /proc/meminfo - the autoscaler pod is genuinely unpinned now (see docker-compose.yml's own removal of host pinning), so a single-node reading would only ever reflect wherever the scheduler happened to place THIS pod, not the pool as a whole. Takes the WORST-CASE (minimum) available-memory percentage across the nodes it reads (those matching AUTOSCALER_NODE_SELECTOR, or every node when it is empty), not an average: a pool average can look healthy while the specific node a new runner pod would actually land on is not - consistent with the algorithm's own existing bias toward lowering eagerly, since lowering never disrupts in-flight jobs. No swap-pressure signal: metrics-server's API has no swap field at all (confirmed against its type, not just "probably not"), and the only alternative (the kubelet's own Summary API) needs a materially broader RBAC grant - node/proxy, effectively "reach anything a kubelet exposes" - for a signal most kubelet versions don't even surface unless the off-by-default NodeSwap feature gate is on. Real cluster-wide swap visibility needs Prometheus/node-exporter, which this fleet doesn't run and which would be genuine scope creep to add here - dropping swap detection is a deliberate, documented trade-off, not an oversight. -# Only the nodes runner pods may be placed on, when the role restricts them by label: a node they cannot use says nothing about where the next one lands, and one whose kubelet is unreachable reports no metrics, which would otherwise fail safe on every poll it is down. +# Only the nodes runner pods may be placed on, when the role restricts them by label: a node they cannot use says nothing about where the next one lands, and one whose kubelet is unreachable reports no metrics, which would otherwise fail safe on every poll it is down. The same holds inside the label for a node that is not Ready: `kubectl top nodes` prints it with figures, and it places no runner pod, which the ceiling already leaves out of its count. The poll reads every node once into NODES_FILE (the ceiling reads the same file below), and a figure counts only when its node is Ready by the one definition both share (scripts/autoscaler-node.jq); a Ready node whose figure cannot be read still fails safe. +NODES_FILE="$WORK/nodes" +kubectl get nodes -o json > "$NODES_FILE" 2>/dev/null || fail_safe "could not list the nodes" +READY_NODES="$(jq -r -L "$JQ_MODULES" 'include "autoscaler-node"; .items[] | select(is_ready) | .metadata.name' "$NODES_FILE")" || fail_safe "could not work out which nodes are Ready" TOP_NODES_ARGS=(top nodes --no-headers) [ -n "$NODE_SELECTOR" ] && TOP_NODES_ARGS+=(-l "$NODE_SELECTOR") # kubectl's own error is kept for the message, since its exit status alone does not say which failure it was: "Metrics API not available" when API discovery lists no usable metrics.k8s.io version (not registered, or registered but marked unavailable), the API server's own error when the list request itself fails, and "metrics not available yet" when the list succeeds with no node in it, which is what a selector matching no node, or matching only nodes whose kubelets are down, comes to. @@ -371,7 +373,8 @@ rm -f "$TOP_NODES_ERR_FILE" || fail_safe "kubectl top nodes${NODE_SELECTOR:+ -l ${NODE_SELECTOR}} failed (${TOP_NODES_ERR% }): the metrics API is missing or unreachable, or no node${NODE_SELECTOR:+ matching the selector} reports metrics" [ -n "$TOP_NODES_OUTPUT" ] || fail_safe "kubectl top nodes${NODE_SELECTOR:+ -l ${NODE_SELECTOR}} returned no nodes" mem_available_pct=100 -while read -r _name _cpu _cpu_pct _mem mem_pct; do +while read -r top_node _cpu _cpu_pct _mem mem_pct; do + case $'\n'"$READY_NODES"$'\n' in *$'\n'"$top_node"$'\n'*) ;; *) continue ;; esac mem_pct="${mem_pct%\%}" case "$mem_pct" in ''|*[!0-9]*) fail_safe "could not parse a node's own MEMORY% from kubectl top nodes ('$mem_pct')" ;; esac node_available_pct=$(( 100 - mem_pct )) diff --git a/tests/unit/test_autoscaler.py b/tests/unit/test_autoscaler.py index fc5f4a8f..5b238a5d 100644 --- a/tests/unit/test_autoscaler.py +++ b/tests/unit/test_autoscaler.py @@ -333,7 +333,32 @@ def test_a_selector_matching_no_node_fails_safe_with_kubectls_error(self) -> Non for namespace in NAMESPACES: self.assertIn(f"would patch {namespace}/release-{namespace}'s maxRunners to {RUNNING}", result.stdout) - def test_an_unreachable_node_inside_the_selector_fails_safe(self) -> None: + def test_a_not_ready_node_inside_the_selector_is_left_out_of_the_memory_check(self) -> None: + for ready in ("False", "Unknown"): + with self.subTest(ready=ready): + nodes = [{"name": "a", "labels": [self.ELIGIBLE], "mem": "20%"}, {"name": "b", "labels": [self.ELIGIBLE], "mem": "", "ready": ready}, {"name": "c", "labels": [], "mem": "10%"}] + result = self.poll({"ns-a": [SCHEDULED_POD], "ns-b": [SCHEDULED_POD]}, ceiling=MAX_RUNNERS * len(NAMESPACES), nodes=nodes, node_selector=self.ELIGIBLE) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertNotIn("FAIL-SAFE", result.stderr) + self.assertIn("mem_available=80% pressure=false", result.stdout) + self.assertNotIn("would patch", result.stdout) + + def test_a_not_ready_node_is_left_out_without_a_selector_too(self) -> None: + nodes = [{"name": "a", "labels": [], "mem": "20%"}, {"name": "b", "labels": [], "mem": "", "ready": "False"}] + result = self.poll({"ns-a": [SCHEDULED_POD], "ns-b": [SCHEDULED_POD]}, ceiling=MAX_RUNNERS * len(NAMESPACES), nodes=nodes) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertNotIn("FAIL-SAFE", result.stderr) + self.assertIn("mem_available=80% pressure=false", result.stdout) + + def test_a_not_ready_node_is_left_out_whatever_figure_it_reports(self) -> None: + # A kubelet that went silent can leave metrics-server's last figure behind: the node places no pod, so its figure says nothing about where the next one lands. + nodes = [{"name": "a", "labels": [self.ELIGIBLE], "mem": "20%"}, {"name": "b", "labels": [self.ELIGIBLE], "mem": "99%", "ready": "False"}] + result = self.poll({"ns-a": [SCHEDULED_POD], "ns-b": [SCHEDULED_POD]}, ceiling=MAX_RUNNERS * len(NAMESPACES), nodes=nodes, node_selector=self.ELIGIBLE) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("mem_available=80% pressure=false", result.stdout) + self.assertNotIn("would patch", result.stdout) + + def test_a_ready_node_inside_the_selector_with_an_unreadable_figure_fails_safe(self) -> None: nodes = [{"name": "a", "labels": [self.ELIGIBLE], "mem": "20%"}, {"name": "b", "labels": [self.ELIGIBLE], "mem": ""}, {"name": "c", "labels": [], "mem": "10%"}] result = self.poll({"ns-a": [SCHEDULED_POD], "ns-b": [SCHEDULED_POD]}, ceiling=MAX_RUNNERS * len(NAMESPACES), nodes=nodes, node_selector=self.ELIGIBLE) self.assertEqual(result.returncode, 0, result.stderr) @@ -341,6 +366,19 @@ def test_an_unreachable_node_inside_the_selector_fails_safe(self) -> None: for namespace in NAMESPACES: self.assertIn(f"would patch {namespace}/release-{namespace}'s maxRunners to {RUNNING}", result.stdout) + def test_a_ready_node_with_an_unreadable_figure_fails_safe_beside_a_not_ready_one(self) -> None: + nodes = [{"name": "a", "labels": [self.ELIGIBLE], "mem": "20%"}, {"name": "b", "labels": [self.ELIGIBLE], "mem": "", "ready": "False"}, {"name": "d", "labels": [self.ELIGIBLE], "mem": ""}] + result = self.poll({"ns-a": [SCHEDULED_POD], "ns-b": [SCHEDULED_POD]}, ceiling=MAX_RUNNERS * len(NAMESPACES), nodes=nodes, node_selector=self.ELIGIBLE) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("FAIL-SAFE: could not parse a node's own MEMORY% from kubectl top nodes ('')", result.stderr) + + def test_a_node_list_that_cannot_be_read_fails_safe(self) -> None: + result = self.poll({"ns-a": [SCHEDULED_POD], "ns-b": [SCHEDULED_POD]}, ceiling=MAX_RUNNERS * len(NAMESPACES), nodes_fail=True) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("FAIL-SAFE: could not list the nodes", result.stderr) + for namespace in NAMESPACES: + self.assertIn(f"would patch {namespace}/release-{namespace}'s maxRunners to {RUNNING}", result.stdout) + def recorded(self) -> dict[str, dict[str, object]]: return json.loads(self.job_memory_out.read_text()) @@ -767,7 +805,7 @@ def test_the_status_records_the_ceiling_and_what_each_node_places(self) -> None: self.assertEqual(status["ceiling_nodes"], [{"name": "a", "runners": 4}, {"name": "b", "runners": 1}, {"name": "c", "not_counted": "does not carry the runner pods' nodeSelector"}]) def test_a_failed_read_keeps_the_last_ceiling_for_lowering_and_never_raises(self) -> None: - for failure in ({"nodes_fail": True}, {"cluster_pods_fail": True}): + for failure in ({"cluster_pods_fail": True},): with self.subTest(failure): lowered = self.ceiling_poll(configmap={"ceiling": "5"}, **failure) self.assertEqual(lowered.returncode, 0, lowered.stderr) @@ -782,14 +820,14 @@ def test_a_failed_read_keeps_the_last_ceiling_for_lowering_and_never_raises(self self.assertEqual(self.stored("raise-confirm-count"), "0") def test_with_no_ceiling_ever_worked_out_and_no_bound_a_failed_read_neither_raises_nor_lowers(self) -> None: - result = self.ceiling_poll(nodes_fail=True, max_runners={"ns-a": 2, "ns-b": 2}, configmap={"raise-confirm-count": "1"}) + result = self.ceiling_poll(cluster_pods_fail=True, max_runners={"ns-a": 2, "ns-b": 2}, configmap={"raise-confirm-count": "1"}) self.assertEqual(result.returncode, 0, result.stderr) self.assertIn("ERROR: could not work out the ceiling from the nodes; not raising", result.stderr) self.assertIn("ceiling=none measured_ceiling=false", result.stdout) self.assertNotIn("would patch", result.stdout) def test_with_a_bound_and_no_ceiling_ever_worked_out_a_failed_read_lowers_to_the_bound(self) -> None: - result = self.ceiling_poll(nodes_fail=True, ceiling=4) + result = self.ceiling_poll(cluster_pods_fail=True, ceiling=4) self.assertIn("ceiling=4 measured_ceiling=false", result.stdout) self.assertEqual(self.pooled_total_after(result), 4) diff --git a/tests/unit/test_autoscaler_image.py b/tests/unit/test_autoscaler_image.py index 9625920b..a90c6526 100644 --- a/tests/unit/test_autoscaler_image.py +++ b/tests/unit/test_autoscaler_image.py @@ -39,6 +39,14 @@ def test_every_file_autoscaler_sh_reads_from_its_own_directory_is_copied_beside_ self.assertTrue((ROOT / "scripts" / name).is_file(), name) self.assertIn(f"COPY scripts/{name} /app/scripts/{name}", dockerfile) + def test_every_jq_module_a_program_the_image_runs_includes_is_copied_beside_it(self) -> None: + dockerfile = DOCKERFILE.read_text() + for program in sorted((ROOT / "scripts").glob("autoscaler-*.jq")): + for module in re.findall(r'^include "([A-Za-z0-9._-]+)";', program.read_text(), re.MULTILINE): + with self.subTest(program=program.name, module=module): + self.assertTrue((ROOT / "scripts" / f"{module}.jq").is_file()) + self.assertIn(f"COPY scripts/{module}.jq /app/scripts/{module}.jq", dockerfile) + def test_every_script_the_role_runs_from_the_image_is_copied_into_it_and_made_executable(self) -> None: dockerfile = DOCKERFILE.read_text() for script in scripts_run_from_the_image():