diff --git a/hack/e2e-chainsaw/_lib/talos-image-cache.sh b/hack/e2e-chainsaw/_lib/talos-image-cache.sh index 670d692f7..f15dd98f3 100644 --- a/hack/e2e-chainsaw/_lib/talos-image-cache.sh +++ b/hack/e2e-chainsaw/_lib/talos-image-cache.sh @@ -144,18 +144,124 @@ _talos_image_cache_reachable_from_tenant() { _talos_image_cache_probe_succeeded "$out" } +# _talos_image_cache_deploy_state: answer "is the mirror Deployment there?" as +# exactly one of present, absent or unknown. +# +# --ignore-not-found is what makes a three-way answer possible: a genuine absence +# becomes exit 0 with no output, which leaves a non-zero exit meaning only "the +# question was never answered" — connection refused, Unauthorized, timed out. +# A plain `get` collapses those two into one exit code, and the caller below then +# reads a blip as an absence, caches it, and sends every tenant worker in the +# suite to the public factory over the live egress the mirror exists to avoid. +# Same idiom as the teardown poll in hack/e2e-chainsaw/_lib/etcd-cleanup.sh. +# +# talos_image_cache_diagnose asks the same question and answers it its own way, +# and the two are separate on purpose. That one runs on the node-join failure +# path under a phase budget, where re-asking spends what the crust-gather +# snapshot after it needs, and it gives up after one attempt. This one runs on +# the happy path, where a re-ask costs a suite nothing and buys it the mirror. +# +# The re-ask is small, and each attempt has to be bounded twice over for the +# re-ask to be worth anything. kubectl's --request-timeout defaults to 0 — no +# deadline on the request at all — and the case this loop exists for is a +# connection that establishes and then stalls, which client-go's dial timeout +# does not bound. The flag on its own is still not a wall-clock bound: against +# an unreachable apiserver kubectl retries discovery several times before it +# gives up, so a call carrying only the flag costs a multiple of it, which +# hack/e2e-chainsaw/_lib/pod-label-census.sh measured and sized its caller's op +# timeout around. Asking three times would inherit that multiple, inside a +# Chainsaw op the rest of the tenant test has to fit in. So each attempt takes +# both bounds, the pairing the reads in run-kubernetes.sh use and explain beside +# its pod list: --request-timeout bounds the HTTP request, timeout bounds the +# client retrying against a wedged apiserver. A kill by timeout exits non-zero +# and is simply another attempt that did not answer. Past that, an apiserver +# that is down does not become reachable by asking longer, and a run whose +# apiserver is down has bigger problems than which factory it pulls from. A +# real absence answers on the first try and waits for nothing. +# +# The wrapper is skipped rather than depended on when `timeout` is not installed, +# for the reason _talos_image_cache_bounded_read gives below and one more that is +# specific to a loop: its exit 127 would otherwise count as an attempt that did +# not answer, three times over, and a missing local binary would pin the whole +# suite to the public factory while every message blamed the apiserver. +# +# The outer bound sits ABOVE the inner one rather than level with it, and that +# gap is the whole reason kubectl ever gets to explain itself. Measured against a +# stub apiserver, the three ways this call fails are not alike. A refused port +# answers in under a second and names the refusal. One that completes TCP but +# stalls in TLS gets kubectl's own handshake deadline, which is shorter than +# either bound and fires twice inside the window, so there is text even though +# the wrapper ends up killing the call. A blackholed address — SYN dropped, no +# RST, which is what a wedged node or a netpol looks like — produces nothing at +# all until some deadline expires, and with the two bounds equal the expiry that +# arrives is the wall-clock kill, at the same instant as the request deadline it +# beats to it. `--request-timeout` then cannot fire in any of the three, which +# makes it decoration. Ten seconds of headroom hands that case back to kubectl: +# the request deadline ends the attempt and says why, and the wrapper stays as +# the backstop for a client that overruns its own deadline. The cost is ten more +# seconds per attempt on a path where the apiserver is already unreachable, +# against a 50m op. +# +# kubectl's stderr is therefore deliberately not swallowed: it is the only thing +# that separates refused from unauthorized from stalled, and suppressing it would +# replace one silent failure with another. It is still not guaranteed — a kill +# can always land before the client has said anything — so the last attempt's +# exit status is named too, in the vocabulary the diagnostic reads below use for +# 124 and 137. Between the two the caller's warning always has something real to +# point at. +_TALOS_IMAGE_CACHE_QUERY_TRIES="${_TALOS_IMAGE_CACHE_QUERY_TRIES:-3}" +_TALOS_IMAGE_CACHE_QUERY_DELAY="${_TALOS_IMAGE_CACHE_QUERY_DELAY:-5}" +_talos_image_cache_deploy_state() { + local out rc try=1 + while :; do + rc=0 + if command -v timeout >/dev/null 2>&1; then + out=$(timeout -k 5 40 kubectl -n kube-system get deploy talos-image-cache \ + --request-timeout=30s --ignore-not-found -o name) || rc=$? + else + out=$(kubectl -n kube-system get deploy talos-image-cache \ + --request-timeout=30s --ignore-not-found -o name) || rc=$? + fi + if [ "$rc" -eq 0 ]; then + if [ -n "$out" ]; then + printf 'present' + else + printf 'absent' + fi + return 0 + fi + if [ "$try" -ge "$_TALOS_IMAGE_CACHE_QUERY_TRIES" ]; then + echo "WARNING: the talos-image-cache lookup gave up after ${_TALOS_IMAGE_CACHE_QUERY_TRIES} attempts; the last one exited ${rc} (124 = cut off at the wall-clock bound, 137 = SIGKILL after it, and both of those print nothing of their own; any other status is kubectl's, and its message is above)" >&2 + printf 'unknown' + return 0 + fi + try=$((try + 1)) + sleep "$_TALOS_IMAGE_CACHE_QUERY_DELAY" + done +} + # resolve_talos_image_factory_url: print the imageFactoryURL to use, or an empty -# string to signal "use the chart default". The decision is resolved once (with a +# string to signal "use the chart default". A decision that was actually reached +# — the mirror is there, or it demonstrably is not — is taken once (with a # bounded wait for the mirror to finish seeding) and cached in a /tmp file that # persists across bats files (they all exec in the same sandbox container), so -# only the first tenant Kubernetes test pays the readiness wait. +# only the first tenant Kubernetes test pays the readiness wait. A lookup that +# never got an answer decides nothing and caches nothing; the next tenant test +# asks again. resolve_talos_image_factory_url() { if [ -f "$_TALOS_IMAGE_FACTORY_DECISION_FILE" ]; then cat "$_TALOS_IMAGE_FACTORY_DECISION_FILE" return 0 fi - local url="" - if kubectl -n kube-system get deploy talos-image-cache >/dev/null 2>&1; then + local url="" state + state=$(_talos_image_cache_deploy_state) + if [ "$state" = unknown ]; then + # Nothing was established, so nothing is cached: this caller falls back for + # itself and the next tenant test asks again. + echo "WARNING: could not determine whether the talos-image-cache mirror is deployed (the lookup's own account of why is above) — this test falls back to public factory.talos.dev; nothing is cached, so the next tenant test asks again" >&2 + return 0 + fi + if [ "$state" = present ]; then # Available only after the seed initContainer has fully fetched the image, so # this is the signal that the mirror is warm. if kubectl -n kube-system rollout status deploy/talos-image-cache --timeout=12m >/dev/null 2>&1; then diff --git a/hack/talos-image-cache_test.bats b/hack/talos-image-cache_test.bats index 6c7e94e67..55b4eeb25 100644 --- a/hack/talos-image-cache_test.bats +++ b/hack/talos-image-cache_test.bats @@ -17,13 +17,26 @@ # and namespace the egress policy selects. If either side drifts, the probe can # pass while real importers stay blocked (a false positive that makes CI worse). # -# Finally they cover the two pure fragments of the decision itself, sourced from +# Finally they cover the decision itself, sourced from # hack/e2e-chainsaw/_lib/talos-image-cache.sh (sourcing it has no cluster side -# effects — it only sets parameter defaults and defines functions): the strict -# 206 gate that decides whether tenants are pointed at the mirror at all, and the -# --overrides JSON builder whose silent breakage would drop every probe to the -# public factory. The kubectl orchestration around them needs a live cluster and -# is covered by the e2e run, matching how the other hack/ helpers are tested. +# effects — it only sets parameter defaults and defines functions). Two pure +# fragments are exercised directly: the strict 206 gate that decides whether +# tenants are pointed at the mirror at all, and the --overrides JSON builder +# whose silent breakage would drop every probe to the public factory. The +# kubectl orchestration around them is driven here too, against a throwaway +# `kubectl` and `timeout` placed on PATH — the decision of which factory a whole +# suite pulls from is worth pinning without waiting for a cluster. +# +# On PATH, not shell functions, and that is not a style choice: the lookup runs +# `timeout -k 5 40 kubectl …` wherever timeout is installed, and an external +# timeout binary execs a real file. +# A `kubectl() { … }` override is invisible to it, so the stub would never be +# reached, and a suite that stubs that way goes quietly vacuous. A stub on PATH +# is also the only way to assert how the real call was spelled, which matters +# for the flags the three-way answer depends on: the stub answers exit 0 with no +# output for an absence no matter what flags it is handed, while a real kubectl +# does that only with --ignore-not-found, so that flag is pinned by inspecting +# the recorded call rather than by the behaviour around it. # # Root cause + fix are from cozystack/cozystack#3254 (@lexfrei); this is the port # to the Chainsaw layout (hack/e2e-chainsaw/_lib/). @@ -124,6 +137,254 @@ done } +_stub_kubectl_dir() { + # Build a throwaway PATH entry whose `kubectl get deploy ... -o name` fails + # its first $2 invocations and then answers with $3 on stdout (empty for a + # genuine absence). $1 is the scratch dir. printf, not a heredoc: cozytest.sh + # translates the file with awk and would read a @test or a bare `}` inside a + # heredoc as real syntax. + mkdir -p "$1/bin" + printf '%s\n' \ + '#!/bin/sh' \ + 'printf "%s\n" "$*" >> "$STUB_CALLS"' \ + 'n=$(cat "$STUB_COUNTER")' \ + 'n=$((n + 1))' \ + 'printf "%s" "$n" > "$STUB_COUNTER"' \ + 'if [ "$n" -le "$STUB_FAILS" ]; then' \ + ' echo "error: the server was unable to return a response" >&2' \ + ' exit 1' \ + 'fi' \ + 'printf "%s" "$STUB_OUT"' \ + > "$1/bin/kubectl" + chmod +x "$1/bin/kubectl" + # A transparent `timeout` that records how it was invoked and then runs the + # command anyway, so a test can pin the wrapper without changing what the + # wrapped call does. Same idiom as hack/run-kubernetes-serial-console_test.bats. + # With STUB_TIMEOUT_RC set it instead exits that status without running the + # command and without printing, which is what a real kill looks like from the + # caller's side: the wrapper says nothing and the command never reaches the + # point where it would have. + printf '%s\n' \ + '#!/bin/sh' \ + 'printf "%s\n" "$*" >> "$STUB_TIMEOUT_CALLS"' \ + 'if [ -n "$STUB_TIMEOUT_RC" ]; then exit "$STUB_TIMEOUT_RC"; fi' \ + 'while [ $# -gt 0 ] && [ "$1" != kubectl ]; do shift; done' \ + 'exec "$@"' \ + > "$1/bin/timeout" + chmod +x "$1/bin/timeout" + printf '0' > "$1/counter" + : > "$1/calls" + : > "$1/timeout.calls" + export STUB_COUNTER="$1/counter" STUB_CALLS="$1/calls" \ + STUB_TIMEOUT_CALLS="$1/timeout.calls" STUB_FAILS="$2" STUB_OUT="$3" \ + STUB_TIMEOUT_RC="" + PATH="$1/bin:$PATH" +} + +@test "deployment lookup reports present when the mirror answers with a name" { + . hack/e2e-chainsaw/_lib/talos-image-cache.sh + d=$(mktemp -d) + _TALOS_IMAGE_CACHE_QUERY_DELAY=0 + _stub_kubectl_dir "$d" 0 'deployment.apps/talos-image-cache' + state=$(_talos_image_cache_deploy_state) + rm -rf "$d" + [ "$state" = present ] +} + +@test "deployment lookup reports absent only on an empty answer the apiserver actually gave" { + . hack/e2e-chainsaw/_lib/talos-image-cache.sh + d=$(mktemp -d) + _TALOS_IMAGE_CACHE_QUERY_DELAY=0 + # --ignore-not-found: a real absence is exit 0 with no output. This is the + # one outcome allowed to turn the whole suite toward the public factory. + _stub_kubectl_dir "$d" 0 '' + state=$(_talos_image_cache_deploy_state) + # The flag has to be pinned by inspection, because the stub gives exit 0 and + # no output whether or not the call asks for it. Drop it from the real call + # and kubectl reports a genuine absence as an error instead: `absent` becomes + # a state production can never reach, while this test keeps passing. + flagged=$(grep -c -- '--ignore-not-found' "$d/calls" || true) + rm -rf "$d" + [ "$state" = absent ] + [ "$flagged" = 1 ] +} + +@test "deployment lookup reports unknown when the apiserver never answers" { + . hack/e2e-chainsaw/_lib/talos-image-cache.sh + d=$(mktemp -d) + _TALOS_IMAGE_CACHE_QUERY_DELAY=0 + # Every attempt refused. Reading this as "absent" is the defect: a blip would + # send every tenant worker in the suite to the public factory. + _stub_kubectl_dir "$d" 99 'deployment.apps/talos-image-cache' + state=$(_talos_image_cache_deploy_state) + rm -rf "$d" + [ "$state" = unknown ] +} + +@test "deployment lookup rides out a blip and reports what a later attempt found" { + . hack/e2e-chainsaw/_lib/talos-image-cache.sh + d=$(mktemp -d) + _TALOS_IMAGE_CACHE_QUERY_DELAY=0 + _stub_kubectl_dir "$d" 1 'deployment.apps/talos-image-cache' + state=$(_talos_image_cache_deploy_state) + tries=$(cat "$d/counter") + rm -rf "$d" + [ "$state" = present ] + [ "$tries" = 2 ] +} + +@test "every attempt at the lookup carries a request deadline" { + . hack/e2e-chainsaw/_lib/talos-image-cache.sh + d=$(mktemp -d) + _TALOS_IMAGE_CACHE_QUERY_DELAY=0 + _stub_kubectl_dir "$d" 99 'deployment.apps/talos-image-cache' + state=$(_talos_image_cache_deploy_state) + # kubectl's --request-timeout defaults to 0, so a connection that + # establishes and then stalls hangs with no deadline, and re-asking would + # multiply that hang instead of riding out a blip. The flag alone is not a + # wall-clock bound either — kubectl retries discovery several times over + # before it gives up — so each attempt needs the wrapper as well. The last + # count pins the number of attempts, not their duration. + bounded=$(grep -c -- '--request-timeout=30s' "$d/calls" || true) + # The wall-clock bound is deliberately above the request deadline beside it: + # level with it, the kill lands first every time and --request-timeout can + # never fire, which is how the one silent failure mode stays silent. + wrapped=$(grep -c '^-k 5 40 kubectl ' "$d/timeout.calls" || true) + total=$(grep -c . "$d/calls" || true) + rm -rf "$d" + [ "$state" = unknown ] + [ "$total" = 3 ] + [ "$bounded" = 3 ] + [ "$wrapped" = 3 ] +} + +@test "a present mirror routes the decision into the reachability probe" { + d=$(mktemp -d) + _TALOS_IMAGE_FACTORY_DECISION_FILE="$d/decision" + . hack/e2e-chainsaw/_lib/talos-image-cache.sh + _TALOS_IMAGE_CACHE_QUERY_DELAY=0 + _stub_kubectl_dir "$d" 0 'deployment.apps/talos-image-cache' + set +x + url=$(resolve_talos_image_factory_url 2>"$d/err") + err=$(cat "$d/err") + rm -rf "$d" + # The state token is a contract between this helper and its two readers. + # Drift on the resolve side is the worst case there is: the present branch + # would fall through to "not deployed", cache an empty URL, and send every + # tenant worker in the suite to the public factory — the very harm the + # three-way answer exists to prevent, with every other test still green. + [ -z "$url" ] + printf '%s' "$err" | grep -q 'Available but a tenant-scoped probe' || { echo "a present mirror must reach the probe, not the not-deployed branch: [$err]" >&2; exit 1; } +} + +@test "an undecidable lookup warns and is not cached for the rest of the suite" { + d=$(mktemp -d) + _TALOS_IMAGE_FACTORY_DECISION_FILE="$d/decision" + . hack/e2e-chainsaw/_lib/talos-image-cache.sh + _TALOS_IMAGE_CACHE_QUERY_DELAY=0 + _stub_kubectl_dir "$d" 99 'deployment.apps/talos-image-cache' + # cozytest.sh runs each test under `set -x`, and xtrace shares the stderr + # this captures. Off for the call, so the assertions below read what the + # function said rather than an echo of the commands it ran, and left off + # afterwards: a test that turns it back on and then fails wedges `bats` + # instead of reporting, which costs a reader the failure they came for. + set +x + url=$(resolve_talos_image_factory_url 2>"$d/err") + cached=no + if [ -f "$d/decision" ]; then cached=yes; fi + err=$(cat "$d/err") + rm -rf "$d" + # Falling back for this one caller is fine; pinning that fallback for every + # later tenant test on the strength of a failed question is not. + [ -z "$url" ] + [ "$cached" = no ] + printf '%s' "$err" | grep -q 'could not determine' || { echo "an undecidable lookup must say so: [$err]" >&2; exit 1; } + # And it must say why. "could not determine" alone leaves an operator with a + # new silent failure in place of the old one: refused, Unauthorized and + # timed-out all read the same until kubectl's own message reaches the log. + printf '%s' "$err" | grep -q 'the server was unable to return a response' || { echo "the reason the lookup failed must reach the log: [$err]" >&2; exit 1; } +} + +@test "a lookup killed by the wall-clock bound still accounts for itself" { + d=$(mktemp -d) + _TALOS_IMAGE_FACTORY_DECISION_FILE="$d/decision" + . hack/e2e-chainsaw/_lib/talos-image-cache.sh + _TALOS_IMAGE_CACHE_QUERY_DELAY=0 + _stub_kubectl_dir "$d" 0 'deployment.apps/talos-image-cache' + # The stall this loop exists for, and the one case where nobody speaks: + # `timeout` prints nothing when it fires, and the kubectl it killed never + # reached the point where it would have printed either. The sibling test + # below pins the reason on a kubectl that failed loudly, which every stub + # does by default -- so without this case "the reason reaches the log" is + # pinned only where it was never in doubt. + export STUB_TIMEOUT_RC=124 + set +x + url=$(resolve_talos_image_factory_url 2>"$d/err") + err=$(cat "$d/err") + spoke=$(grep -c . "$d/calls" || true) + rm -rf "$d" + [ -z "$url" ] + # kubectl never ran, so anything the warning points at has to come from the + # lookup itself. Drop this and the case degrades into the sibling below. + [ "$spoke" = 0 ] + printf '%s' "$err" | grep -q 'the last one exited 124' || { echo "a lookup cut off by its own bound must say so: [$err]" >&2; exit 1; } +} + +@test "a genuine absence still resolves once and stays cached" { + d=$(mktemp -d) + _TALOS_IMAGE_FACTORY_DECISION_FILE="$d/decision" + . hack/e2e-chainsaw/_lib/talos-image-cache.sh + _TALOS_IMAGE_CACHE_QUERY_DELAY=0 + _stub_kubectl_dir "$d" 0 '' + set +x + url=$(resolve_talos_image_factory_url 2>"$d/err") + cached=no + if [ -f "$d/decision" ]; then cached=yes; fi + err=$(cat "$d/err") + rm -rf "$d" + [ -z "$url" ] + [ "$cached" = yes ] + printf '%s' "$err" | grep -q 'not deployed' || { echo "a real absence must report itself as such: [$err]" >&2; exit 1; } +} + +@test "with timeout off PATH the lookup still asks instead of answering unknown" { + . hack/e2e-chainsaw/_lib/talos-image-cache.sh + d=$(mktemp -d) + _TALOS_IMAGE_CACHE_QUERY_DELAY=0 + _stub_kubectl_dir "$d" 0 'deployment.apps/talos-image-cache' + rm -f "$d/bin/timeout" + # Everything the kubectl stub itself needs, staged from known directories so + # the stripped PATH below loses `timeout` and nothing else. Resolved this way + # rather than through `command -v` for the reason the same idiom gives in + # hack/run-kubernetes-node-join_test.bats: a name mocked as a shell function + # answers with the function and no link gets made. + for c in cat sleep; do + for p in /bin /usr/bin /usr/local/bin /opt/homebrew/bin; do + if [ -x "$p/$c" ]; then ln -sf "$p/$c" "$d/bin/$c"; break; fi + done + done + if [ ! -x "$d/bin/cat" ]; then + echo "could not stage a real cat in the stripped PATH; the check below would be vacuous" >&2 + rm -rf "$d" + exit 1 + fi + # Stripped for the call and restored right after: the assertions below need a + # `grep` this PATH deliberately does not have. + oldpath=$PATH + PATH="$d/bin" + state=$(_talos_image_cache_deploy_state) + PATH=$oldpath + wrapped=$(grep -c . "$d/timeout.calls" || true) + rm -rf "$d" + # Calling the wrapper anyway when it is not installed makes every attempt + # exit 127, and three of those read as an apiserver that never answered: a + # missing local binary would pin the whole suite to the public factory while + # every message blamed the cluster. Asserting the state rather than the exit + # is what distinguishes the two — 127 lands on `unknown`, not on an error. + [ "$state" = present ] + [ "$wrapped" = 0 ] +} + @test "probe overrides builder emits valid JSON with the image, command and restricted PSA" { . hack/e2e-chainsaw/_lib/talos-image-cache.sh img=example.invalid/img:tag