Skip to content

Commit 4d7d336

Browse files
themoriartithemoriarti
committed
fix(e2e): tell a failed pool existence read from an absent pool release
The failure-path report checked the pool release with `get hr -o name` and discarded the exit status and stderr, so a timeout, a kill, a Forbidden or a refused connection all printed that the run ended before the worker pool was applied. That is the conflation the guard wrapper exists to refuse, on the path where a slow apiserver is most likely. Read it with --ignore-not-found instead: exit 0 with empty output is an absent release, exit 0 with a name is one to classify, and any non-zero exit is a failed read named through _cozy_guard_read_failure. The pool reads also skipped the report's own deadline and checked only the phase clock, which says yes on every path that opens no phase. Check the deadline before the existence read, like the listing and the addons do, and name what was not read. The stub's ABSENT marker now answers the flag the way kubectl does. New cases pin the failed read and the deadline; the two budget cases are re-timed for the extra clock reading. Signed-off-by: Marian Koreniuk <[email protected]> Assisted-by: LLM
1 parent cac1520 commit 4d7d336

2 files changed

Lines changed: 129 additions & 19 deletions

File tree

‎hack/e2e-chainsaw/_lib/run-kubernetes.sh‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5512,7 +5512,8 @@ _COZY_GUARD_REPORT_DONE=0
55125512
# failing path would replace the caller's own exit status, and the tenant
55135513
# crust-gather snapshot hangs off that exit.
55145514
#
5515-
# Bounded on its own clock, checked before the listing and before each release.
5515+
# Bounded on its own clock, checked before the pool release, before the listing
5516+
# and before each addon release.
55165517
# What comes after this on the exit path is the tenant snapshot, which needs the
55175518
# rest of the op, so a namespace whose apiserver answers slowly must cost a
55185519
# bounded number of seconds here rather than however many releases happen to be
@@ -5532,7 +5533,7 @@ cozy_report_helmrelease_remediation() {
55325533
local _ns="$1"
55335534
local _parent="$2"
55345535
local _label="${3:-}"
5535-
local _addon_hrs _hr _pool _deadline _budget
5536+
local _addon_hrs _hr _pool _pool_found _read_rc _deadline _budget
55365537
[ "${_COZY_GUARD_REPORT_DONE}" -eq 0 ] || return 0
55375538
# An empty parent name means the run failed before the release was created, so
55385539
# there is no history anywhere to read yet.
@@ -5560,13 +5561,24 @@ cozy_report_helmrelease_remediation() {
55605561
cozy_guard_helmrelease "$_ns" "${_parent}" required || true
55615562
# The pool release is created by a later apply than the parent, so a run that
55625563
# failed between the two has none to read. Its absence is then a fact about
5563-
# where the run stopped, not a failed read, and is reported as such.
5564+
# where the run stopped, not a failed read, and is reported as such. Only an
5565+
# exit 0 with empty output means absent: --ignore-not-found makes NotFound
5566+
# that, so any non-zero exit is a read that failed and is named as one.
55645567
_pool=$(cozy_pool_helmrelease_name "${_parent}")
55655568
cozy_diag_phase_has_time "HelmRelease remediation footprint of ${_pool}" || return 0
5566-
if _cozy_guard_kubectl get hr -n "$_ns" "${_pool}" -o name >/dev/null 2>&1; then
5567-
cozy_guard_helmrelease "$_ns" "${_pool}" required || true
5568-
else
5569+
if [ "$(date +%s)" -ge "${_deadline}" ]; then
5570+
echo "=== HelmRelease remediation footprint: stopped after ${_budget}s; ${_pool} and the addon releases of ${_parent} were not read, so nothing was observed about any of them ===" >&2
5571+
return 0
5572+
fi
5573+
_cozy_guard_read_bounds
5574+
_read_rc=0
5575+
_pool_found=$(_cozy_guard_kubectl get hr -n "$_ns" "${_pool}" --ignore-not-found -o name) || _read_rc=$?
5576+
if [ "${_read_rc}" -ne 0 ]; then
5577+
echo "Checking whether ${_pool} exists failed - $(_cozy_guard_read_failure "${_read_rc}"); nothing was observed about it either way." >&2
5578+
elif [ -z "${_pool_found}" ]; then
55695579
echo "» ${_pool} does not exist - the run ended before the worker pool was applied, or its HelmRelease was never created; nothing to read"
5580+
else
5581+
cozy_guard_helmrelease "$_ns" "${_pool}" required || true
55705582
fi
55715583
cozy_diag_phase_has_time "the addon HelmRelease listing of ${_parent}" || return 0
55725584
if [ "$(date +%s)" -ge "${_deadline}" ]; then

‎hack/run-kubernetes-remediation_test.bats‎

Lines changed: 111 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,10 @@ load test_helper
3333
# (from ALL_HR), one release's install strategy and Ready condition together
3434
# (<name>.strategy and <name>.ready, joined the way the guard's jsonpath joins
3535
# them), its history statuses (<name>.history), and whether it exists at all
36-
# (`-o name`, which an ABSENT marker turns into NotFound). A missing fixture file
37-
# stands for a field the object does not carry, which is what kubectl prints for
38-
# an absent jsonpath - nothing.
36+
# (`--ignore-not-found -o name`, which a <name>.ABSENT marker turns into empty
37+
# output, and whose read the markers below take as <name>.exists). A missing
38+
# fixture file stands for a field the object does not carry, which is what
39+
# kubectl prints for an absent jsonpath - nothing.
3940
#
4041
# A FAILS marker file makes the matching read exit non-zero with a message on
4142
# stderr, which is how the API answers a timeout or a denied request. That
@@ -105,6 +106,7 @@ case "$mode" in
105106
list) markers=LIST ;;
106107
history) markers="$name.history" ;;
107108
probe) markers="$name.strategy $name.ready" ;;
109+
exists) markers="$name.exists" ;;
108110
*) markers= ;;
109111
esac
110112
for m in $markers; do
@@ -148,9 +150,14 @@ case "$mode" in
148150
"$(cat "$FIXTURES/$name.ready" 2>/dev/null || true)"
149151
;;
150152
exists)
151-
# `get hr <name> -o name`: NotFound is a non-zero exit with the message on
152-
# stderr, which an ABSENT marker reproduces; anything else exists.
153+
# `get hr <name> --ignore-not-found -o name`: an absent release prints
154+
# nothing and exits 0, which an ABSENT marker reproduces; anything else
155+
# exists. Without the flag kubectl answers NotFound with a non-zero exit,
156+
# so a read that drops it is told apart here the way the cluster would.
153157
if [ -f "$FIXTURES/$name.ABSENT" ]; then
158+
case " $* " in
159+
*' --ignore-not-found '*) exit 0 ;;
160+
esac
154161
echo "Error from server (NotFound): helmreleases.helm.toolkit.fluxcd.io \"$name\" not found" >&2
155162
exit 1
156163
fi
@@ -411,6 +418,96 @@ STUB
411418
rm -rf "$tmp"
412419
}
413420

