diff --git a/README.md b/README.md index 4eb3dd4a..d84f6970 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,7 @@ Its image, `github_runner_arc_node_recovery_image`, is pulled without a pull Sec `fleet-role-probe` and `fleet-role` (an optional DaemonSet and Deployment in the platform namespace, see `roles/github_runner_arc/templates/fleet-role.yaml.j2`, `scripts/fleet-role-probe.sh` and `scripts/fleet-role.sh`; `github_runner_cluster_role_controller`, off by default, which `github_runner_arc_role_controller_enabled` takes as its default) measure which hosts are fit to hold an etcd vote and report the swap of an unfit voter for a fit standby that the controller would make. They only report: the pods' ClusterRoles can `get` and `list` nodes and nothing else, and the only objects they write are their own two ConfigMaps. The controller refuses to start with any setting other than `FLEET_ROLE_DRY_RUN=true`, and runs unchanged when no host is marked eligible, when it reports the voters' fitness alone. -The probe runs on every Linux node that has a kubelet (a server regraded to etcd-only has none, and its side is read from its peers), on the node's own network, but records only on a voter or a host marked eligible (a node that is neither has no entry, and loses one it had), so the ConfigMap, which holds at most a mebibyte, grows with the hosts that decide the control plane and not with the fleet. Every `github_runner_arc_role_controller_sample_seconds` (15) it appends one sample to its node's key of the `fleet-role-probe` ConfigMap, which keeps the last window of them so that a replaced pod loses nothing. A sample holds the TCP connect time to every voter's etcd peer port (a connect that does not finish within the election timeout is recorded as the election timeout, so loss shows as tail latency); on a voter, what its etcd metrics (the `127.0.0.1:2381` listener) gained since the last sample: the WAL fsync and backend commit histograms, the round trip to each peer, leader changes, and whether it has a leader, is a learner and knows as many members as there are voters (a learner or a member that has not started shows as an extra known member); on a node that is not a voter, an fdatasync reading from fio with the write size etcd's own disk check uses, once per window; the memory pressure and `MemAvailable` the regrade core reads, with the core's own functions; and whether the interface the default route leaves by is `wired`, `wifi` or `unknown` (an interface that is virtual, such as the k3s container's veth, hides the link beneath it). The link is read from the interface's own `/sys/class/net` entry and never by walking `/sys`, whose symlinks loop. `scripts/fleet-role-probe.sh check link|connect
|memory|etcd|fsync` takes one of these readings alone, with no cluster access, to compare with a reading taken by hand on the host. +The probe runs on every Linux node that is an etcd voter (k3s's `node-role.kubernetes.io/etcd` label) or is marked eligible and has a kubelet (a server regraded to etcd-only has none, and its side is read from its peers), on the node's own network, through a node affinity on those two labels, so a node that is neither (a Unraid host, an image-flashed agent) has no pod and nothing is asked of its kernel. A pod stopped on a node that has lost both labels, or is gone, removes the node's entry as it ends, so the ConfigMap, which holds at most a mebibyte, grows with the hosts that decide the control plane and not with the fleet. Every `github_runner_arc_role_controller_sample_seconds` (15) it appends one sample to its node's key of the `fleet-role-probe` ConfigMap, which keeps the last window of them so that a replaced pod loses nothing. A sample holds the TCP connect time to every voter's etcd peer port (a connect that does not finish within the election timeout is recorded as the election timeout, so loss shows as tail latency); on a voter, what its etcd metrics (the `127.0.0.1:2381` listener) gained since the last sample: the WAL fsync and backend commit histograms, the round trip to each peer, leader changes, and whether it has a leader, is a learner and knows as many members as there are voters (a learner or a member that has not started shows as an extra known member); on a node that is not a voter, an fdatasync reading from fio with the write size etcd's own disk check uses, once per window; the memory pressure (`/proc/pressure/memory` of the pod's own procfs, which serves the whole node's, so nothing is mounted) and `MemAvailable` the regrade core reads, with the core's own functions, where a node on a kernel without pressure stall information gives `MemAvailable` and `MemTotal` and no pressure figure, which the controller counts as not over the memory limit; and whether the interface the default route leaves by is `wired`, `wifi` or `unknown` (an interface that is virtual, such as the k3s container's veth, hides the link beneath it). The link is read from the interface's own `/sys/class/net` entry and never by walking `/sys`, whose symlinks loop. `scripts/fleet-role-probe.sh check link|connect
|memory|etcd|fsync` takes one of these readings alone, with no cluster access, to compare with a reading taken by hand on the host. The controller reads the Nodes and the probes' samples every sample period and publishes `fleet-role-status`, which the heartbeat republishes in its gist beside the autoscaler's status. For every voter and every eligible host it gives a verdict, `fit`, `unfit` or `unknown`, with each reason and the numbers behind it. Every limit derives from etcd's guidance and the cluster's own timing, not chosen numbers: diff --git a/roles/github_runner_arc/README.md b/roles/github_runner_arc/README.md index 8a3c59bb..35abcafc 100644 --- a/roles/github_runner_arc/README.md +++ b/roles/github_runner_arc/README.md @@ -481,9 +481,9 @@ A pruned node returns only when its k3s agent process restarts. A running k3s ag ## Role controller -With `github_runner_cluster_role_controller: true` (or `github_runner_arc_role_controller_enabled: true`, for a host that does not run the cluster role) the role installs `fleet-role-probe`, a DaemonSet on every Linux node that has a kubelet, and `fleet-role`, a Deployment, both in `github_runner_arc_platform_namespace` and both from `github_runner_arc_role_controller_image`. Together they report, for every etcd voter and every host marked eligible, whether it is fit to hold an etcd vote and why, and the swap of an unfit voter for a fit standby that the controller would make. They change nothing in the cluster: each has a ClusterRole that can `get` and `list` nodes and nothing else, and a Role that reaches only the two ConfigMaps it uses by name (`get` and `patch`, never `create`; the role creates both empty and keeps them). `false` removes the workloads, their RBAC and both ConfigMaps. The repository README's Role controller section explains each signal, how it is read, the thresholds and when a swap is held back. +With `github_runner_cluster_role_controller: true` (or `github_runner_arc_role_controller_enabled: true`, for a host that does not run the cluster role) the role installs `fleet-role-probe`, a DaemonSet on every Linux node that is an etcd voter or is marked eligible (`github_runner_arc_role_controller_eligible_label`) and has a kubelet, and `fleet-role`, a Deployment, both in `github_runner_arc_platform_namespace` and both from `github_runner_arc_role_controller_image`. Together they report, for every etcd voter and every host marked eligible, whether it is fit to hold an etcd vote and why, and the swap of an unfit voter for a fit standby that the controller would make. They change nothing in the cluster: each has a ClusterRole that can `get` and `list` nodes and nothing else, and a Role that reaches only the two ConfigMaps it uses by name (`get` and `patch`, never `create`; the role creates both empty and keeps them). `false` removes the workloads, their RBAC and both ConfigMaps. The repository README's Role controller section explains each signal, how it is read, the thresholds and when a swap is held back. -The probe runs with the node's network and as root with every capability dropped, because the directory it measures the disk in (`github_runner_arc_role_controller_data_dir`, on the filesystem k3s keeps its data on) is created by the kubelet and owned by root. It mounts the host's `/proc/pressure` read-only. The controller runs as an unprivileged user with a read-only root filesystem. The heartbeat, when it is installed, reads the controller's status ConfigMap too and republishes it in its gist as `fleet-role-status.json`. +The probe runs with the node's network and as root with every capability dropped, because the directory it measures the disk in (`github_runner_arc_role_controller_data_dir`, on the filesystem k3s keeps its data on) is created by the kubelet and owned by root. It mounts nothing for memory pressure: `/proc/pressure/memory` of its own procfs is the node's, as `/proc/meminfo` is, and a `hostPath` mount of `/proc/pressure` would stop the pod starting on a kernel without pressure stall information, since the container runtime then tries to create the directory. The controller runs as an unprivileged user with a read-only root filesystem. The heartbeat, when it is installed, reads the controller's status ConfigMap too and republishes it in its gist as `fleet-role-status.json`. The check in `tasks/validate.yml` refuses a target (`github_runner_arc_role_controller_voters`) that is even, under 3 or without a place of headroom below `github_runner_arc_role_controller_max_servers`, a period, election timeout or maximum that is not a whole number of at least 1, and two labels that are not label keys or are the same. Nothing is checked while the controller is off. diff --git a/roles/github_runner_arc/tasks/install_psi_reporter.yml b/roles/github_runner_arc/tasks/install_psi_reporter.yml index 4bccaf6f..aa1ea0eb 100644 --- a/roles/github_runner_arc/tasks/install_psi_reporter.yml +++ b/roles/github_runner_arc/tasks/install_psi_reporter.yml @@ -1,5 +1,5 @@ --- -# The per-node pressure reporter (scripts/psi-reporter.sh) and what the autoscaler needs to read it, installed when github_runner_arc_autoscaler_node_pressure_enabled is on and a profile is autoscaled, removed otherwise. One DaemonSet pod on each node runner pods may use, with the host's /proc/pressure mounted read-only; each patches its own node's key of one shared ConfigMap. Like the autoscaler's own state ConfigMap, that one is pre-created empty, so no ServiceAccount here is granted `create`, only get/patch scoped by resourceNames. +# The per-node pressure reporter (scripts/psi-reporter.sh) and what the autoscaler needs to read it, installed when github_runner_arc_autoscaler_node_pressure_enabled is on and a profile is autoscaled, removed otherwise. One DaemonSet pod on each node runner pods may use, reading /proc/pressure of its own procfs, with nothing mounted; each patches its own node's key of one shared ConfigMap. Like the autoscaler's own state ConfigMap, that one is pre-created empty, so no ServiceAccount here is granted `create`, only get/patch scoped by resourceNames. - name: "Platform: decide whether the node pressure reporter is wanted" ansible.builtin.set_fact: @@ -156,10 +156,6 @@ value: "{{ github_runner_arc_node_pressure_configmap }}" - name: REPORT_SECONDS value: "{{ github_runner_arc_psi_reporter_seconds | string }}" - volumeMounts: - - name: pressure - mountPath: /host-pressure - readOnly: true resources: requests: cpu: "10m" @@ -169,7 +165,3 @@ cpu: "50m" memory: "32Mi" ephemeral-storage: "{{ github_runner_arc_platform_ephemeral_storage_limit }}" - volumes: - - name: pressure - hostPath: - path: /proc/pressure diff --git a/roles/github_runner_arc/templates/fleet-role.yaml.j2 b/roles/github_runner_arc/templates/fleet-role.yaml.j2 index fc312b91..1fed9085 100644 --- a/roles/github_runner_arc/templates/fleet-role.yaml.j2 +++ b/roles/github_runner_arc/templates/fleet-role.yaml.j2 @@ -5,6 +5,8 @@ {% set namespace = github_runner_arc_platform_namespace %} {% set probe_configmap = github_runner_arc_role_controller_probe_configmap %} {% set status_configmap = github_runner_arc_role_controller_status_configmap %} +{# The label k3s puts, with the value true, on every node that is an etcd member: the controller (scripts/fleet-role.sh) and the probe (scripts/fleet-role-probe.sh) each name it, and tests/unit/test_fleet_role_arc.py holds the three together. #} +{% set voter_label = 'node-role.kubernetes.io/etcd' %} {# A ClusterRole can only name nodes cluster-wide; the controller and the probe both list them to find the voters and the standbys, and neither can write one. #} {% set node_rules = [{'apiGroups': [''], 'resources': ['nodes'], 'verbs': ['get', 'list']}] %} {% for name in [controller, probe] %} @@ -175,7 +177,15 @@ spec: hostNetwork: true dnsPolicy: ClusterFirstWithHostNet nodeSelector: {{ {'kubernetes.io/os': 'linux'} | to_json }} - # Every node with a kubelet, whatever it is tainted for; a server regraded to etcd-only has no kubelet, so no pod, and its side is read from its peers. + # Only where the controller needs a reading: a node that is an etcd voter (the label k3s gives every member) or that the owner marked eligible (the label the controller reads to find its candidates). The terms of a node affinity are alternatives, so a node matching either gets a pod and any other gets none. A node that stops matching loses its pod, which removes the node's entry from the probe ConfigMap as it ends (scripts/fleet-role-probe.sh). + affinity: + nodeAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + nodeSelectorTerms: +{% for label in [voter_label, github_runner_arc_role_controller_eligible_label] %} + - matchExpressions: {{ [{'key': label, 'operator': 'In', 'values': ['true']}] | to_json }} +{% endfor %} + # Whatever such a node is tainted for; a server regraded to etcd-only has no kubelet, so no pod, and its side is read from its peers. tolerations: - operator: Exists securityContext: @@ -196,6 +206,8 @@ spec: value: {{ github_runner_arc_role_controller_election_timeout_ms | string | to_json }} - name: ELIGIBLE_LABEL value: {{ github_runner_arc_role_controller_eligible_label | to_json }} + - name: VOTER_LABEL + value: {{ (voter_label ~ '=true') | to_json }} - name: PROBE_CONFIGMAP value: {{ probe_configmap | to_json }} - name: REPORT_SECONDS @@ -224,9 +236,6 @@ spec: volumeMounts: - name: tmp mountPath: /tmp - - name: pressure - mountPath: /host-pressure - readOnly: true - name: fsync mountPath: /fsync volumes: @@ -234,9 +243,6 @@ spec: emptyDir: medium: Memory sizeLimit: 8Mi - - name: pressure - hostPath: - path: /proc/pressure - name: fsync hostPath: path: {{ github_runner_arc_role_controller_data_dir | to_json }} diff --git a/scripts/fleet-role-probe.sh b/scripts/fleet-role-probe.sh index d405a03d..f8b3f38b 100755 --- a/scripts/fleet-role-probe.sh +++ b/scripts/fleet-role-probe.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash -# The role controller's per-node probe (see roles/github_runner_arc/templates/fleet-role.yaml.j2): the `fleet-role-probe` DaemonSet runs this on every node that has a kubelet, with host networking, so it measures the path the node's own etcd and kubelet use. Every REPORT_SECONDS it takes one sample and appends it to this node's entry in the shared probe ConfigMap, which scripts/fleet-role.sh reads. An entry keeps the last window of samples, so it survives this pod being replaced. +# The role controller's per-node probe (see roles/github_runner_arc/templates/fleet-role.yaml.j2): the `fleet-role-probe` DaemonSet runs this on every node that is an etcd voter or marked eligible and has a kubelet, with host networking, so it measures the path the node's own etcd and kubelet use. Every REPORT_SECONDS it takes one sample and appends it to this node's entry in the shared probe ConfigMap, which scripts/fleet-role.sh reads. An entry keeps the last window of samples, so it survives this pod being replaced. # -# Only a node the controller judges is measured: a voter, or a host the owner marks eligible (ELIGIBLE_LABEL, with the value true). Any other node has nothing recorded, and an entry it has from when it was one is removed, so the ConfigMap, which holds at most a mebibyte, grows with the hosts that decide the cluster's control plane and not with the fleet. +# Only a node the controller judges is measured: a voter, or a host the owner marks eligible (ELIGIBLE_LABEL, with the value true). The DaemonSet schedules a pod on no other node, and a pod that finds its node is neither (a label removed while it runs) records nothing and removes the entry it has from when it was one, as does a pod that is stopped on a node that is neither any more or is gone, so the ConfigMap, which holds at most a mebibyte, grows with the hosts that decide the cluster's control plane and not with the fleet. # # A sample holds, per node: the TCP connect time to every voter's etcd peer port (a connect that does not complete within the election timeout is recorded as the election timeout, so loss shows as tail latency); on a voter, what etcd's own metrics gained since the last sample (the WAL fsync and backend commit histograms, the round trip to each peer, leader changes) and whether it has a leader, is a learner and is connected to its peers; on a node that is not a voter, an fdatasync probe in the k3s data directory once per window, with the write size etcd's own fio check uses; the memory pressure and MemAvailable the regrade core reads; and the link the default route leaves by. # @@ -20,7 +20,7 @@ # - ETCD_PEER_PORT: etcd's peer port (default 2380). # - FSYNC_DIR: a directory on the filesystem k3s keeps its data on, which the fdatasync probe writes in (default /var/lib/rancher/k3s/fleet-role-probe). # - SYS_CLASS_NET, PROC_NET_ROUTE: where the host's /sys/class/net and /proc/net/route are (the defaults). -# - FLEET_REGRADE_CORE: the regrade core (roles/github_runner_cluster/files/regrade/fleet-regrade-core.sh), whose own psi_avg300 and meminfo_kb read the memory signals (default /app/fleet-regrade-core.sh). PSI_FILE and MEMINFO_FILE, which it reads, default here to the host's /proc/pressure mounted at /host-pressure and the node's /proc/meminfo. +# - FLEET_REGRADE_CORE: the regrade core (roles/github_runner_cluster/files/regrade/fleet-regrade-core.sh), whose own psi_avg300 and meminfo_kb read the memory signals (default /app/fleet-regrade-core.sh). PSI_FILE and MEMINFO_FILE, which it reads, default here to /proc/pressure/memory and /proc/meminfo, which a pod's own procfs serves for the whole node as it does /proc/meminfo, so nothing is mounted for them. A kernel without pressure stall information has no /proc/pressure, and the sample then has no pressure reading and the memory figures from /proc/meminfo alone, which the controller counts as not over the limit. set -euo pipefail PROBE_CONFIGMAP="${PROBE_CONFIGMAP:-fleet-role-probe}" @@ -33,7 +33,7 @@ FSYNC_DIR="${FSYNC_DIR:-/var/lib/rancher/k3s/fleet-role-probe}" SYS_CLASS_NET="${SYS_CLASS_NET:-/sys/class/net}" PROC_NET_ROUTE="${PROC_NET_ROUTE:-/proc/net/route}" FLEET_REGRADE_CORE="${FLEET_REGRADE_CORE:-/app/fleet-regrade-core.sh}" -PSI_FILE="${PSI_FILE:-/host-pressure/memory}" +PSI_FILE="${PSI_FILE:-/proc/pressure/memory}" MEMINFO_FILE="${MEMINFO_FILE:-/proc/meminfo}" # The core reads its settings from an environment file when there is one; the probe has none, and must not pick up another by the core's default path. # shellcheck disable=SC2034 # read by the sourced core @@ -101,6 +101,40 @@ fsync_ms() { | jq -r --arg percentile "${tail_percentile}.000000" '.jobs[0].sync.lat_ns.percentile[$percentile] / 1000000' } +# The nodes JSON's voters, as a JSON array of name and InternalIP. +voters_of() { + jq -c --arg label "${VOTER_LABEL%%=*}" --arg value "${VOTER_LABEL#*=}" '[.items[] | select((.metadata.labels // {})[$label] == $value) | {name: .metadata.name, ip: ([.status.addresses[]? | select(.type == "InternalIP") | .address] | .[0])}]' <<< "$1" +} + +# Whether this node is among the voters (a JSON array from voters_of). +is_voter_in() { + jq --arg node "$NODE_NAME" 'any(.[]; .name == $node)' <<< "$1" +} + +# Whether this node is marked eligible in the nodes JSON. A node the JSON does not list is not. +is_eligible_in() { + jq --arg node "$NODE_NAME" --arg label "$ELIGIBLE_LABEL" 'any(.items[]; .metadata.name == $node and (.metadata.labels // {})[$label] == "true")' <<< "$1" +} + +# Removes this node's entry from the ConfigMap. A key set to null in a merge patch is removed. +remove_entry() { + local patch + patch="$(mktemp)" + jq -n --arg node "$NODE_NAME" '{data: {($node): null}}' > "$patch" + kubectl patch configmap "$PROBE_CONFIGMAP" -n "$NAMESPACE" --type merge --patch-file "$patch" > /dev/null || { rm -f "$patch"; return 1; } + rm -f "$patch" +} + +# What the pod does when it is stopped: if its node is neither a voter nor eligible any more (the DaemonSet stops the pod of a node that has lost both labels) or is gone, its entry is removed, so the ConfigMap holds only the hosts that decide the control plane; a pod stopped for any other reason (a new image, the node shutting down) leaves the entry for the pod that follows it to extend. The API can fail here, and a pod that is going away has nothing to retry it with. +retire() { + local nodes_json + trap - TERM INT + if nodes_json="$(kubectl get nodes -o json)" && [ "$(is_voter_in "$(voters_of "$nodes_json")")" != true ] && [ "$(is_eligible_in "$nodes_json")" != true ]; then + remove_entry || log "could not remove this node's entry" + fi + exit 0 +} + # Takes one sample and publishes it. $1 is "true" on the first pass of this process, which for a node that is not a voter is the only sign this probe has of k3s having restarted (a voter sees it in etcd's counters going backwards). report() { local first=$1 now entry voters is_voter eligible tcp='{}' peer_name peer_ip ms etcd='null' restarted=false cumulative='null' etcd_id='null' @@ -114,16 +148,12 @@ report() { entry="$kept_entry" fi nodes_json="$(kubectl get nodes -o json)" || return 1 - voters="$(jq -c --arg label "${VOTER_LABEL%%=*}" --arg value "${VOTER_LABEL#*=}" '[.items[] | select((.metadata.labels // {})[$label] == $value) | {name: .metadata.name, ip: ([.status.addresses[]? | select(.type == "InternalIP") | .address] | .[0])}]' <<< "$nodes_json")" - is_voter="$(jq --arg node "$NODE_NAME" 'any(.[]; .name == $node)' <<< "$voters")" - eligible="$(jq --arg node "$NODE_NAME" --arg label "$ELIGIBLE_LABEL" 'any(.items[]; .metadata.name == $node and (.metadata.labels // {})[$label] == "true")' <<< "$nodes_json")" + voters="$(voters_of "$nodes_json")" + is_voter="$(is_voter_in "$voters")" + eligible="$(is_eligible_in "$nodes_json")" if [ "$is_voter" != true ] && [ "$eligible" != true ]; then - # A key set to null in a merge patch is removed. if [ "$entry" != null ]; then - patch="$(mktemp)" - jq -n --arg node "$NODE_NAME" '{data: {($node): null}}' > "$patch" - kubectl patch configmap "$PROBE_CONFIGMAP" -n "$NAMESPACE" --type merge --patch-file "$patch" > /dev/null - rm -f "$patch" + remove_entry kept_entry=null fi return 0 @@ -189,7 +219,12 @@ if [ "${1:-}" = check ]; then jq -nc --arg interface "$iface" --arg link "$(link_type "$iface")" '{interface: $interface, link: $link}' ;; connect) connect_ms "${3:?usage: check connect
}" ;; - memory) jq -nc --arg psi "$(psi_avg300)" --arg avail "$(meminfo_kb MemAvailable)" --arg total "$(meminfo_kb MemTotal)" '{psi_avg300: ($psi | tonumber), mem_available_kb: ($avail | tonumber), mem_total_kb: ($total | tonumber)}' ;; + memory) + # No pressure reading on a kernel without pressure stall information, as in a sample. + psi='' + if [ -r "$PSI_FILE" ]; then psi="$(psi_avg300)"; fi + jq -nc --arg psi "$psi" --arg avail "$(meminfo_kb MemAvailable)" --arg total "$(meminfo_kb MemTotal)" '{psi_avg300: (if $psi == "" then null else $psi | tonumber end), mem_available_kb: ($avail | tonumber), mem_total_kb: ($total | tonumber)}' + ;; etcd) curl --silent --show-error --fail --max-time "$REPORT_SECONDS" "$ETCD_METRICS_URL" | jq -Rsc -L "$MODULES" 'include "fleet-role-stats"; etcd_reading | {id, has_leader, is_learner, known_members, active_peers, quota_bytes, leader_changes, wal_fsync_p99_ms: (.wal | per_bucket | histogram_quantile(tail_percentile) * 1000), backend_commit_p99_ms: (.commit | per_bucket | histogram_quantile(tail_percentile) * 1000)}' ;; fsync) fsync_ms ;; *) echo "usage: $0 check link|connect
|memory|etcd|fsync" >&2; exit 2 ;; @@ -202,6 +237,8 @@ fi : "${ELECTION_TIMEOUT_MS:?ELECTION_TIMEOUT_MS is required}" : "${ELIGIBLE_LABEL:?ELIGIBLE_LABEL is required}" +trap retire TERM INT + # This node's entry as the ConfigMap holds it, read once and then kept (see report). entry_loaded=false kept_entry=null @@ -215,5 +252,7 @@ while true; do log "could not record a sample; trying again in ${REPORT_SECONDS}s" fi [ "$REPORT_ONCE" = true ] && exit 0 - sleep "$REPORT_SECONDS" + # In the background and waited for, so that a signal is handled at once and not when the sleep ends. + sleep "$REPORT_SECONDS" & + wait $! || true done diff --git a/scripts/psi-reporter.sh b/scripts/psi-reporter.sh index 19405852..517b6d72 100755 --- a/scripts/psi-reporter.sh +++ b/scripts/psi-reporter.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Publishes this node's own pressure stall information for the autoscaler. Runs in the `psi-reporter` DaemonSet, one pod per node a runner pod may use, with the host's /proc/pressure mounted read-only. +# Publishes this node's own pressure stall information for the autoscaler. Runs in the `psi-reporter` DaemonSet, one pod per node a runner pod may use, reading /proc/pressure of its own procfs, which serves the whole node's pressure as /proc/meminfo serves its memory, so nothing is mounted: a hostPath volume of /proc/pressure makes the container runtime create the directory when the kernel has none (no pressure stall information), and the pod then never starts. # # The reading comes from the node's own /proc/pressure, not from the kubelet's summary API: on a cgroup v1 node that API reports every pressure field as 0 whatever the node is doing, so a reader built on it would find no pressure on exactly the nodes that have some. # @@ -7,7 +7,7 @@ # - NODE_NAME: the node this pod runs on (the downward API's spec.nodeName). Required. # - NAMESPACE: the namespace of the ConfigMap. Required. # - NODE_PRESSURE_CONFIGMAP: the shared ConfigMap, pre-created empty, that every reporter patches its own key of (default: autoscaler-node-pressure). -# - PRESSURE_DIR: where the host's /proc/pressure is mounted (default: /host-pressure). +# - PRESSURE_DIR: where the node's pressure files are (default: /proc/pressure, absent on a kernel without pressure stall information, which gives a report with no pressure figures). # - REPORT_SECONDS: seconds between reports (default: 15). The autoscaler treats a report older than a few polls as no report. # - REPORT_ONCE: when "true", report once and exit, for tests. # @@ -17,7 +17,7 @@ set -euo pipefail : "${NODE_NAME:?NODE_NAME is required}" : "${NAMESPACE:?NAMESPACE is required}" NODE_PRESSURE_CONFIGMAP="${NODE_PRESSURE_CONFIGMAP:-autoscaler-node-pressure}" -PRESSURE_DIR="${PRESSURE_DIR:-/host-pressure}" +PRESSURE_DIR="${PRESSURE_DIR:-/proc/pressure}" REPORT_SECONDS="${REPORT_SECONDS:-15}" REPORT_ONCE="${REPORT_ONCE:-false}" diff --git a/tests/fleet_role/image-smoke.sh b/tests/fleet_role/image-smoke.sh index 97901f3c..bafa97d8 100755 --- a/tests/fleet_role/image-smoke.sh +++ b/tests/fleet_role/image-smoke.sh @@ -24,6 +24,12 @@ chmod 644 "$work/meminfo" "$work/pressure" docker run --rm -v "$work/meminfo:/meminfo:ro" -v "$work/pressure:/pressure:ro" -e MEMINFO_FILE=/meminfo -e PSI_FILE=/pressure \ --entrypoint /app/scripts/fleet-role-probe.sh "$image" check memory | jq -e '.psi_avg300 == 1.5 and .mem_available_kb == 400' > /dev/null +# With no file named, the probe reads the pod's own /proc/pressure/memory, which is the node's, so the DaemonSet mounts nothing for it: the container sees pressure exactly when this host's kernel has it (a mounted /proc/pressure would stop a pod on a kernel without it from starting) and the reading is then absent, never an error. +host_has_psi=false +[ -r /proc/pressure/memory ] && host_has_psi=true +docker run --rm --entrypoint /app/scripts/fleet-role-probe.sh "$image" check memory \ + | jq -e --argjson psi "$host_has_psi" '(.psi_avg300 != null) == $psi and .mem_total_kb > 0' > /dev/null + # A kubectl that answers an empty cluster and keeps the patch it is given, in place of the image's own on the PATH. mkdir "$work/out" chmod 777 "$work/out" diff --git a/tests/unit/test_fleet_role_arc.py b/tests/unit/test_fleet_role_arc.py index 41451d6f..8d7b4dd1 100644 --- a/tests/unit/test_fleet_role_arc.py +++ b/tests/unit/test_fleet_role_arc.py @@ -174,7 +174,7 @@ def test_the_controller_and_the_probe_agree_on_the_election_timeout_and_the_peri self.assertEqual(controller["FLEET_ROLE_ELECTION_TIMEOUT_MS"], probe["ELECTION_TIMEOUT_MS"]) self.assertEqual((controller["FLEET_ROLE_SAMPLE_SECONDS"], probe["REPORT_SECONDS"]), ("30", "30")) - def test_the_probe_measures_from_the_nodes_own_network_on_every_linux_node_with_a_kubelet(self) -> None: + def test_the_probe_measures_from_the_nodes_own_network_and_tolerates_every_taint(self) -> None: daemonset = of_kind(self.manifests, "DaemonSet")[0] pod = daemonset["spec"]["template"]["spec"] self.assertTrue(pod["hostNetwork"]) @@ -184,6 +184,50 @@ def test_the_probe_measures_from_the_nodes_own_network_on_every_linux_node_with_ self.assertEqual(container(daemonset)["command"], ["/app/scripts/fleet-role-probe.sh"]) self.assertEqual(env_of(daemonset)["NODE_NAME"], {"fieldRef": {"fieldPath": "spec.nodeName"}}) + @staticmethod + def terms(manifests: list[dict[str, Any]]) -> list[list[dict[str, Any]]]: + """The alternatives of the probe's node affinity, each the expressions a node must meet.""" + pod = of_kind(manifests, "DaemonSet")[0]["spec"]["template"]["spec"] + return [term["matchExpressions"] for term in pod["affinity"]["nodeAffinity"]["requiredDuringSchedulingIgnoredDuringExecution"]["nodeSelectorTerms"]] + + @staticmethod + def scheduled(terms: list[list[dict[str, Any]]], labels: dict[str, str]) -> bool: + """Whether a node with these labels meets one of the terms, as the scheduler reads In expressions.""" + return any(all(e["operator"] == "In" and labels.get(e["key"]) in e["values"] for e in term) for term in terms) + + def test_the_probe_is_scheduled_only_on_a_voter_or_an_eligible_node(self) -> None: + terms = self.terms(self.manifests) + voter, eligible = "node-role.kubernetes.io/etcd", "github-runner.exadev.com/voter-eligible" + self.assertTrue(self.scheduled(terms, {voter: "true"}), "a voter that is not marked eligible") + self.assertTrue(self.scheduled(terms, {eligible: "true"}), "an eligible standby that is not a voter") + self.assertTrue(self.scheduled(terms, {voter: "true", eligible: "true"})) + self.assertFalse(self.scheduled(terms, {}), "a node that is neither: an agent, or a host that is neither voter nor eligible") + self.assertFalse(self.scheduled(terms, {eligible: "false"}), "the label with another value is not the owner's mark") + self.assertFalse(self.scheduled(terms, {"node-role.kubernetes.io/control-plane": "true", "node-role.kubernetes.io/master": "true"}), "a control-plane node that holds no etcd member") + + def test_the_eligible_term_follows_the_controllers_label_name(self) -> None: + manifests = render(**{f"{ARC}eligible_label": "example.com/standby"}) + terms = self.terms(manifests) + self.assertTrue(self.scheduled(terms, {"example.com/standby": "true"})) + self.assertFalse(self.scheduled(terms, {"github-runner.exadev.com/voter-eligible": "true"})) + controller = env_of(of_kind(manifests, "Deployment")[0]) + self.assertEqual(controller["FLEET_ROLE_ELIGIBLE_LABEL"], "example.com/standby") + + def test_the_probe_reads_the_nodes_pressure_from_its_own_procfs_so_a_kernel_without_it_cannot_fail_the_pod(self) -> None: + # A hostPath volume of /proc/pressure makes the container runtime create the directory when the kernel has none, and the pod then sits in CreateContainerError. + daemonset = of_kind(self.manifests, "DaemonSet")[0] + pod = daemonset["spec"]["template"]["spec"] + self.assertNotIn("/proc", json.dumps(pod["volumes"])) + self.assertNotIn("pressure", {v["name"] for v in pod["volumes"]} | {m["name"] for m in container(daemonset)["volumeMounts"]}) + self.assertNotIn("PSI_FILE", env_of(daemonset)) + self.assertIn('PSI_FILE="${PSI_FILE:-/proc/pressure/memory}"', (ROOT / "scripts" / "fleet-role-probe.sh").read_text()) + + def test_the_voter_label_the_template_schedules_on_is_the_one_the_controller_and_the_probe_read(self) -> None: + voter = "node-role.kubernetes.io/etcd" + self.assertIn(f'voter_label: "{voter}"', (ROOT / "scripts" / "fleet-role.sh").read_text()) + self.assertIn(f'VOTER_LABEL="${{VOTER_LABEL:-{voter}=true}}"', (ROOT / "scripts" / "fleet-role-probe.sh").read_text()) + self.assertEqual(env_of(of_kind(self.manifests, "DaemonSet")[0])["VOTER_LABEL"], f"{voter}=true") + def test_the_probe_is_root_only_for_its_own_directory_and_holds_no_capability(self) -> None: daemonset = of_kind(self.manifests, "DaemonSet")[0] self.assertEqual( @@ -192,16 +236,14 @@ def test_the_probe_is_root_only_for_its_own_directory_and_holds_no_capability(se ) self.assertNotIn("privileged", container(daemonset)["securityContext"]) - def test_the_probe_sees_the_hosts_pressure_read_only_and_writes_only_in_its_own_directory(self) -> None: + def test_the_probe_writes_only_in_its_own_directory(self) -> None: daemonset = of_kind(self.manifests, "DaemonSet")[0] pod = daemonset["spec"]["template"]["spec"] mounts = {m["name"]: m for m in container(daemonset)["volumeMounts"]} volumes = {v["name"]: v for v in pod["volumes"]} - self.assertEqual(volumes["pressure"]["hostPath"], {"path": "/proc/pressure"}) - self.assertTrue(mounts["pressure"]["readOnly"]) self.assertEqual(volumes["fsync"]["hostPath"], {"path": "/var/lib/rancher/k3s/fleet-role-probe", "type": "DirectoryOrCreate"}) self.assertEqual(mounts["fsync"]["mountPath"], env_of(daemonset)["FSYNC_DIR"]) - self.assertEqual(sorted(volumes), ["fsync", "pressure", "tmp"]) + self.assertEqual(sorted(volumes), ["fsync", "tmp"]) self.assertEqual({name for name, mount in mounts.items() if not mount.get("readOnly")}, {"tmp", "fsync"}) def test_the_controller_and_the_probe_run_one_image(self) -> None: diff --git a/tests/unit/test_fleet_role_probe.py b/tests/unit/test_fleet_role_probe.py index 4fd36bf5..3c2dabce 100644 --- a/tests/unit/test_fleet_role_probe.py +++ b/tests/unit/test_fleet_role_probe.py @@ -42,6 +42,7 @@ sample, ) +SCRIPT = Path(__file__).resolve().parents[2] / "scripts" / "fleet-role-probe.sh" FIXTURES = Path(__file__).resolve().parents[1] / "fleet_role" / "etcd-metrics" # The members of the cluster the fixtures were captured from (etcd 3.6.15, three members on one host); the page is the first member's. FIXTURE_SERVER_ID = "d5811b26ae265d01" @@ -607,6 +608,25 @@ def test_a_kernel_without_pressure_stall_information_gives_a_sample_without_a_pr self.assertIsNone(latest["psi"]) self.assertEqual(latest["avail_kb"], 8192000) + def test_without_a_psi_file_setting_the_pressure_is_read_from_this_hosts_own_procfs(self) -> None: + # The DaemonSet sets no PSI_FILE and mounts nothing, so the probe reads /proc/pressure/memory of its own procfs: with this host's kernel, a figure when it has pressure stall information and none when it has not (as on a Mac or a kernel booted with psi=0). + self.assertIn('PSI_FILE="${PSI_FILE:-/proc/pressure/memory}"', SCRIPT.read_text()) + completed = self.probe(PSI_FILE="") + self.assertEqual(completed.returncode, 0, completed.stderr) + if os.access("/proc/pressure/memory", os.R_OK): + self.assertIsNotNone(self.newest()["psi"], "the kernel has pressure stall information and the probe left it out") + else: + self.assertIsNone(self.newest()["psi"], "the kernel has none, and the sample has the memory figures alone") + + def test_a_node_without_pressure_stall_information_still_records_memory_and_publishes_its_entry(self) -> None: + # The case the DaemonSet met on a host whose kernel has no /proc/pressure: the pod must run and sample, with no pressure figure and the figures /proc/meminfo gives. + self.psi.unlink() + completed = self.probe(PSI_FILE=str(self.cluster.dir / "no-such-dir" / "pressure" / "memory")) + self.assertEqual(completed.returncode, 0, completed.stderr) + self.assertEqual(completed.stderr, "", "reading the pressure file that is not there must not even complain") + latest = self.newest() + self.assertEqual((latest["psi"], latest["avail_kb"], latest["total_kb"]), (None, 8192000, 16384000)) + class RingTest(ProbeCase): def setUp(self) -> None: @@ -688,10 +708,84 @@ def test_a_sample_after_the_first_costs_two_api_calls_the_nodes_and_the_patch(se reads = [c[:2] for c in calls if c[0] == "get"] patches = [c for c in calls if c[0] == "patch"] self.assertEqual(reads.count(["get", "configmap"]), 1, "this node's entry is read once, when the process starts") - # The process may have been stopped between a sample's read of the nodes and its patch, so the nodes were read once for each patch and at most once more. - self.assertIn(reads.count(["get", "nodes"]) - len(patches), (0, 1), reads) + # The process may have been stopped between a sample's read of the nodes and its patch, and it reads the nodes once more as it stops (to see whether its entry is to go), so the nodes were read once for each patch and at most twice more. + self.assertIn(reads.count(["get", "nodes"]) - len(patches), (0, 1, 2), reads) self.assertGreaterEqual(len(patches), len(samples)) + def stop_loop(self, **env: str) -> subprocess.Popen[bytes]: + """Starts the probe as the DaemonSet does, waits for its first recorded sample and returns it still running.""" + self.cluster.nodes = self.nodes + self.cluster.write_store() + process = subprocess.Popen( + [str(SCRIPT)], + env=self.cluster.env( + NODE_NAME="me", NAMESPACE=NAMESPACE, ELECTION_TIMEOUT_MS=str(ELECTION_TIMEOUT_MS), ELIGIBLE_LABEL=ELIGIBLE_LABEL, REPORT_SECONDS="600", ETCD_PEER_PORT="1", FSYNC_DIR=str(self.fsync_dir), + SYS_CLASS_NET=str(self.sys / "class" / "net"), PROC_NET_ROUTE=str(self.route), FLEET_REGRADE_CORE=str(REGRADE_CORE), PSI_FILE=str(self.psi), MEMINFO_FILE=str(self.meminfo), **env, + ), + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + self.addCleanup(process.kill) + deadline = time.monotonic() + 60 + while time.monotonic() < deadline and not self.recorded(): + time.sleep(0.2) + self.assertTrue(self.recorded(), "the probe never recorded a sample") + return process + + def stored_probes(self) -> dict[str, Any]: + """The probe ConfigMap as the stand-in cluster holds it now, which the running probe has patched.""" + data = self.cluster.read_store()["configmaps"][f"{NAMESPACE}/fleet-role-probe"]["data"] + return {name: json.loads(value) for name, value in data.items()} + + def change_nodes(self, nodes: list[dict[str, Any]]) -> None: + """Changes the Nodes the running probe will read, leaving what it has patched.""" + store = self.cluster.read_store() + store["nodes"] = nodes + self.cluster.store_path.write_text(json.dumps(store)) + + def stop(self, process: subprocess.Popen[bytes]) -> None: + started = time.monotonic() + process.terminate() + self.assertEqual(process.wait(timeout=20), 0) + self.assertLess(time.monotonic() - started, 20, "the signal waited for the sleep between samples") + + def test_a_pod_stopped_on_a_node_that_lost_both_labels_removes_its_entry_and_no_other(self) -> None: + self.cluster.set_probes({"me": {**entry([]), "fsync_at": int(time.time())}, "other": entry([sample(5)], "other-id")}) + process = self.stop_loop() + self.change_nodes([node("me")]) + self.stop(process) + self.assertEqual(sorted(self.stored_probes()), ["other"]) + + def test_a_pod_stopped_on_a_node_that_is_gone_removes_its_entry(self) -> None: + self.cluster.set_probes({"me": {**entry([]), "fsync_at": int(time.time())}}) + process = self.stop_loop() + self.change_nodes([]) + self.stop(process) + self.assertEqual(self.stored_probes(), {}) + + def test_a_pod_stopped_on_a_node_that_is_still_eligible_keeps_its_entry_for_the_pod_that_follows(self) -> None: + self.cluster.set_probes({"me": {**entry([]), "fsync_at": int(time.time())}}) + process = self.stop_loop() + self.stop(process) + self.assertEqual(len(self.stored_probes()["me"]["samples"]), 1) + + def test_a_pod_stopped_on_a_node_that_is_a_voter_keeps_its_entry(self) -> None: + self.cluster.set_probes({"me": {**entry([]), "fsync_at": int(time.time())}}) + self.nodes = [node("me", voter=True)] + process = self.stop_loop(ETCD_METRICS_URL="http://127.0.0.1:1/metrics") + self.stop(process) + self.assertIn("me", self.stored_probes()) + + def test_a_pod_stopped_when_the_api_refuses_the_removal_exits_and_leaves_the_entry(self) -> None: + refusals = self.cluster.dir / "refusals" + refusals.write_text("0") + self.cluster.set_probes({"me": {**entry([]), "fsync_at": int(time.time())}}) + process = self.stop_loop(FAKE_FAIL_PATCHES=str(refusals)) + self.change_nodes([node("me")]) + refusals.write_text("1") + self.stop(process) + self.assertIn("me", self.stored_probes()) + def test_a_patch_the_api_refuses_is_retried_next_period_without_the_process_exiting_and_the_first_sample_still_marks_the_restart(self) -> None: refusals = self.cluster.dir / "refusals" refusals.write_text("2") @@ -783,6 +877,12 @@ def test_link_prints_the_interface_and_its_link_type(self) -> None: self.assertEqual(completed.returncode, 0, completed.stderr) self.assertEqual(json.loads(completed.stdout), {"interface": "wlan0", "link": "wifi"}) + def test_memory_without_pressure_stall_information_prints_the_memory_figures_and_no_pressure(self) -> None: + self.meminfo.write_text("MemTotal: 32768000 kB\nMemAvailable: 4096000 kB\n") + completed = self.check("memory", PSI_FILE=str(self.cluster.dir / "no-such-dir" / "memory")) + self.assertEqual(completed.returncode, 0, completed.stderr) + self.assertEqual(json.loads(completed.stdout), {"psi_avg300": None, "mem_available_kb": 4096000, "mem_total_kb": 32768000}) + def test_connect_prints_the_milliseconds_to_a_listener(self) -> None: with listener() as port: completed = self.check("connect", "127.0.0.1", ETCD_PEER_PORT=str(port)) diff --git a/tests/unit/test_psi_reporter.py b/tests/unit/test_psi_reporter.py index 0e3aa90e..5559473b 100644 --- a/tests/unit/test_psi_reporter.py +++ b/tests/unit/test_psi_reporter.py @@ -75,6 +75,28 @@ def test_a_missing_pressure_file_leaves_that_resource_out(self) -> None: assert isinstance(data, dict) self.assertEqual(sorted(key for key in json.loads(data["node-a"]) if key != "at"), ["io_full_avg10", "io_some_avg10"]) + def test_a_kernel_without_pressure_stall_information_reports_the_time_alone(self) -> None: + # No /proc/pressure at all: the reporter must run and publish a report with no figures, which the autoscaler reads as no pressure. + with tempfile.TemporaryDirectory() as work: + root = Path(work) + (root / "bin").mkdir() + kubectl = root / "bin" / "kubectl" + kubectl.write_text(FAKE_KUBECTL) + kubectl.chmod(0o755) + env = {**os.environ, "PATH": f"{root / 'bin'}:{os.environ['PATH']}", "NODE_NAME": "node-a", "NAMESPACE": "platform", "PRESSURE_DIR": str(root / "no-such-dir"), "REPORT_ONCE": "true", "FAKE_PATCH_OUT": str(root / "patch.json"), "FAKE_ARGS_OUT": str(root / "args.txt")} + result = subprocess.run(["bash", str(SCRIPT)], env=env, capture_output=True, text=True, check=False) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stderr, "") + data = json.loads((root / "patch.json").read_text())["data"] + self.assertEqual(list(json.loads(data["node-a"])), ["at"]) + + def test_the_pod_reads_the_nodes_own_procfs_and_mounts_nothing_that_a_kernel_without_it_would_fail(self) -> None: + # A hostPath volume of /proc/pressure makes the container runtime create the directory when the kernel has none, and the pod then never starts. + self.assertIn('PRESSURE_DIR="${PRESSURE_DIR:-/proc/pressure}"', SCRIPT.read_text()) + task = (SCRIPT.parents[1] / "roles" / "github_runner_arc" / "tasks" / "install_psi_reporter.yml").read_text() + self.assertNotIn("hostPath", task) + self.assertNotIn("host-pressure", task) + def test_the_configmap_name_and_namespace_are_configurable(self) -> None: _, args = self.report({"io": IO}, NODE_PRESSURE_CONFIGMAP="pressure-x", NAMESPACE="other") self.assertIn("patch configmap pressure-x -n other", args)