From 8badba39438868ad330450521f0004863d5af837 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 1 Sep 2026 08:02:12 +0200 Subject: [PATCH 1/6] feat(tests): track and ratchet the control-plane request footprint (backend#2870) Co-Authored-By: Claude Opus 4.8 --- Makefile | 2 +- scripts/tests/control-plane-footprint.bats | 101 +++++++++++++++ scripts/tests/control-plane-footprint.sh | 135 ++++++++++++++++++++ scripts/tests/sum_control_plane_requests.py | 55 ++++++++ 4 files changed, 292 insertions(+), 1 deletion(-) create mode 100644 scripts/tests/control-plane-footprint.bats create mode 100755 scripts/tests/control-plane-footprint.sh create mode 100644 scripts/tests/sum_control_plane_requests.py diff --git a/Makefile b/Makefile index 1175ae38..a7babc47 100644 --- a/Makefile +++ b/Makefile @@ -350,7 +350,7 @@ lint-warnings: # # `|`-separated because each guard is a multi-word command. One entry per guard, # and this is the only place they are written down. -DRIFT_GUARDS := scripts/gen-manifest.sh --check|scripts/check-facts.sh --check|bash scripts/check-style.sh|bash scripts/tests/check-drift.sh|bash scripts/tests/env-vocabulary-agreement.sh|bash scripts/tests/telemetry-vocabulary-agreement.sh|bash scripts/tests/k3s-components-agreement.sh|bash scripts/tests/collector-class-a-agreement.sh|bash scripts/tests/openshift-scc-coverage.sh|bash scripts/tests/collector-offsets-persisted.sh|bash scripts/tests/node-agents-tenancy.sh|bash scripts/tests/node-agents-pull-secret.sh|bash scripts/tests/telemetry-token-agreement.sh|bash scripts/tests/node-agents-namespace-safety.sh|bash scripts/tests/automount-token-explicit.sh|bash scripts/tests/collector-redaction-floor.sh|bash scripts/tests/collector-redaction-derived.sh|bash scripts/tests/telemetry-token-bootstrap.sh|bash scripts/tests/node-jsonpath-agreement.sh|bash scripts/tests/preflight-not-privilege-gated.sh|bash scripts/tests/kubelet-arg-map-safety.sh|bash scripts/tests/kubelet-config-agreement.sh|bash scripts/tests/kubelet-config-mutations.sh|bash scripts/tests/cronjob-failures-are-readable.sh|bash scripts/tests/release-name-equals-namespace.sh|bash scripts/tests/helm-unittest-error-assertions.sh|bash scripts/tests/helm-unittest-gated.sh|bash scripts/tests/mirror-enumeration-complete.sh|bash scripts/tests/reparent-requires-rotation.sh|bash scripts/tests/edgeuser-drop-readiness-verdicts.sh|bash scripts/tests/regcred-migration-verdicts.sh|bash scripts/tests/client-credentials-have-a-secret-tier.sh|bash scripts/tests/gate-byenv-resolution.sh|bash scripts/tests/gpu-limit-semantics-documented.sh|bash scripts/tests/gate-default-prose-agreement.sh|bash scripts/tests/gate-default-prose-mutations.sh|bash scripts/tests/guards-survive-spaced-paths.sh|bash scripts/tests/auto-upgrade-inflight-vs-wedge.sh +DRIFT_GUARDS := scripts/gen-manifest.sh --check|scripts/check-facts.sh --check|bash scripts/check-style.sh|bash scripts/tests/check-drift.sh|bash scripts/tests/env-vocabulary-agreement.sh|bash scripts/tests/telemetry-vocabulary-agreement.sh|bash scripts/tests/k3s-components-agreement.sh|bash scripts/tests/collector-class-a-agreement.sh|bash scripts/tests/openshift-scc-coverage.sh|bash scripts/tests/collector-offsets-persisted.sh|bash scripts/tests/node-agents-tenancy.sh|bash scripts/tests/node-agents-pull-secret.sh|bash scripts/tests/telemetry-token-agreement.sh|bash scripts/tests/node-agents-namespace-safety.sh|bash scripts/tests/automount-token-explicit.sh|bash scripts/tests/collector-redaction-floor.sh|bash scripts/tests/collector-redaction-derived.sh|bash scripts/tests/telemetry-token-bootstrap.sh|bash scripts/tests/node-jsonpath-agreement.sh|bash scripts/tests/preflight-not-privilege-gated.sh|bash scripts/tests/kubelet-arg-map-safety.sh|bash scripts/tests/kubelet-config-agreement.sh|bash scripts/tests/kubelet-config-mutations.sh|bash scripts/tests/cronjob-failures-are-readable.sh|bash scripts/tests/release-name-equals-namespace.sh|bash scripts/tests/helm-unittest-error-assertions.sh|bash scripts/tests/helm-unittest-gated.sh|bash scripts/tests/mirror-enumeration-complete.sh|bash scripts/tests/reparent-requires-rotation.sh|bash scripts/tests/edgeuser-drop-readiness-verdicts.sh|bash scripts/tests/regcred-migration-verdicts.sh|bash scripts/tests/client-credentials-have-a-secret-tier.sh|bash scripts/tests/gate-byenv-resolution.sh|bash scripts/tests/gpu-limit-semantics-documented.sh|bash scripts/tests/gate-default-prose-agreement.sh|bash scripts/tests/gate-default-prose-mutations.sh|bash scripts/tests/guards-survive-spaced-paths.sh|bash scripts/tests/auto-upgrade-inflight-vs-wedge.sh|bash scripts/tests/control-plane-footprint.sh # EXPORTED, not interpolated. The recipe reads $$DRIFT_GUARDS from the # environment; it used to do `guards='$(DRIFT_GUARDS)'`, which Make expands diff --git a/scripts/tests/control-plane-footprint.bats b/scripts/tests/control-plane-footprint.bats new file mode 100644 index 00000000..a2b6d436 --- /dev/null +++ b/scripts/tests/control-plane-footprint.bats @@ -0,0 +1,101 @@ +#!/usr/bin/env bats +# The control-plane-footprint guard, tested (backend#2870). +# +# The guard runs in DRIFT_GUARDS (the required `Source-of-truth drift` job), so +# per this repo's rule it must have a test of its own rather than only its own +# green run against the real tree (backend#1729). Two things are exercised: +# +# 1 the summer (sum_control_plane_requests.py) parses a KNOWN manifest to a +# known total -- Gi->MiB, DaemonSet counted x1, Deployment x replicas, and +# Job/CronJob EXCLUDED. This is the arithmetic the whole guard rests on, and +# helm is not needed to test it: it reads rendered YAML on stdin. +# 2 the guard's RATCHET and FAIL-CLOSED branches, driven through the real +# script via TB_CP_FOOTPRINT_ROOT / the ceiling overrides -- never a copy. + +setup() { + HERE="$(cd "$(dirname "$BATS_TEST_FILENAME")" && pwd)" + SUMMER="$HERE/sum_control_plane_requests.py" + GUARD="$HERE/control-plane-footprint.sh" +} + +# A manifest with one of each shape, so the exclusions and multipliers are all +# exercised by a total that is wrong under any one of them. +fixture() { + cat <<'YAML' +apiVersion: apps/v1 +kind: Deployment +metadata: {name: a} +spec: + replicas: 2 + template: + spec: + containers: + - name: c1 + resources: {requests: {memory: 1Gi, cpu: 250m}} +--- +apiVersion: apps/v1 +kind: DaemonSet +metadata: {name: b} +spec: + template: + spec: + containers: + - name: c2 + resources: {requests: {memory: 128Mi, cpu: 50m}} +--- +apiVersion: batch/v1 +kind: Job +metadata: {name: hook} +spec: + template: + spec: + containers: + - name: probe + resources: {requests: {memory: 512Mi, cpu: 500m}} +YAML +} + +@test "summer: Gi->MiB, Deployment x replicas, DaemonSet x1, Job excluded" { + # Deployment 1Gi x2 = 2048 MiB, 250m x2 = 500m ; DaemonSet 128 MiB / 50m x1 + # Job (512 MiB / 500m) MUST NOT count. Expected: 2176 MiB / 550 m / 2 containers. + local f; f="$(mktemp)"; fixture > "$f" + run bash -c "python3 '$SUMMER' < '$f'" + rm -f "$f" + [ "$status" -eq 0 ] + [ "$output" = "2176 550 2" ] +} + +@test "summer: an empty render sums to zero containers (the guard treats that as a finding)" { + run bash -c "printf '' | python3 '$SUMMER'" + [ "$status" -eq 0 ] + [ "$output" = "0 0 0" ] +} + +@test "guard: the real render is at or under the recorded ceiling (green today)" { + run bash "$GUARD" + [ "$status" -eq 0 ] + [[ "$output" == *"control-plane-footprint: OK"* ]] +} + +@test "guard: RATCHET reddens when the footprint would exceed the ceiling" { + TB_CP_FOOTPRINT_MEM_CEIL=3000 run bash "$GUARD" + [ "$status" -eq 1 ] + [[ "$output" == *"exceed the recorded ceiling"* ]] +} + +@test "guard: it reports the gap against the reserve, not asserts schedulability" { + run bash "$GUARD" + [ "$status" -eq 0 ] + [[ "$output" == *"OVER"* || "$output" == *"headroom"* ]] +} + +@test "guard: FAIL-CLOSED when the installer reserve cannot be read" { + root="$(mktemp -d)" + mkdir -p "$root/client/ci" "$root/scripts/lib" + cp -r "$HERE/../../client/." "$root/client/" 2>/dev/null || true + : > "$root/scripts/lib/install-client-helm.sh" # present but carries no reserve constant + TB_CP_FOOTPRINT_ROOT="$root" run bash "$GUARD" + [ "$status" -eq 1 ] + [[ "$output" == *"could not read _TB_ENVELOPE_OVERHEAD"* ]] + rm -rf "$root" +} diff --git a/scripts/tests/control-plane-footprint.sh b/scripts/tests/control-plane-footprint.sh new file mode 100755 index 00000000..ef15ad09 --- /dev/null +++ b/scripts/tests/control-plane-footprint.sh @@ -0,0 +1,135 @@ +#!/usr/bin/env bash +# +# control-plane-footprint.sh — how much memory and CPU the chart's own control +# plane REQUESTS, summed from the rendered chart, ratcheted so it cannot grow +# worse without a conscious decision (backend#2870, part of RFC-BACKEND-664). +# +# WHY THIS EXISTS +# --------------- +# A training pod is sized `node_allocatable − overhead`, where `overhead` is a +# single embedded constant (`_TB_ENVELOPE_OVERHEAD_MEM_BYTES`, 3 GiB) meant to +# stand in for everything that is NOT the training pod: kubelet, the runtime, +# and this chart's control plane. backend#2870's finding is that NOTHING sums +# what the control plane actually requests, so nobody can tell whether that one +# constant still covers it. Grep the three consumers for `3008`, `platform +# footprint`, `control plane requests`: zero hits. The number was invisible. +# +# Measured here by rendering the chart: the steady-state control plane requests +# ~3136 MiB, already ABOVE the 3 GiB (3072 MiB) the envelope reserves for it. +# That 64 MiB overshoot is the memory half of the reason a training pod on a +# freshly-installed single-node edge can sit `Pending / Insufficient memory` +# (backend#2870). CPU fits: 900m requested against a 1000m reserve. +# +# WHAT THIS GUARD DOES, AND DELIBERATELY DOES NOT +# ----------------------------------------------- +# It does NOT assert schedulability (`footprint <= overhead`). That assertion is +# RED today by construction, and making it green needs measured, per-platform +# kubelet reservations (backend#2460) and/or a control-plane request trim under +# load (backend#2461) -- both empirical campaigns, neither a code change. Landing +# a red gate would train people to skip the tier (org rule 4). +# +# It RATCHETS instead: the footprint may not grow past the recorded ceiling. A +# chart change that adds a component or raises a request -- making an already +# tight fit worse, silently, on every edge -- reddens here and forces the author +# to see the schedulability cost and bump the ceiling on purpose. It also REPORTS +# the current gap against the reserve, so the number backend#2460/#2461 must +# close is tracked rather than rediscovered. +# +# IT DERIVES, IT DOES NOT RESTATE (backend#1729). The footprint is summed from +# `helm template`, the ground truth of what installs; the reserve is parsed from +# the installer's own embedded constant, not a second copy. The only written-down +# numbers are the ratchet ceilings, and a ceiling is a bound to hold under, not a +# restatement of the render -- raising a request is what moves it. +# +set -euo pipefail + +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# TB_CP_FOOTPRINT_ROOT lets this guard's own test point it at a fixture tree. +root="${TB_CP_FOOTPRINT_ROOT:-$(cd "$here/../.." && pwd)}" +chart="$root/client" +installer="$root/scripts/lib/install-client-helm.sh" + +# THE RATCHET CEILINGS. Current measured steady-state control-plane requests, +# consistent across every client/ci/*-values.yaml profile (2026-09-01). A change +# that pushes the footprint above either of these must raise the ceiling in the +# same PR -- which is the moment to weigh whether the training envelope can still +# afford it. Overridable so the guard's own test can drive a lower ceiling and +# watch a real render breach it. +MEM_CEIL_MIB="${TB_CP_FOOTPRINT_MEM_CEIL:-3136}" +CPU_CEIL_MILLI="${TB_CP_FOOTPRINT_CPU_CEIL:-900}" + +command -v helm >/dev/null 2>&1 || { echo "[ERROR] helm is required to render the chart footprint" >&2; exit 3; } +command -v python3 >/dev/null 2>&1 || { echo "[ERROR] python3 is required to sum the rendered requests" >&2; exit 3; } +[ -d "$chart" ] || { echo "[ERROR] no chart at $chart -- refusing to report a footprint I could not render" >&2; exit 1; } +[ -f "$installer" ] || { echo "[ERROR] no installer at $installer -- cannot read the reserve to compare against" >&2; exit 1; } + +# THE RESERVE, DERIVED from the installer's embedded constant, not restated. A +# `\b` word match on the exact assignment; missing it is a finding, not a pass. +reserve_bytes="$(sed -n 's/^_TB_ENVELOPE_OVERHEAD_MEM_BYTES=\([0-9]\{1,\}\).*/\1/p' "$installer" | head -1)" +reserve_cpu_milli="$(sed -n 's/^_TB_ENVELOPE_OVERHEAD_CPU_MILLI=\([0-9]\{1,\}\).*/\1/p' "$installer" | head -1)" +if [ -z "$reserve_bytes" ] || [ -z "$reserve_cpu_milli" ]; then + echo "[ERROR] could not read _TB_ENVELOPE_OVERHEAD_{MEM_BYTES,CPU_MILLI} from $installer." >&2 + echo " The reserve moved or was renamed; this guard compares against it and cannot 'cannot tell' into a pass." >&2 + exit 1 +fi +reserve_mib=$(( reserve_bytes / 1024 / 1024 )) + +profiles=("$chart"/ci/*-values.yaml) +[ -e "${profiles[0]}" ] || { echo "[ERROR] no client/ci/*-values.yaml to render against -- this guard would check nothing" >&2; exit 1; } + +worst_mem=0 +worst_cpu=0 +fail=0 +for vf in "${profiles[@]}"; do + prof="$(basename "$vf" -values.yaml)" + rendered="$(helm template be "$chart" --set image.tag=footprint-probe -f "$vf" 2>/dev/null || true)" + if [ -z "$rendered" ]; then + echo "[ERROR] $prof: the chart rendered nothing -- a footprint cannot be summed from an empty render." >&2 + fail=1 + continue + fi + summed="$(printf '%s' "$rendered" | python3 "$here/sum_control_plane_requests.py")" + read -r mem cpu n <<<"$summed" + if [ -z "${n:-}" ] || [ "${n:-0}" -eq 0 ]; then + echo "[ERROR] $prof: rendered workloads carried ZERO resource requests -- the sum walked nothing, which is not the same as a footprint of zero." >&2 + fail=1 + continue + fi + echo " $prof: control plane requests ${mem} MiB / ${cpu} m (from ${n} container(s))" + (( mem > worst_mem )) && worst_mem=$mem + (( cpu > worst_cpu )) && worst_cpu=$cpu +done + +[ "$fail" -eq 0 ] || exit 1 + +# THE RATCHET. +if (( worst_mem > MEM_CEIL_MIB )); then + echo "[ERROR] control-plane memory requests ${worst_mem} MiB exceed the recorded ceiling ${MEM_CEIL_MIB} MiB." >&2 + echo " A chart change raised the footprint. Every MiB here is a MiB the training envelope loses on every edge," >&2 + echo " and the footprint is ALREADY above the ${reserve_mib} MiB reserve (backend#2870). If the increase is intended," >&2 + echo " raise TB_CP_FOOTPRINT_MEM_CEIL's default in this file -- and weigh whether backend#2460/#2461 must land first." >&2 + fail=1 +fi +if (( worst_cpu > CPU_CEIL_MILLI )); then + echo "[ERROR] control-plane cpu requests ${worst_cpu} m exceed the recorded ceiling ${CPU_CEIL_MILLI} m (backend#2870)." >&2 + echo " Raise TB_CP_FOOTPRINT_CPU_CEIL's default in this file if intended." >&2 + fail=1 +fi +[ "$fail" -eq 0 ] || exit 1 + +# THE GAP, reported not asserted (see the header). Positive = the control plane +# already out-requests its reserve; that overshoot is what backend#2460 (measured +# kubelet reservation) and backend#2461 (control-plane trim) exist to close. +mem_gap=$(( worst_mem - reserve_mib )) +cpu_gap=$(( worst_cpu - reserve_cpu_milli )) +if (( mem_gap > 0 )); then + echo " note: the control plane requests ${worst_mem} MiB against a ${reserve_mib} MiB reserve -- ${mem_gap} MiB OVER." + echo " This is the memory half of the unschedulable gap (backend#2870); closing it is backend#2460/#2461, not this guard." +else + echo " note: control-plane memory ${worst_mem} MiB fits within the ${reserve_mib} MiB reserve (${mem_gap#-} MiB headroom)." +fi +(( cpu_gap > 0 )) \ + && echo " note: control-plane cpu ${worst_cpu} m is ${cpu_gap} m OVER the ${reserve_cpu_milli} m reserve." \ + || echo " note: control-plane cpu ${worst_cpu} m fits the ${reserve_cpu_milli} m reserve (${cpu_gap#-} m headroom)." + +echo "control-plane-footprint: OK -- ${worst_mem} MiB / ${worst_cpu} m, at or under the recorded ceiling." diff --git a/scripts/tests/sum_control_plane_requests.py b/scripts/tests/sum_control_plane_requests.py new file mode 100644 index 00000000..fa816d5e --- /dev/null +++ b/scripts/tests/sum_control_plane_requests.py @@ -0,0 +1,55 @@ +#!/usr/bin/env python3 +"""Sum the STEADY-STATE control-plane resource requests from a rendered chart. + +Reads `helm template` output on stdin, prints " ". +Used by control-plane-footprint.sh (backend#2870); kept a separate file so the +render-and-sum can be unit-tested against a fixed manifest without helm. + +STEADY STATE ONLY -- Deployment / StatefulSet / DaemonSet. Not Job / CronJob: the +egress-reachability and storage-assertions hooks are one-shot conformance runs +that exit, so they are not resident beside a training pod. A DaemonSet is one +replica per node; on the single-node edge backend#2870 is about, that is x1. +""" +import sys +import re +import yaml + + +def mib(v): + if not v: + return 0.0 + m = re.match(r'^(\d+(?:\.\d+)?)(Ki|Mi|Gi|Ti)?$', str(v)) + if not m: + return 0.0 + unit = {'Ki': 1 / 1024, 'Mi': 1, 'Gi': 1024, 'Ti': 1024 * 1024}[m.group(2) or 'Mi'] + return float(m.group(1)) * unit + + +def milli(v): + if not v: + return 0.0 + v = str(v) + return float(v[:-1]) if v.endswith('m') else float(v) * 1000 + + +def main(): + mem = cpu = 0.0 + n = 0 + for d in yaml.safe_load_all(sys.stdin): + if not d: + continue + if d.get('kind') not in ('Deployment', 'StatefulSet', 'DaemonSet'): + continue + reps = 1 if d.get('kind') == 'DaemonSet' else (d.get('spec', {}).get('replicas', 1) or 1) + sp = d.get('spec', {}).get('template', {}).get('spec', {}) or {} + for c in (sp.get('containers', []) or []) + (sp.get('initContainers', []) or []): + req = (c.get('resources', {}) or {}).get('requests') or {} + if req.get('memory') or req.get('cpu'): + n += 1 + mem += mib(req.get('memory')) * reps + cpu += milli(req.get('cpu')) * reps + print(f"{mem:.0f} {cpu:.0f} {n}") + + +if __name__ == '__main__': + main() From 174481c8ba30d83b2cd1dc2d4648002160fac653 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 1 Sep 2026 08:28:37 +0200 Subject: [PATCH 2/6] fix(tests): inline the footprint summer with a PyYAML preflight, fail closed on a failed render (backend#2870) Co-Authored-By: Claude Opus 4.8 --- scripts/tests/control-plane-footprint.bats | 90 +++++++++------------ scripts/tests/control-plane-footprint.sh | 83 ++++++++++++++++++- scripts/tests/sum_control_plane_requests.py | 55 ------------- 3 files changed, 119 insertions(+), 109 deletions(-) delete mode 100644 scripts/tests/sum_control_plane_requests.py diff --git a/scripts/tests/control-plane-footprint.bats b/scripts/tests/control-plane-footprint.bats index a2b6d436..3e757fa4 100644 --- a/scripts/tests/control-plane-footprint.bats +++ b/scripts/tests/control-plane-footprint.bats @@ -3,99 +3,89 @@ # # The guard runs in DRIFT_GUARDS (the required `Source-of-truth drift` job), so # per this repo's rule it must have a test of its own rather than only its own -# green run against the real tree (backend#1729). Two things are exercised: +# green run against the real tree (backend#1729). # -# 1 the summer (sum_control_plane_requests.py) parses a KNOWN manifest to a -# known total -- Gi->MiB, DaemonSet counted x1, Deployment x replicas, and -# Job/CronJob EXCLUDED. This is the arithmetic the whole guard rests on, and -# helm is not needed to test it: it reads rendered YAML on stdin. -# 2 the guard's RATCHET and FAIL-CLOSED branches, driven through the real -# script via TB_CP_FOOTPRINT_ROOT / the ceiling overrides -- never a copy. +# Every assertion ends in `|| return 1`: bats-hygiene.bats requires it, and on +# bash 3.2 a bare `[[ … ]]` as a test's last statement can pass vacuously. +# +# The ARITHMETIC is driven through TB_CP_FOOTPRINT_FIXTURE -- a known manifest fed +# to the real guard in place of a helm render -- so the sum (Gi->MiB, replicas, +# DaemonSet x1, Job excluded) is exercised without helm. The RATCHET and +# FAIL-CLOSED branches are driven through the real script via its env overrides. setup() { HERE="$(cd "$(dirname "$BATS_TEST_FILENAME")" && pwd)" - SUMMER="$HERE/sum_control_plane_requests.py" GUARD="$HERE/control-plane-footprint.sh" } -# A manifest with one of each shape, so the exclusions and multipliers are all -# exercised by a total that is wrong under any one of them. -fixture() { - cat <<'YAML' +# One of each shape, so the exclusions and multipliers all matter: Deployment 1Gi +# x2 = 2048 MiB / 500m ; DaemonSet 128 MiB / 50m x1 ; Job (512 MiB / 500m) EXCLUDED. +# Expected steady-state total: 2176 MiB / 550 m across 2 requesting containers. +write_fixture() { + cat > "$1" <<'YAML' apiVersion: apps/v1 kind: Deployment metadata: {name: a} spec: replicas: 2 - template: - spec: - containers: - - name: c1 - resources: {requests: {memory: 1Gi, cpu: 250m}} + template: {spec: {containers: [{name: c1, resources: {requests: {memory: 1Gi, cpu: 250m}}}]}} --- apiVersion: apps/v1 kind: DaemonSet metadata: {name: b} spec: - template: - spec: - containers: - - name: c2 - resources: {requests: {memory: 128Mi, cpu: 50m}} + template: {spec: {containers: [{name: c2, resources: {requests: {memory: 128Mi, cpu: 50m}}}]}} --- apiVersion: batch/v1 kind: Job metadata: {name: hook} spec: - template: - spec: - containers: - - name: probe - resources: {requests: {memory: 512Mi, cpu: 500m}} + template: {spec: {containers: [{name: probe, resources: {requests: {memory: 512Mi, cpu: 500m}}}]}} YAML } -@test "summer: Gi->MiB, Deployment x replicas, DaemonSet x1, Job excluded" { - # Deployment 1Gi x2 = 2048 MiB, 250m x2 = 500m ; DaemonSet 128 MiB / 50m x1 - # Job (512 MiB / 500m) MUST NOT count. Expected: 2176 MiB / 550 m / 2 containers. - local f; f="$(mktemp)"; fixture > "$f" - run bash -c "python3 '$SUMMER' < '$f'" - rm -f "$f" - [ "$status" -eq 0 ] - [ "$output" = "2176 550 2" ] -} - -@test "summer: an empty render sums to zero containers (the guard treats that as a finding)" { - run bash -c "printf '' | python3 '$SUMMER'" - [ "$status" -eq 0 ] - [ "$output" = "0 0 0" ] +@test "arithmetic: Gi->MiB, Deployment x replicas, DaemonSet x1, Job excluded" { + local fx; fx="$(mktemp)"; write_fixture "$fx" + # A ceiling above the fixture total keeps the ratchet green; we assert the SUM. + TB_CP_FOOTPRINT_FIXTURE="$fx" TB_CP_FOOTPRINT_MEM_CEIL=9999 TB_CP_FOOTPRINT_CPU_CEIL=9999 run bash "$GUARD" + rm -f "$fx" + [ "$status" -eq 0 ] || { echo "$output"; return 1; } + printf '%s\n' "$output" | grep -q '2176 MiB / 550 m' || { echo "wrong sum: $output"; return 1; } } @test "guard: the real render is at or under the recorded ceiling (green today)" { run bash "$GUARD" - [ "$status" -eq 0 ] - [[ "$output" == *"control-plane-footprint: OK"* ]] + [ "$status" -eq 0 ] || { echo "$output"; return 1; } + printf '%s\n' "$output" | grep -q 'control-plane-footprint: OK' || { echo "$output"; return 1; } } @test "guard: RATCHET reddens when the footprint would exceed the ceiling" { TB_CP_FOOTPRINT_MEM_CEIL=3000 run bash "$GUARD" - [ "$status" -eq 1 ] - [[ "$output" == *"exceed the recorded ceiling"* ]] + [ "$status" -eq 1 ] || { echo "expected exit 1, got $status: $output"; return 1; } + printf '%s\n' "$output" | grep -q 'exceed the recorded ceiling' || { echo "$output"; return 1; } } @test "guard: it reports the gap against the reserve, not asserts schedulability" { run bash "$GUARD" - [ "$status" -eq 0 ] - [[ "$output" == *"OVER"* || "$output" == *"headroom"* ]] + [ "$status" -eq 0 ] || { echo "$output"; return 1; } + printf '%s\n' "$output" | grep -qE 'OVER|headroom' || { echo "$output"; return 1; } +} + +@test "guard: FAIL-CLOSED when a render exits non-zero (partial/failed helm)" { + # A fixture path that does not exist makes `cat` in _render_profile exit non-zero, + # standing in for a failed helm render -- which must refuse, not undercount. + TB_CP_FOOTPRINT_FIXTURE="/no/such/manifest.$$" run bash "$GUARD" + [ "$status" -eq 1 ] || { echo "expected exit 1, got $status: $output"; return 1; } + printf '%s\n' "$output" | grep -q 'exited non-zero' || { echo "$output"; return 1; } } @test "guard: FAIL-CLOSED when the installer reserve cannot be read" { - root="$(mktemp -d)" + local root; root="$(mktemp -d)" mkdir -p "$root/client/ci" "$root/scripts/lib" cp -r "$HERE/../../client/." "$root/client/" 2>/dev/null || true : > "$root/scripts/lib/install-client-helm.sh" # present but carries no reserve constant TB_CP_FOOTPRINT_ROOT="$root" run bash "$GUARD" - [ "$status" -eq 1 ] - [[ "$output" == *"could not read _TB_ENVELOPE_OVERHEAD"* ]] + [ "$status" -eq 1 ] || { echo "expected exit 1, got $status: $output"; rm -rf "$root"; return 1; } + printf '%s\n' "$output" | grep -q 'could not read _TB_ENVELOPE_OVERHEAD' || { echo "$output"; rm -rf "$root"; return 1; } rm -rf "$root" } diff --git a/scripts/tests/control-plane-footprint.sh b/scripts/tests/control-plane-footprint.sh index ef15ad09..05065817 100755 --- a/scripts/tests/control-plane-footprint.sh +++ b/scripts/tests/control-plane-footprint.sh @@ -65,8 +65,12 @@ command -v python3 >/dev/null 2>&1 || { echo "[ERROR] python3 is required to sum # THE RESERVE, DERIVED from the installer's embedded constant, not restated. A # `\b` word match on the exact assignment; missing it is a finding, not a pass. -reserve_bytes="$(sed -n 's/^_TB_ENVELOPE_OVERHEAD_MEM_BYTES=\([0-9]\{1,\}\).*/\1/p' "$installer" | head -1)" -reserve_cpu_milli="$(sed -n 's/^_TB_ENVELOPE_OVERHEAD_CPU_MILLI=\([0-9]\{1,\}\).*/\1/p' "$installer" | head -1)" +# awk, not `sed ... | head -1`: `head` closes the pipe early, which the org +# pipefail gate flags (SIGPIPE on the upstream would be masked under pipefail). +# awk `exit`s after the FIRST match -- one process, no pipe -- and is portable +# across the BSD sed on macOS and GNU sed in CI (the `{s;q}` sed form is not). +reserve_bytes="$(awk -F= '$1=="_TB_ENVELOPE_OVERHEAD_MEM_BYTES"{v=$2; sub(/[^0-9].*/,"",v); print v; exit}' "$installer")" +reserve_cpu_milli="$(awk -F= '$1=="_TB_ENVELOPE_OVERHEAD_CPU_MILLI"{v=$2; sub(/[^0-9].*/,"",v); print v; exit}' "$installer")" if [ -z "$reserve_bytes" ] || [ -z "$reserve_cpu_milli" ]; then echo "[ERROR] could not read _TB_ENVELOPE_OVERHEAD_{MEM_BYTES,CPU_MILLI} from $installer." >&2 echo " The reserve moved or was renamed; this guard compares against it and cannot 'cannot tell' into a pass." >&2 @@ -80,15 +84,86 @@ profiles=("$chart"/ci/*-values.yaml) worst_mem=0 worst_cpu=0 fail=0 +# The summer, INLINE in this .sh with a shell-level PyYAML preflight (Bugbot, +# backend#2870). A separate `.py` sidecar carried a top-level `import yaml` that +# `pyyaml-preflight.bats` does not scan (it reads `.sh`/`.bats`), so a runner with +# python3 but no PyYAML would die as a bare traceback. Inlined, the import lives in +# the heredoc the preflight guard already checks, and a missing module is a NAMED +# refusal. STEADY STATE ONLY -- Deployment/StatefulSet/DaemonSet, never Job/CronJob +# (the egress-reachability and storage-assertions hooks are one-shot and exit); a +# DaemonSet is one replica per node, x1 on the single-node edge this ticket is about. +# $1 = a file of rendered manifests. Read from the FILE, not stdin: `python3 - +# <<'PY'` already occupies stdin with the heredoc, so a piped manifest would never +# arrive -- the sibling guards pass their input as an argv for exactly this reason. +_sum_requests() { + python3 - "$1" <<'PY' +import sys, re +try: + import yaml +except ImportError: + sys.exit("[ERROR] PyYAML required (pip install pyyaml)") +def mib(v): + if not v: return 0.0 + m=re.match(r'^(\d+(?:\.\d+)?)(Ki|Mi|Gi|Ti)?$', str(v)) + if not m: return 0.0 + return float(m.group(1))*{'Ki':1/1024,'Mi':1,'Gi':1024,'Ti':1024*1024}[m.group(2) or 'Mi'] +def milli(v): + if not v: return 0.0 + v=str(v); return float(v[:-1]) if v.endswith('m') else float(v)*1000 +mem=cpu=0.0; n=0 +with open(sys.argv[1], encoding="utf-8") as fh: + for d in yaml.safe_load_all(fh): + if not d: continue + if d.get('kind') not in ('Deployment','StatefulSet','DaemonSet'): continue + reps=1 if d.get('kind')=='DaemonSet' else (d.get('spec',{}).get('replicas',1) or 1) + sp=d.get('spec',{}).get('template',{}).get('spec',{}) or {} + for c in (sp.get('containers',[]) or [])+(sp.get('initContainers',[]) or []): + r=(c.get('resources',{}) or {}).get('requests') or {} + if r.get('memory') or r.get('cpu'): n+=1 + mem+=mib(r.get('memory'))*reps; cpu+=milli(r.get('cpu'))*reps +print(f"{mem:.0f} {cpu:.0f} {n}") +PY +} + +# Render one profile, or -- when TB_CP_FOOTPRINT_FIXTURE points at a manifest -- +# read that instead, so the arithmetic can be tested on a KNOWN render without helm. +_render_profile() { + local vf="$1" + if [ -n "${TB_CP_FOOTPRINT_FIXTURE:-}" ]; then + cat "$TB_CP_FOOTPRINT_FIXTURE" + return # propagate cat's status -- an unreadable fixture is a failed render + fi + helm template be "$chart" --set image.tag=footprint-probe -f "$vf" +} + for vf in "${profiles[@]}"; do prof="$(basename "$vf" -values.yaml)" - rendered="$(helm template be "$chart" --set image.tag=footprint-probe -f "$vf" 2>/dev/null || true)" + # A FAILED render is not a smaller footprint (Bugbot, backend#2870). helm streams + # documents, so a template error partway through still emits YAML for the earlier + # resources -- an undercount that would sit under the ceiling and print OK while + # hiding the error. Capture helm's EXIT CODE and refuse on non-zero, rather than + # `|| true`-ing it into a success. + rendered="" + if ! rendered="$(_render_profile "$vf" 2>/tmp/cp-footprint-helm.$$)"; then + echo "[ERROR] $prof: the render exited non-zero, so its footprint is incomplete: $(tail -1 "/tmp/cp-footprint-helm.$$" 2>/dev/null)" >&2 + rm -f "/tmp/cp-footprint-helm.$$" + fail=1 + continue + fi + rm -f "/tmp/cp-footprint-helm.$$" if [ -z "$rendered" ]; then echo "[ERROR] $prof: the chart rendered nothing -- a footprint cannot be summed from an empty render." >&2 fail=1 continue fi - summed="$(printf '%s' "$rendered" | python3 "$here/sum_control_plane_requests.py")" + # `_sum_requests` exits non-zero (the PyYAML refusal) rather than printing a sum; + # propagate that as a finding instead of reading an empty `summed` as zero. + mf="$(mktemp)"; printf '%s' "$rendered" > "$mf" + if ! summed="$(_sum_requests "$mf")"; then + echo "[ERROR] $prof: could not sum the rendered requests ($summed)." >&2 + rm -f "$mf"; fail=1; continue + fi + rm -f "$mf" read -r mem cpu n <<<"$summed" if [ -z "${n:-}" ] || [ "${n:-0}" -eq 0 ]; then echo "[ERROR] $prof: rendered workloads carried ZERO resource requests -- the sum walked nothing, which is not the same as a footprint of zero." >&2 diff --git a/scripts/tests/sum_control_plane_requests.py b/scripts/tests/sum_control_plane_requests.py deleted file mode 100644 index fa816d5e..00000000 --- a/scripts/tests/sum_control_plane_requests.py +++ /dev/null @@ -1,55 +0,0 @@ -#!/usr/bin/env python3 -"""Sum the STEADY-STATE control-plane resource requests from a rendered chart. - -Reads `helm template` output on stdin, prints " ". -Used by control-plane-footprint.sh (backend#2870); kept a separate file so the -render-and-sum can be unit-tested against a fixed manifest without helm. - -STEADY STATE ONLY -- Deployment / StatefulSet / DaemonSet. Not Job / CronJob: the -egress-reachability and storage-assertions hooks are one-shot conformance runs -that exit, so they are not resident beside a training pod. A DaemonSet is one -replica per node; on the single-node edge backend#2870 is about, that is x1. -""" -import sys -import re -import yaml - - -def mib(v): - if not v: - return 0.0 - m = re.match(r'^(\d+(?:\.\d+)?)(Ki|Mi|Gi|Ti)?$', str(v)) - if not m: - return 0.0 - unit = {'Ki': 1 / 1024, 'Mi': 1, 'Gi': 1024, 'Ti': 1024 * 1024}[m.group(2) or 'Mi'] - return float(m.group(1)) * unit - - -def milli(v): - if not v: - return 0.0 - v = str(v) - return float(v[:-1]) if v.endswith('m') else float(v) * 1000 - - -def main(): - mem = cpu = 0.0 - n = 0 - for d in yaml.safe_load_all(sys.stdin): - if not d: - continue - if d.get('kind') not in ('Deployment', 'StatefulSet', 'DaemonSet'): - continue - reps = 1 if d.get('kind') == 'DaemonSet' else (d.get('spec', {}).get('replicas', 1) or 1) - sp = d.get('spec', {}).get('template', {}).get('spec', {}) or {} - for c in (sp.get('containers', []) or []) + (sp.get('initContainers', []) or []): - req = (c.get('resources', {}) or {}).get('requests') or {} - if req.get('memory') or req.get('cpu'): - n += 1 - mem += mib(req.get('memory')) * reps - cpu += milli(req.get('cpu')) * reps - print(f"{mem:.0f} {cpu:.0f} {n}") - - -if __name__ == '__main__': - main() From cf410f00274b4e0594a83580271212ef41b28392 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 1 Sep 2026 08:59:22 +0200 Subject: [PATCH 3/6] fix(tests): parse the full k8s memory grammar and fail closed on the unparseable (backend#2870) Co-Authored-By: Claude Opus 4.8 --- scripts/tests/control-plane-footprint.bats | 39 +++++++++++++++ scripts/tests/control-plane-footprint.sh | 58 ++++++++++++++++------ 2 files changed, 81 insertions(+), 16 deletions(-) diff --git a/scripts/tests/control-plane-footprint.bats b/scripts/tests/control-plane-footprint.bats index 3e757fa4..3fb79eb0 100644 --- a/scripts/tests/control-plane-footprint.bats +++ b/scripts/tests/control-plane-footprint.bats @@ -53,6 +53,45 @@ YAML printf '%s\n' "$output" | grep -q '2176 MiB / 550 m' || { echo "wrong sum: $output"; return 1; } } +@test "arithmetic: decimal-SI and plain-byte quantities are summed, not counted as zero" { + # 250M (decimal) = 238 MiB, 268435456 bytes = 256 MiB, on a DaemonSet (x1). + # The old Ki|Mi|Gi|Ti-only parser returned 0 for both -- an undercount that + # slips the ratchet (Bugbot, backend#2870). Total: 238 + 256 = 494 MiB. + local fx; fx="$(mktemp)" + cat > "$fx" <<'YAML' +apiVersion: apps/v1 +kind: DaemonSet +metadata: {name: dec} +spec: + template: {spec: {containers: [{name: c, resources: {requests: {memory: "250M"}}}]}} +--- +apiVersion: apps/v1 +kind: Deployment +metadata: {name: bytes} +spec: + template: {spec: {containers: [{name: c, resources: {requests: {memory: "268435456"}}}]}} +YAML + TB_CP_FOOTPRINT_FIXTURE="$fx" TB_CP_FOOTPRINT_MEM_CEIL=9999 TB_CP_FOOTPRINT_CPU_CEIL=9999 run bash "$GUARD" + rm -f "$fx" + [ "$status" -eq 0 ] || { echo "$output"; return 1; } + printf '%s\n' "$output" | grep -q '494 MiB' || { echo "wrong sum: $output"; return 1; } +} + +@test "arithmetic: an UNPARSEABLE quantity fails closed, never sums to zero" { + local fx; fx="$(mktemp)" + cat > "$fx" <<'YAML' +apiVersion: apps/v1 +kind: Deployment +metadata: {name: bad} +spec: + template: {spec: {containers: [{name: c, resources: {requests: {memory: "25Xy"}}}]}} +YAML + TB_CP_FOOTPRINT_FIXTURE="$fx" run bash "$GUARD" + rm -f "$fx" + [ "$status" -eq 1 ] || { echo "expected exit 1, got $status: $output"; return 1; } + printf '%s\n' "$output" | grep -q 'cannot parse' || { echo "$output"; return 1; } +} + @test "guard: the real render is at or under the recorded ceiling (green today)" { run bash "$GUARD" [ "$status" -eq 0 ] || { echo "$output"; return 1; } diff --git a/scripts/tests/control-plane-footprint.sh b/scripts/tests/control-plane-footprint.sh index 05065817..809453b2 100755 --- a/scripts/tests/control-plane-footprint.sh +++ b/scripts/tests/control-plane-footprint.sh @@ -102,25 +102,51 @@ try: import yaml except ImportError: sys.exit("[ERROR] PyYAML required (pip install pyyaml)") +# The FULL Kubernetes memory-quantity grammar, and FAIL CLOSED on anything else +# (Bugbot, backend#2870). The first version matched only Ki|Mi|Gi|Ti and silently +# returned 0 otherwise -- so a valid `250M` (decimal) or a plain-byte integer would +# count as ZERO, undercount the footprint and slip the ratchet, which is the exact +# blind spot this guard exists to remove. A quantity that is present but +# unrecognised is a finding, not a free pass: raise so the guard refuses. +_BIN={'Ki':2**10,'Mi':2**20,'Gi':2**30,'Ti':2**40,'Pi':2**50,'Ei':2**60} +_DEC={'k':1e3,'M':1e6,'G':1e9,'T':1e12,'P':1e15,'E':1e18} +_MIB=2**20 def mib(v): - if not v: return 0.0 - m=re.match(r'^(\d+(?:\.\d+)?)(Ki|Mi|Gi|Ti)?$', str(v)) - if not m: return 0.0 - return float(m.group(1))*{'Ki':1/1024,'Mi':1,'Gi':1024,'Ti':1024*1024}[m.group(2) or 'Mi'] + if v in (None, ''): return 0.0 + s=str(v).strip() + m=re.match(r'^(\d+(?:\.\d+)?(?:[eE][-+]?\d+)?)([A-Za-z]+)?$', s) + if not m: + raise ValueError("unparseable memory quantity %r" % v) + num=float(m.group(1)); suf=m.group(2) + if suf is None: bytes_=num # a bare number is BYTES (k8s) + elif suf in _BIN: bytes_=num*_BIN[suf] + elif suf in _DEC: bytes_=num*_DEC[suf] + else: raise ValueError("unknown memory unit %r in %r" % (suf, v)) + return bytes_/_MIB def milli(v): - if not v: return 0.0 - v=str(v); return float(v[:-1]) if v.endswith('m') else float(v)*1000 + if v in (None, ''): return 0.0 + s=str(v).strip() + if s.endswith('m'): + try: return float(s[:-1]) + except ValueError: raise ValueError("unparseable cpu quantity %r" % v) + try: return float(s)*1000 # cores -> millicores + except ValueError: raise ValueError("unparseable cpu quantity %r" % v) mem=cpu=0.0; n=0 -with open(sys.argv[1], encoding="utf-8") as fh: - for d in yaml.safe_load_all(fh): - if not d: continue - if d.get('kind') not in ('Deployment','StatefulSet','DaemonSet'): continue - reps=1 if d.get('kind')=='DaemonSet' else (d.get('spec',{}).get('replicas',1) or 1) - sp=d.get('spec',{}).get('template',{}).get('spec',{}) or {} - for c in (sp.get('containers',[]) or [])+(sp.get('initContainers',[]) or []): - r=(c.get('resources',{}) or {}).get('requests') or {} - if r.get('memory') or r.get('cpu'): n+=1 - mem+=mib(r.get('memory'))*reps; cpu+=milli(r.get('cpu'))*reps +try: + with open(sys.argv[1], encoding="utf-8") as fh: + for d in yaml.safe_load_all(fh): + if not d: continue + if d.get('kind') not in ('Deployment','StatefulSet','DaemonSet'): continue + reps=1 if d.get('kind')=='DaemonSet' else (d.get('spec',{}).get('replicas',1) or 1) + sp=d.get('spec',{}).get('template',{}).get('spec',{}) or {} + for c in (sp.get('containers',[]) or [])+(sp.get('initContainers',[]) or []): + r=(c.get('resources',{}) or {}).get('requests') or {} + if r.get('memory') or r.get('cpu'): n+=1 + mem+=mib(r.get('memory'))*reps; cpu+=milli(r.get('cpu'))*reps +except ValueError as exc: + # FAIL CLOSED: a quantity we cannot read is not a footprint of zero. Refusing + # is the whole posture of this guard -- a silent 0 would undercount and pass. + sys.exit("[ERROR] %s -- refusing to sum a footprint from a quantity this guard cannot parse" % exc) print(f"{mem:.0f} {cpu:.0f} {n}") PY } From 868a0d5c757bf485cc687656a39474063c2e4f64 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 1 Sep 2026 09:32:12 +0200 Subject: [PATCH 4/6] test(footprint): make the DaemonSet x1 arm falsifiable (backend#2870) The fixture DaemonSet had no replicas, so the special case and the missing-replicas fallback both yielded 1 and deleting the arm changed nothing. A stray replicas: 3 makes the two disagree. Co-Authored-By: Claude Opus 4.8 --- scripts/tests/control-plane-footprint.bats | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/scripts/tests/control-plane-footprint.bats b/scripts/tests/control-plane-footprint.bats index 3fb79eb0..977be7b0 100644 --- a/scripts/tests/control-plane-footprint.bats +++ b/scripts/tests/control-plane-footprint.bats @@ -19,8 +19,11 @@ setup() { } # One of each shape, so the exclusions and multipliers all matter: Deployment 1Gi -# x2 = 2048 MiB / 500m ; DaemonSet 128 MiB / 50m x1 ; Job (512 MiB / 500m) EXCLUDED. +# x2 = 2048 MiB / 500m ; DaemonSet 128 MiB / 50m x1 DESPITE carrying `replicas: 3` +# ; Job (512 MiB / 500m) EXCLUDED. # Expected steady-state total: 2176 MiB / 550 m across 2 requesting containers. +# Drop the DaemonSet special case and it becomes 2432 / 650 -- which is what makes +# the `x1` in this test's name a claim the fixture can actually falsify. write_fixture() { cat > "$1" <<'YAML' apiVersion: apps/v1 @@ -34,6 +37,17 @@ apiVersion: apps/v1 kind: DaemonSet metadata: {name: b} spec: + # A STRAY `replicas`, deliberately (Bugbot, #944). A DaemonSet has no replica + # count -- but this fixture carries one so the `x1` arm is actually TESTED. + # Without it the guard's DaemonSet branch and its missing-replicas fallback + # both yield 1, so deleting the special case changed nothing and the arm the + # test name advertises was pinned by nothing at all. + # + # With `replicas: 3` here the two disagree: the special case still counts the + # DaemonSet ONCE (total unchanged at 2176/550), while the fallback would count + # it three times (2432 MiB / 650 m). The expected total below is therefore the + # assertion that the special case is present and doing the work. + replicas: 3 template: {spec: {containers: [{name: c2, resources: {requests: {memory: 128Mi, cpu: 50m}}}]}} --- apiVersion: batch/v1 From ab756ead7401b39af4162beb67cc2ef9b454bfe4 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 1 Sep 2026 10:05:16 +0200 Subject: [PATCH 5/6] fix(tests): mktemp the footprint render stderr, not a PID-predictable path (backend#2870) Co-Authored-By: Claude Opus 4.8 --- scripts/tests/control-plane-footprint.sh | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/scripts/tests/control-plane-footprint.sh b/scripts/tests/control-plane-footprint.sh index 809453b2..0147ac33 100755 --- a/scripts/tests/control-plane-footprint.sh +++ b/scripts/tests/control-plane-footprint.sh @@ -169,14 +169,17 @@ for vf in "${profiles[@]}"; do # resources -- an undercount that would sit under the ceiling and print OK while # hiding the error. Capture helm's EXIT CODE and refuse on non-zero, rather than # `|| true`-ing it into a success. + # `mktemp`, not /tmp/...$$ (Bugbot Low): a PID-predictable path in a world- + # writable dir is a symlink-clobber vector and can collide; mktemp is unguessable. rendered="" - if ! rendered="$(_render_profile "$vf" 2>/tmp/cp-footprint-helm.$$)"; then - echo "[ERROR] $prof: the render exited non-zero, so its footprint is incomplete: $(tail -1 "/tmp/cp-footprint-helm.$$" 2>/dev/null)" >&2 - rm -f "/tmp/cp-footprint-helm.$$" + errf="$(mktemp "${TMPDIR:-/tmp}/cp-footprint-helm.XXXXXX")" + if ! rendered="$(_render_profile "$vf" 2>"$errf")"; then + echo "[ERROR] $prof: the render exited non-zero, so its footprint is incomplete: $(tail -1 "$errf" 2>/dev/null)" >&2 + rm -f "$errf" fail=1 continue fi - rm -f "/tmp/cp-footprint-helm.$$" + rm -f "$errf" if [ -z "$rendered" ]; then echo "[ERROR] $prof: the chart rendered nothing -- a footprint cannot be summed from an empty render." >&2 fail=1 From 9f5f103376355fa6c68527f866248a2cd1293aba Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 1 Sep 2026 10:11:01 +0200 Subject: [PATCH 6/6] fix(footprint): the scheduler max()es init containers, it does not add them (backend#2870) @aptracebloc nit 1, and it is more than a nit. A pod effective request per resource is max( sum(app containers), max(init containers) ): init containers run to completion ONE AT A TIME before the app containers start, so they are never resident alongside them. This summed both, which CONTRADICTED the guard own "STEADY STATE ONLY" note and inflated the figure the ceiling is compared against. Conservative is not harmless here -- this number is the input to backend#2460/#2461, so an inflated one sends that work after MiB that no scheduler ever reserves. PER-RESOURCE, not per-pod: k8s takes the max independently for memory and cpu. THE NUMBER DOES NOT MOVE: 3136 MiB / 900 m either way, because this chart init containers carry no requests. So the "64 MiB OVER" finding STANDS -- the method was wrong, the conclusion was not. Worth saying rather than leaving the doubt open. WHICH MEANS THE FIX IS INERT ON THE REAL RENDER, and a green run proves nothing about the formula. So the new fixture is built where the two answers disagree: app 300Mi/200m, init max 1000Mi/500m -> scheduler 1000 MiB / 500 m against sum-everything 1350 MiB / 750 m. BOTH numbers are asserted rather than "under the ceiling", which would pass on either formula. The 1350/750 figure was MEASURED by running the previous code in place, not computed by hand. Mutation-proved: max -> + reddens the fixture test. Only the formula, rebased onto ab756ea. My own mktemp fix for the same Bugbot Low is DISCARDED -- the co-driver got there first and theirs is better, using mktemp "${TMPDIR:-/tmp}/cp-footprint-helm.XXXXXX" rather than a bare mktemp. control-plane-footprint.bats 9 passing. gen-manifest --check clean, shellcheck -S warning -x clean, bash -n clean. The ~1600-test client bats suite is CI job. Co-Authored-By: Claude Opus 4.8 --- scripts/tests/control-plane-footprint.bats | 49 ++++++++++++++++++++++ scripts/tests/control-plane-footprint.sh | 33 ++++++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/scripts/tests/control-plane-footprint.bats b/scripts/tests/control-plane-footprint.bats index 977be7b0..50c61308 100644 --- a/scripts/tests/control-plane-footprint.bats +++ b/scripts/tests/control-plane-footprint.bats @@ -67,6 +67,55 @@ YAML printf '%s\n' "$output" | grep -q '2176 MiB / 550 m' || { echo "wrong sum: $output"; return 1; } } +@test "arithmetic: init containers are max()'d against the app sum, not added to it" { + # THE SCHEDULER'S FORMULA (@aptracebloc on client#944). A pod's effective request + # per resource is max( sum(app containers), max(init containers) ) -- init + # containers run to completion one at a time BEFORE the app containers, so they + # are never resident alongside them. Summing them both inflates the figure this + # guard hands to backend#2460/#2461. + # + # THIS FIXTURE EXISTS BECAUSE THE REAL CHART CANNOT TELL THE TWO APART. On the + # live render the number is 3136 MiB either way -- the chart's init containers + # carry no requests -- so the fix is INERT there and a green run proves nothing + # about the formula. Here the two answers differ, deliberately: + # + # app containers : 100Mi + 200Mi = 300Mi , 100m + 100m = 200m + # init containers : max(1000Mi, 50Mi) = 1000Mi , max(50m, 500m) = 500m + # sum-everything : 1350 MiB / 750 m <- the old, wrong answer + # scheduler : max(300,1000)=1000 MiB , max(200,500)=500 m + # + # And it is PER-RESOURCE: memory comes from the init side, cpu also from the init + # side here, but a fixture where they came from opposite sides would pass equally + # -- which is why both numbers are asserted rather than a single total. + local fx; fx="$(mktemp)" + cat > "$fx" <<'YAML' +apiVersion: apps/v1 +kind: Deployment +metadata: {name: cp} +spec: + replicas: 1 + template: + spec: + initContainers: + - name: migrate + resources: {requests: {memory: 1000Mi, cpu: 50m}} + - name: seed + resources: {requests: {memory: 50Mi, cpu: 500m}} + containers: + - name: api + resources: {requests: {memory: 100Mi, cpu: 100m}} + - name: sidecar + resources: {requests: {memory: 200Mi, cpu: 100m}} +YAML + TB_CP_FOOTPRINT_FIXTURE="$fx" TB_CP_FOOTPRINT_MEM_CEIL=9999 TB_CP_FOOTPRINT_CPU_CEIL=9999 run bash "$GUARD" + rm -f "$fx" + [ "$status" -eq 0 ] || { echo "$output"; return 1; } + # 1000 MiB / 500 m -- NOT 1350 / 750. Asserting the exact pair is what separates + # the two formulas; "under the ceiling" would pass on both. + printf '%s\n' "$output" | grep -q '1000 MiB / 500 m' || { + echo "expected 1000 MiB / 500 m (scheduler formula), got: $output"; return 1; } +} + @test "arithmetic: decimal-SI and plain-byte quantities are summed, not counted as zero" { # 250M (decimal) = 238 MiB, 268435456 bytes = 256 MiB, on a DaemonSet (x1). # The old Ki|Mi|Gi|Ti-only parser returned 0 for both -- an undercount that diff --git a/scripts/tests/control-plane-footprint.sh b/scripts/tests/control-plane-footprint.sh index 0147ac33..4b8c4e48 100755 --- a/scripts/tests/control-plane-footprint.sh +++ b/scripts/tests/control-plane-footprint.sh @@ -139,10 +139,39 @@ try: if d.get('kind') not in ('Deployment','StatefulSet','DaemonSet'): continue reps=1 if d.get('kind')=='DaemonSet' else (d.get('spec',{}).get('replicas',1) or 1) sp=d.get('spec',{}).get('template',{}).get('spec',{}) or {} - for c in (sp.get('containers',[]) or [])+(sp.get('initContainers',[]) or []): + # THE SCHEDULER'S FORMULA, NOT sum(everything) (@aptracebloc nit 1). + # + # A pod's effective request per resource is + # max( sum(app containers), max(init containers) ) + # because init containers run to completion ONE AT A TIME before the + # app containers start -- they are never resident alongside them. + # Summing both CONTRADICTED this guard's own "STEADY STATE ONLY" note + # and inflated the figure the ceiling is compared against. Conservative + # is not harmless here: this number is the input to backend#2460/#2461, + # so an inflated one sends that work after MiB no scheduler reserves. + # + # PER-RESOURCE, not per-pod: k8s takes the max independently for memory + # and cpu, so a pod can take its memory from the init side and its cpu + # from the app side. + # + # THE NUMBER DOES NOT MOVE ON THIS CHART -- 3136 MiB / 900 m either way, + # because these init containers carry no requests. So the "64 MiB OVER" + # finding stands; the method was wrong, the conclusion was not. It also + # means the fix is INERT on the real render, which is why the bats + # fixture is built so the two formulas disagree. + def _req(c, key): + return ((c.get('resources', {}) or {}).get('requests') or {}).get(key) + apps = sp.get('containers', []) or [] + inits = sp.get('initContainers', []) or [] + csum_m = sum(mib(_req(c, 'memory')) for c in apps) + csum_c = sum(milli(_req(c, 'cpu')) for c in apps) + imax_m = max([mib(_req(c, 'memory')) for c in inits] or [0.0]) + imax_c = max([milli(_req(c, 'cpu')) for c in inits] or [0.0]) + for c in apps + inits: r=(c.get('resources',{}) or {}).get('requests') or {} if r.get('memory') or r.get('cpu'): n+=1 - mem+=mib(r.get('memory'))*reps; cpu+=milli(r.get('cpu'))*reps + mem += max(csum_m, imax_m)*reps + cpu += max(csum_c, imax_c)*reps except ValueError as exc: # FAIL CLOSED: a quantity we cannot read is not a footprint of zero. Refusing # is the whole posture of this guard -- a silent 0 would undercount and pass.