421+
@test "a failed existence read of the pool release is not reported as absence" {
422+
# The report runs on the failure path, where a slow apiserver is most
423+
# likely. A cut-off existence read said "does not exist" there, which sends
424+
# the reader to the wrong stage of the run; it has to be named as the failed
425+
# read it is, the way the parent's and the listing's are, and the walk goes
426+
# on to the addons.
427+
. hack/e2e-chainsaw/_lib/run-kubernetes.sh
428+
tmp=$(mktemp -d)
429+
cozy_make_kubectl_stub "$tmp"
430+
export FIXTURES="$tmp/fixtures"
431+
printf 'kubernetes-test-latest-version-coredns\n' > "$FIXTURES/ALL_HR"
432+
printf 'RetryOnFailure' > "$FIXTURES/kubernetes-test-latest-version.strategy"
433+
printf 'deployed\n' > "$FIXTURES/kubernetes-test-latest-version.history"
434+
printf 'deployed\n' > "$FIXTURES/kubernetes-test-latest-version-coredns.history"
435+
: > "$FIXTURES/kubernetes-nodes-test-latest-version-md0.exists.CUTOFF"
436+
437+
rc=0
438+
out=$(PATH="$tmp/bin:$PATH" cozy_report_helmrelease_remediation tenant-test kubernetes-test-latest-version 2>&1) || rc=$?
439+
if [ "$rc" -ne 0 ]; then
440+
echo "expected the report to return zero, got rc=$rc: $out" >&2
441+
exit 1
442+
fi
443+
if ! printf '%s\n' "$out" | grep -q -F 'Checking whether kubernetes-nodes-test-latest-version-md0 exists failed - the read did not finish'; then
444+
echo "expected the cut-off existence read to be named as a failed read, got: $out" >&2
445+
exit 1
446+
fi
447+
if printf '%s\n' "$out" | grep -q -F 'kubernetes-nodes-test-latest-version-md0 does not exist'; then
448+
echo "expected a failed read not to be reported as absence, got: $out" >&2
449+
exit 1
450+
fi
451+
if grep -F 'kubernetes-nodes-test-latest-version-md0' "$FIXTURES/CALLS" | grep -q -v -F -e '-o name'; then
452+
echo "expected no classification read of a release whose existence is unknown, calls were: $(cat "$FIXTURES/CALLS")" >&2
453+
exit 1
454+
fi
455+
if ! printf '%s\n' "$out" | grep -q 'HelmRelease kubernetes-test-latest-version-coredns '; then
456+
echo "expected the addons to be read after the failed pool read, got: $out" >&2
457+
exit 1
458+
fi
459+
460+
rm -rf "$tmp"
461+
}
462+
463+
@test "the failure-path report does not read the pool release past its budget" {
464+
# The parent's reads are issued before the first clock check, so on a path
465+
# that opens no phase they can spend the whole budget on their own. The pool
466+
# release is three more bounded reads, and starting them past the deadline
467+
# takes them out of the tenant snapshot's time.
468+
. hack/e2e-chainsaw/_lib/run-kubernetes.sh
469+
tmp=$(mktemp -d)
470+
cozy_make_kubectl_stub "$tmp"
471+
export FIXTURES="$tmp/fixtures"
472+
printf 'kubernetes-test-latest-version-coredns\n' > "$FIXTURES/ALL_HR"
473+
printf 'RetryOnFailure' > "$FIXTURES/kubernetes-test-latest-version.strategy"
474+
printf 'deployed\n' > "$FIXTURES/kubernetes-test-latest-version.history"
475+
printf 'RetryOnFailure' > "$FIXTURES/kubernetes-nodes-test-latest-version-md0.strategy"
476+
printf 'failed\ndeployed\n' > "$FIXTURES/kubernetes-nodes-test-latest-version-md0.history"
477+
# The deadline is stamped at 1000+90 and the pool release is checked one
478+
# reading later, at 1100.
479+
date_ticks="$tmp/ticks"
480+
printf '0' > "$date_ticks"
481+
date_now=1000
482+
date_step=100
483+
COZY_GUARD_REPORT_BUDGET=90
484+
485+
rc=0
486+
out=$(PATH="$tmp/bin:$PATH" cozy_report_helmrelease_remediation tenant-test kubernetes-test-latest-version 2>&1) || rc=$?
487+
date_now=
488+
date_step=0
489+
date_ticks=
490+
unset COZY_GUARD_REPORT_BUDGET
491+
if [ "$rc" -ne 0 ]; then
492+
echo "expected the report to return zero, got rc=$rc: $out" >&2
493+
exit 1
494+
fi
495+
if ! printf '%s\n' "$out" | grep -q 'HelmRelease kubernetes-test-latest-version (install strategy'; then
496+
echo "expected the parent to have been classified before the budget ran out, got: $out" >&2
497+
exit 1
498+
fi
499+
if grep -q -F 'kubernetes-nodes-test-latest-version-md0' "$FIXTURES/CALLS"; then
500+
echo "expected no read of the pool release once the budget was spent, calls were: $(cat "$FIXTURES/CALLS")" >&2
501+
exit 1
502+
fi
503+
if ! printf '%s\n' "$out" | grep -q -F 'stopped after 90s; kubernetes-nodes-test-latest-version-md0 and the addon releases of kubernetes-test-latest-version were not read'; then
504+
echo "expected the skipped pool release to be named, got: $out" >&2
505+
exit 1
506+
fi
507+
508+
rm -rf "$tmp"
509+
}
510+
414511
@test "single-release guard fails a RetryOnFailure release that failed and recovered" {
415512
# The entry point the parent kubernetes-<test> release goes through, and the
416513
# shape the rule exists for. The apiserver stamps RetryOnFailure on every
@@ -1269,14 +1366,15 @@ STUB
12691366
printf 'deployed\n' > "$FIXTURES/kubernetes-test-latest-version.history"
12701367
printf 'deployed\n' > "$FIXTURES/kubernetes-test-latest-version-cilium.history"
12711368
printf 'deployed\n' > "$FIXTURES/kubernetes-test-latest-version-coredns.history"
1272-
# 30s per reading against a 90s budget: the report stamps its deadline at
1273-
# 1000+90, the listing is checked at 1030, the first addon at 1060 and the
1274-
# second at 1090, which is the first reading the deadline refuses.
1369+
# 30s per reading against a 120s budget: the report stamps its deadline at
1370+
# 1000+120, the pool release is checked at 1030, the listing at 1060, the
1371+
# first addon at 1090 and the second at 1120, which is the first reading
1372+
# the deadline refuses.
12751373
date_ticks="$tmp/ticks"
12761374
printf '0' > "$date_ticks"
12771375
date_now=1000
12781376
date_step=30
1279-
COZY_GUARD_REPORT_BUDGET=90
1377+
COZY_GUARD_REPORT_BUDGET=120
12801378

12811379
rc=0
12821380
out=$(PATH="$tmp/bin:$PATH" cozy_report_helmrelease_remediation tenant-test kubernetes-test-latest-version 2>&1) || rc=$?
@@ -1296,7 +1394,7 @@ STUB
12961394
echo "expected the second addon to fall outside the budget, got: $out" >&2
12971395
exit 1
12981396
fi
1299-
if ! printf '%s\n' "$out" | grep -q -F 'stopped after 90s; kubernetes-test-latest-version-coredns and every release after it were not read'; then
1397+
if ! printf '%s\n' "$out" | grep -q -F 'stopped after 120s; kubernetes-test-latest-version-coredns and every release after it were not read'; then
13001398
echo "expected the cut-off release to be named, got: $out" >&2
13011399
exit 1
13021400
fi
@@ -1479,13 +1577,13 @@ $out" >&2
14791577
printf 'RetryOnFailure' > "$FIXTURES/kubernetes-test-latest-version.strategy"
14801578
printf 'deployed\n' > "$FIXTURES/kubernetes-test-latest-version.history"
14811579
printf 'uninstalled\ndeployed\n' > "$FIXTURES/kubernetes-test-latest-version-cilium.history"
1482-
# The deadline is stamped at 1000+90 and the listing is checked one reading
1483-
# later, at 1100.
1580+
# The deadline is stamped at 1000+150, the pool release is admitted at 1100
1581+
# and the listing is checked one reading later, at 1200.
14841582
date_ticks="$tmp/ticks"
14851583
printf '0' > "$date_ticks"
14861584
date_now=1000
14871585
date_step=100
1488-
COZY_GUARD_REPORT_BUDGET=90
1586+
COZY_GUARD_REPORT_BUDGET=150
14891587

14901588
rc=0
14911589
out=$(PATH="$tmp/bin:$PATH" cozy_report_helmrelease_remediation tenant-test kubernetes-test-latest-version 2>&1) || rc=$?

0 commit comments

Comments
 (0)