From bde44dc9fe4ec49a384c7ac72e84f77bef13e605 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 6 Oct 2026 08:22:55 +0200 Subject: [PATCH 1/2] feat(deploy): read back the declared kubelet settings on every node A green machine-config sync does not prove every running kubelet took the settings the repository declares: two autoscaler nodes ran without the disk eviction thresholds for weeks and nothing reported it. After each cluster update the deploy now reads every node's live kubelet configuration and compares it with the cluster-wide declared settings. A node that cannot be read is a failure, never a pass. The deploy step starts observe-only, so a finding is a warning until one clean production reading is on record. Part of #3137 Co-Authored-By: Claude Opus 5.5 --- .github/actions/deploy-prod/action.yml | 16 ++ .github/workflows/ci.yaml | 7 + scripts/tests/test-kubelet-config-readback.sh | 215 ++++++++++++++++++ scripts/verify-kubelet-config-readback.sh | 156 +++++++++++++ 4 files changed, 394 insertions(+) create mode 100755 scripts/tests/test-kubelet-config-readback.sh create mode 100755 scripts/verify-kubelet-config-readback.sh diff --git a/.github/actions/deploy-prod/action.yml b/.github/actions/deploy-prod/action.yml index 230326a286..17969c9f29 100644 --- a/.github/actions/deploy-prod/action.yml +++ b/.github/actions/deploy-prod/action.yml @@ -459,6 +459,22 @@ runs: shell: bash run: ./scripts/verify-talos-schematic-readback.sh + - name: 🧾 Read back the kubelet settings on every node + # A green machine-config sync does not prove that every running kubelet + # took the declared settings: two autoscaler nodes ran without the disk + # eviction thresholds for weeks after the fix deployed, and nothing + # reported it (#3137). Observe-only for now: the comparison has not yet + # been read against production, so a finding is a warning on a green + # deploy. Drop KUBELET_READBACK_ENFORCE once one deploy has logged a + # clean PASS (tracked on #3137). The check only reads Kubernetes. + if: >- + steps.cluster_update.outcome == 'success' && + steps.wait_prod_api_stability.outcome == 'success' + shell: bash + env: + KUBELET_READBACK_ENFORCE: "false" + run: ./scripts/verify-kubelet-config-readback.sh + - name: 📣 Report suppressed machine-config sync # The step above is the pipeline's ONLY Talos machine-config sync, and the # rollout gate skips it. A skipped step inside a green deploy reads as diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 787667d246..d9eaf1630a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -957,6 +957,8 @@ jobs: - 'scripts/verify-talos-schematic-readback.sh' - 'scripts/tests/test-talos-factory-schematic-registration.sh' - 'scripts/tests/test-talos-schematic-readback.sh' + - 'scripts/verify-kubelet-config-readback.sh' + - 'scripts/tests/test-kubelet-config-readback.sh' - 'scripts/tests/test-talos-apparmor-lsm.sh' - 'ksail.prod.yaml' - 'ksail.yaml' @@ -2842,6 +2844,11 @@ jobs: shellcheck scripts/verify-talos-schematic-readback.sh scripts/tests/test-talos-schematic-readback.sh bash scripts/tests/test-talos-schematic-readback.sh + - name: 🧾 Validate kubelet settings readback + run: | + shellcheck scripts/verify-kubelet-config-readback.sh scripts/tests/test-kubelet-config-readback.sh + bash scripts/tests/test-kubelet-config-readback.sh + - name: 🧱 Prove folded Talos render and invalid-patch rejection run: | shellcheck scripts/tests/test-talos-render-kernel-args.sh diff --git a/scripts/tests/test-kubelet-config-readback.sh b/scripts/tests/test-kubelet-config-readback.sh new file mode 100755 index 0000000000..c2d234ed0c --- /dev/null +++ b/scripts/tests/test-kubelet-config-readback.sh @@ -0,0 +1,215 @@ +#!/usr/bin/env bash +set -euo pipefail + +root_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd) +readback="$root_dir/scripts/verify-kubelet-config-readback.sh" +deploy_action="$root_dir/.github/actions/deploy-prod/action.yml" +ci_workflow="$root_dir/.github/workflows/ci.yaml" + +[[ -x "$readback" ]] || { + printf 'kubelet settings readback is missing or not executable\n' >&2 + exit 1 +} + +if ! grep -Fq 'scripts/verify-kubelet-config-readback.sh' "$deploy_action" || + ! grep -Fq "'scripts/tests/test-kubelet-config-readback.sh'" "$ci_workflow" || + ! grep -Fq 'bash scripts/tests/test-kubelet-config-readback.sh' "$ci_workflow"; then + printf 'CI and deployment must execute the kubelet settings readback contract\n' >&2 + exit 1 +fi + +stability_line=$(grep -nF './scripts/wait-for-prod-api-stability.sh' "$deploy_action" | cut -d: -f1) +readback_line=$(grep -nF './scripts/verify-kubelet-config-readback.sh' "$deploy_action" | cut -d: -f1) +if [[ -z "$stability_line" || -z "$readback_line" ]] || (( stability_line >= readback_line )); then + printf 'kubelet settings readback must follow cluster update and API stability\n' >&2 + exit 1 +fi + +tmp_dir=$(mktemp -d) +trap 'rm -rf "$tmp_dir"' EXIT + +# The mock answers the two reads the check makes and nothing else. A node's +# reply is the file named after it; a missing file is a refused read. +mock_kubectl="$tmp_dir/kubectl" +cat >"$mock_kubectl" <<'MOCK' +#!/usr/bin/env bash +set -euo pipefail +[[ "$1 $2 $3" == "--context admin@prod --request-timeout=15s" ]] || exit 64 +shift 3 +if [[ "$*" == "get nodes -o json" ]]; then + if [[ -n "${MOCK_FIRST_DIR:-}" && ! -f "$MOCK_DIR/first-used" ]]; then + cat "$MOCK_FIRST_DIR/nodes.json" + else + cat "$MOCK_DIR/nodes.json" + fi + exit 0 +fi +[[ "$1 $2" == "get --raw" && "$3" == /api/v1/nodes/*/proxy/configz && $# -eq 3 ]] || exit 64 +node=${3#/api/v1/nodes/} +node=${node%/proxy/configz} +dir=$MOCK_DIR +if [[ -n "${MOCK_FIRST_DIR:-}" && ! -f "$MOCK_DIR/first-used" ]]; then + dir=$MOCK_FIRST_DIR + [[ "$node" != "${MOCK_LAST_NODE:-}" ]] || : >"$MOCK_DIR/first-used" +fi +[[ -f "$dir/$node.json" ]] || { printf 'Error from server (Forbidden)\n' >&2; exit 1; } +cat "$dir/$node.json" +MOCK +chmod +x "$mock_kubectl" + +nodes=(prod-control-plane-2 prod-worker-1 autoscale-cx43-example) + +# What a healthy kubelet reports for the settings the repository declares +# today, plus fields the check must ignore. +healthy='{"kubeletconfig":{ + "systemReserved":{"memory":"256Mi","cpu":"50m"}, + "kubeReserved":{"memory":"256Mi"}, + "evictionSoft":{"memory.available":"500Mi"}, + "evictionSoftGracePeriod":{"memory.available":"1m30s"}, + "evictionMinimumReclaim":{"memory.available":"200Mi"}, + "evictionMaxPodGracePeriod":60, + "mergeDefaultEvictionSettings":true, + "evictionHard":{"memory.available":"100Mi","nodefs.available":"10%","nodefs.inodesFree":"5%","imagefs.available":"15%","imagefs.inodesFree":"5%"}, + "imageGCHighThresholdPercent":75, + "imageGCLowThresholdPercent":70, + "maxPods":110}}' +# The exact shape #3137 found on two autoscaler nodes. +drifted=$(jq '.kubeletconfig.mergeDefaultEvictionSettings = false + | .kubeletconfig.evictionHard = {"memory.available":"100Mi"}' <<<"$healthy") + +new_fixture() { # : every node healthy + mkdir -p "$1" + printf '%s\n' "${nodes[@]}" | jq -R '{metadata: {name: .}}' | jq -s '{items: .}' >"$1/nodes.json" + for node in "${nodes[@]}"; do printf '%s\n' "$healthy" >"$1/$node.json"; done +} + +run() { # [env assignments...]: sets rc and output + local dir=$1 + shift + rc=0 + output=$(env KUBECTL_BIN="$mock_kubectl" MOCK_DIR="$dir" \ + KUBELET_READBACK_ATTEMPTS=1 KUBELET_READBACK_INTERVAL_SECONDS=0 "$@" "$readback" 2>&1) || rc=$? +} + +expect() { # ... + local name=$1 want=$2 + shift 2 + if [[ "$rc" -ne "$want" ]]; then + printf 'FAIL %s: expected exit %s, got %s:\n%s\n' "$name" "$want" "$rc" "$output" >&2 + exit 1 + fi + for fragment in "$@"; do + # A here-string, not a pipe: grep -q stops at the first match. + if ! grep -qF -- "$fragment" <<<"$output"; then + printf 'FAIL %s: missing "%s":\n%s\n' "$name" "$fragment" "$output" >&2 + exit 1 + fi + done + printf 'ok: %s\n' "$name" +} + +new_fixture "$tmp_dir/healthy" +run "$tmp_dir/healthy" +expect 'every node matches the declared settings' 0 \ + 'PASS: 3 node(s), 1 of them autoscaler-provisioned' 'mergeDefaultEvictionSettings' + +new_fixture "$tmp_dir/drift" +printf '%s\n' "$drifted" >"$tmp_dir/drift/autoscale-cx43-example.json" +run "$tmp_dir/drift" +expect 'the drift #3137 found on an autoscaler node fails and names it' 1 \ + 'autoscale-cx43-example: mergeDefaultEvictionSettings: declared true, live false' \ + 'autoscale-cx43-example: evictionHard: live has no nodefs.available threshold' \ + 'autoscale-cx43-example: evictionHard: live has no imagefs.available threshold' +if grep -qF 'prod-worker-1:' <<<"$output"; then + printf 'FAIL: a healthy node was reported beside the drifted one:\n%s\n' "$output" >&2 + exit 1 +fi + +new_fixture "$tmp_dir/value" +jq '.kubeletconfig.evictionSoft["memory.available"] = "100Mi"' <<<"$healthy" >"$tmp_dir/value/prod-worker-1.json" +run "$tmp_dir/value" +expect 'a changed value inside a declared map fails' 1 \ + 'prod-worker-1: evictionSoft: declared {"memory.available":"500Mi"}, live {"memory.available":"100Mi"}' + +new_fixture "$tmp_dir/absent" +jq 'del(.kubeletconfig.imageGCHighThresholdPercent)' <<<"$healthy" >"$tmp_dir/absent/prod-worker-1.json" +run "$tmp_dir/absent" +expect 'a declared setting the kubelet does not report fails' 1 \ + 'prod-worker-1: imageGCHighThresholdPercent: declared 75, live null' + +# The reproduction in #3137 printed nothing on a refused read. That must +# never read as a pass. +new_fixture "$tmp_dir/refused" +rm "$tmp_dir/refused/autoscale-cx43-example.json" +run "$tmp_dir/refused" +expect 'a node whose kubelet cannot be read fails' 1 \ + 'autoscale-cx43-example: kubelet configuration could not be read' + +new_fixture "$tmp_dir/garbage" +printf 'not json\n' >"$tmp_dir/garbage/prod-worker-1.json" +run "$tmp_dir/garbage" +expect 'a reply that is not JSON fails' 1 'prod-worker-1: kubelet configuration reply is not valid JSON' + +new_fixture "$tmp_dir/shapeless" +printf '{}\n' >"$tmp_dir/shapeless/prod-worker-1.json" +run "$tmp_dir/shapeless" +expect 'a reply without a kubelet configuration fails' 1 'prod-worker-1: no kubeletconfig in the reply' + +mkdir "$tmp_dir/empty" +printf '{"items": []}\n' >"$tmp_dir/empty/nodes.json" +run "$tmp_dir/empty" +expect 'a cluster reporting no nodes fails' 1 'the cluster reported no nodes' + +mkdir "$tmp_dir/unreadable" +printf 'not json\n' >"$tmp_dir/unreadable/nodes.json" +run "$tmp_dir/unreadable" +expect 'an unreadable node list fails' 1 'Kubernetes nodes could not be read from context admin@prod' + +mkdir "$tmp_dir/hostile" +printf '{"items": [{"metadata": {"name": "a/../../b"}}]}\n' >"$tmp_dir/hostile/nodes.json" +run "$tmp_dir/hostile" +expect 'a node name that is not a node name is never queried' 1 'unexpected node name; refusing to query it' + +# A replacement node that is not answering yet converges within the window. +new_fixture "$tmp_dir/late-first" +rm "$tmp_dir/late-first/autoscale-cx43-example.json" +new_fixture "$tmp_dir/late" +run "$tmp_dir/late" MOCK_FIRST_DIR="$tmp_dir/late-first" MOCK_LAST_NODE=autoscale-cx43-example \ + KUBELET_READBACK_ATTEMPTS=3 +expect 'a node that answers on a later attempt passes' 0 'PASS: 3 node(s)' +[[ -f "$tmp_dir/late/first-used" ]] || { + printf 'FAIL: the late-node case never served its first, incomplete reading\n' >&2 + exit 1 +} + +# Observe-only: the finding is reported as a warning and the deploy stays green. +run "$tmp_dir/drift" KUBELET_READBACK_ENFORCE=false +expect 'observe-only reports drift as a warning and exits 0' 0 \ + '::warning title=Kubelet settings readback (observe-only)::' \ + 'autoscale-cx43-example: mergeDefaultEvictionSettings: declared true, live false' +run "$tmp_dir/healthy" KUBELET_READBACK_ENFORCE=false +expect 'observe-only still passes a healthy cluster without a warning' 0 'PASS: 3 node(s)' +if grep -qF '::warning' <<<"$output"; then + printf 'FAIL: a healthy observe-only run printed a warning:\n%s\n' "$output" >&2 + exit 1 +fi +run "$tmp_dir/healthy" KUBELET_READBACK_ENFORCE=maybe +expect 'an unknown enforce value is refused' 2 'invalid kubelet readback' + +# Nothing declared means nothing was compared; that is not a pass. +mkdir -p "$tmp_dir/talos-none/cluster" +printf 'machine:\n network: {}\n' >"$tmp_dir/talos-none/cluster/other.yaml" +run "$tmp_dir/healthy" TALOS_CONFIG_DIR="$tmp_dir/talos-none" +expect 'no declared kubelet settings is refused' 2 'nothing to compare' + +mkdir -p "$tmp_dir/talos-missing" +run "$tmp_dir/healthy" TALOS_CONFIG_DIR="$tmp_dir/talos-missing" +expect 'a missing patch directory is refused' 2 'no cluster-wide Talos patches found' + +mkdir -p "$tmp_dir/talos-role/cluster" "$tmp_dir/talos-role/workers" +cp "$root_dir/talos/cluster/evict-pods-before-oom.yaml" "$tmp_dir/talos-role/cluster/" +printf 'machine:\n kubelet:\n extraConfig:\n maxPods: 200\n' >"$tmp_dir/talos-role/workers/more-pods.yaml" +run "$tmp_dir/healthy" TALOS_CONFIG_DIR="$tmp_dir/talos-role" +expect 'a per-role kubelet setting is refused, not half-compared' 2 'are not modelled by this check' + +printf 'All kubelet settings readback cases passed.\n' diff --git a/scripts/verify-kubelet-config-readback.sh b/scripts/verify-kubelet-config-readback.sh new file mode 100755 index 0000000000..cd1b94f835 --- /dev/null +++ b/scripts/verify-kubelet-config-readback.sh @@ -0,0 +1,156 @@ +#!/usr/bin/env bash +set -euo pipefail + +# A green machine-config sync does not prove that every running kubelet took +# the settings the repository declares. #3137: two autoscaler nodes kept +# running without the disk eviction thresholds for weeks after the fix was +# deployed, and nothing reported it. This reads each node's live kubelet +# configuration and requires every declared setting to be present in it. +# +# Declared settings are the cluster-wide machine.kubelet.extraConfig patches. +# The comparison is literal and one-directional: a declared key must exist +# live with exactly the declared value, and anything else the kubelet reports +# is ignored. Every setting declared today is a number, a boolean or a string +# the kubelet reports back unchanged. One it rewrites (a duration such as 2m, +# reported as 2m0s) will fail here on its first deploy, visibly, and has to be +# declared in the kubelet's own spelling. +# +# A node that cannot be read is a failure, never a pass: the reproduction in +# #3137 printed nothing when the read was refused, which looked clean. +# +# Exit codes: 0 every node matches; 1 drift or an unreadable node; 2 nothing +# to compare against, or a declaration this check does not model. +root_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +talos_dir=${TALOS_CONFIG_DIR:-$root_dir/talos} +kubectl_bin=${KUBECTL_BIN:-kubectl} +# Deployments restore a kubeconfig that contains admin@prod, but its current +# context is not a trusted cluster selector. +kube_context=${KUBE_CONTEXT:-admin@prod} +max_attempts=${KUBELET_READBACK_ATTEMPTS:-30} +interval_seconds=${KUBELET_READBACK_INTERVAL_SECONDS:-10} +max_seconds=${KUBELET_READBACK_MAX_SECONDS:-300} +# false reports a failure as a warning and exits 0. The production deploy +# starts that way until one clean production reading is on record (#3137). +enforce=${KUBELET_READBACK_ENFORCE:-true} + +[[ "$max_attempts" =~ ^[1-9][0-9]*$ && + "$interval_seconds" =~ ^(0|[1-9][0-9]*)$ && + "$max_seconds" =~ ^[1-9][0-9]*$ && + "$enforce" =~ ^(true|false)$ ]] || { + printf 'invalid kubelet readback attempt, interval, deadline, or enforce setting\n' >&2 + exit 2 +} + +shopt -s nullglob +cluster_patches=("$talos_dir"/cluster/*.yaml) +role_patches=("$talos_dir"/control-planes/*.yaml "$talos_dir"/workers/*.yaml) +shopt -u nullglob + +((${#cluster_patches[@]} > 0)) || { + printf 'no cluster-wide Talos patches found under %s/cluster\n' "$talos_dir" >&2 + exit 2 +} + +# A per-role kubelet setting would make "every node" the wrong expectation. +# Refuse it instead of comparing those nodes against half their declaration. +for patch in "${role_patches[@]}"; do + if [[ "$(yq eval '.machine.kubelet.extraConfig | length' "$patch")" != 0 ]]; then + printf 'per-role kubelet settings in %s are not modelled by this check\n' "$patch" >&2 + exit 2 + fi +done + +# shellcheck disable=SC2016 # yq program, not shell +declared=$(yq eval-all -o=json -I=0 \ + '. as $doc ireduce ({}; . * ($doc.machine.kubelet.extraConfig // {}))' \ + "${cluster_patches[@]}") +jq -e 'type == "object" and length > 0' <<<"$declared" >/dev/null 2>&1 || { + printf 'no declared kubelet settings found under %s/cluster; nothing to compare\n' "$talos_dir" >&2 + exit 2 +} + +# shellcheck disable=SC2016 # jq program, not shell +compare=' + def covers($want): + . as $have + | if ($want | type) == "object" then + ($have | type) == "object" and + all($want | to_entries[]; . as $e | ($have | has($e.key)) and ($have[$e.key] | covers($e.value))) + else $have == $want end; + (.kubeletconfig // null) as $live + | if ($live | type) != "object" then ["no kubeletconfig in the reply"] + else + [ $declared | to_entries[] | . as $e + | select(($live | has($e.key) and (.[$e.key] | covers($e.value))) | not) + | "\($e.key): declared \($e.value | tojson), live \($live[$e.key] | tojson)" ] + # Without these three the kubelet never evicts on disk pressure, which + # is the condition #3137 found. They are kubelet defaults, so they are + # only expected when the declaration asks for the defaults to be kept. + + ( if $declared.mergeDefaultEvictionSettings == true then + [ ("nodefs.available", "nodefs.inodesFree", "imagefs.available") as $signal + | select((($live.evictionHard // {}) | has($signal)) | not) + | "evictionHard: live has no \($signal) threshold" ] + else [] end ) + end + | .[]' + +report='' +node_count=0 +autoscaler_count=0 +deadline_epoch=$(( $(date +%s) + max_seconds )) +for ((attempt = 1; attempt <= max_attempts; attempt++)); do + report='' + node_count=0 + autoscaler_count=0 + # A node the autoscaler has just registered may not answer yet. Never accept + # an unread node, but allow a normal replacement to converge within a + # finite window. + if names=$("$kubectl_bin" --context "$kube_context" --request-timeout=15s \ + get nodes -o json 2>/dev/null | jq -r '.items | if type == "array" then .[].metadata.name else error("no items") end'); then + while IFS= read -r node; do + [[ -n "$node" ]] || continue + node_count=$((node_count + 1)) + if [[ ! "$node" =~ ^[a-z0-9]([a-z0-9.-]*[a-z0-9])?$ ]]; then + report+="unexpected node name; refusing to query it"$'\n' + continue + fi + [[ "$node" == prod-control-plane-* || "$node" == prod-worker-* ]] || + autoscaler_count=$((autoscaler_count + 1)) + if ! configz=$("$kubectl_bin" --context "$kube_context" --request-timeout=15s \ + get --raw "/api/v1/nodes/$node/proxy/configz" 2>/dev/null); then + report+="$node: kubelet configuration could not be read"$'\n' + continue + fi + if ! drift=$(jq -r --argjson declared "$declared" "$compare" <<<"$configz" 2>/dev/null); then + report+="$node: kubelet configuration reply is not valid JSON"$'\n' + continue + fi + while IFS= read -r line; do + [[ -z "$line" ]] || report+="$node: $line"$'\n' + done <<<"$drift" + done <<<"$names" + ((node_count > 0)) || report='the cluster reported no nodes'$'\n' + else + report="Kubernetes nodes could not be read from context $kube_context"$'\n' + fi + + if [[ -z "$report" ]]; then + printf 'PASS: %d node(s), %d of them autoscaler-provisioned, run every declared kubelet setting (%s)\n' \ + "$node_count" "$autoscaler_count" "$(jq -r 'keys | join(", ")' <<<"$declared")" + exit 0 + fi + + if ((attempt == max_attempts || $(date +%s) >= deadline_epoch)); then + break + fi + sleep "$interval_seconds" +done + +printf 'Kubelet settings readback did not pass after %d attempt(s):\n%s' "$attempt" "$report" >&2 +if [[ "$enforce" == false ]]; then + # One annotation, so the finding is visible on a deploy that stays green. + printf '::warning title=Kubelet settings readback (observe-only)::%s\n' \ + "$(printf '%s' "$report" | tr '\n' ';')" + exit 0 +fi +exit 1 From fa0931735eff1aed4e16240cb9bac68a0140e0ea Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 6 Oct 2026 08:27:50 +0200 Subject: [PATCH 2/2] fix(deploy): refuse an empty kubelet reply and keep observe-only green An empty reply printed no differences and read as a match. Every refusal now goes through one exit, so observe-only cannot fail a deploy and skip the steps after it. Drift is reported on the first reading; only a read that did not complete is retried. Role files are read per document. Part of #3137 Co-Authored-By: Claude Opus 5.5 --- scripts/tests/test-kubelet-config-readback.sh | 54 +++++++++++++- scripts/verify-kubelet-config-readback.sh | 74 ++++++++++++------- 2 files changed, 100 insertions(+), 28 deletions(-) diff --git a/scripts/tests/test-kubelet-config-readback.sh b/scripts/tests/test-kubelet-config-readback.sh index c2d234ed0c..9983196934 100755 --- a/scripts/tests/test-kubelet-config-readback.sh +++ b/scripts/tests/test-kubelet-config-readback.sh @@ -25,6 +25,17 @@ if [[ -z "$stability_line" || -z "$readback_line" ]] || (( stability_line >= rea exit 1 fi +# The deploy step must stay observe-only until the switch is made on purpose, +# and must run only after a successful update and a stable API. +step=$(yq -o=json -I=0 '.runs.steps[] | select(.run == "./scripts/verify-kubelet-config-readback.sh")' "$deploy_action") +if [[ "$(jq -s 'length' <<<"$step")" != 1 ]] || + ! jq -e '.env.KUBELET_READBACK_ENFORCE == "false" and + (.if | contains("steps.cluster_update.outcome == \u0027success\u0027") and + contains("steps.wait_prod_api_stability.outcome == \u0027success\u0027"))' <<<"$step" >/dev/null; then + printf 'the deploy must run the kubelet readback once, observe-only, after a successful update\n' >&2 + exit 1 +fi + tmp_dir=$(mktemp -d) trap 'rm -rf "$tmp_dir"' EXIT @@ -37,6 +48,7 @@ set -euo pipefail [[ "$1 $2 $3" == "--context admin@prod --request-timeout=15s" ]] || exit 64 shift 3 if [[ "$*" == "get nodes -o json" ]]; then + [[ -z "${MOCK_NODES_FAIL:-}" ]] || exit 1 if [[ -n "${MOCK_FIRST_DIR:-}" && ! -f "$MOCK_DIR/first-used" ]]; then cat "$MOCK_FIRST_DIR/nodes.json" else @@ -145,6 +157,16 @@ run "$tmp_dir/refused" expect 'a node whose kubelet cannot be read fails' 1 \ 'autoscale-cx43-example: kubelet configuration could not be read' +# jq prints nothing for an empty document, so an empty reply must be refused +# before it is compared. +new_fixture "$tmp_dir/blank" +: >"$tmp_dir/blank/prod-worker-1.json" +printf ' \n' >"$tmp_dir/blank/prod-control-plane-2.json" +run "$tmp_dir/blank" +expect 'an empty or blank reply fails' 1 \ + 'prod-worker-1: kubelet configuration reply is empty' \ + 'prod-control-plane-2: kubelet configuration reply is empty' + new_fixture "$tmp_dir/garbage" printf 'not json\n' >"$tmp_dir/garbage/prod-worker-1.json" run "$tmp_dir/garbage" @@ -165,6 +187,9 @@ printf 'not json\n' >"$tmp_dir/unreadable/nodes.json" run "$tmp_dir/unreadable" expect 'an unreadable node list fails' 1 'Kubernetes nodes could not be read from context admin@prod' +run "$tmp_dir/healthy" MOCK_NODES_FAIL=1 +expect 'a failed node list read fails' 1 'Kubernetes nodes could not be read from context admin@prod' + mkdir "$tmp_dir/hostile" printf '{"items": [{"metadata": {"name": "a/../../b"}}]}\n' >"$tmp_dir/hostile/nodes.json" run "$tmp_dir/hostile" @@ -182,6 +207,12 @@ expect 'a node that answers on a later attempt passes' 0 'PASS: 3 node(s)' exit 1 } +# Drift cannot improve by waiting, so it is reported on the first reading. +run "$tmp_dir/drift" KUBELET_READBACK_ATTEMPTS=3 +expect 'drift is reported without waiting out the retry window' 1 'did not pass after 1 attempt(s)' +run "$tmp_dir/refused" KUBELET_READBACK_ATTEMPTS=3 +expect 'an unread node is retried until the last attempt' 1 'did not pass after 3 attempt(s)' + # Observe-only: the finding is reported as a warning and the deploy stays green. run "$tmp_dir/drift" KUBELET_READBACK_ENFORCE=false expect 'observe-only reports drift as a warning and exits 0' 0 \ @@ -194,7 +225,7 @@ if grep -qF '::warning' <<<"$output"; then exit 1 fi run "$tmp_dir/healthy" KUBELET_READBACK_ENFORCE=maybe -expect 'an unknown enforce value is refused' 2 'invalid kubelet readback' +expect 'an unknown enforce value is refused' 2 'invalid KUBELET_READBACK_ENFORCE' # Nothing declared means nothing was compared; that is not a pass. mkdir -p "$tmp_dir/talos-none/cluster" @@ -212,4 +243,25 @@ printf 'machine:\n kubelet:\n extraConfig:\n maxPods: 200\n' >"$tmp_dir run "$tmp_dir/healthy" TALOS_CONFIG_DIR="$tmp_dir/talos-role" expect 'a per-role kubelet setting is refused, not half-compared' 2 'are not modelled by this check' +# Observe-only must not fail the deploy on a refusal either: a red step skips +# the steps after it. +run "$tmp_dir/healthy" TALOS_CONFIG_DIR="$tmp_dir/talos-role" KUBELET_READBACK_ENFORCE=false +expect 'observe-only reports a refusal as a warning and exits 0' 0 \ + '::warning title=Kubelet settings readback (observe-only)::' 'are not modelled by this check' +run "$tmp_dir/healthy" TALOS_CONFIG_DIR="$tmp_dir/talos-missing" KUBELET_READBACK_ENFORCE=false +expect 'observe-only reports a missing patch directory as a warning' 0 '::warning' + +# A role file with several documents and no kubelet settings is not a refusal. +mkdir -p "$tmp_dir/talos-multi/cluster" "$tmp_dir/talos-multi/workers" +cp "$root_dir"/talos/cluster/*.yaml "$tmp_dir/talos-multi/cluster/" +printf 'machine:\n network: {}\n---\nmachine:\n sysctls: {}\n' >"$tmp_dir/talos-multi/workers/two-documents.yaml" +run "$tmp_dir/healthy" TALOS_CONFIG_DIR="$tmp_dir/talos-multi" +expect 'a multi-document role file without kubelet settings passes' 0 'PASS: 3 node(s)' +printf 'machine:\n network: {}\n---\nmachine:\n kubelet:\n extraConfig:\n maxPods: 200\n' >"$tmp_dir/talos-multi/workers/two-documents.yaml" +run "$tmp_dir/healthy" TALOS_CONFIG_DIR="$tmp_dir/talos-multi" +expect 'a kubelet setting in a later document of a role file is refused' 2 'are not modelled by this check' +printf 'machine: [unclosed\n' >"$tmp_dir/talos-multi/workers/two-documents.yaml" +run "$tmp_dir/healthy" TALOS_CONFIG_DIR="$tmp_dir/talos-multi" +expect 'a role file that cannot be parsed is refused, not read as empty' 2 'to rule out per-role kubelet settings' + printf 'All kubelet settings readback cases passed.\n' diff --git a/scripts/verify-kubelet-config-readback.sh b/scripts/verify-kubelet-config-readback.sh index cd1b94f835..cc4460dbfa 100755 --- a/scripts/verify-kubelet-config-readback.sh +++ b/scripts/verify-kubelet-config-readback.sh @@ -33,41 +33,56 @@ max_seconds=${KUBELET_READBACK_MAX_SECONDS:-300} # starts that way until one clean production reading is on record (#3137). enforce=${KUBELET_READBACK_ENFORCE:-true} -[[ "$max_attempts" =~ ^[1-9][0-9]*$ && - "$interval_seconds" =~ ^(0|[1-9][0-9]*)$ && - "$max_seconds" =~ ^[1-9][0-9]*$ && - "$enforce" =~ ^(true|false)$ ]] || { - printf 'invalid kubelet readback attempt, interval, deadline, or enforce setting\n' >&2 +[[ "$enforce" =~ ^(true|false)$ ]] || { + printf 'invalid KUBELET_READBACK_ENFORCE: expected true or false\n' >&2 exit 2 } +# Every way this check can end without a pass goes through here, so that +# observe-only really cannot fail a deploy: a red step would skip the steps +# after it, and those reassert production credentials. +give_up() { # + printf '%s\n' "$2" >&2 + if [[ "$enforce" == false ]]; then + # One annotation, so the finding is visible on a deploy that stays green. + printf '::warning title=Kubelet settings readback (observe-only)::%s\n' \ + "$(printf '%s' "$2" | tr '\n' ';')" + exit 0 + fi + exit "$1" +} + +[[ "$max_attempts" =~ ^[1-9][0-9]*$ && + "$interval_seconds" =~ ^(0|[1-9][0-9]*)$ && + "$max_seconds" =~ ^[1-9][0-9]*$ ]] || + give_up 2 'invalid kubelet readback attempt, interval, or deadline setting' + shopt -s nullglob cluster_patches=("$talos_dir"/cluster/*.yaml) role_patches=("$talos_dir"/control-planes/*.yaml "$talos_dir"/workers/*.yaml) shopt -u nullglob -((${#cluster_patches[@]} > 0)) || { - printf 'no cluster-wide Talos patches found under %s/cluster\n' "$talos_dir" >&2 - exit 2 -} +((${#cluster_patches[@]} > 0)) || + give_up 2 "no cluster-wide Talos patches found under $talos_dir/cluster" # A per-role kubelet setting would make "every node" the wrong expectation. # Refuse it instead of comparing those nodes against half their declaration. +# A file may hold several documents, and a read that fails is not a zero. for patch in "${role_patches[@]}"; do - if [[ "$(yq eval '.machine.kubelet.extraConfig | length' "$patch")" != 0 ]]; then - printf 'per-role kubelet settings in %s are not modelled by this check\n' "$patch" >&2 - exit 2 - fi + role_settings=$(yq eval -o=json -I=0 '.machine.kubelet.extraConfig // {}' "$patch" 2>/dev/null | + jq -s 'map(length) | add // 0' 2>/dev/null) || + give_up 2 "could not read $patch to rule out per-role kubelet settings" + [[ "$role_settings" == 0 ]] || + give_up 2 "per-role kubelet settings in $patch are not modelled by this check" done # shellcheck disable=SC2016 # yq program, not shell declared=$(yq eval-all -o=json -I=0 \ '. as $doc ireduce ({}; . * ($doc.machine.kubelet.extraConfig // {}))' \ - "${cluster_patches[@]}") -jq -e 'type == "object" and length > 0' <<<"$declared" >/dev/null 2>&1 || { - printf 'no declared kubelet settings found under %s/cluster; nothing to compare\n' "$talos_dir" >&2 - exit 2 -} + "${cluster_patches[@]}" 2>/dev/null) || + give_up 2 "could not read the cluster-wide Talos patches under $talos_dir/cluster" +jq -e 'type == "object" and length > 0' <<<"$declared" >/dev/null 2>&1 || + give_up 2 "no declared kubelet settings found under $talos_dir/cluster; nothing to compare" # shellcheck disable=SC2016 # jq program, not shell compare=' @@ -95,11 +110,13 @@ compare=' | .[]' report='' +unread=false node_count=0 autoscaler_count=0 deadline_epoch=$(( $(date +%s) + max_seconds )) for ((attempt = 1; attempt <= max_attempts; attempt++)); do report='' + unread=false node_count=0 autoscaler_count=0 # A node the autoscaler has just registered may not answer yet. Never accept @@ -119,6 +136,13 @@ for ((attempt = 1; attempt <= max_attempts; attempt++)); do if ! configz=$("$kubectl_bin" --context "$kube_context" --request-timeout=15s \ get --raw "/api/v1/nodes/$node/proxy/configz" 2>/dev/null); then report+="$node: kubelet configuration could not be read"$'\n' + unread=true + continue + fi + # jq prints nothing for an empty reply, which would read as no drift. + if [[ -z "${configz//[[:space:]]/}" ]]; then + report+="$node: kubelet configuration reply is empty"$'\n' + unread=true continue fi if ! drift=$(jq -r --argjson declared "$declared" "$compare" <<<"$configz" 2>/dev/null); then @@ -132,6 +156,7 @@ for ((attempt = 1; attempt <= max_attempts; attempt++)); do ((node_count > 0)) || report='the cluster reported no nodes'$'\n' else report="Kubernetes nodes could not be read from context $kube_context"$'\n' + unread=true fi if [[ -z "$report" ]]; then @@ -140,17 +165,12 @@ for ((attempt = 1; attempt <= max_attempts; attempt++)); do exit 0 fi - if ((attempt == max_attempts || $(date +%s) >= deadline_epoch)); then + # Only a read that did not complete can improve by waiting. Drift is a + # finding, and waiting out the window would only delay reporting it. + if [[ "$unread" == false ]] || ((attempt == max_attempts || $(date +%s) >= deadline_epoch)); then break fi sleep "$interval_seconds" done -printf 'Kubelet settings readback did not pass after %d attempt(s):\n%s' "$attempt" "$report" >&2 -if [[ "$enforce" == false ]]; then - # One annotation, so the finding is visible on a deploy that stays green. - printf '::warning title=Kubelet settings readback (observe-only)::%s\n' \ - "$(printf '%s' "$report" | tr '\n' ';')" - exit 0 -fi -exit 1 +give_up 1 "Kubelet settings readback did not pass after $attempt attempt(s):"$'\n'"${report%$'\n'}"