Files
cozystack/hack/capture-dataplane.bats
T
Aleksei Sviridkin 575b41e1cd test(hack): let each bats file declare the EXIT traps it still holds
The ban on EXIT-trap cleanup in hack/*.bats was enforced by two lists
inside hack/cozyreport.bats: one naming the files known to be clean, one
freezing an exact trap count for each file that was not. Both had to be
edited from whatever change moved them, and that is where the guard kept
failing. A change adding a trap to its own file had to edit a string in
a suite it otherwise never touches, so two changes sharing no line still
invalidated each other: each stayed green against its own base, git
merged both cleanly, and the guard went red only once the second landed.
A file arriving with traps hit the same wall from the other side -- the
inventory did notice it, since the string it compared was built by
scanning the directory, but absorbing it meant an edit in a file nothing
in the author's diff pointed at.

Replace both lists with a declaration each file makes about itself: one
"# EXIT-TRAP DEBT: N" comment, exact rather than a ceiling. A file
carrying none must install no EXIT trap, which covers the converted
files, the files that never had one, and every file added later.
Growing or shedding a trap now fails in the file the change already
edits, so two changes that disagree about a count collide textually
instead of silently.

The declaration is read from the leading comment block, not from
anywhere in the file. A .bats file is shell that writes shell, so the
same line turns up inside a heredoc, a fixture writer or an
expected-output string, where it is data belonging to one test. Reading
it there as a statement about the whole file would let an unrelated
fixture excuse a real trap, and would do it silently, since nothing in
that test's own diff looks like a declaration. A comment block is the
region with no interior: stopping instead at the first @test would still
read a line out of a helper's heredoc.

Not every counted handler is debt. A trap inside an explicit subshell
does not replace the bats binary's own, so a test failing inside
`( ... )` still prints its `not ok`. hack/e2e-test-openapi.bats kills a
backgrounded kubectl proxy that way, and moving the kill to the end of
the body would leak a process holding a fixed port. Its declaration
records that rather than scheduling a conversion, and because the
ratchet is exact, removing the trap fails too -- the count protects the
construct. What the count cannot do is tell the two apart: substituting
a test-level trap for the subshell one keeps the total at 1 and stays
green, which the header states rather than leaves to be found.

Counting bounds the keyword and the signal the same way, at any
character that cannot be part of an identifier, and matches the signal
in either case. Whitespace on the right missed `trap ... EXIT; cd
"$tmp"`; whitespace on the left missed `tmp=$(mktemp -d);trap ... EXIT`
and `(trap ... EXIT; true)`; upper case missed `trap ... exit`, which
bash and dash both install. All are real handlers that scored zero, and
the left boundary matters most, since the inventory being replaced had
none and caught the semicolon form. A bare word boundary is not enough
either way: `bootstrap ` ends in `trap `, and it must keep scoring
nothing. A quoted signal counts for the same reason as the rest.

Two handlers sharing one line are reported rather than counted, since
the count is a count of lines and the second would otherwise arrive
without moving the total; splitting the line properly needs a shell
parser, a semicolon inside a handler's own action not being a
separator, and guessing wrong undercounts -- the one direction a ratchet
cannot afford.

The scan recurses, so hack/e2e-apps/*.bats is covered rather than
sitting one directory below the guard that claims the tree. It reads
.bats and nothing else, so a handler arriving through a sourced .sh
stays outside it -- hack/e2e-chainsaw/_lib/run-kubernetes.sh installs
two, each benign for its own reason rather than by design: one sits in
a function declared with `(` and so runs in a subshell, the other in a
brace function no @test calls. That boundary is stated in the header
rather than papered over.

The guard moves to hack/bats-no-exit-trap.bats: its subject is every
unit suite under hack/, not the report collector it grew up in. Its
fixture helpers assemble the trap keyword and the signal from separate
arguments, because the guard scans its own source and a fixture written
as a literal would be counted as a real trap in it.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-07 23:53:40 +03:00

271 lines
11 KiB
Bash

#!/usr/bin/env bats
# EXIT-TRAP DEBT: 1 -- see hack/bats-no-exit-trap.bats; lower it as the traps go, delete it at zero.
# -----------------------------------------------------------------------------
# Unit tests for the pure decision/parsing helpers in
# hack/e2e-capture-dataplane.sh -- specifically the LoadBalancer-datapath
# section, which fires for a Service type=LoadBalancer whose external IP is
# unreachable while its backend stays Ready (the NotReady-pod path never sees
# this case). Only the pure logic is unit-testable here:
#
# - lb_filter_services -- keep only LoadBalancer rows with an ingress IP;
# - lb_first_ready_endpoint -- pick the first addressed, non-NotReady endpoint;
# - lb_announcer_node -- the speaker node of the most recent announce;
# - lb_capture_decision -- capture only when every probe failed.
# - pod_filter_affected -- retain scheduled NotReady pods even without IPs.
#
# The kubectl exec / tcpdump capture itself is not unit-testable (it needs a
# live cluster); these tests pin the derivations that decide WHICH node to
# capture on and WHETHER to capture at all, fed mock kubectl/log output.
#
# Strategy: the script is sourced once with E2E_CAPTURE_DATAPLANE_LIB set, which
# the script's sourcing guard honours by defining the helpers and returning
# before it touches $1 or runs any capture -- so no cluster is required and the
# capture body never executes. Each @test then calls the helpers directly and
# asserts with `[ ... ]`, matching this repo's plain-shell bats convention (no
# `run` helper). Mock IPs use the RFC 5737 / RFC 3849 documentation ranges.
#
# Title syntax constraints (inherited from cozytest.sh's awk parser):
# - Titles delimited by ASCII double quotes; embedded quotes truncate.
# - Only [A-Za-z0-9] from the title survives into the function name, so keep
# titles distinctive in their alphanumeric run.
#
# Run with: hack/cozytest.sh hack/capture-dataplane.bats
# (or `bats hack/capture-dataplane.bats` if the bats binary is
# installed; cozytest.sh is the CI path.)
# -----------------------------------------------------------------------------
HACK_DIR="$(cd "$(dirname "${BATS_TEST_FILENAME:-$0}")" && pwd)"
SCRIPT="$HACK_DIR/e2e-capture-dataplane.sh"
# Load the pure helpers. The guard returns before the capture body, so this is
# side-effect-free and needs no cluster.
E2E_CAPTURE_DATAPLANE_LIB=1
# shellcheck source=/dev/null
. "$SCRIPT"
@test "pod_filter_affected keeps a scheduled NotReady pod without a podIP" {
rows="$(printf '%s\n' \
'tenant|no-ip||node-a|False|Pending' \
'tenant|with-ip|192.0.2.80|node-b|False|Running')"
out="$(printf '%s\n' "$rows" | pod_filter_affected)"
[ "$(printf '%s\n' "$out" | awk 'END { print NR }')" -eq 2 ]
printf '%s\n' "$out" | awk '$0 == "tenant|no-ip||node-a|False|Pending" { found = 1 } END { exit !found }'
}
@test "pod_filter_affected drops Ready unscheduled and terminal pods" {
rows="$(printf '%s\n' \
'tenant|ready|192.0.2.81|node-a|True|Running' \
'tenant|unscheduled|||False|Pending' \
'tenant|succeeded|192.0.2.82|node-b|False|Succeeded' \
'tenant|failed|192.0.2.83|node-b|False|Failed')"
[ -z "$(printf '%s\n' "$rows" | pod_filter_affected)" ]
}
@test "runtime checks LoadBalancers when there are no affected pods" {
tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' EXIT
calls="$tmp/kubectl.calls"
mkdir -p "$tmp/bin"
cat > "$tmp/bin/kubectl" <<'EOF'
#!/bin/sh
printf '%s\n' "$*" >> "$MOCK_KUBECTL_CALLS"
exit 0
EOF
chmod +x "$tmp/bin/kubectl"
MOCK_KUBECTL_CALLS="$calls" PATH="$tmp/bin:$PATH" \
"$SCRIPT" "$tmp/out" > "$tmp/stdout" 2>&1
awk '$0 ~ /^get svc -A / { found = 1 } END { exit !found }' "$calls"
awk 'index($0, "checking LoadBalancers independently") { found = 1 } END { exit !found }' "$tmp/stdout"
awk 'index($0, "no Service type=LoadBalancer") { found = 1 } END { exit !found }' "$tmp/stdout"
}
@test "lb_filter_services keeps only LoadBalancer rows that have an ingress IP" {
rows="$(printf '%s\n' \
'tenant|app|LoadBalancer|192.0.2.50|80|31000|Cluster' \
'kube-system|kube-dns|ClusterIP||53||Cluster' \
'tenant|pending|LoadBalancer||443|31443|Local' \
'tenant|db|LoadBalancer|192.0.2.51|5432|31543|Local')"
out="$(printf '%s\n' "$rows" | lb_filter_services)"
[ "$(printf '%s\n' "$out" | grep -c .)" -eq 2 ]
printf '%s\n' "$out" | grep -q '^tenant|app|LoadBalancer|192.0.2.50|'
printf '%s\n' "$out" | grep -q '^tenant|db|LoadBalancer|192.0.2.51|'
# `! cmd` is vacuous under cozytest's `set -e` (errexit is suppressed for a
# `!`-negated pipeline), so a filter regression that let these rows through
# would not fail the test. Assert the absence via `if cmd; then ...; false`.
if printf '%s\n' "$out" | grep -q 'kube-dns'; then echo "FAIL: lb_filter_services must drop the kube-dns row"; false; fi
if printf '%s\n' "$out" | grep -q 'pending'; then echo "FAIL: lb_filter_services must drop the pending (no external IP) row"; false; fi
}
@test "lb_first_ready_endpoint picks the first addressed endpoint that is not NotReady" {
eps="$(printf '%s\n' \
'||tenant|virt-launcher-x|true' \
'192.0.2.60|worker-1|tenant|virt-launcher-x|false' \
'192.0.2.61|worker-2|tenant|virt-launcher-y|true' \
'192.0.2.62|worker-3|tenant|virt-launcher-z|true')"
out="$(printf '%s\n' "$eps" | lb_first_ready_endpoint)"
[ "$out" = "192.0.2.61|worker-2|tenant|virt-launcher-y" ]
}
@test "lb_first_ready_endpoint treats a blank ready column as ready" {
eps="$(printf '%s\n' '192.0.2.70|worker-9|tenant|app-0|')"
out="$(printf '%s\n' "$eps" | lb_first_ready_endpoint)"
[ "$out" = "192.0.2.70|worker-9|tenant|app-0" ]
}
@test "lb_announcer_node returns the speaker node of the most recent announce" {
logs="$(printf '%s\t%s\n' \
'node-a' '{"event":"serviceAnnounced","ips":["192.0.2.50"],"node":"node-a"}' \
'node-b' '{"event":"serviceAnnounced","ips":["192.0.2.50"],"node":"node-b"}')"
out="$(printf '%s\n' "$logs" | lb_announcer_node '192.0.2.50')"
[ "$out" = "node-b" ]
}
@test "lb_announcer_node ignores withdraw lines and announces for other IPs" {
logs="$(printf '%s\t%s\n' \
'node-x' '{"event":"serviceAnnounced","ips":["198.51.100.9"],"node":"node-x"}' \
'node-a' '{"event":"serviceAnnounced","ips":["192.0.2.50"],"node":"node-a"}' \
'node-b' '{"event":"serviceWithdrawn","ips":["192.0.2.50"],"node":"node-b"}' \
'node-c' '{"event":"serviceAnnounced","ips":["192.0.2.50"],"node":"node-c"}')"
out="$(printf '%s\n' "$logs" | lb_announcer_node '192.0.2.50')"
[ "$out" = "node-c" ]
}
@test "lb_announcer_node emits nothing when the IP was never announced" {
logs="$(printf '%s\t%s\n' \
'node-a' '{"event":"serviceAnnounced","ips":["198.51.100.9"],"node":"node-a"}')"
out="$(printf '%s\n' "$logs" | lb_announcer_node '192.0.2.50')"
[ -z "$out" ]
}
@test "lb_announcer_node excludes a node whose last own event is a withdraw" {
# Case A: node-a announces the IP then withdraws it and never re-announces.
# Its last own IP-event is the withdraw, so it is NOT the current announcer and
# nothing must be reported. The pre-polish logic kept node-a (a withdraw did
# not retract a prior announce), so this pins the failover fix.
logs="$(printf '%s\t%s\n' \
'node-a' '{"event":"serviceAnnounced","ips":["192.0.2.50"],"node":"node-a"}' \
'node-a' '{"event":"serviceWithdrawn","ips":["192.0.2.50"],"node":"node-a"}')"
out="$(printf '%s\n' "$logs" | lb_announcer_node '192.0.2.50')"
[ -z "$out" ]
}
@test "lb_announcer_node ignores a stale owner that appears later in concat order" {
# Case B: node-c is the true current owner (announce, never withdrawn). Later
# in the input -- speaker logs are concatenated per-pod, NOT globally time-
# sorted -- a stale node-a announce+withdraw block appears. node-a's last own
# event is a withdraw, so the announcer is node-c, NOT the later-in-input
# node-a. The pre-polish logic returned node-a (most recent announce by concat
# order), so this pins robustness to concat order.
logs="$(printf '%s\t%s\n' \
'node-c' '{"event":"serviceAnnounced","ips":["192.0.2.50"],"node":"node-c"}' \
'node-a' '{"event":"serviceAnnounced","ips":["192.0.2.50"],"node":"node-a"}' \
'node-a' '{"event":"serviceWithdrawn","ips":["192.0.2.50"],"node":"node-a"}')"
out="$(printf '%s\n' "$logs" | lb_announcer_node '192.0.2.50')"
[ "$out" = "node-c" ]
}
@test "lb_announcer_node matches the IP as a whole token not a substring" {
# Case C: querying 192.0.2.5 must NOT match a 192.0.2.50 announce (the
# pre-polish index()/substring match returned node-d here). The exact 192.0.2.5
# announce on node-e is matched and wins.
logs="$(printf '%s\t%s\n' \
'node-d' '{"event":"serviceAnnounced","ips":["192.0.2.50"],"node":"node-d"}' \
'node-e' '{"event":"serviceAnnounced","ips":["192.0.2.5"],"node":"node-e"}')"
no_substr="$(printf '%s\n' "$logs" | lb_announcer_node '192.0.2.5')"
no_match="$(printf '%s\t%s\n' \
'node-d' '{"event":"serviceAnnounced","ips":["192.0.2.50"],"node":"node-d"}' \
| lb_announcer_node '192.0.2.5')"
[ "$no_substr" = "node-e" ]
[ -z "$no_match" ]
}
@test "lb_capture_decision returns capture when every probe failed" {
[ "$(printf 'fail\nfail\nfail\n' | lb_capture_decision)" = "capture" ]
[ "$(printf 'fail\n' | lb_capture_decision)" = "capture" ]
}
@test "lb_capture_decision returns skip when any probe succeeded" {
[ "$(printf 'fail\nok\nfail\n' | lb_capture_decision)" = "skip" ]
[ "$(printf 'ok\n' | lb_capture_decision)" = "skip" ]
}
@test "lb_capture_decision returns skip when no probe ran at all" {
[ "$(printf '' | lb_capture_decision)" = "skip" ]
}
@test "lb_budget_ok yes below the cap and no once it is reached" {
[ "$(lb_budget_ok 0 6)" = "yes" ]
[ "$(lb_budget_ok 5 6)" = "yes" ]
[ "$(lb_budget_ok 6 6)" = "no" ]
[ "$(lb_budget_ok 7 6)" = "no" ]
}
@test "capture budget counts captured LBs so a late broken LB is still captured" {
# The MAX_LBS cap must bound LBs actually CAPTURED, not LBs enumerated: reachable
# (skipped) LBs must not consume the budget, so a broken LB enumerated after many
# reachable ones is still characterised. This mirrors the per-LB loop in
# capture_lb_datapath, which only consults lb_budget_ok / increments the counter
# on the capture branch -- here driven by a fixed decision sequence so no cluster
# is needed. With the pre-polish "increment per enumerated LB" logic and max=2,
# the loop would have broken at the 3rd LB and never reached the broken one.
max=2
captured=0
last=""
for decision in skip skip skip skip skip capture; do
if [ "$decision" != "capture" ]; then
last="skip"
continue
fi
if [ "$(lb_budget_ok "$captured" "$max")" != "yes" ]; then
last="dropped"
continue
fi
captured=$((captured + 1))
last="captured"
done
[ "$captured" -eq 1 ]
[ "$last" = "captured" ]
}
@test "capture budget stops after the cap worth of broken LBs" {
# Once MAX_LBS broken LBs are captured, further unreachable LBs are not
# characterised (left to the outer wall-clock backstop). max=2, three broken.
max=2
captured=0
dropped=0
for decision in capture capture capture; do
if [ "$(lb_budget_ok "$captured" "$max")" != "yes" ]; then
dropped=$((dropped + 1))
continue
fi
captured=$((captured + 1))
done
[ "$captured" -eq 2 ]
[ "$dropped" -eq 1 ]
}