Skip to content

Commit 3598f2c

Browse files
lexfreilexfrei
committed
fix(e2e): keep a failed mirror lookup from pinning the public factory
resolve_talos_image_factory_url asked whether the in-cluster Talos image mirror exists with a plain `kubectl get deploy`, where a connection refused, an Unauthorized and a real absence all arrive as the same non-zero exit. It then cached that reading for the whole suite, so one unlucky moment sent every tenant worker to the public Image Factory over live egress -- the dependency the mirror exists to remove -- while the run still looked ordinary. Ask with --ignore-not-found -o name instead: a real absence answers exit 0 with no output, which leaves a non-zero exit meaning only that the question failed, and re-ask a couple of times before giving up. Only the empty answer turns the suite toward the public factory. An unanswered lookup warns, falls back for its own caller alone, and is not written to the decision file, so the next tenant test asks again rather than inheriting a guess. Each attempt takes both bounds the reads in this tree use. kubectl defaults --request-timeout to 0, so a connection that establishes and then stalls has no deadline; the flag alone is not a wall-clock bound either, because kubectl retries discovery several times before giving up, so three attempts would inherit that multiple inside a Chainsaw op the rest of the tenant test has to fit in. Where timeout is not installed the read runs unwrapped rather than through it: 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. kubectl's stderr is left alone: the warning says the lookup failed, and only that text says whether it was refused, unauthorized or timed out. The diagnostic path asks the same question and keeps its own answer to it. That one gives up after a single attempt, under a phase budget where re-asking spends what the snapshot behind it needs; this one runs on the happy path, where a re-ask costs a suite nothing and buys it the mirror. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent 4bcea3d commit 3598f2c

1 file changed

Lines changed: 69 additions & 4 deletions

File tree

‎hack/e2e-chainsaw/_lib/talos-image-cache.sh‎

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,76 @@ _talos_image_cache_reachable_from_tenant() {
147147
# _talos_image_cache_deploy_state: answer "is the mirror Deployment there?" as
148148
# exactly one of present, absent or unknown.
149149
#
150-
# NOT IMPLEMENTED — this placeholder always claims present. The caller below is
151-
# wired to it so the distinction has one place to live, but until the body is
152-
# real it gets an answer that was never asked for.
150+
# --ignore-not-found is what makes a three-way answer possible: a genuine absence
151+
# becomes exit 0 with no output, which leaves a non-zero exit meaning only "the
152+
# question was never answered" — connection refused, Unauthorized, timed out.
153+
# A plain `get` collapses those two into one exit code, and the caller below then
154+
# reads a blip as an absence, caches it, and sends every tenant worker in the
155+
# suite to the public factory over the live egress the mirror exists to avoid.
156+
# Same idiom as the teardown poll in hack/e2e-chainsaw/_lib/etcd-cleanup.sh.
157+
#
158+
# talos_image_cache_diagnose asks the same question and answers it its own way,
159+
# and the two are separate on purpose. That one runs on the node-join failure
160+
# path under a phase budget, where re-asking spends what the crust-gather
161+
# snapshot after it needs, and it gives up after one attempt. This one runs on
162+
# the happy path, where a re-ask costs a suite nothing and buys it the mirror.
163+
#
164+
# The re-ask is small, and each attempt has to be bounded twice over for the
165+
# re-ask to be worth anything. kubectl's --request-timeout defaults to 0 — no
166+
# deadline on the request at all — and the case this loop exists for is a
167+
# connection that establishes and then stalls, which client-go's dial timeout
168+
# does not bound. The flag on its own is still not a wall-clock bound: against
169+
# an unreachable apiserver kubectl retries discovery several times before it
170+
# gives up, so a call carrying only the flag costs a multiple of it, which
171+
# hack/e2e-chainsaw/_lib/pod-label-census.sh measured and sized its caller's op
172+
# timeout around. Asking three times would inherit that multiple, inside a
173+
# Chainsaw op the rest of the tenant test has to fit in. So each attempt takes
174+
# both bounds, the pairing the reads in run-kubernetes.sh use and explain beside
175+
# its pod list: --request-timeout bounds the HTTP request, timeout bounds the
176+
# client retrying against a wedged apiserver. A kill by timeout exits non-zero
177+
# and is simply another attempt that did not answer. Past that, an apiserver
178+
# that is down does not become reachable by asking longer, and a run whose
179+
# apiserver is down has bigger problems than which factory it pulls from. A
180+
# real absence answers on the first try and waits for nothing.
181+
#
182+
# The wrapper is skipped rather than depended on when `timeout` is not installed,
183+
# for the reason _talos_image_cache_bounded_read gives below and one more that is
184+
# specific to a loop: its exit 127 would otherwise count as an attempt that did
185+
# not answer, three times over, and a missing local binary would pin the whole
186+
# suite to the public factory while every message blamed the apiserver.
187+
#
188+
# kubectl's stderr is deliberately not swallowed: the warning the caller prints
189+
# says the question failed, and only this text says whether it was refused,
190+
# unauthorized or timed out. Suppressing it would replace one silent failure
191+
# with another.
192+
_TALOS_IMAGE_CACHE_QUERY_TRIES="${_TALOS_IMAGE_CACHE_QUERY_TRIES:-3}"
193+
_TALOS_IMAGE_CACHE_QUERY_DELAY="${_TALOS_IMAGE_CACHE_QUERY_DELAY:-5}"
153194
_talos_image_cache_deploy_state() {
154-
printf 'present'
195+
local out rc try=1
196+
while :; do
197+
rc=0
198+
if command -v timeout >/dev/null 2>&1; then
199+
out=$(timeout -k 5 30 kubectl -n kube-system get deploy talos-image-cache \
200+
--request-timeout=30s --ignore-not-found -o name) || rc=$?
201+
else
202+
out=$(kubectl -n kube-system get deploy talos-image-cache \
203+
--request-timeout=30s --ignore-not-found -o name) || rc=$?
204+
fi
205+
if [ "$rc" -eq 0 ]; then
206+
if [ -n "$out" ]; then
207+
printf 'present'
208+
else
209+
printf 'absent'
210+
fi
211+
return 0
212+
fi
213+
if [ "$try" -ge "$_TALOS_IMAGE_CACHE_QUERY_TRIES" ]; then
214+
printf 'unknown'
215+
return 0
216+
fi
217+
try=$((try + 1))
218+
sleep "$_TALOS_IMAGE_CACHE_QUERY_DELAY"
219+
done
155220
}
156221

157222
# resolve_talos_image_factory_url: print the imageFactoryURL to use, or an empty

0 commit comments

Comments
 (0)